Changeset 241860 in webkit

Timestamp:

Feb 20, 2019 7:32:50 PM (5 years ago)

Author:

Simon Fraser

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -1170 +1170 @@
         }
         
-        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()) {
@@ -1186 +1189 @@
         // 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;