Summary: | [Cocoa] CFPrefs synchronization does not always work for global preference changes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||||||
Component: | WebKit Misc. | Assignee: | Per Arne Vollan <pvollan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bfulgham, darin, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Per Arne Vollan
2020-06-04 14:44:22 PDT
Created attachment 401078 [details]
Patch
Comment on attachment 401078 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401078&action=review > Source/WebKit/ChangeLog:9 > + This is caused by the KVO notification being sent to the incorrect NSUserDefault object, which leads us to believe a non-global preference It's not incorrect! Global changes affect all domains. We're just inferring too much :D > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:923 > + if (domain.isEmpty()) I think you need to merge this with my change. (In reply to Tim Horton from comment #3) > Comment on attachment 401078 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401078&action=review > > > Source/WebKit/ChangeLog:9 > > + This is caused by the KVO notification being sent to the incorrect NSUserDefault object, which leads us to believe a non-global preference > > It's not incorrect! Global changes affect all domains. We're just inferring > too much :D > Right :) > > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:923 > > + if (domain.isEmpty()) > > I think you need to merge this with my change. Ah, yes, this needs a rebase. Thanks for reviewing! Created attachment 401203 [details]
Patch
Created attachment 401343 [details]
Patch
Committed r262724: <https://trac.webkit.org/changeset/262724> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401343 [details]. *** Bug 212627 has been marked as a duplicate of this bug. *** Comment on attachment 401203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401203&action=review > Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:72 > + if (!m_observer) > + return; Don’t we want "continue" here instead of return? If we want return, then why not do this outside the loop? (In reply to Darin Adler from comment #9) > Comment on attachment 401203 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401203&action=review > > > Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:72 > > + if (!m_observer) > > + return; > > Don’t we want "continue" here instead of return? If we want return, then why > not do this outside the loop? You're right! This check should be moved outside the loop. |