[webkit-reviews] review granted: [Bug 211075] [Cocoa] Global preferences are not accessible in the WebContent process when CFPrefs direct mode is enabled : [Attachment 397848] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 211075: [Cocoa] Global preferences are not accessible in the WebContent
process when CFPrefs direct mode is enabled
https://bugs.webkit.org/show_bug.cgi?id=211075

Attachment 397848: Patch

https://bugs.webkit.org/attachment.cgi?id=397848&action=review




--- Comment #6 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 397848
  --> https://bugs.webkit.org/attachment.cgi?id=397848
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397848&action=review

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:481
> +    for (size_t i = 0; i < WTF_ARRAY_LENGTH(keys); ++i) {

I learned from Darin yesterday that in modern C++ (or Objective-C++) we can
just do: std::size(keys).

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:489
> +	   auto data = retainPtr([NSKeyedArchiver
archivedDataWithRootObject:(__bridge NSMutableDictionary
*)globalPreferencesDictionary.get() requiringSecureCoding:YES error:&e]);

I suggest logging if we have an error here. These can be the source of
difficult problems to track down otherwise! :-)

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:182
> +    if (err)

Ditto: I'd log this error so we can find it if something is going wrong. It
should be very rare.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:184
> +    for (NSString *key in globalPreferencesDictionary) {

I think you can do:

[globalPreferencesDictionary enumerateKeysAndObjectsUsingBlock: ^(NSString
*key, id value, BOOL* stop) {
    if (value)
       CFPreferencesSetAppValue(static_cast<CFStringRef>(key),
static_cast<CFPropertyListRef>(value), CFSTR("kCFPreferencesAnyApplication"));
}];