Attachment 361020 Details for Bug 193665 – patch

[patch] patch

frame-scroll-container-4.patch (text/plain), 41.89 KB, created by Antti Koivisto on 2019-02-03 10:02:17 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-02-03 10:02:17 PST

Size: 41.89 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 240901)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,54 @@
>+2019-02-03  Antti Koivisto  <antti@apple.com>
>+
>+        [iOS] Tiles not created in large scrollable iframes
>+        https://bugs.webkit.org/show_bug.cgi?id=193665
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We are not syncing scroll position back to the graphics layer tree correctly.
>+
>+        Test by FrÃ©dÃ©ric Wang.
>+
>+        * page/scrolling/AsyncScrollingCoordinator.cpp:
>+        (WebCore::AsyncScrollingCoordinator::frameViewRootLayerDidChange):
>+        (WebCore::AsyncScrollingCoordinator::reconcileScrollingState):
>+        (WebCore::AsyncScrollingCoordinator::setScrollPositionToScrollingLayer):
>+        (WebCore::AsyncScrollingCoordinator::syncScrollPositionToScrollingLayer):
>+
>+        Factor setting and syncing scrolling layer positions into functions.
>+        Use bounds.origin scrolling mechanic when scrollContainerLayer is present.
>+
>+        (WebCore::AsyncScrollingCoordinator::scrollableAreaScrollbarLayerDidChange):
>+        (WebCore::AsyncScrollingCoordinator::setSynchronousScrollingReasons):
>+        (WebCore::AsyncScrollingCoordinator::updateScrollLayerPosition): Deleted.
>+        * page/scrolling/AsyncScrollingCoordinator.h:
>+        * page/scrolling/ScrollingCoordinator.cpp:
>+        (WebCore::ScrollingCoordinator::scrollContainerLayerForFrameView):
>+        (WebCore::ScrollingCoordinator::scrolledContentsLayerForFrameView):
>+        (WebCore::ScrollingCoordinator::scrollLayerForFrameView): Deleted.
>+        * page/scrolling/ScrollingCoordinator.h:
>+        * rendering/RenderLayerCompositor.cpp:
>+
>+        Rename scrollLayer to scrolledContentsLayer according to out preferred naming scheme and use it in that role only.
>+        Add scrollContainerLayer as a separate layer. It is only constructed when using async scrolling on iOS.
>+
>+        (WebCore::RenderLayerCompositor::~RenderLayerCompositor):
>+        (WebCore::RenderLayerCompositor::customPositionForVisibleRectComputation const):
>+        (WebCore::RenderLayerCompositor::visibleRectForLayerFlushing const):
>+        (WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
>+        (WebCore::RenderLayerCompositor::frameViewDidChangeSize):
>+        (WebCore::RenderLayerCompositor::updateScrollLayerPosition):
>+        (WebCore::RenderLayerCompositor::frameViewDidScroll):
>+        (WebCore::RenderLayerCompositor::updateLayerForTopOverhangArea):
>+        (WebCore::RenderLayerCompositor::updateLayerForBottomOverhangArea):
>+        (WebCore::RenderLayerCompositor::updateLayerForHeader):
>+        (WebCore::RenderLayerCompositor::updateLayerForFooter):
>+        (WebCore::RenderLayerCompositor::updateOverflowControlsLayers):
>+        (WebCore::RenderLayerCompositor::ensureRootLayer):
>+        (WebCore::RenderLayerCompositor::destroyRootLayer):
>+        (WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingRole):
>+        * rendering/RenderLayerCompositor.h:
>+
> 2019-02-02  Simon Fraser  <simon.fraser@apple.com>
> 
>         Tidy up data memebers of FrameView and related classes to shrink class sizes
>Index: Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	(revision 240901)
>+++ Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	(working copy)
>@@ -226,7 +226,8 @@ void AsyncScrollingCoordinator::frameVie
>     ScrollingCoordinator::frameViewRootLayerDidChange(frameView);
> 
>     auto* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollingNodeID()));
>-    node->setScrolledContentsLayer(scrollLayerForFrameView(frameView));
>+    node->setScrollContainerLayer(scrollContainerLayerForFrameView(frameView));
>+    node->setScrolledContentsLayer(scrolledContentsLayerForFrameView(frameView));
>     node->setRootContentsLayer(rootContentsLayerForFrameView(frameView));
>     node->setCounterScrollingLayer(counterScrollingLayerForFrameView(frameView));
>     node->setInsetClipLayer(insetClipLayerForFrameView(frameView));
>@@ -413,8 +414,7 @@ void AsyncScrollingCoordinator::reconcil
>             reconcileViewportConstrainedLayerPositions(scrollingNodeID, LayoutRect(layoutViewportRect.value()), scrollingLayerPositionAction);
>     }
> 
>-    auto* scrollLayer = scrollLayerForFrameView(frameView);
>-    if (!scrollLayer)
>+    if (!scrolledContentsLayerForFrameView(frameView))
>         return;
> 
>     auto* counterScrollingLayer = counterScrollingLayerForFrameView(frameView);
>@@ -438,7 +438,8 @@ void AsyncScrollingCoordinator::reconcil
>         FrameView::yPositionForFooterLayer(scrollPosition, topContentInset, frameView.totalContentsSize().height(), frameView.footerHeight()));
> 
>     if (programmaticScroll || scrollingLayerPositionAction == ScrollingLayerPositionAction::Set) {
>-        scrollLayer->setPosition(-frameView.scrollPosition());
>+        setScrollPositionToScrollingLayer(frameView);
>+
>         if (counterScrollingLayer)
>             counterScrollingLayer->setPosition(scrollPositionForFixed);
>         if (insetClipLayer)
>@@ -452,7 +453,8 @@ void AsyncScrollingCoordinator::reconcil
>         if (footerLayer)
>             footerLayer->setPosition(positionForFooterLayer);
>     } else {
>-        scrollLayer->syncPosition(-frameView.scrollPosition());
>+        syncScrollPositionToScrollingLayer(frameView);
>+
>         if (counterScrollingLayer)
>             counterScrollingLayer->syncPosition(scrollPositionForFixed);
>         if (insetClipLayer)
>@@ -468,6 +470,29 @@ void AsyncScrollingCoordinator::reconcil
>     }
> }
> 
>+void AsyncScrollingCoordinator::setScrollPositionToScrollingLayer(FrameView& frameView)
>+{
>+#if PLATFORM(IOS_FAMILY)
>+    // Doing all scrolling like this (UIScrollView style) would simplify code.
>+    if (auto* scrollContainerLayer = scrollContainerLayerForFrameView(frameView))
>+        scrollContainerLayer->setBoundsOrigin(frameView.scrollPosition());
>+#else
>+    if (auto* scrolledContentsLayer = scrolledContentsLayerForFrameView(frameView))
>+        scrolledContentsLayer->setPosition(-frameView.scrollPosition());
>+#endif
>+}
>+
>+void AsyncScrollingCoordinator::syncScrollPositionToScrollingLayer(FrameView& frameView)
>+{
>+#if PLATFORM(IOS_FAMILY)
>+    if (auto* scrollContainerLayer = scrollContainerLayerForFrameView(frameView))
>+        scrollContainerLayer->syncBoundsOrigin(frameView.scrollPosition());
>+#else
>+    if (auto* scrolledContentsLayer = scrolledContentsLayerForFrameView(frameView))
>+        scrolledContentsLayer->syncPosition(-frameView.scrollPosition());
>+#endif
>+}
>+
> void AsyncScrollingCoordinator::scrollableAreaScrollbarLayerDidChange(ScrollableArea& scrollableArea, ScrollbarOrientation orientation)
> {
>     ASSERT(isMainThread());
>@@ -480,7 +505,6 @@ void AsyncScrollingCoordinator::scrollab
>             scrollingNode.setVerticalScrollbarLayer(scrollableArea.layerForVerticalScrollbar());
>         else
>             scrollingNode.setHorizontalScrollbarLayer(scrollableArea.layerForHorizontalScrollbar());
>-
>     }
> 
>     if (&scrollableArea == m_page->mainFrame().view()) {
>@@ -659,17 +683,10 @@ void AsyncScrollingCoordinator::setSynch
>     // at this point. So we'll update it before we switch back to main thread scrolling
>     // in order to avoid layer positioning bugs.
>     if (reasons)
>-        updateScrollLayerPosition(frameView);
>+        setScrollPositionToScrollingLayer(frameView);
>     scrollingStateNode->setSynchronousScrollingReasons(reasons);
> }
> 
>-void AsyncScrollingCoordinator::updateScrollLayerPosition(FrameView& frameView)
>-{
>-    ASSERT(isMainThread());
>-    if (auto* scrollLayer = scrollLayerForFrameView(frameView))
>-        scrollLayer->setPosition(-frameView.scrollPosition());
>-}
>-
> bool AsyncScrollingCoordinator::isRubberBandInProgress() const
> {
>     return scrollingTree()->isRubberBandInProgress();
>Index: Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h
>===================================================================
>--- Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h	(revision 240901)
>+++ Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h	(working copy)
>@@ -113,6 +113,8 @@ private:
>     WEBCORE_EXPORT void setViewportConstraintedNodeGeometry(ScrollingNodeID, const ViewportConstraints&) override;
> 
>     WEBCORE_EXPORT void reconcileScrollingState(FrameView&, const FloatPoint&, const LayoutViewportOriginOrOverrideRect&, bool programmaticScroll, ViewportRectStability, ScrollingLayerPositionAction) override;
>+    void setScrollPositionToScrollingLayer(FrameView&);
>+    void syncScrollPositionToScrollingLayer(FrameView&);
> 
>     bool isRubberBandInProgress() const override;
>     void setScrollPinningBehavior(ScrollPinningBehavior) override;
>@@ -129,7 +131,6 @@ private:
>     virtual void scheduleTreeStateCommit() = 0;
> 
>     void ensureRootStateNodeForFrameView(FrameView&);
>-    void updateScrollLayerPosition(FrameView&);
> 
>     void updateScrollPositionAfterAsyncScrollTimerFired();
>     void setEventTrackingRegionsDirty();
>Index: Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingCoordinator.cpp	(revision 240901)
>+++ Source/WebCore/page/scrolling/ScrollingCoordinator.cpp	(working copy)
>@@ -224,10 +224,17 @@ void ScrollingCoordinator::frameViewFixe
>     updateSynchronousScrollingReasons(frameView);
> }
> 
>-GraphicsLayer* ScrollingCoordinator::scrollLayerForFrameView(FrameView& frameView)
>+GraphicsLayer* ScrollingCoordinator::scrollContainerLayerForFrameView(FrameView& frameView)
> {
>     if (auto* renderView = frameView.frame().contentRenderer())
>-        return renderView->compositor().scrollLayer();
>+        return renderView->compositor().scrollContainerLayer();
>+    return nullptr;
>+}
>+
>+GraphicsLayer* ScrollingCoordinator::scrolledContentsLayerForFrameView(FrameView& frameView)
>+{
>+    if (auto* renderView = frameView.frame().contentRenderer())
>+        return renderView->compositor().scrolledContentsLayer();
>     return nullptr;
> }
> 
>Index: Source/WebCore/page/scrolling/ScrollingCoordinator.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingCoordinator.h	(revision 240901)
>+++ Source/WebCore/page/scrolling/ScrollingCoordinator.h	(working copy)
>@@ -192,7 +192,8 @@ public:
> protected:
>     explicit ScrollingCoordinator(Page*);
> 
>-    GraphicsLayer* scrollLayerForFrameView(FrameView&);
>+    GraphicsLayer* scrollContainerLayerForFrameView(FrameView&);
>+    GraphicsLayer* scrolledContentsLayerForFrameView(FrameView&);
>     GraphicsLayer* counterScrollingLayerForFrameView(FrameView&);
>     GraphicsLayer* insetClipLayerForFrameView(FrameView&);
>     GraphicsLayer* rootContentsLayerForFrameView(FrameView&);
>Index: Source/WebCore/rendering/RenderLayerCompositor.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderLayerCompositor.cpp	(revision 240901)
>+++ Source/WebCore/rendering/RenderLayerCompositor.cpp	(working copy)
>@@ -291,7 +291,7 @@ RenderLayerCompositor::~RenderLayerCompo
> {
>     // Take care that the owned GraphicsLayers are deleted first as their destructors may call back here.
>     m_clipLayer = nullptr;
>-    m_scrollLayer = nullptr;
>+    m_scrolledContentsLayer = nullptr;
>     ASSERT(m_rootLayerAttachment == RootLayerUnattached);
> }
> 
>@@ -411,7 +411,7 @@ bool RenderLayerCompositor::didRecalcSty
> 
> void RenderLayerCompositor::customPositionForVisibleRectComputation(const GraphicsLayer* graphicsLayer, FloatPoint& position) const
> {
>-    if (graphicsLayer != m_scrollLayer.get())
>+    if (graphicsLayer != m_scrolledContentsLayer.get())
>         return;
> 
>     FloatPoint scrollPosition = -position;
>@@ -453,8 +453,8 @@ FloatRect RenderLayerCompositor::visible
> #if PLATFORM(IOS_FAMILY)
>     return frameView.exposedContentRect();
> #else
>-    // Having a m_scrollLayer indicates that we're doing scrolling via GraphicsLayers.
>-    FloatRect visibleRect = m_scrollLayer ? FloatRect({ }, frameView.sizeForVisibleContent()) : frameView.visibleContentRect();
>+    // Having a m_scrolledContentsLayer indicates that we're doing scrolling via GraphicsLayers.
>+    FloatRect visibleRect = m_scrolledContentsLayer ? FloatRect({ }, frameView.sizeForVisibleContent()) : frameView.visibleContentRect();
> 
>     if (frameView.viewExposedRect())
>         visibleRect.intersect(frameView.viewExposedRect().value());
>@@ -549,10 +549,9 @@ void RenderLayerCompositor::didChangePla
>     if (auto nodeID = backing->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling)) {
>         // FIXME: would be nice to not have to special-case the root.
>         ScrollingCoordinator::NodeLayers nodeLayers;
>-        if (layer.isRenderViewLayer()) {
>-            // FIXME: Reorganize the layers and pass the scrollContainerLayer.
>-            nodeLayers = { nullptr, nullptr, m_scrollLayer.get(), fixedRootBackgroundLayer(), clipLayer(), m_rootContentsLayer.get() };
>-        } else
>+        if (layer.isRenderViewLayer())
>+            nodeLayers = { nullptr, scrollContainerLayer(), scrolledContentsLayer(), fixedRootBackgroundLayer(), clipLayer(), rootContentsLayer() };
>+        else
>             nodeLayers = { layer.backing()->graphicsLayer(), backing->scrollContainerLayer(), backing->scrolledContentsLayer() };
> 
>         scrollingCoordinator->setNodeLayers(nodeID, nodeLayers);
>@@ -1800,7 +1799,7 @@ void RenderLayerCompositor::frameViewDid
>     if (auto* layer = m_renderView.layer())
>         layer->setNeedsCompositingGeometryUpdate();
> 
>-    if (m_scrollLayer) {
>+    if (m_scrolledContentsLayer) {
>         updateScrollLayerClipping();
>         frameViewDidScroll();
>         updateOverflowControlsLayers();
>@@ -1823,12 +1822,13 @@ bool RenderLayerCompositor::hasCoordinat
> 
> void RenderLayerCompositor::updateScrollLayerPosition()
> {
>-    ASSERT(m_scrollLayer);
>+    ASSERT(!hasCoordinatedScrolling());
>+    ASSERT(m_scrolledContentsLayer);
> 
>     auto& frameView = m_renderView.frameView();
>     IntPoint scrollPosition = frameView.scrollPosition();
> 
>-    m_scrollLayer->setPosition(FloatPoint(-scrollPosition.x(), -scrollPosition.y()));
>+    m_scrolledContentsLayer->setPosition(FloatPoint(-scrollPosition.x(), -scrollPosition.y()));
> 
>     if (auto* fixedBackgroundLayer = fixedRootBackgroundLayer())
>         fixedBackgroundLayer->setPosition(frameView.scrollPositionForFixedPosition());
>@@ -1855,7 +1855,7 @@ FloatPoint RenderLayerCompositor::positi
> 
> void RenderLayerCompositor::frameViewDidScroll()
> {
>-    if (!m_scrollLayer)
>+    if (!m_scrolledContentsLayer)
>         return;
> 
>     // If there's a scrolling coordinator that manages scrolling for this frame view,
>@@ -3123,7 +3123,7 @@ GraphicsLayer* RenderLayerCompositor::up
>     if (!m_layerForTopOverhangArea) {
>         m_layerForTopOverhangArea = GraphicsLayer::create(graphicsLayerFactory(), *this);
>         m_layerForTopOverhangArea->setName("top overhang");
>-        m_scrollLayer->addChildBelow(*m_layerForTopOverhangArea, m_rootContentsLayer.get());
>+        m_scrolledContentsLayer->addChildBelow(*m_layerForTopOverhangArea, m_rootContentsLayer.get());
>     }
> 
>     return m_layerForTopOverhangArea.get();
>@@ -3142,7 +3142,7 @@ GraphicsLayer* RenderLayerCompositor::up
>     if (!m_layerForBottomOverhangArea) {
>         m_layerForBottomOverhangArea = GraphicsLayer::create(graphicsLayerFactory(), *this);
>         m_layerForBottomOverhangArea->setName("bottom overhang");
>-        m_scrollLayer->addChildBelow(*m_layerForBottomOverhangArea, m_rootContentsLayer.get());
>+        m_scrolledContentsLayer->addChildBelow(*m_layerForBottomOverhangArea, m_rootContentsLayer.get());
>     }
> 
>     m_layerForBottomOverhangArea->setPosition(FloatPoint(0, m_rootContentsLayer->size().height() + m_renderView.frameView().headerHeight()
>@@ -3170,7 +3170,7 @@ GraphicsLayer* RenderLayerCompositor::up
>     if (!m_layerForHeader) {
>         m_layerForHeader = GraphicsLayer::create(graphicsLayerFactory(), *this);
>         m_layerForHeader->setName("header");
>-        m_scrollLayer->addChildAbove(*m_layerForHeader, m_rootContentsLayer.get());
>+        m_scrolledContentsLayer->addChildAbove(*m_layerForHeader, m_rootContentsLayer.get());
>         m_renderView.frameView().addPaintPendingMilestones(DidFirstFlushForHeaderLayer);
>     }
> 
>@@ -3207,7 +3207,7 @@ GraphicsLayer* RenderLayerCompositor::up
>     if (!m_layerForFooter) {
>         m_layerForFooter = GraphicsLayer::create(graphicsLayerFactory(), *this);
>         m_layerForFooter->setName("footer");
>-        m_scrollLayer->addChildAbove(*m_layerForFooter, m_rootContentsLayer.get());
>+        m_scrolledContentsLayer->addChildAbove(*m_layerForFooter, m_rootContentsLayer.get());
>     }
> 
>     float totalContentHeight = m_rootContentsLayer->size().height() + m_renderView.frameView().headerHeight() + m_renderView.frameView().footerHeight();
>@@ -3341,7 +3341,7 @@ void RenderLayerCompositor::updateOverfl
>             m_contentShadowLayer->setAnchorPoint(FloatPoint3D());
>             m_contentShadowLayer->setCustomAppearance(GraphicsLayer::CustomAppearance::ScrollingShadow);
> 
>-            m_scrollLayer->addChildBelow(*m_contentShadowLayer, m_rootContentsLayer.get());
>+            m_scrolledContentsLayer->addChildBelow(*m_contentShadowLayer, m_rootContentsLayer.get());
>         }
>     } else
>         GraphicsLayer::unparentAndClear(m_contentShadowLayer);
>@@ -3432,36 +3432,40 @@ void RenderLayerCompositor::ensureRootLa
> 
>     if (requiresScrollLayer(expectedAttachment)) {
>         if (!m_overflowControlsHostLayer) {
>-            ASSERT(!m_scrollLayer);
>+            ASSERT(!m_scrolledContentsLayer);
>             ASSERT(!m_clipLayer);
> 
>             // Create a layer to host the clipping layer and the overflow controls layers.
>             m_overflowControlsHostLayer = GraphicsLayer::create(graphicsLayerFactory(), *this);
>             m_overflowControlsHostLayer->setName("overflow controls host");
> 
>-            auto scrollLayerType = GraphicsLayer::Type::Normal;
>+            m_scrolledContentsLayer = GraphicsLayer::create(graphicsLayerFactory(), *this);
>+            m_scrolledContentsLayer->setName("scrolled contents layer");
>+            m_scrolledContentsLayer->setAnchorPoint({ });
>+
> #if PLATFORM(IOS_FAMILY)
>-            if (m_renderView.settings().asyncFrameScrollingEnabled())
>-                scrollLayerType = GraphicsLayer::Type::Scrolling;
>+            if (m_renderView.settings().asyncFrameScrollingEnabled()) {
>+                m_scrollContainerLayer = GraphicsLayer::create(graphicsLayerFactory(), *this, GraphicsLayer::Type::Scrolling);
>+
>+                m_scrollContainerLayer->setName("scroll containers layer");
>+                m_scrollContainerLayer->setMasksToBounds(true);
>+                m_scrollContainerLayer->setAnchorPoint({ });
>+
>+                m_scrollContainerLayer->addChild(*m_scrolledContentsLayer);
>+                m_overflowControlsHostLayer->addChild(*m_scrollContainerLayer);
>+            }
> #endif
>-            m_scrollLayer = GraphicsLayer::create(graphicsLayerFactory(), *this, scrollLayerType);
>-            m_scrollLayer->setName("frame scrolling");
>-            m_scrollLayer->setAnchorPoint({ });
>-
>-            if (scrollLayerType == GraphicsLayer::Type::Scrolling) {
>-                // Scroll layer clips so there is no need for a separate clipping layer.
>-                m_overflowControlsHostLayer->addChild(*m_scrollLayer);
>-            } else {
>+            if (!m_scrollContainerLayer) {
>                 m_clipLayer = GraphicsLayer::create(graphicsLayerFactory(), *this);
>                 m_clipLayer->setName("frame clipping");
>                 m_clipLayer->setMasksToBounds(true);
>                 m_clipLayer->setAnchorPoint({ });
> 
>-                m_clipLayer->addChild(*m_scrollLayer);
>+                m_clipLayer->addChild(*m_scrolledContentsLayer);
>                 m_overflowControlsHostLayer->addChild(*m_clipLayer);
>             }
> 
>-            m_scrollLayer->addChild(*m_rootContentsLayer);
>+            m_scrolledContentsLayer->addChild(*m_rootContentsLayer);
> 
>             updateScrollLayerClipping();
>             updateOverflowControlsLayers();
>@@ -3475,7 +3479,8 @@ void RenderLayerCompositor::ensureRootLa
>         if (m_overflowControlsHostLayer) {
>             GraphicsLayer::unparentAndClear(m_overflowControlsHostLayer);
>             GraphicsLayer::unparentAndClear(m_clipLayer);
>-            GraphicsLayer::unparentAndClear(m_scrollLayer);
>+            GraphicsLayer::unparentAndClear(m_scrollContainerLayer);
>+            GraphicsLayer::unparentAndClear(m_scrolledContentsLayer);
>         }
>     }
> 
>@@ -3521,9 +3526,10 @@ void RenderLayerCompositor::destroyRootL
>     if (m_overflowControlsHostLayer) {
>         GraphicsLayer::unparentAndClear(m_overflowControlsHostLayer);
>         GraphicsLayer::unparentAndClear(m_clipLayer);
>-        GraphicsLayer::unparentAndClear(m_scrollLayer);
>+        GraphicsLayer::unparentAndClear(m_scrollContainerLayer);
>+        GraphicsLayer::unparentAndClear(m_scrolledContentsLayer);
>     }
>-    ASSERT(!m_scrollLayer);
>+    ASSERT(!m_scrolledContentsLayer);
>     GraphicsLayer::unparentAndClear(m_rootContentsLayer);
> 
>     m_layerUpdater = nullptr;
>@@ -4012,7 +4018,7 @@ ScrollingNodeID RenderLayerCompositor::u
>         }
> 
>         if (changes & ScrollingNodeChangeFlags::Layer)
>-            scrollingCoordinator->setNodeLayers(newNodeID, { nullptr, nullptr, m_scrollLayer.get(), fixedRootBackgroundLayer(), clipLayer(), m_rootContentsLayer.get() });
>+            scrollingCoordinator->setNodeLayers(newNodeID, { nullptr, scrollContainerLayer(), scrolledContentsLayer(), fixedRootBackgroundLayer(), clipLayer(), rootContentsLayer() });
> 
>         if (changes & ScrollingNodeChangeFlags::LayerGeometry) {
>             ScrollingCoordinator::ScrollingGeometry scrollingGeometry;
>Index: Source/WebCore/rendering/RenderLayerCompositor.h
>===================================================================
>--- Source/WebCore/rendering/RenderLayerCompositor.h	(revision 240901)
>+++ Source/WebCore/rendering/RenderLayerCompositor.h	(working copy)
>@@ -241,11 +241,12 @@ public:
>     WEBCORE_EXPORT RenderLayer& rootRenderLayer() const;
>     GraphicsLayer* rootGraphicsLayer() const;
> 
>-    GraphicsLayer* scrollLayer() const { return m_scrollLayer.get(); }
>+    GraphicsLayer* scrollContainerLayer() const { return m_scrollContainerLayer.get(); }
>+    GraphicsLayer* scrolledContentsLayer() const { return m_scrolledContentsLayer.get(); }
>     GraphicsLayer* clipLayer() const { return m_clipLayer.get(); }
>     GraphicsLayer* rootContentsLayer() const { return m_rootContentsLayer.get(); }
> 
>-    GraphicsLayer* layerForClipping() const {  return m_clipLayer ? m_clipLayer.get() : m_scrollLayer.get();  }
>+    GraphicsLayer* layerForClipping() const {  return m_clipLayer ? m_clipLayer.get() : m_scrollContainerLayer.get();  }
> 
> #if ENABLE(RUBBER_BANDING)
>     GraphicsLayer* headerLayer() const { return m_layerForHeader.get(); }
>@@ -564,7 +565,8 @@ private:
> 
>     // Enclosing clipping layer for iframe content
>     RefPtr<GraphicsLayer> m_clipLayer;
>-    RefPtr<GraphicsLayer> m_scrollLayer;
>+    RefPtr<GraphicsLayer> m_scrollContainerLayer;
>+    RefPtr<GraphicsLayer> m_scrolledContentsLayer;
> 
>     // Enclosing layer for overflow controls and the clipping layer
>     RefPtr<GraphicsLayer> m_overflowControlsHostLayer;
>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 240902)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2019-02-03  Antti Koivisto  <antti@apple.com>
>+
>+        [iOS] Tiles not created in large scrollable iframes
>+        https://bugs.webkit.org/show_bug.cgi?id=193665
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm:
>+        (WebKit::ScrollingTreeFrameScrollingNodeRemoteIOS::commitStateBeforeChildren):
>+        * UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
>+        (WebKit::ScrollingTreeScrollingNodeDelegateIOS::commitStateBeforeChildren):
>+
>+        We now use scrollContainerLayer consistently so remove the special cases.
>+
> 2019-02-03  Megan Gardner  <megan_gardner@apple.com>
> 
>         Turn on Smart Paste
>Index: Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm
>===================================================================
>--- Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm	(revision 240901)
>+++ Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm	(working copy)
>@@ -52,9 +52,8 @@ void ScrollingTreeFrameScrollingNodeRemo
> {
>     ScrollingTreeFrameScrollingNodeIOS::commitStateBeforeChildren(stateNode);
> 
>-    // FIXME: Should be ScrollContainerLayer.
>-    if (stateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrolledContentsLayer)) {
>-        if (scrolledContentsLayer() && [[scrolledContentsLayer() delegate] isKindOfClass:[UIScrollView self]])
>+    if (stateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer)) {
>+        if (scrollContainerLayer())
>             m_scrollingNodeDelegate = std::make_unique<ScrollingTreeScrollingNodeDelegateIOS>(*this);
>         else
>             m_scrollingNodeDelegate = nullptr;
>Index: Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm
>===================================================================
>--- Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm	(revision 240901)
>+++ Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm	(working copy)
>@@ -215,20 +215,8 @@ void ScrollingTreeScrollingNodeDelegateI
> 
> void ScrollingTreeScrollingNodeDelegateIOS::commitStateBeforeChildren(const ScrollingStateScrollingNode& scrollingStateNode)
> {
>-    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer)) {
>-        RetainPtr<CALayer> layer;
>-        layer = scrollingStateNode.scrollContainerLayer();
>-        if ([[layer delegate] isKindOfClass:[UIScrollView self]])
>-            m_scrollLayer = layer;
>-    }
>-
>-    // FIMXE: ScrollContainerLayer should always be the UIScrollView layer.
>-    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrolledContentsLayer)) {
>-        RetainPtr<CALayer> layer;
>-        layer = scrollingStateNode.scrolledContentsLayer();
>-        if ([[layer delegate] isKindOfClass:[UIScrollView self]])
>-            m_scrollLayer = layer;
>-    }
>+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer))
>+        m_scrollLayer = scrollingStateNode.scrollContainerLayer();
> }
> 
> void ScrollingTreeScrollingNodeDelegateIOS::commitStateAfterChildren(const ScrollingStateScrollingNode& scrollingStateNode)
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 240903)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2019-02-03  Antti Koivisto  <antti@apple.com>
>+
>+        [iOS] Tiles not created in large scrollable iframes
>+        https://bugs.webkit.org/show_bug.cgi?id=193665
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test by FrÃ©dÃ©ric Wang.
>+
>+        * fast/scrolling/ios/scroll-iframe-expected.html:
>+        * fast/scrolling/ios/scroll-iframe.html:
>+        * platform/ios-wk2/compositing/iframes/scrolling-iframe-expected.txt:
>+        * platform/ios-wk2/compositing/tiling/tiled-drawing-async-frame-scrolling-expected.txt:
>+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/fixed-inside-frame-expected.txt:
>+
> 2019-02-03  Megan Gardner  <megan_gardner@apple.com>
> 
>         Turn on Smart Paste
>Index: LayoutTests/fast/scrolling/ios/scroll-iframe-expected.html
>===================================================================
>--- LayoutTests/fast/scrolling/ios/scroll-iframe-expected.html	(revision 240901)
>+++ LayoutTests/fast/scrolling/ios/scroll-iframe-expected.html	(working copy)
>@@ -18,6 +18,7 @@
>         <div class=testdiv></div>
>         <div class=testdiv></div>
>         <div class=testdiv></div>
>+        <div class=testdiv></div>
>     </div>
> </body>
> </html>
>Index: LayoutTests/fast/scrolling/ios/scroll-iframe.html
>===================================================================
>--- LayoutTests/fast/scrolling/ios/scroll-iframe.html	(revision 240901)
>+++ LayoutTests/fast/scrolling/ios/scroll-iframe.html	(working copy)
>@@ -24,18 +24,24 @@
>          var c = centerOf("maxScrollX");
>          await touchAndDragFromPointToPoint(c.x, c.y, c.x - 150, c.y);
>          await liftUpAtPoint(c.x - 150, c.y);
>+
>          c = centerOf("maxScrollY");
>          await touchAndDragFromPointToPoint(c.x, c.y, c.x, c.y - 150);
>          await liftUpAtPoint(c.x, c.y - 150);
>+         
>          c = centerOf("maxScrollXY");
>          await touchAndDragFromPointToPoint(c.x, c.y, c.x - 150, c.y - 150);
>          await liftUpAtPoint(c.x - 150, c.y - 150);
> 
>+         c = centerOf("largeContent");
>+         await touchAndDragFromPointToPoint(c.x, c.y, c.x, c.y - 2000);
>+         await liftUpAtPoint(c.x, c.y - 2000);
>+
>          // Wait for scrolling to stabilize and for scrollbars to disappear.
>          setTimeout(() => {testRunner.notifyDone(); }, 2000);
>        }
> 
>-       var frameToLoadCount = 3;
>+       var frameToLoadCount = 4;
>        function newFrameLoaded() {
>            frameToLoadCount--;
>            if (frameToLoadCount == 0)
>@@ -70,6 +76,15 @@
>                 <div style='left: 100px; top: 100px; position: absolute; width: 100px; height: 100px; background: green;'></div>
>             </body>" onload="newFrameLoaded()">
>         </iframe>
>+        <iframe id="largeContent" scrolling="yes" srcdoc="
>+            <body style='margin: 0;'>
>+            <div style='width: 100px; height: 500px; background: red;'></div>
>+            <div style='width: 100px; height: 500px; background: blue;'></div>
>+            <div style='width: 100px; height: 500px; background: red;'></div>
>+            <div style='width: 100px; height: 500px; background: blue;'></div>
>+            <div style='width: 100px; height: 500px; background: green;'></div>
>+            </body>" onload="newFrameLoaded()">
>+        </iframe>
>     </div>
> </body>
> </html>
>Index: LayoutTests/platform/ios-wk2/compositing/iframes/scrolling-iframe-expected.txt
>===================================================================
>--- LayoutTests/platform/ios-wk2/compositing/iframes/scrolling-iframe-expected.txt	(revision 240901)
>+++ LayoutTests/platform/ios-wk2/compositing/iframes/scrolling-iframe-expected.txt	(working copy)
>@@ -20,7 +20,6 @@
>                   (bounds 300.00 150.00)
>                   (children 1
>                     (GraphicsLayer
>-                      (position -80.00 -80.00)
>                       (anchor 0.00 0.00)
>                       (children 1
>                         (GraphicsLayer
>Index: LayoutTests/platform/ios-wk2/compositing/tiling/tiled-drawing-async-frame-scrolling-expected.txt
>===================================================================
>--- LayoutTests/platform/ios-wk2/compositing/tiling/tiled-drawing-async-frame-scrolling-expected.txt	(revision 240901)
>+++ LayoutTests/platform/ios-wk2/compositing/tiling/tiled-drawing-async-frame-scrolling-expected.txt	(working copy)
>@@ -44,38 +44,48 @@
>                   (bounds 300.00 150.00)
>                   (backingStoreAttached 1)
>                   (visible rect 0.00, 0.00 300.00 x 150.00)
>-                  (coverage rect -10.00, -10.00 800.00 x 600.00)
>+                  (coverage rect 0.00, 0.00 300.00 x 150.00)
>                   (intersects coverage rect 1)
>                   (contentsScale 2.00)
>                   (children 1
>                     (GraphicsLayer
>                       (anchor 0.00 0.00)
>-                      (bounds 400.00 300.00)
>-                      (backingStoreAttached 1)
>-                      (visible rect 0.00, 0.00 400.00 x 300.00)
>-                      (coverage rect 0.00, 0.00 400.00 x 300.00)
>-                      (intersects coverage rect 1)
>+                      (backingStoreAttached 0)
>+                      (visible rect 0.00, 0.00 0.00 x 0.00)
>+                      (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                      (intersects coverage rect 0)
>                       (contentsScale 2.00)
>                       (children 1
>                         (GraphicsLayer
>+                          (anchor 0.00 0.00)
>                           (bounds 400.00 300.00)
>-                          (drawsContent 1)
>                           (backingStoreAttached 1)
>-                          (visible rect 0.00, 0.00 400.00 x 300.00)
>-                          (coverage rect 0.00, 0.00 400.00 x 300.00)
>+                          (visible rect 0.00, 0.00 300.00 x 150.00)
>+                          (coverage rect 0.00, 0.00 300.00 x 150.00)
>                           (intersects coverage rect 1)
>                           (contentsScale 2.00)
>-                          (tile cache coverage 0, 0 400 x 300)
>-                          (tile size 512 x 512)
>-                          (top left tile 0, 0 tiles grid 1 x 1)
>-                          (in window 1)
>                           (children 1
>                             (GraphicsLayer
>-                              (backingStoreAttached 0)
>-                              (visible rect 0.00, 0.00 0.00 x 0.00)
>-                              (coverage rect 0.00, 0.00 400.00 x 300.00)
>-                              (intersects coverage rect 0)
>+                              (bounds 400.00 300.00)
>+                              (drawsContent 1)
>+                              (backingStoreAttached 1)
>+                              (visible rect 0.00, 0.00 300.00 x 150.00)
>+                              (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                              (intersects coverage rect 1)
>                               (contentsScale 2.00)
>+                              (tile cache coverage 0, 0 400 x 300)
>+                              (tile size 512 x 512)
>+                              (top left tile 0, 0 tiles grid 1 x 1)
>+                              (in window 1)
>+                              (children 1
>+                                (GraphicsLayer
>+                                  (backingStoreAttached 0)
>+                                  (visible rect 0.00, 0.00 0.00 x 0.00)
>+                                  (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                                  (intersects coverage rect 0)
>+                                  (contentsScale 2.00)
>+                                )
>+                              )
>                             )
>                           )
>                         )
>@@ -110,38 +120,48 @@
>                   (bounds 300.00 150.00)
>                   (backingStoreAttached 1)
>                   (visible rect 0.00, 0.00 300.00 x 150.00)
>-                  (coverage rect -318.00, -10.00 800.00 x 600.00)
>+                  (coverage rect 0.00, 0.00 300.00 x 150.00)
>                   (intersects coverage rect 1)
>                   (contentsScale 2.00)
>                   (children 1
>                     (GraphicsLayer
>                       (anchor 0.00 0.00)
>-                      (bounds 400.00 300.00)
>-                      (backingStoreAttached 1)
>-                      (visible rect 0.00, 0.00 400.00 x 300.00)
>-                      (coverage rect 0.00, 0.00 400.00 x 300.00)
>-                      (intersects coverage rect 1)
>+                      (backingStoreAttached 0)
>+                      (visible rect 0.00, 0.00 0.00 x 0.00)
>+                      (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                      (intersects coverage rect 0)
>                       (contentsScale 2.00)
>                       (children 1
>                         (GraphicsLayer
>+                          (anchor 0.00 0.00)
>                           (bounds 400.00 300.00)
>-                          (drawsContent 1)
>                           (backingStoreAttached 1)
>-                          (visible rect 0.00, 0.00 400.00 x 300.00)
>-                          (coverage rect 0.00, 0.00 400.00 x 300.00)
>+                          (visible rect 0.00, 0.00 300.00 x 150.00)
>+                          (coverage rect 0.00, 0.00 300.00 x 150.00)
>                           (intersects coverage rect 1)
>                           (contentsScale 2.00)
>-                          (tile cache coverage 0, 0 400 x 300)
>-                          (tile size 512 x 512)
>-                          (top left tile 0, 0 tiles grid 1 x 1)
>-                          (in window 1)
>                           (children 1
>                             (GraphicsLayer
>-                              (backingStoreAttached 0)
>-                              (visible rect 0.00, 0.00 0.00 x 0.00)
>-                              (coverage rect 0.00, 0.00 400.00 x 300.00)
>-                              (intersects coverage rect 0)
>+                              (bounds 400.00 300.00)
>+                              (drawsContent 1)
>+                              (backingStoreAttached 1)
>+                              (visible rect 0.00, 0.00 300.00 x 150.00)
>+                              (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                              (intersects coverage rect 1)
>                               (contentsScale 2.00)
>+                              (tile cache coverage 0, 0 400 x 300)
>+                              (tile size 512 x 512)
>+                              (top left tile 0, 0 tiles grid 1 x 1)
>+                              (in window 1)
>+                              (children 1
>+                                (GraphicsLayer
>+                                  (backingStoreAttached 0)
>+                                  (visible rect 0.00, 0.00 0.00 x 0.00)
>+                                  (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                                  (intersects coverage rect 0)
>+                                  (contentsScale 2.00)
>+                                )
>+                              )
>                             )
>                           )
>                         )
>@@ -176,38 +196,48 @@
>                   (bounds 300.00 150.00)
>                   (backingStoreAttached 1)
>                   (visible rect 0.00, 0.00 300.00 x 150.00)
>-                  (coverage rect -10.00, -169.00 800.00 x 600.00)
>+                  (coverage rect 0.00, 0.00 300.00 x 150.00)
>                   (intersects coverage rect 1)
>                   (contentsScale 2.00)
>                   (children 1
>                     (GraphicsLayer
>                       (anchor 0.00 0.00)
>-                      (bounds 400.00 300.00)
>-                      (backingStoreAttached 1)
>-                      (visible rect 0.00, 0.00 400.00 x 300.00)
>-                      (coverage rect 0.00, 0.00 400.00 x 300.00)
>-                      (intersects coverage rect 1)
>+                      (backingStoreAttached 0)
>+                      (visible rect 0.00, 0.00 0.00 x 0.00)
>+                      (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                      (intersects coverage rect 0)
>                       (contentsScale 2.00)
>                       (children 1
>                         (GraphicsLayer
>+                          (anchor 0.00 0.00)
>                           (bounds 400.00 300.00)
>-                          (drawsContent 1)
>                           (backingStoreAttached 1)
>-                          (visible rect 0.00, 0.00 400.00 x 300.00)
>-                          (coverage rect 0.00, 0.00 400.00 x 300.00)
>+                          (visible rect 0.00, 0.00 300.00 x 150.00)
>+                          (coverage rect 0.00, 0.00 300.00 x 150.00)
>                           (intersects coverage rect 1)
>                           (contentsScale 2.00)
>-                          (tile cache coverage 0, 0 400 x 300)
>-                          (tile size 512 x 512)
>-                          (top left tile 0, 0 tiles grid 1 x 1)
>-                          (in window 1)
>                           (children 1
>                             (GraphicsLayer
>-                              (backingStoreAttached 0)
>-                              (visible rect 0.00, 0.00 0.00 x 0.00)
>-                              (coverage rect 0.00, 0.00 400.00 x 300.00)
>-                              (intersects coverage rect 0)
>+                              (bounds 400.00 300.00)
>+                              (drawsContent 1)
>+                              (backingStoreAttached 1)
>+                              (visible rect 0.00, 0.00 300.00 x 150.00)
>+                              (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                              (intersects coverage rect 1)
>                               (contentsScale 2.00)
>+                              (tile cache coverage 0, 0 400 x 300)
>+                              (tile size 512 x 512)
>+                              (top left tile 0, 0 tiles grid 1 x 1)
>+                              (in window 1)
>+                              (children 1
>+                                (GraphicsLayer
>+                                  (backingStoreAttached 0)
>+                                  (visible rect 0.00, 0.00 0.00 x 0.00)
>+                                  (coverage rect 0.00, 0.00 300.00 x 150.00)
>+                                  (intersects coverage rect 0)
>+                                  (contentsScale 2.00)
>+                                )
>+                              )
>                             )
>                           )
>                         )
>Index: LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/fixed-inside-frame-expected.txt
>===================================================================
>--- LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/fixed-inside-frame-expected.txt	(revision 240901)
>+++ LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/fixed-inside-frame-expected.txt	(working copy)
>@@ -68,7 +68,6 @@
>                   (bounds 480.00 400.00)
>                   (children 1
>                     (GraphicsLayer
>-                      (position 0.00 -120.00)
>                       (anchor 0.00 0.00)
>                       (children 1
>                         (GraphicsLayer

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193665: 359724 | 359729 | 359730 | 360596 | 360604 | 361002 | 361006 | 361010 | 361020 | 361040