Summary: | ERROR: Unhandled web process message 'WebPage:SetUseDarkAppearance' when browsing in dark mode | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, bdakin, commit-queue, ggaren, thorton, timothy, webkit-bug-importer, wenson_hsieh | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2018-07-25 16:30:26 PDT
Created attachment 345800 [details]
Patch
Comment on attachment 345800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345800&action=review > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1316 > + m_page->setUseDarkAppearance(effectiveAppearanceIsDark()); Doesn't this mean that the WebPageCreationParameters sent over when initializing the web process won't have the right value for `useDarkAppearance`? Comment on attachment 345800 [details]
Patch
Will check.
Comment on attachment 345800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345800&action=review >> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1316 >> + m_page->setUseDarkAppearance(effectiveAppearanceIsDark()); > > Doesn't this mean that the WebPageCreationParameters sent over when initializing the web process won't have the right value for `useDarkAppearance`? We shouldn't even need to set this if it's in the WebPageCreationParameters Comment on attachment 345800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345800&action=review >>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1316 >>> + m_page->setUseDarkAppearance(effectiveAppearanceIsDark()); >> >> Doesn't this mean that the WebPageCreationParameters sent over when initializing the web process won't have the right value for `useDarkAppearance`? > > We shouldn't even need to set this if it's in the WebPageCreationParameters Hm...I'm not sure I understand this comment. In WebPageProxy::creationParameters(), we have: parameters.useDarkAppearance = m_useDarkAppearance; ...and m_useDarkAppearance may not be the right value then if we haven't called WebPageProxy::setUseDarkAppearance yet. It seems that either: - we should set m_useDarkAppearance to the right value but avoid sending IPC to WebPage before sending WebPageCreationParameters, or... - maybe it doesn't matter that the useDarkAppearance bit is the right value during web process creation, since we'll just end up setting it in the next IPC message anyways and this tweak here is okay. I did not realize it was part of WebPageCreationParameters. In this case, I will avoid sending the IPC if this is called before initialization. If it is called later ton (e.g. preference changes while Safari is running), then we still want to send the IPC. (In reply to Chris Dumez from comment #6) > I did not realize it was part of WebPageCreationParameters. In this case, I > will avoid sending the IPC if this is called before initialization. If it is > called later ton (e.g. preference changes while Safari is running), then we > still want to send the IPC. Hmm, looks like there is currently no flag telling if initializeWebPage() was called yet or not. Created attachment 345857 [details]
Patch
Comment on attachment 345857 [details] Patch Clearing flags on attachment: 345857 Committed r234268: <https://trac.webkit.org/changeset/234268> All reviewed patches have been landed. Closing bug. |