Ensure RemoteScrollbarsController is used by scrollable areas with as…

…ync scrolling https://bugs.webkit.org/show_bug.cgi?id=272935 rdar://126719107 Reviewed by Simon Fraser. After https://commits.webkit.org/277015@main added use of usesAsyncScrolling to determine the scrollbars controller type, and since usesAsyncScrolling is not correct prior to layer creation, ensure creation of a RemoteScrollbarsController after layer creation if the scrollable area does use async scrolling. * Source/WebCore/platform/ScrollableArea.h: (WebCore::ScrollableArea::updateScrollbarsControllerForLayerCreation): (WebCore::ScrollableArea::hasRemoteScrollbarsController): * Source/WebCore/platform/ScrollbarsController.h: (WebCore::ScrollbarsController::isRemoteScrollbarsController): * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingRole): * Source/WebCore/rendering/RenderLayerScrollableArea.cpp: (WebCore::RenderLayerScrollableArea::updateScrollbarsControllerForLayerCreation): (WebCore::RenderLayerScrollableArea::createScrollbarsController): * Source/WebCore/rendering/RenderLayerScrollableArea.h: * Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp: (WebKit::WebChromeClient::createScrollbarsController const): * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollbarsController.h: Canonical link: https://commits.webkit.org/278517@main
WebKit · May 8, 2024 · 5b9a246 · 5b9a246
1 parent 22dd032
commit 5b9a246
Show file tree

Hide file tree

Showing 21 changed files with 176 additions and 40 deletions.
diff --git a/LayoutTests/fast/scrolling/mac/scrollbars/scrollbars-controller-type-expected.txt b/LayoutTests/fast/scrolling/mac/scrollbars/scrollbars-controller-type-expected.txt
@@ -0,0 +1,9 @@
+Ensure scrollbars controller state is correct for scroller type
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.internals.scrollbarsControllerTypeForNode(svgScroller) is "ScrollbarsControllerMac"
+PASS window.internals.scrollbarsControllerTypeForNode(scroller) is "RemoteScrollbarsController"
+PASS window.internals.scrollbarsControllerTypeForNode() is "RemoteScrollbarsController"
+
diff --git a/LayoutTests/fast/scrolling/mac/scrollbars/scrollbars-controller-type.html b/LayoutTests/fast/scrolling/mac/scrollbars/scrollbars-controller-type.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ MockScrollbarsEnabled=false AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+        body {
+            height: 1000px;
+        }
+        .scroller {
+            width: 100px;
+            height: 100px;
+            border: 1px solid black;
+            overflow: scroll;
+        }
+        .contents {
+            width: 100%;
+            height: 200%;
+        }
+    </style>
+    <script src="../../../../resources/js-test.js"></script>
+    <script src="../../../../resources/ui-helper.js"></script>
+
+    <script>
+        jsTestIsAsync = true;
+
+        if (window.internals)
+            internals.setUsesOverlayScrollbars(true);
+
+        async function doTest()
+        {
+            description('Ensure scrollbars controller state is correct for scroller type');
+            if (window.testRunner)
+                testRunner.waitUntilDone();
+            await UIHelper.renderingUpdate();
+            svgScroller = document.getElementById('svgScroller');
+            shouldBeEqualToString('window.internals.scrollbarsControllerTypeForNode(svgScroller)', 'ScrollbarsControllerMac');
+
+            scroller = document.getElementById('scroller');
+            shouldBeEqualToString('window.internals.scrollbarsControllerTypeForNode(scroller)', window.internals.isUsingUISideCompositing() ? 'RemoteScrollbarsController' : 'ScrollbarsControllerMac');
+            shouldBeEqualToString('window.internals.scrollbarsControllerTypeForNode()', window.internals.isUsingUISideCompositing() ? 'RemoteScrollbarsController' : 'ScrollbarsControllerMac');
+
+            testRunner.notifyDone();
+        }
+
+        window.addEventListener('load', () => {
+            doTest();
+        }, false);
+    </script>
+</head>
+<body>
+    <div id="scroller" class="scroller">
+        <div class="contents"></div>
+    </div>
+
+    <svg id="hello" width="400" height="400">
+        <foreignObject x="0" y="0" width="400" height="400">
+
+            <div id="svgScroller" class="scroller">
+                <div class="contents">
+                </div>
+            </div>
+        </foreignObject>
+    </svg>
+    <div id="console"></div>
+</body>
+</html>
diff --git a/...rm/mac-monterey-wk2/fast/scrolling/mac/scrollbars/scrollbars-controller-type-expected.txt b/...rm/mac-monterey-wk2/fast/scrolling/mac/scrollbars/scrollbars-controller-type-expected.txt
@@ -0,0 +1,9 @@
+Ensure scrollbars controller state is correct for scroller type
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.internals.scrollbarsControllerTypeForNode(svgScroller) is "ScrollbarsControllerMac"
+PASS window.internals.scrollbarsControllerTypeForNode(scroller) is "ScrollbarsControllerMac"
+PASS window.internals.scrollbarsControllerTypeForNode() is "ScrollbarsControllerMac"
+
diff --git a/...orm/mac-ventura-wk2/fast/scrolling/mac/scrollbars/scrollbars-controller-type-expected.txt b/...orm/mac-ventura-wk2/fast/scrolling/mac/scrollbars/scrollbars-controller-type-expected.txt
@@ -0,0 +1,9 @@
+Ensure scrollbars controller state is correct for scroller type
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.internals.scrollbarsControllerTypeForNode(svgScroller) is "ScrollbarsControllerMac"
+PASS window.internals.scrollbarsControllerTypeForNode(scroller) is "ScrollbarsControllerMac"
+PASS window.internals.scrollbarsControllerTypeForNode() is "ScrollbarsControllerMac"
+
diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
@@ -2497,7 +2497,7 @@
 		5CA126C128DBCE2900B65AD3 /* PCMSites.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CA126C028DBCE2800B65AD3 /* PCMSites.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		5CA1DEC61F71F1C700E71BD3 /* HTTPHeaderField.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CA1DEC41F71E68700E71BD3 /* HTTPHeaderField.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		5CB05C9329CA1C2800CC1AA0 /* LegacyWebSocketInspectorInstrumentation.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CB05C9129CA1C2800CC1AA0 /* LegacyWebSocketInspectorInstrumentation.h */; settings = {ATTRIBUTES = (Private, ); }; };
-		5CB37FFF1C62D2A100F20188 /* ScrollbarsControllerMock.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CB37FFD1C62D27800F20188 /* ScrollbarsControllerMock.h */; };
+		5CB37FFF1C62D2A100F20188 /* ScrollbarsControllerMock.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CB37FFD1C62D27800F20188 /* ScrollbarsControllerMock.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		5CBC8DAD1AAA302200E1C803 /* MediaAccessibilitySoftLink.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CBC8DAB1AAA302200E1C803 /* MediaAccessibilitySoftLink.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		5CBD59592280E926002B22AA /* CustomHeaderFields.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5C5D2386227A077C000B9BDA /* CustomHeaderFields.cpp */; };
 		5CC7900E266AFA89006924FE /* TimingAllowOrigin.h in Headers */ = {isa = PBXBuildFile; fileRef = 5CC7900B266AFA88006924FE /* TimingAllowOrigin.h */; settings = {ATTRIBUTES = (Private, ); }; };

diff --git a/Source/WebCore/page/ChromeClient.cpp b/Source/WebCore/page/ChromeClient.cpp
@@ -50,9 +50,10 @@ RefPtr<ImageBuffer> ChromeClient::sinkIntoImageBuffer(std::unique_ptr<WebCore::S
     return SerializedImageBuffer::sinkIntoImageBuffer(WTFMove(imageBuffer));
 }
 
-std::unique_ptr<ScrollbarsController> ChromeClient::createScrollbarsController(Page&, ScrollableArea&) const
+
+void ChromeClient::ensureScrollbarsController(Page&, ScrollableArea& area) const
 {
-    return nullptr;
+    area.ScrollableArea::createScrollbarsController();
 }
 
 }
diff --git a/Source/WebCore/page/ChromeClient.h b/Source/WebCore/page/ChromeClient.h
@@ -433,7 +433,7 @@ class ChromeClient {
     virtual bool layerTreeStateIsFrozen() const { return false; }
 
     virtual RefPtr<ScrollingCoordinator> createScrollingCoordinator(Page&) const { return nullptr; }
-    WEBCORE_EXPORT virtual std::unique_ptr<ScrollbarsController> createScrollbarsController(Page&, ScrollableArea&) const;
+    WEBCORE_EXPORT virtual void ensureScrollbarsController(Page&, ScrollableArea&) const;
 
     virtual bool canEnterVideoFullscreen(HTMLMediaElementEnums::VideoFullscreenMode) const { return false; }
     virtual bool supportsVideoFullscreen(HTMLMediaElementEnums::VideoFullscreenMode) { return false; }

diff --git a/Source/WebCore/page/LocalFrameView.cpp b/Source/WebCore/page/LocalFrameView.cpp
@@ -5599,14 +5599,10 @@ void LocalFrameView::setScrollingPerformanceTestingEnabled(bool scrollingPerform
 void LocalFrameView::createScrollbarsController()
 {
     auto* page = m_frame->page();
-    if (page) {
-        if (auto scrollbarController = page->chrome().client().createScrollbarsController(*page, *this)) {
-            setScrollbarsController(WTFMove(scrollbarController));
-            return;
-        }
-    }
+    if (!page)
+        return;
 
-    ScrollView::createScrollbarsController();
+    page->chrome().client().ensureScrollbarsController(*page, *this);
 }
 
 void LocalFrameView::didAddScrollbar(Scrollbar* scrollbar, ScrollbarOrientation orientation)

diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
@@ -224,6 +224,8 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
     ScrollAnimator* existingScrollAnimator() const { return m_scrollAnimator.get(); }
 
     WEBCORE_EXPORT ScrollbarsController& scrollbarsController() const;
+    ScrollbarsController* existingScrollbarsController() const { return m_scrollbarsController.get(); }
+    WEBCORE_EXPORT virtual void createScrollbarsController();
 
     virtual bool isActive() const = 0;
     WEBCORE_EXPORT virtual void invalidateScrollbar(Scrollbar&, const IntRect&);
@@ -434,6 +436,8 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
     virtual void updateScrollPositionForScrollAnchoringController() { }
     virtual void invalidateScrollAnchoringElement() { }
 
+    WEBCORE_EXPORT void setScrollbarsController(std::unique_ptr<ScrollbarsController>&&);
+
 protected:
     WEBCORE_EXPORT ScrollableArea();
     WEBCORE_EXPORT virtual ~ScrollableArea();
@@ -452,9 +456,6 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
 
     bool hasLayerForScrollCorner() const;
 
-    WEBCORE_EXPORT virtual void createScrollbarsController();
-    WEBCORE_EXPORT void setScrollbarsController(std::unique_ptr<ScrollbarsController>&&);
-
     WEBCORE_EXPORT LayoutRect getRectToExposeForScrollIntoView(const LayoutRect& visibleBounds, const LayoutRect& exposeRect, const ScrollAlignment& alignX, const ScrollAlignment& alignY, const std::optional<LayoutRect> = std::nullopt) const;
 
 private:

diff --git a/Source/WebCore/platform/ScrollbarsController.h b/Source/WebCore/platform/ScrollbarsController.h
@@ -52,6 +52,10 @@ class ScrollbarsController {
     bool scrollbarAnimationsUnsuspendedByUserInteraction() const { return m_scrollbarAnimationsUnsuspendedByUserInteraction; }
     void setScrollbarAnimationsUnsuspendedByUserInteraction(bool unsuspended) { m_scrollbarAnimationsUnsuspendedByUserInteraction = unsuspended; }
 
+    WEBCORE_EXPORT virtual bool isRemoteScrollbarsController() const { return false; }
+    WEBCORE_EXPORT virtual bool isScrollbarsControllerMac() const { return false; }
+    WEBCORE_EXPORT virtual bool isScrollbarsControllerMock() const { return false; }
+
     bool shouldSuspendScrollbarAnimations() const;
 
     virtual void notifyContentAreaScrolled(const FloatSize&) { }
@@ -92,7 +96,7 @@ class ScrollbarsController {
 
     WEBCORE_EXPORT virtual String horizontalScrollbarStateForTesting() const { return emptyString(); }
     WEBCORE_EXPORT virtual String verticalScrollbarStateForTesting() const { return emptyString(); }
-    
+
     WEBCORE_EXPORT virtual void setScrollbarVisibilityState(ScrollbarOrientation, bool) { }
 
     WEBCORE_EXPORT virtual bool shouldDrawIntoScrollbarLayer(Scrollbar&) const { return true; }

diff --git a/Source/WebCore/platform/mac/ScrollbarsControllerMac.h b/Source/WebCore/platform/mac/ScrollbarsControllerMac.h
@@ -49,6 +49,8 @@ class ScrollbarsControllerMac final : public ScrollbarsController {
     explicit ScrollbarsControllerMac(ScrollableArea&);
     ~ScrollbarsControllerMac();
 
+    bool isScrollbarsControllerMac() const final { return true; }
+
     void notifyContentAreaScrolled(const FloatSize& delta) final;
 
     void cancelAnimations() final;
@@ -125,4 +127,8 @@ class ScrollbarsControllerMac final : public ScrollbarsController {
 
 } // namespace WebCore
 
+SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::ScrollbarsControllerMac)
+    static bool isType(const WebCore::ScrollbarsController& controller) { return controller.isScrollbarsControllerMac(); }
+SPECIALIZE_TYPE_TRAITS_END()
+
 #endif // PLATFORM(MAC)
diff --git a/Source/WebCore/platform/mock/ScrollbarsControllerMock.h b/Source/WebCore/platform/mock/ScrollbarsControllerMock.h
@@ -45,6 +45,7 @@ class ScrollbarsControllerMock final : public ScrollbarsController {
 public:
     ScrollbarsControllerMock(ScrollableArea&, Function<void(const String&)>&&);
     virtual ~ScrollbarsControllerMock();
+    bool isScrollbarsControllerMock() const final { return true; }
 
 private:
 
@@ -67,3 +68,6 @@ class ScrollbarsControllerMock final : public ScrollbarsController {
 
 } // namespace WebCore
 
+SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::ScrollbarsControllerMock)
+    static bool isType(const WebCore::ScrollbarsController& controller) { return controller.isScrollbarsControllerMock(); }
+SPECIALIZE_TYPE_TRAITS_END()
diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -5303,6 +5303,8 @@ ScrollingNodeID RenderLayerCompositor::updateScrollingNodeForScrollingRole(Rende
             if (auto* scrollableArea = layer.scrollableArea())
                 scrollingCoordinator->setScrollingNodeScrollableAreaGeometry(newNodeID, *scrollableArea);
         }
+        if (auto* scrollableArea = layer.scrollableArea())
+            page().chrome().client().ensureScrollbarsController(page(), *scrollableArea);
     }
 
     return newNodeID;

diff --git a/Source/WebCore/rendering/RenderLayerScrollableArea.cpp b/Source/WebCore/rendering/RenderLayerScrollableArea.cpp
@@ -847,14 +847,7 @@ bool RenderLayerScrollableArea::canShowNonOverlayScrollbars() const
 
 void RenderLayerScrollableArea::createScrollbarsController()
 {
-    if (usesAsyncScrolling()) {
-        if (auto scrollbarController = m_layer.page().chrome().client().createScrollbarsController(m_layer.page(), *this)) {
-            setScrollbarsController(WTFMove(scrollbarController));
-            return;
-        }
-    }
-
-    ScrollableArea::createScrollbarsController();
+    m_layer.page().chrome().client().ensureScrollbarsController(m_layer.page(), *this);
 }
 
 static inline RenderElement* rendererForScrollbar(RenderLayerModelObject& renderer)

diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
@@ -199,6 +199,7 @@
 #include "SWClientConnection.h"
 #include "ScriptController.h"
 #include "ScriptedAnimationController.h"
+#include "ScrollbarsControllerMock.h"
 #include "ScrollingCoordinator.h"
 #include "ScrollingMomentumCalculator.h"
 #include "SecurityOrigin.h"
@@ -356,6 +357,7 @@
 
 #if PLATFORM(MAC)
 #include "GraphicsChecksMac.h"
+#include "ScrollbarsControllerMac.h"
 #endif
 
 #if PLATFORM(IOS_FAMILY)
@@ -3383,6 +3385,27 @@ ExceptionOr<String> Internals::verticalScrollbarState(Node* node) const
     return scrollableArea->verticalScrollbarStateForTesting();
 }
 
+static String scrollbarsControllerTypeString(ScrollbarsController& controller)
+{
+#if PLATFORM(MAC)
+    if (is<ScrollbarsControllerMac>(controller))
+        return "ScrollbarsControllerMac"_s;
+#endif
+    if (is<ScrollbarsControllerMock>(controller))
+        return "ScrollbarsControllerMock"_s;
+    return "RemoteScrollbarsController"_s;
+}
+
+ExceptionOr<String> Internals::scrollbarsControllerTypeForNode(Node* node) const
+{
+    auto areaOrException = scrollableAreaForNode(node);
+    if (areaOrException.hasException())
+        return areaOrException.releaseException();
+
+    auto* scrollableArea = areaOrException.releaseReturnValue();
+    return scrollbarsControllerTypeString(scrollableArea->scrollbarsController());
+}
+
 ExceptionOr<String> Internals::scrollingStateTreeAsText() const
 {
     Document* document = contextDocument();

diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
@@ -515,6 +515,8 @@ class Internals final
     ExceptionOr<String> horizontalScrollbarState(Node*) const;
     ExceptionOr<String> verticalScrollbarState(Node*) const;
 
+    ExceptionOr<String> scrollbarsControllerTypeForNode(Node*) const;
+
     ExceptionOr<String> scrollingStateTreeAsText() const;
     ExceptionOr<String> scrollingTreeAsText() const;
     ExceptionOr<bool> haveScrollingTree() const;

diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
@@ -713,6 +713,8 @@ typedef (FetchRequest or FetchResponse) FetchObject;
     boolean scrollbarUsingDarkAppearance(optional Node? node = null);
     DOMString horizontalScrollbarState(optional Node? node = null);
     DOMString verticalScrollbarState(optional Node? node = null);
+
+    DOMString scrollbarsControllerTypeForNode(optional Node? node = null);
 
     DOMString scrollingStateTreeAsText();
     DOMString scrollingTreeAsText();

diff --git a/Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm b/Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm
@@ -1668,12 +1668,7 @@ static String mutationObserverNotificationString()
     if (!page)
         return;
 
-    if (auto scrollbarController = page->chrome().client().createScrollbarsController(*page, *this)) {
-        setScrollbarsController(WTFMove(scrollbarController));
-        return;
-    }
-
-    PDFPluginBase::createScrollbarsController();
+    page->chrome().client().ensureScrollbarsController(*page, *this);
 }
 
 DelegatedScrollingMode UnifiedPDFPlugin::scrollingMode() const

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
@@ -101,7 +101,6 @@
 #include <WebCore/Settings.h>
 #include <WebCore/TextIndicator.h>
 #include <WebCore/TextRecognitionOptions.h>
-
 #include <WebCore/WindowFeatures.h>
 
 #if HAVE(WEBGPU_IMPLEMENTATION)
@@ -156,6 +155,7 @@
 
 #if PLATFORM(MAC)
 #include "RemoteScrollbarsController.h"
+#include <WebCore/ScrollbarsControllerMock.h>
 #endif
 
 namespace WebKit {
@@ -1173,22 +1173,31 @@ RefPtr<WebCore::ScrollingCoordinator> WebChromeClient::createScrollingCoordinato
 #endif
 
 #if PLATFORM(MAC)
-std::unique_ptr<ScrollbarsController> WebChromeClient::createScrollbarsController(Page& corePage, ScrollableArea& area) const
+void WebChromeClient::ensureScrollbarsController(Page& corePage, ScrollableArea& area) const
 {
     auto page = protectedPage();
     ASSERT(page->corePage() == &corePage);
-
-    if (area.mockScrollbarsControllerEnabled())
-        return nullptr;
+    auto* currentScrollbarsController = area.existingScrollbarsController();
+    if (area.mockScrollbarsControllerEnabled()) {
+        ASSERT(!currentScrollbarsController || is<ScrollbarsControllerMock>(currentScrollbarsController));
+        return;
+    }
 
     switch (page->drawingArea()->type()) {
-    case DrawingAreaType::RemoteLayerTree:
-        return makeUnique<RemoteScrollbarsController>(area, corePage.scrollingCoordinator());
-    default:
-        return nullptr;
+    case DrawingAreaType::RemoteLayerTree: {
+        if (!area.usesAsyncScrolling() && (!currentScrollbarsController || is<RemoteScrollbarsController>(currentScrollbarsController)))
+            area.setScrollbarsController(ScrollbarsController::create(area));
+        else if (area.usesAsyncScrolling() && (!currentScrollbarsController || !is<RemoteScrollbarsController>(currentScrollbarsController)))
+            area.setScrollbarsController(makeUnique<RemoteScrollbarsController>(area, corePage.scrollingCoordinator()));
+        return;
+    }
+    default: {
+        if (!currentScrollbarsController)
+            area.setScrollbarsController(ScrollbarsController::create(area));
+    }
     }
-    return nullptr;
 }
+
 #endif
 
 

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
@@ -297,7 +297,7 @@ class WebChromeClient final : public WebCore::ChromeClient {
 #endif
 
 #if PLATFORM(MAC)
-    std::unique_ptr<WebCore::ScrollbarsController> createScrollbarsController(WebCore::Page&, WebCore::ScrollableArea&) const final;
+    void ensureScrollbarsController(WebCore::Page&, WebCore::ScrollableArea&) const final;
 #endif
 
 #if ENABLE(VIDEO_PRESENTATION_MODE)