Attachment 345857 Details for Bug 188028 – Patch

[patch] Patch

bug-188028-20180726115141.patch (text/plain), 8.91 KB, created by Chris Dumez on 2018-07-26 11:51:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-07-26 11:51:42 PDT

Size: 8.91 KB

patch

obsolete

>Subversion Revision: 234265
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 6f9ae099526b9fb2dd77431d86bc92573357681c..04a5a3fc59a469f93b9d0ceaf7cc9ca42decef6e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,36 @@
>+2018-07-26  Chris Dumez  <cdumez@apple.com>
>+
>+        ERROR: Unhandled web process message 'WebPage:SetUseDarkAppearance' when browsing in dark mode
>+        https://bugs.webkit.org/show_bug.cgi?id=188028
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The issue was that the WebViewImpl constructor was calling setUseDarkAppearance() *before* calling
>+        initializeWebPage(), which would cause us to send IPC the WebContent process for a pageID that does
>+        not exist on WebProcess side yet.
>+
>+        To address the issue, the WebViewImpl constructor no longer calls setUseDarkAppearance() and WebPageProxy
>+        no longer holds a flag indicates is dark appearance is used. Instead, the WebPageProxy asks the WebView
>+        for the value whenever it needs to, via the PageClient.
>+
>+        * UIProcess/Cocoa/WebViewImpl.h:
>+        * UIProcess/Cocoa/WebViewImpl.mm:
>+        (WebKit::WebViewImpl::WebViewImpl):
>+        (WebKit::WebViewImpl::effectiveAppearanceDidChange):
>+        (WebKit::WebViewImpl::setUseDarkAppearance): Deleted.
>+        * UIProcess/PageClient.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::creationParameters):
>+        (WebKit::WebPageProxy::useDarkAppearance const):
>+        (WebKit::WebPageProxy::effectiveAppearanceDidChange):
>+        (WebKit::WebPageProxy::showPlaybackTargetPicker):
>+        (WebKit::WebPageProxy::setUseDarkAppearance): Deleted.
>+        * UIProcess/WebPageProxy.h:
>+        (WebKit::WebPageProxy::useDarkAppearance const): Deleted.
>+        * UIProcess/mac/PageClientImplMac.h:
>+        * UIProcess/mac/PageClientImplMac.mm:
>+        (WebKit::PageClientImpl::effectiveAppearanceIsDark const):
>+
> 2018-07-26  Sihui Liu  <sihui_liu@apple.com>
> 
>         Remove a forward protocol declaration of '_WKWebViewPrintProvider'
>diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h
>index 98e27a1c31f2d93cd1f11d73c213e08220e171bf..7289bfd2d4b9dd7442039e842761d55a5891d5c2 100644
>--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h
>+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h
>@@ -539,8 +539,6 @@ public:
>     void setUseSystemAppearance(bool);
>     bool useSystemAppearance();
> 
>-    void setUseDarkAppearance(bool);
>-
>     void effectiveAppearanceDidChange();
>     bool effectiveAppearanceIsDark();
> 
>diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>index db6d523423ab981b2c74003f5a596a5142a4d43b..e8f0e1bae9996338fdcff044f1284c0a26f32782 100644
>--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>@@ -1312,8 +1312,6 @@ WebViewImpl::WebViewImpl(NSView <WebViewImplDelegate> *view, WKWebView *outerWeb
> 
>     m_page->setAddsVisitedLinks(processPool.historyClient().addsVisitedLinks());
> 
>-    m_page->setUseDarkAppearance(effectiveAppearanceIsDark());
>-
>     m_page->initializeWebPage();
> 
>     registerDraggedTypes();
>@@ -5023,7 +5021,7 @@ bool WebViewImpl::useSystemAppearance()
> 
> void WebViewImpl::effectiveAppearanceDidChange()
> {
>-    setUseDarkAppearance(effectiveAppearanceIsDark());
>+    m_page->effectiveAppearanceDidChange();
> }
> 
> bool WebViewImpl::effectiveAppearanceIsDark()
>@@ -5036,11 +5034,6 @@ bool WebViewImpl::effectiveAppearanceIsDark()
> #endif
> }
> 
>-void WebViewImpl::setUseDarkAppearance(bool useDarkAppearance)
>-{
>-    m_page->setUseDarkAppearance(useDarkAppearance);
>-}
>-
> } // namespace WebKit
> 
> #endif // PLATFORM(MAC)
>diff --git a/Source/WebKit/UIProcess/PageClient.h b/Source/WebKit/UIProcess/PageClient.h
>index 1add80fc8403fde1e12781745e8371af9399c737..877adfd263ec735c968127013d3e55aa112f9a05 100644
>--- a/Source/WebKit/UIProcess/PageClient.h
>+++ b/Source/WebKit/UIProcess/PageClient.h
>@@ -266,6 +266,8 @@ public:
>     virtual NSWindow *platformWindow() = 0;
>     virtual void setShouldSuppressFirstResponderChanges(bool) = 0;
> 
>+    virtual bool effectiveAppearanceIsDark() const = 0;
>+
> #if WK_API_ENABLED
>     virtual NSView *inspectorAttachmentView() = 0;
>     virtual _WKRemoteObjectRegistry *remoteObjectRegistry() = 0;
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 87770f2df72b4b32dde2253aef99f5b464663f6d..f1ca5391d6b4499e5c52296fdfe4b7690c8d7d5a 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -6247,7 +6247,7 @@ WebPageCreationParameters WebPageProxy::creationParameters()
> #if PLATFORM(MAC)
>     parameters.colorSpace = m_pageClient.colorSpace();
>     parameters.useSystemAppearance = m_useSystemAppearance;
>-    parameters.useDarkAppearance = m_useDarkAppearance;
>+    parameters.useDarkAppearance = useDarkAppearance();
> #endif
> #if PLATFORM(IOS)
>     parameters.screenSize = screenSize();
>@@ -6862,6 +6862,11 @@ void WebPageProxy::handleAlternativeTextUIResult(const String& result)
>         m_process->send(Messages::WebPage::HandleAlternativeTextUIResult(result), m_pageID);
> }
> 
>+bool WebPageProxy::useDarkAppearance() const
>+{
>+    return m_pageClient.effectiveAppearanceIsDark();
>+}
>+
> #if USE(DICTATION_ALTERNATIVES)
> void WebPageProxy::showDictationAlternativeUI(const WebCore::FloatRect& boundingBoxOfDictatedText, uint64_t dictationContext)
> {
>@@ -7371,16 +7376,12 @@ void WebPageProxy::setUseSystemAppearance(bool useSystemAppearance)
>     m_process->send(Messages::WebPage::SetUseSystemAppearance(useSystemAppearance), m_pageID);
> }
>     
>-void WebPageProxy::setUseDarkAppearance(bool useDarkAppearance)
>+void WebPageProxy::effectiveAppearanceDidChange()
> {
>     if (!isValid())
>         return;
> 
>-    if (useDarkAppearance == m_useDarkAppearance)
>-        return;
>-
>-    m_useDarkAppearance = useDarkAppearance;
>-    m_process->send(Messages::WebPage::SetUseDarkAppearance(useDarkAppearance), m_pageID);
>+    m_process->send(Messages::WebPage::SetUseDarkAppearance(useDarkAppearance()), m_pageID);
> }
> 
> void WebPageProxy::setHeaderBannerHeightForTesting(int height)
>@@ -7428,7 +7429,7 @@ void WebPageProxy::removePlaybackTargetPickerClient(uint64_t contextId)
> 
> void WebPageProxy::showPlaybackTargetPicker(uint64_t contextId, const WebCore::FloatRect& rect, bool hasVideo)
> {
>-    m_pageClient.mediaSessionManager().showPlaybackTargetPicker(*this, contextId, m_pageClient.rootViewToScreen(IntRect(rect)), hasVideo, m_useDarkAppearance);
>+    m_pageClient.mediaSessionManager().showPlaybackTargetPicker(*this, contextId, m_pageClient.rootViewToScreen(IntRect(rect)), hasVideo, useDarkAppearance());
> }
> 
> void WebPageProxy::playbackTargetPickerClientStateDidChange(uint64_t contextId, WebCore::MediaProducer::MediaStateFlags state)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 186522d2f39114b2f7304a159b73ef98890d7813..0d2bddccce2da6188b9605b9c466a8107590b9b9 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -836,8 +836,8 @@ public:
> #if PLATFORM(MAC)
>     void setUseSystemAppearance(bool);
>     bool useSystemAppearance() const { return m_useSystemAppearance; }
>-    void setUseDarkAppearance(bool);
>-    bool useDarkAppearance() const { return m_useDarkAppearance; }
>+    void effectiveAppearanceDidChange();
>+    bool useDarkAppearance() const;
> #endif
> 
> #if PLATFORM(COCOA)
>@@ -1891,7 +1891,6 @@ private:
>         
> #if PLATFORM(MAC)
>     bool m_useSystemAppearance { false };
>-    bool m_useDarkAppearance { false };
> #endif
> 
> #if ENABLE(APPLE_PAY)
>diff --git a/Source/WebKit/UIProcess/mac/PageClientImplMac.h b/Source/WebKit/UIProcess/mac/PageClientImplMac.h
>index fd998be49dc3416926df0da522683558293bb711..066fa9122ff48b478a492a7509e7ed1b63399698 100644
>--- a/Source/WebKit/UIProcess/mac/PageClientImplMac.h
>+++ b/Source/WebKit/UIProcess/mac/PageClientImplMac.h
>@@ -231,6 +231,7 @@ private:
>     NSWindow *platformWindow() override;
> 
>     WebCore::UserInterfaceLayoutDirection userInterfaceLayoutDirection() override;
>+    bool effectiveAppearanceIsDark() const override;
> 
> #if WK_API_ENABLED
>     NSView *inspectorAttachmentView() override;
>diff --git a/Source/WebKit/UIProcess/mac/PageClientImplMac.mm b/Source/WebKit/UIProcess/mac/PageClientImplMac.mm
>index f6fc25b0f08e29dede7992d3563eadfa869d0aed..62c239ed1d6bdb70e9ea2ba312495eb4f597c271 100644
>--- a/Source/WebKit/UIProcess/mac/PageClientImplMac.mm
>+++ b/Source/WebKit/UIProcess/mac/PageClientImplMac.mm
>@@ -894,6 +894,11 @@ WebCore::UserInterfaceLayoutDirection PageClientImpl::userInterfaceLayoutDirecti
>     return (m_view.userInterfaceLayoutDirection == NSUserInterfaceLayoutDirectionLeftToRight) ? WebCore::UserInterfaceLayoutDirection::LTR : WebCore::UserInterfaceLayoutDirection::RTL;
> }
> 
>+bool PageClientImpl::effectiveAppearanceIsDark() const
>+{
>+    return m_impl->effectiveAppearanceIsDark();
>+}
>+
> } // namespace WebKit
> 
> #endif // PLATFORM(MAC)

Actions: View | Formatted Diff | Diff

Attachments on bug 188028: 345800 | 345857