-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add GULUserDefaults to GoogleUtilities. #1812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is a workaround for NSUserDefaults crashing when setting values in the background in some situations on iOS 11. This uses the C API in order to avoid an NSNotification firing (which a class eventually tries to update the UI from the thread that set the value).
Will wait until Travis is green before adding reviewers. |
|
||
@interface GULUserDefaults () | ||
|
||
/// |
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.
Comment?
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.
Oops nice catch, will fix.
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.
Fixed in latest commit.
CFStringRef _appNameRef; | ||
} | ||
|
||
+ (nonnull GULUserDefaults *)standardUserDefaults { |
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.
Should we just use the NS_ASSUME_NONNULL
for this file and others?
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.
Thanks, good call! Actually just caught an issue there too.
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.
Fixed in latest commit.
From travis, it looks like it doesn't work on macOS |
@bstpierr Travis is green and I addressed your comments. PTAL when you get a chance! :) |
GULUDMessageCodeInvalidKeySet = 2, | ||
GULUDMessageCodeInvalidObjectSet = 3, | ||
GULUDMessageCodeSynchronizeFailed = 4, | ||
GULUDMessageCodeLibraryDirNotFound = 5, |
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: It looks like this message code is never used. Not sure if this was used in a previous commit.
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.
Great catch! Fixed.
This reverts commit ed30bc9.
This is a workaround for NSUserDefaults crashing when setting values
in the background in some situations on iOS 11. This uses the C API
in order to avoid an NSNotification firing (which a class eventually
tries to update the UI from the thread that set the value).