Changeset 288536 in webkit

Timestamp:

Jan 24, 2022, 8:59:14 PM (5 years ago)

Author:

Simon Fraser

Message:

REGRESSION (r286932): Fixed position elements jiggle sometimes (Twitter, Facebook)
​https://bugs.webkit.org/show_bug.cgi?id=235543
<rdar://87981122>

Reviewed by Tim Horton.

In r286932 we ensure that the scrolling layer's position gets committed in the scrolling
thread to reduce stutters. However, we also have to do the same for fixed and sticky
layers, because they need to committed in synchrony with the scrolling layer.

Also change some 'override' to 'final' and add locking annotations.

• page/scrolling/cocoa/ScrollingTreeFixedNode.h:
• page/scrolling/cocoa/ScrollingTreeFixedNode.mm:

(WebCore::ScrollingTreeFixedNode::applyLayerPositions):

• page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h:
• page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm:

(WebCore::ScrollingTreeStickyNodeCocoa::applyLayerPositions):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/scrolling/cocoa/ScrollingTreeFixedNode.h (modified) (2 diffs)
• page/scrolling/cocoa/ScrollingTreeFixedNode.mm (modified) (2 diffs)
• page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h (modified) (2 diffs)
• page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-24  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r286932): Fixed position elements jiggle sometimes (Twitter, Facebook)
+        https://bugs.webkit.org/show_bug.cgi?id=235543
+        <rdar://87981122>
+
+        Reviewed by Tim Horton.
+
+        In r286932 we ensure that the scrolling layer's position gets committed in the scrolling
+        thread to reduce stutters. However, we also have to do the same for fixed and sticky
+        layers, because they need to committed in synchrony with the scrolling layer.
+
+        Also change some 'override' to 'final' and add locking annotations.
+
+        * page/scrolling/cocoa/ScrollingTreeFixedNode.h:
+        * page/scrolling/cocoa/ScrollingTreeFixedNode.mm:
+        (WebCore::ScrollingTreeFixedNode::applyLayerPositions):
+        * page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h:
+        * page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm:
+        (WebCore::ScrollingTreeStickyNodeCocoa::applyLayerPositions):
+
 2022-01-24  Diego Pino Garcia  <dpino@igalia.com>
 

trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h

@@ -29 +29 @@
 
 #include "ScrollingConstraints.h"
+#include "ScrollingTree.h"
 #include "ScrollingTreeNode.h"
 #include <wtf/RetainPtr.h>
@@ -47 +48 @@
     ScrollingTreeFixedNode(ScrollingTree&, ScrollingNodeID);
 
-    void commitStateBeforeChildren(const ScrollingStateNode&) override;
-    void applyLayerPositions() override;
+    void commitStateBeforeChildren(const ScrollingStateNode&) final;
+    void applyLayerPositions() final WTF_REQUIRES_LOCK(scrollingTree().treeLock());
 
-    void dumpProperties(WTF::TextStream&, OptionSet<ScrollingStateTreeAsTextBehavior>) const override;
+    void dumpProperties(WTF::TextStream&, OptionSet<ScrollingStateTreeAsTextBehavior>) const final;
 
     FixedPositionViewportConstraints m_constraints;

trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm

@@ -31 +31 @@
 #import "Logging.h"
 #import "ScrollingStateFixedNode.h"
+#import "ScrollingThread.h"
 #import "ScrollingTree.h"
 #import "ScrollingTreeFrameScrollingNode.h"
@@ -127 +128 @@
     LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " relatedNodeScrollPositionDidChange: viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " layerPosition " << layerPosition);
 
+#if ENABLE(SCROLLING_THREAD)
+    if (ScrollingThread::isCurrentThread()) {
+        // Match the behavior of ScrollingTreeFrameScrollingNodeMac::repositionScrollingLayers().
+        if (!scrollingTree().isScrollingSynchronizedWithMainThread())
+            [m_layer _web_setLayerTopLeftPosition:CGPointZero];
+    }
+#endif
+
     [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
 }

trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h

@@ -29 +29 @@
 
 #include "ScrollingConstraints.h"
+#include "ScrollingTree.h"
 #include "ScrollingTreeStickyNode.h"
 #include <wtf/RetainPtr.h>
@@ -47 +48 @@
     ScrollingTreeStickyNodeCocoa(ScrollingTree&, ScrollingNodeID);
 
-    void commitStateBeforeChildren(const ScrollingStateNode&) override;
-    void applyLayerPositions() override;
-    FloatPoint layerTopLeft() const override;
+    void commitStateBeforeChildren(const ScrollingStateNode&) final;
+    void applyLayerPositions() final WTF_REQUIRES_LOCK(scrollingTree().treeLock());
+    FloatPoint layerTopLeft() const final;
 
     RetainPtr<CALayer> m_layer;

trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm

@@ -31 +31 @@
 #import "Logging.h"
 #import "ScrollingStateStickyNode.h"
+#import "ScrollingThread.h"
 #import "ScrollingTree.h"
 #import "WebCoreCALayerExtras.h"
@@ -63 +64 @@
     LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeStickyNodeCocoa " << scrollingNodeID() << " constrainingRectAtLastLayout " << m_constraints.constrainingRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " layerPosition " << layerPosition);
 
+#if ENABLE(SCROLLING_THREAD)
+    if (ScrollingThread::isCurrentThread()) {
+        // Match the behavior of ScrollingTreeFrameScrollingNodeMac::repositionScrollingLayers().
+        if (!scrollingTree().isScrollingSynchronizedWithMainThread())
+            [m_layer _web_setLayerTopLeftPosition:CGPointZero];
+    }
+#endif
+
     [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
 }