Changeset 182799 in webkit

Timestamp:

Apr 14, 2015 10:25:31 AM (9 years ago)

Author:

Simon Fraser

Message:

[iOS WK2] Interactive elements of developer.apple.com are broken
​https://bugs.webkit.org/show_bug.cgi?id=143692
Source/WebCore:

rdar://problem/19320087

Reviewed by Sam Weinig.

When a composited RenderLayer had nodes in the scrolling tree by virtue of
both position and overflow:scroll, and one of those reasons disappeared,
we'd fail to remove the corresponding node from the scrolling tree. This
could leave an overflow:scroll element behaving as if it were position:fixed.

Fix by having RenderLayerCompositor::updateScrollCoordinationForThisFrame()
detach the layer on a per-role basis.

Test: platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html

• rendering/RenderLayerBacking.cpp:

(WebCore::RenderLayerBacking::detachFromScrollingCoordinatorForRole):

• rendering/RenderLayerBacking.h:
• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::detachScrollCoordinatedLayerForRole):
(WebCore::RenderLayerCompositor::detachScrollCoordinatedLayer): Just moved.
(WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer): Call detachScrollCoordinatedLayerForRole()
if the layer doesn't have the relevant scrolling reasons.

• rendering/RenderLayerCompositor.h:

LayoutTests:

Reviewed by Sam Weinig.

• platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt: Added.
• platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios-simulator-wk2/scrolling (added)
• LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt (added)
• LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerBacking.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerBacking.h (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderLayerCompositor.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-04-13  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Interactive elements of developer.apple.com are broken
+        https://bugs.webkit.org/show_bug.cgi?id=143692
+
+        Reviewed by Sam Weinig.
+
+        * platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt: Added.
+        * platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html: Added.
+
 2015-04-14  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-04-13  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Interactive elements of developer.apple.com are broken
+        https://bugs.webkit.org/show_bug.cgi?id=143692
+        rdar://problem/19320087
+
+        Reviewed by Sam Weinig.
+        
+        When a composited RenderLayer had nodes in the scrolling tree by virtue of
+        both position and overflow:scroll, and one of those reasons disappeared,
+        we'd fail to remove the corresponding node from the scrolling tree. This
+        could leave an overflow:scroll element behaving as if it were position:fixed.
+        
+        Fix by having RenderLayerCompositor::updateScrollCoordinationForThisFrame()
+        detach the layer on a per-role basis.
+
+        Test: platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html
+
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::detachFromScrollingCoordinatorForRole):
+        * rendering/RenderLayerBacking.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::detachScrollCoordinatedLayerForRole):
+        (WebCore::RenderLayerCompositor::detachScrollCoordinatedLayer): Just moved.
+        (WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer): Call detachScrollCoordinatedLayerForRole()
+        if the layer doesn't have the relevant scrolling reasons.
+        * rendering/RenderLayerCompositor.h:
+
 2015-04-14  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebCore/rendering/RenderLayerBacking.cpp

@@ -1571 +1571 @@
 }
 
+void RenderLayerBacking::detachFromScrollingCoordinatorForRole(ScrollingNodeType role)
+{
+    ScrollingNodeID nodeID = scrollingNodeIDForRole(role);
+    if (!nodeID)
+        return;
+
+    ScrollingCoordinator* scrollingCoordinator = scrollingCoordinatorFromLayer(m_owningLayer);
+    if (!scrollingCoordinator)
+        return;
+
+    scrollingCoordinator->detachFromStateTree(nodeID);
+    setScrollingNodeIDForRole(0, role);
+}
+
 GraphicsLayerPaintingPhase RenderLayerBacking::paintingPhaseForPrimaryLayer() const
 {

trunk/Source/WebCore/rendering/RenderLayerBacking.h

@@ -109 +109 @@
 
     void detachFromScrollingCoordinator();
+    void detachFromScrollingCoordinatorForRole(ScrollingNodeType);
     
     ScrollingNodeID scrollingNodeIDForRole(ScrollingNodeType nodeType) const

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -3814 +3814 @@
 }
 
+void RenderLayerCompositor::detachScrollCoordinatedLayerForRole(RenderLayer& layer, ScrollingNodeType role)
+{
+    RenderLayerBacking* backing = layer.backing();
+    if (!backing)
+        return;
+
+    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(role))
+        m_scrollingNodeToLayerMap.remove(nodeID);
+
+    backing->detachFromScrollingCoordinatorForRole(role);
+}
+
+void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer)
+{
+    RenderLayerBacking* backing = layer.backing();
+    if (!backing)
+        return;
+
+    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FrameScrollingNode))
+        m_scrollingNodeToLayerMap.remove(nodeID);
+
+    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FixedNode))
+        m_scrollingNodeToLayerMap.remove(nodeID);
+
+    backing->detachFromScrollingCoordinator();
+}
+
 void RenderLayerCompositor::updateScrollCoordinationForThisFrame(ScrollingNodeID parentNodeID)
 {
@@ -3884 +3911 @@
         
         parentNodeID = nodeID;
-    }
+    } else
+        detachScrollCoordinatedLayerForRole(layer, FixedNode);
 
     if (reasons & Scrolling) {
@@ -3909 +3937 @@
             scrollingCoordinator->updateOverflowScrollingNode(nodeID, backing->scrollingLayer(), backing->scrollingContentsLayer(), &scrollingGeometry);
         }
-    }
-}
-
-void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer)
-{
-    RenderLayerBacking* backing = layer.backing();
-    if (!backing)
-        return;
-
-    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FrameScrollingNode))
-        m_scrollingNodeToLayerMap.remove(nodeID);
-
-    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FixedNode))
-        m_scrollingNodeToLayerMap.remove(nodeID);
-
-    backing->detachFromScrollingCoordinator();
+    } else
+        detachScrollCoordinatedLayerForRole(layer, OverflowScrollingNode);
 }
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.h

@@ -435 +435 @@
     void updateScrollCoordinatedLayer(RenderLayer&, ScrollCoordinationReasons);
     void detachScrollCoordinatedLayer(RenderLayer&);
+    void detachScrollCoordinatedLayerForRole(RenderLayer&, ScrollingNodeType);
     void reattachSubframeScrollLayers();