Attachment 419553 Details for Bug 221067 – Patch

[patch] Patch

bug-221067-20210208132450.patch (text/plain), 5.83 KB, created by Fujii Hironori on 2021-02-07 20:24:51 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Fujii Hironori

Created: 2021-02-07 20:24:51 PST

Size: 5.83 KB

patch

obsolete

>Subversion Revision: 272478
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index b7b49c554215fc988808b0546814f31e2be7cd33..162265d817e8ae04ad5aa630a55030cd5096eed7 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,39 @@
>+2021-02-07  Fujii Hironori  <Hironori.Fujii@sony.com>
>+
>+        [GTK] Page scrolling by wheel events doesn't work in non-AC mode pages if Hardware Acceleration Policy is changed dynamically from "always" to "never"
>+        https://bugs.webkit.org/show_bug.cgi?id=221067
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        GTK port is still supporting non-AC mode in WebKit2, and it can
>+        enable/disable accelerated compositing dynamically.
>+
>+        r271235 fixed the bug that wheel event scrolling didn't work in
>+        non-AC mode pages by checking if EventDispatcher's
>+        m_scrollingTrees has a scrolling tree for a WebKit::WebPage.
>+        However, if accelerated compositing is disabled dynamically,
>+        m_scrollingTrees keeps to have a scrolling tree for the WebPage.
>+        m_scrollingTrees should remove the item if accelerated compositing
>+        is disabled.
>+
>+        r271235 added a code to check m_scrollingTrees, but it didn't lock
>+        m_scrollingTreesMutex. The code can be removed. Removed it by
>+        moving to above code.
>+
>+        EventDispatcher::wheelEvent was using a local lambda expression.
>+        However the return value wasn't used. It can be a do-while
>+        statement.
>+
>+        * WebProcess/WebPage/EventDispatcher.cpp:
>+        (WebKit::EventDispatcher::wheelEvent): Replaced the lambda
>+        expression with a do-while statement. Unified code of the checking
>+        scrolling tree existence.
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::m_limitsNavigationsToAppBoundDomains):
>+        (WebKit::WebPage::enterAcceleratedCompositingMode):
>+        (WebKit::WebPage::exitAcceleratedCompositingMode):
>+        (WebKit::WebPage::close):
>+
> 2021-02-07  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [CoordinatedGraphics] The whole content is unnecessarily repainted by animations in non-AC mode pages
>diff --git a/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp b/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp
>index 57f79ffd4a3294a3f77a51448c17abf7c28fed34..ac11dbb4c4c680fe3f6f178df71a4f385e567325 100644
>--- a/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp
>@@ -121,12 +121,14 @@ void EventDispatcher::wheelEvent(PageIdentifier pageID, const WebWheelEvent& whe
> 
>     auto processingSteps = OptionSet<WebCore::WheelEventProcessingSteps> { WheelEventProcessingSteps::MainThreadForScrolling, WheelEventProcessingSteps::MainThreadForBlockingDOMEventDispatch };
> #if ENABLE(SCROLLING_THREAD)
>-    processingSteps = [&]() -> OptionSet<WheelEventProcessingSteps> {
>+    do {
>         LockHolder locker(m_scrollingTreesMutex);
> 
>         auto scrollingTree = m_scrollingTrees.get(pageID);
>-        if (!scrollingTree)
>-            return { WheelEventProcessingSteps::MainThreadForScrolling, WheelEventProcessingSteps::MainThreadForBlockingDOMEventDispatch };
>+        if (!scrollingTree) {
>+            dispatchWheelEventViaMainThread(pageID, wheelEvent, processingSteps);
>+            break;
>+        }
>         
>         // FIXME: It's pretty horrible that we're updating the back/forward state here.
>         // WebCore should always know the current state and know when it changes so the
>@@ -159,13 +161,7 @@ void EventDispatcher::wheelEvent(PageIdentifier pageID, const WebWheelEvent& whe
>             // respond to the UI process that the event was handled.
>             protectedThis->sendDidReceiveEvent(pageID, wheelEvent.type(), result.wasHandled);
>         });
>-
>-        return processingSteps;
>-    }();
>-
>-    auto scrollingTree = m_scrollingTrees.get(pageID);
>-    if (!scrollingTree)
>-        dispatchWheelEventViaMainThread(pageID, wheelEvent, processingSteps);
>+    } while (false);
> #else
>     UNUSED_PARAM(canRubberBandAtLeft);
>     UNUSED_PARAM(canRubberBandAtRight);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 571c103f638655e3e93f5313f4d149edd5f5cd1f..4152e2806ef83cb62650ab1a5365cff35e41877c 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -748,11 +748,6 @@ WebPage::WebPage(PageIdentifier pageID, WebPageCreationParameters&& parameters)
>     webPageCounter.increment();
> #endif
> 
>-#if ENABLE(SCROLLING_THREAD)
>-    if (m_useAsyncScrolling)
>-        webProcess.eventDispatcher().addScrollingTreeForPage(this);
>-#endif
>-
>     for (auto& mimeType : parameters.mimeTypesWithCustomContentProviders)
>         m_mimeTypesWithCustomContentProviders.add(mimeType);
> 
>@@ -1406,11 +1401,21 @@ void WebPage::clearMainFrameName()
> 
> void WebPage::enterAcceleratedCompositingMode(GraphicsLayer* layer)
> {
>+#if ENABLE(SCROLLING_THREAD)
>+    if (m_useAsyncScrolling)
>+        WebProcess::singleton().eventDispatcher().addScrollingTreeForPage(this);
>+#endif
>+
>     m_drawingArea->setRootCompositingLayer(layer);
> }
> 
> void WebPage::exitAcceleratedCompositingMode()
> {
>+#if ENABLE(SCROLLING_THREAD)
>+    if (m_useAsyncScrolling)
>+        WebProcess::singleton().eventDispatcher().removeScrollingTreeForPage(this);
>+#endif
>+
>     m_drawingArea->setRootCompositingLayer(nullptr);
> }
> 
>@@ -1507,10 +1512,6 @@ void WebPage::close()
>     if (m_remoteObjectRegistry)
>         m_remoteObjectRegistry->close();
>     ASSERT(!m_remoteObjectRegistry);
>-#endif
>-#if ENABLE(SCROLLING_THREAD)
>-    if (m_useAsyncScrolling)
>-        webProcess.eventDispatcher().removeScrollingTreeForPage(this);
> #endif
>     webProcess.removeMessageReceiver(Messages::WebPage::messageReceiverName(), m_identifier);
>     // FIXME: This should be done in the object destructors, and the objects themselves should be message receivers.

Actions: View | Formatted Diff | Diff

Attachments on bug 221067: 418598 | 418609 | 419553 | 420247 | 420581