As a step towards blocking the distributed notifications daemon in the WebContent process, color preference changes should be observed in the UI process. The UI process should notify the WebContent process about color preference changes.
<rdar://problem/73721275>
Created attachment 418666 [details] Patch
Comment on attachment 418666 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418666&action=review > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:107 > +static CFStringRef AppleColorPreferencesChangedNotification = CFSTR("AppleColorPreferencesChangedNotification"); Should this be in one of our PAL SPI files? > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:595 > + auto* pool = reinterpret_cast<WebProcessPool*>(observer); Could pool be null (perhaps during launch or shutdown)? Maybe this should be: if (auto* pool = ....) pool->sendToAllProcesses This might apply to the 'backlightLevelDidChangeCallback', too. > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1022 > + CFNotificationCenterPostNotification(CFNotificationCenterGetLocalCenter(), CFSTR("NSColorLocalPreferencesChangedNotification"), nullptr, nullptr, true); Should 'NSColorLocalPreferencesChangedNotification' be something we put in PAL SPI somewhere? Is this a real thing used elsewhere in the system?
(In reply to Brent Fulgham from comment #3) > Comment on attachment 418666 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=418666&action=review > > > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:107 > > +static CFStringRef AppleColorPreferencesChangedNotification = CFSTR("AppleColorPreferencesChangedNotification"); > > Should this be in one of our PAL SPI files? > The constant has not been assigned an official name in the system. Even so, should we add this to an SPI file? > > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:595 > > + auto* pool = reinterpret_cast<WebProcessPool*>(observer); > > Could pool be null (perhaps during launch or shutdown)? Maybe this should be: > if (auto* pool = ....) > pool->sendToAllProcesses > > This might apply to the 'backlightLevelDidChangeCallback', too. > That is a good point. In this particular case it seems pool cannot be null, since it is being initialized with 'this' in 'CFNotificationCenterAddObserver(CFNotificationCenterGetDistributedCenter(), this, colorPreferencesDidChangeCallback, AppleColorPreferencesChangedNotification, nullptr, CFNotificationSuspensionBehaviorCoalesce)'. > > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1022 > > + CFNotificationCenterPostNotification(CFNotificationCenterGetLocalCenter(), CFSTR("NSColorLocalPreferencesChangedNotification"), nullptr, nullptr, true); > > Should 'NSColorLocalPreferencesChangedNotification' be something we put in > PAL SPI somewhere? Is this a real thing used elsewhere in the system? It is used in the system, but it seems it has not been assigned an official name. Would you prefer it being added to an SPI file? Thanks for reviewing!
Comment on attachment 418666 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418666&action=review r=me >>> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:107 >>> +static CFStringRef AppleColorPreferencesChangedNotification = CFSTR("AppleColorPreferencesChangedNotification"); >> >> Should this be in one of our PAL SPI files? > > The constant has not been assigned an official name in the system. Even so, should we add this to an SPI file? No, not if it's our own thing. >>> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:595 >>> + auto* pool = reinterpret_cast<WebProcessPool*>(observer); >> >> Could pool be null (perhaps during launch or shutdown)? Maybe this should be: >> if (auto* pool = ....) >> pool->sendToAllProcesses >> >> This might apply to the 'backlightLevelDidChangeCallback', too. > > That is a good point. In this particular case it seems pool cannot be null, since it is being initialized with 'this' in 'CFNotificationCenterAddObserver(CFNotificationCenterGetDistributedCenter(), this, colorPreferencesDidChangeCallback, AppleColorPreferencesChangedNotification, nullptr, CFNotificationSuspensionBehaviorCoalesce)'. Makes sense.
Comment on attachment 418666 [details] Patch Thanks for reviewing!
Committed r272154: <https://trac.webkit.org/changeset/272154> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418666 [details].