REGRESSION (240698): Fixed position banners flicker and move when scrolling on iOS
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Feb 2019 03:32:50 +0000 (03:32 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Feb 2019 03:32:50 +0000 (03:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194889
rdar://problem/47755552

Reviewed by Tim Horton.

After r240698 we could commit scrolling changes for a fixed node where the "viewportRectAtLastLayout" and the layer
position were mismatched; this happened when AsyncScrollingCoordinator::reconcileScrollingState() came back from the UI process
with an unstable update and set a new layoutViewport, then some other layout triggered a compositing tree update. During the tree
update, we'd update the fixed scrolling node with the new viewport, and an old layer position.

Fix by ensuring that we only update the geometry info for a scrolling tree node when we update layer geometry for the corresponding
layer.

Not currently testable.

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateBackingAndHierarchy):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241860 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp

index 7ceda86..26277ea 100644 (file)
@@ -1,3 +1,24 @@
+2019-02-20  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (240698): Fixed position banners flicker and move when scrolling on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=194889
+        rdar://problem/47755552
+
+        Reviewed by Tim Horton.
+        
+        After r240698 we could commit scrolling changes for a fixed node where the "viewportRectAtLastLayout" and the layer
+        position were mismatched; this happened when AsyncScrollingCoordinator::reconcileScrollingState() came back from the UI process
+        with an unstable update and set a new layoutViewport, then some other layout triggered a compositing tree update. During the tree
+        update, we'd update the fixed scrolling node with the new viewport, and an old layer position.
+        
+        Fix by ensuring that we only update the geometry info for a scrolling tree node when we update layer geometry for the corresponding
+        layer.
+
+        Not currently testable.
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateBackingAndHierarchy):
+
 2019-02-20  Dean Jackson  <dino@apple.com>
 
         Rotation animations sometimes use the wrong origin (affects apple.com)
index 6dd8d15..5d079a1 100644 (file)
@@ -1169,8 +1169,11 @@ void RenderLayerCompositor::updateBackingAndHierarchy(RenderLayer& layer, Vector
             layerBacking->updateDebugIndicators(m_showDebugBorders, m_showRepaintCounter);
         }
         
-        if (layerNeedsUpdate || layer.needsCompositingGeometryUpdate() || layer.needsScrollingTreeUpdate())
+        OptionSet<ScrollingNodeChangeFlags> scrollingNodeChanges = { ScrollingNodeChangeFlags::Layer };
+        if (layerNeedsUpdate || layer.needsCompositingGeometryUpdate() || layer.needsScrollingTreeUpdate()) {
             layerBacking->updateGeometry();
+            scrollingNodeChanges.add(ScrollingNodeChangeFlags::LayerGeometry);
+        }
 
         if (auto* reflection = layer.reflectionLayer()) {
             if (auto* reflectionBacking = reflection->backing()) {
@@ -1185,7 +1188,7 @@ void RenderLayerCompositor::updateBackingAndHierarchy(RenderLayer& layer, Vector
 
         // FIXME: do based on dirty flags. Need to do this for changes of geometry, configuration and hierarchy.
         // Need to be careful to do the right thing when a scroll-coordinated layer loses a scroll-coordinated ancestor.
-        stateForDescendants.parentNodeID = updateScrollCoordinationForLayer(layer, scrollingTreeState, layerBacking->coordinatedScrollingRoles(), { ScrollingNodeChangeFlags::Layer, ScrollingNodeChangeFlags::LayerGeometry });
+        stateForDescendants.parentNodeID = updateScrollCoordinationForLayer(layer, scrollingTreeState, layerBacking->coordinatedScrollingRoles(), scrollingNodeChanges);
         stateForDescendants.nextChildIndex = 0;
 
 #if !LOG_DISABLED