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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288536 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 24e69f7..88652e8 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+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>
Unreviewed, fix non-unified build after r288458
diff --git a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h
index 343fff0..78bea3b 100644
--- a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h
+++ b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h
@@ -28,6 +28,7 @@
#if ENABLE(ASYNC_SCROLLING)
#include "ScrollingConstraints.h"
+#include "ScrollingTree.h"
#include "ScrollingTreeNode.h"
#include <wtf/RetainPtr.h>
@@ -46,10 +47,10 @@
private:
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;
RetainPtr<CALayer> m_layer;
diff --git a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm
index 36bc5b7..42f1903 100644
--- a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm
+++ b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm
@@ -30,6 +30,7 @@
#import "Logging.h"
#import "ScrollingStateFixedNode.h"
+#import "ScrollingThread.h"
#import "ScrollingTree.h"
#import "ScrollingTreeFrameScrollingNode.h"
#import "ScrollingTreeOverflowScrollProxyNode.h"
@@ -126,6 +127,14 @@
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()];
}
diff --git a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h
index af98798..1c8c0e9 100644
--- a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h
+++ b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.h
@@ -28,6 +28,7 @@
#if ENABLE(ASYNC_SCROLLING)
#include "ScrollingConstraints.h"
+#include "ScrollingTree.h"
#include "ScrollingTreeStickyNode.h"
#include <wtf/RetainPtr.h>
@@ -46,9 +47,9 @@
private:
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;
};
diff --git a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm
index b98fe9f..5dda7f4 100644
--- a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm
+++ b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNodeCocoa.mm
@@ -30,6 +30,7 @@
#import "Logging.h"
#import "ScrollingStateStickyNode.h"
+#import "ScrollingThread.h"
#import "ScrollingTree.h"
#import "WebCoreCALayerExtras.h"
#import <wtf/text/TextStream.h>
@@ -62,6 +63,14 @@
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()];
}
