Bug 188028 – ERROR: Unhandled web process message 'WebPage:SetUseDarkAppearance' when browsing in dark mode

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

ERROR: Unhandled web process message 'WebPage:SetUseDarkAppearance' when browsing in dark mode.

[Chris Dumez]

Created attachment 345800 [details]
Patch

[Wenson Hsieh]

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`?

[Chris Dumez]

Comment on attachment 345800 [details]
Patch

Will check.

[Alex Christensen]

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

[Wenson Hsieh]

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.

[Chris Dumez]

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.

[Chris Dumez]

(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.

[Chris Dumez]

Created attachment 345857 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 345857 [details]
Patch

Clearing flags on attachment: 345857

Committed r234268: <https://trac.webkit.org/changeset/234268>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/42634537>