-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[ios] Set backup exclusion flag after initializing DefaultFileSource #5124
Conversation
Speculative fix for App Store apps backing-up the offline/ambient cache.
[cacheURL getResourceValue:&exclusionFlag | ||
forKey:NSURLIsExcludedFromBackupKey | ||
error:&error]; | ||
XCTAssertTrue([exclusionFlag boolValue], @"Backup exclusion flag should be set for cache database."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: check for nil
before downcasting to a BOOL
. Either would be a failure in this case, but this is a reminder in case we’ve made a similar assumption elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which would be the runtime behavior when exclusionFlag == nil
?
(lldb) po [exclusionFlag boolValue]
<nil>
(lldb) p [exclusionFlag boolValue]
(BOOL) $2 = NO
nil
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When exclusionFlag
is nil
, [exclusionFlag boolValue]
is NO
, just as if exclusionFlag
had been equal to @NO
. By invoking the po
command, you’re treating the BOOL
as an object pointer and attempting to print its -description
, which is nil
. If we ever have to test that a boolValue
is NO
, either in tests or in production code, be sure to distinguish between nil
and @NO
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an extra check that exclusionFlag
exists in 6becc07.
NSNumber could be fed some non-value that it then coerces into a boolean.
Speculatively fixes #5112, where apps mistakenly backup the offline/ambient cache.
This moves the backup exclusion flag to a point just after
DefaultFileSource
has been created/initialized, ensuring that this flag is set on every launch.As with the other offline storage tests, the new
-testBackupExclusion
must be run in alphabetical order — or, at least after the cache has been initialized. The cache file will persist in the simulator when you run these tests locally./cc @1ec5