[webkit-reviews] review granted: [Bug 209244] [Cocoa] Fix launch time regression with CF prefs direct mode enabled : [Attachment 394134] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 20 15:22:39 PDT 2020
Darin Adler <darin at apple.com> has granted Per Arne Vollan <pvollan at apple.com>'s
request for review:
Bug 209244: [Cocoa] Fix launch time regression with CF prefs direct mode
enabled
https://bugs.webkit.org/show_bug.cgi?id=209244
Attachment 394134: Patch
https://bugs.webkit.org/attachment.cgi?id=394134&action=review
--- Comment #14 from Darin Adler <darin at apple.com> ---
Comment on attachment 394134
--> https://bugs.webkit.org/attachment.cgi?id=394134
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=394134&action=review
>
Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:-537
> - (global-name "com.apple.cfprefsd.daemon")
Seems strange that this is not inside a conditional.
> Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:87
> std::initializer_list<NSString*> domains = {
How can we determine if we have registered the correct set of domains, and keep
this list updated over time? Do we have to notice bugs due to incorrect
behavior or is there a system for detecting mistakes in the list?
Is this a scalable approach? Will we run into performance problems again later
if we have to add more domains? Created by someone else not thinking about
WebKit when they start fetching more preferences in some component used by
WebKit?
> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:681
> +#if ENABLE(CFPREFS_DIRECT_MODE)
> + m_activationObserver = [[NSNotificationCenter defaultCenter]
addObserverForName:@"UIApplicationDidBecomeActiveNotification" object:nil
queue:[NSOperationQueue currentQueue] usingBlock:^(NSNotification
*notification) {
> +
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
^{
> + // Start observing preference changes.
> + [WKPreferenceObserver sharedInstance];
> + });
> + }];
> +#endif
Can we cut down on the copied and pasted code somehow? Maybe a helper function
named startObservingPreferenceChanges?
Is there a race condition for preferences that we fetch before we call this
block and create a WKPreferenceObserver, where we can miss an early change?
> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:-680
> - (global-name "com.apple.cfprefsd.daemon")
Seems strange that this is not inside a conditional.
More information about the webkit-reviews
mailing list