- Revision
- 243539
- Author
- simon.fra...@apple.com
- Date
- 2019-03-27 08:22:03 -0700 (Wed, 27 Mar 2019)
Log Message
[iOS WK2] Fixed elements in frames can be misplaced sometimes
https://bugs.webkit.org/show_bug.cgi?id=196290
Reviewed by Frédéric Wang.
Source/WebCore:
In a page containing position:fixed inside an async-scrolling iframe, if the
main page is scrolled down, and you reload, then the fixed element in the iframe can
get misplaced or disappear.
The bug was that the reconcileViewportConstrainedLayerPositions() recursive state node
walk would cross frame boundaries, hitting subframe ScrollingStateFixedNodes with a viewport rect
for the main page.
Fix by giving ScrollingStateTree the responsibility for the recursive tree walk, and
have it bail at at frame boundaries.
Test: scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html
* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions):
* page/scrolling/ScrollingStateFixedNode.cpp:
(WebCore::ScrollingStateFixedNode::reconcileLayerPositionForViewportRect):
* page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::reconcileLayerPositionForViewportRect): Deleted.
* page/scrolling/ScrollingStateNode.h:
(WebCore::ScrollingStateNode::reconcileLayerPositionForViewportRect):
* page/scrolling/ScrollingStateStickyNode.cpp:
(WebCore::ScrollingStateStickyNode::reconcileLayerPositionForViewportRect):
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::reconcileLayerPositionsRecursive):
(WebCore::ScrollingStateTree::reconcileViewportConstrainedLayerPositions):
* page/scrolling/ScrollingStateTree.h:
LayoutTests:
* scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position-expected.txt: Added.
* scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (243538 => 243539)
--- trunk/LayoutTests/ChangeLog 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/LayoutTests/ChangeLog 2019-03-27 15:22:03 UTC (rev 243539)
@@ -1,3 +1,13 @@
+2019-03-26 Simon Fraser <simon.fra...@apple.com>
+
+ [iOS WK2] Fixed elements in frames can be misplaced sometimes
+ https://bugs.webkit.org/show_bug.cgi?id=196290
+
+ Reviewed by Frédéric Wang.
+
+ * scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position-expected.txt: Added.
+ * scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html: Added.
+
2019-03-26 Chris Dumez <cdu...@apple.com>
Add basic layout test coverage for File Picker on iOS
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position-expected.txt (0 => 243539)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position-expected.txt 2019-03-27 15:22:03 UTC (rev 243539)
@@ -0,0 +1,53 @@
+(GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 320.00 2021.00)
+ (children 1
+ (GraphicsLayer
+ (bounds 320.00 2021.00)
+ (contentsOpaque 1)
+ (children 1
+ (GraphicsLayer
+ (position 8.00 313.00)
+ (bounds 304.00 154.00)
+ (drawsContent 1)
+ (children 1
+ (GraphicsLayer
+ (position 2.00 2.00)
+ (children 1
+ (GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 300.00 150.00)
+ (children 1
+ (GraphicsLayer
+ (anchor 0.00 0.00)
+ (children 1
+ (GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 300.00 1016.00)
+ (children 1
+ (GraphicsLayer
+ (bounds 300.00 1016.00)
+ (drawsContent 1)
+ (children 1
+ (GraphicsLayer
+ (position 8.00 10.00)
+ (bounds 200.00 140.00)
+ (contentsOpaque 1)
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+)
+
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html (0 => 243539)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html 2019-03-27 15:22:03 UTC (rev 243539)
@@ -0,0 +1,52 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+ <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
+ <style>
+ body {
+ height: 2000px;
+ }
+ iframe {
+ margin-top: 300px;
+ }
+ </style>
+ <script src=""
+ <script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ }
+
+ async function doTest()
+ {
+ await UIHelper.immediateUnstableScrollTo(0, 100);
+ await UIHelper.ensurePresentationUpdate();
+
+ document.getElementById('layer-tree').textContent = internals.layerTreeAsText(document);
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', doTest, false);
+ </script>
+</head>
+<body>
+<pre id="layer-tree"></pre>
+ <iframe srcdoc="
+ <style>
+ body {
+ height: 1000px;
+ }
+ .fixed {
+ position: fixed;
+ top: 10px;
+ }
+ .box {
+ width: 200px;
+ height: 200px;
+ background-color: blue;
+ }
+ </style>
+ <div id='box' class='fixed box'></div>"
+ ></iframe>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (243538 => 243539)
--- trunk/Source/WebCore/ChangeLog 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/ChangeLog 2019-03-27 15:22:03 UTC (rev 243539)
@@ -1,3 +1,38 @@
+2019-03-26 Simon Fraser <simon.fra...@apple.com>
+
+ [iOS WK2] Fixed elements in frames can be misplaced sometimes
+ https://bugs.webkit.org/show_bug.cgi?id=196290
+
+ Reviewed by Frédéric Wang.
+
+ In a page containing position:fixed inside an async-scrolling iframe, if the
+ main page is scrolled down, and you reload, then the fixed element in the iframe can
+ get misplaced or disappear.
+
+ The bug was that the reconcileViewportConstrainedLayerPositions() recursive state node
+ walk would cross frame boundaries, hitting subframe ScrollingStateFixedNodes with a viewport rect
+ for the main page.
+
+ Fix by giving ScrollingStateTree the responsibility for the recursive tree walk, and
+ have it bail at at frame boundaries.
+
+ Test: scrollingcoordinator/ios/fixed-in-frame-layer-reconcile-layer-position.html
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions):
+ * page/scrolling/ScrollingStateFixedNode.cpp:
+ (WebCore::ScrollingStateFixedNode::reconcileLayerPositionForViewportRect):
+ * page/scrolling/ScrollingStateNode.cpp:
+ (WebCore::ScrollingStateNode::reconcileLayerPositionForViewportRect): Deleted.
+ * page/scrolling/ScrollingStateNode.h:
+ (WebCore::ScrollingStateNode::reconcileLayerPositionForViewportRect):
+ * page/scrolling/ScrollingStateStickyNode.cpp:
+ (WebCore::ScrollingStateStickyNode::reconcileLayerPositionForViewportRect):
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::reconcileLayerPositionsRecursive):
+ (WebCore::ScrollingStateTree::reconcileViewportConstrainedLayerPositions):
+ * page/scrolling/ScrollingStateTree.h:
+
2019-03-27 Philippe Normand <pnorm...@igalia.com>
Build failure with gstreamer 1.12.5 if USE_GSTREAMER_GL is enabled
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-03-27 15:22:03 UTC (rev 243539)
@@ -539,13 +539,9 @@
void AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions(ScrollingNodeID scrollingNodeID, const LayoutRect& viewportRect, ScrollingLayerPositionAction action)
{
- auto* scrollingNode = m_scrollingStateTree->stateNodeForID(scrollingNodeID);
- if (!scrollingNode)
- return;
-
LOG_WITH_STREAM(Scrolling, stream << getCurrentProcessID() << " AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions for viewport rect " << viewportRect << " and node " << scrollingNodeID);
- scrollingNode->reconcileLayerPositionForViewportRect(viewportRect, action);
+ m_scrollingStateTree->reconcileViewportConstrainedLayerPositions(scrollingNodeID, viewportRect, action);
}
void AsyncScrollingCoordinator::ensureRootStateNodeForFrameView(FrameView& frameView)
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateFixedNode.cpp 2019-03-27 15:22:03 UTC (rev 243539)
@@ -77,8 +77,6 @@
void ScrollingStateFixedNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action)
{
- ScrollingStateNode::reconcileLayerPositionForViewportRect(viewportRect, action);
-
FloatPoint position = m_constraints.layerPositionForViewportRect(viewportRect);
if (layer().representsGraphicsLayer()) {
auto* graphicsLayer = static_cast<GraphicsLayer*>(layer());
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2019-03-27 15:22:03 UTC (rev 243539)
@@ -151,15 +151,6 @@
return m_children->find(&childNode);
}
-void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action)
-{
- if (!m_children)
- return;
-
- for (auto& child : *m_children)
- child->reconcileLayerPositionForViewportRect(viewportRect, action);
-}
-
void ScrollingStateNode::setLayer(const LayerRepresentation& layerRepresentation)
{
if (layerRepresentation == m_layer)
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2019-03-27 15:22:03 UTC (rev 243539)
@@ -217,7 +217,7 @@
ChangedProperties changedProperties() const { return m_changedProperties; }
void setChangedProperties(ChangedProperties changedProperties) { m_changedProperties = changedProperties; }
- virtual void reconcileLayerPositionForViewportRect(const LayoutRect& /*viewportRect*/, ScrollingLayerPositionAction);
+ virtual void reconcileLayerPositionForViewportRect(const LayoutRect& /*viewportRect*/, ScrollingLayerPositionAction) { }
const LayerRepresentation& layer() const { return m_layer; }
WEBCORE_EXPORT void setLayer(const LayerRepresentation&);
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp 2019-03-27 15:22:03 UTC (rev 243539)
@@ -77,8 +77,6 @@
void ScrollingStateStickyNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action)
{
- ScrollingStateNode::reconcileLayerPositionForViewportRect(viewportRect, action);
-
FloatPoint position = m_constraints.layerPositionForConstrainingRect(viewportRect);
if (layer().representsGraphicsLayer()) {
auto* graphicsLayer = static_cast<GraphicsLayer*>(layer());
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2019-03-27 15:22:03 UTC (rev 243539)
@@ -364,6 +364,31 @@
return it->value;
}
+void ScrollingStateTree::reconcileLayerPositionsRecursive(ScrollingStateNode& currNode, const LayoutRect& layoutViewport, ScrollingLayerPositionAction action)
+{
+ currNode.reconcileLayerPositionForViewportRect(layoutViewport, action);
+
+ if (!currNode.children())
+ return;
+
+ for (auto& child : *currNode.children()) {
+ // Never need to cross frame boundaries, since viewport rect reconciliation is per frame.
+ if (is<ScrollingStateFrameScrollingNode>(child))
+ continue;
+
+ reconcileLayerPositionsRecursive(*child, layoutViewport, action);
+ }
+}
+
+void ScrollingStateTree::reconcileViewportConstrainedLayerPositions(ScrollingNodeID scrollingNodeID, const LayoutRect& layoutViewport, ScrollingLayerPositionAction action)
+{
+ auto* scrollingNode = stateNodeForID(scrollingNodeID);
+ if (!scrollingNode)
+ return;
+
+ reconcileLayerPositionsRecursive(*scrollingNode, layoutViewport, action);
+}
+
} // namespace WebCore
#ifndef NDEBUG
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (243538 => 243539)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2019-03-27 12:14:07 UTC (rev 243538)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2019-03-27 15:22:03 UTC (rev 243539)
@@ -75,6 +75,8 @@
LayerRepresentation::Type preferredLayerRepresentation() const { return m_preferredLayerRepresentation; }
void setPreferredLayerRepresentation(LayerRepresentation::Type representation) { m_preferredLayerRepresentation = representation; }
+ void reconcileViewportConstrainedLayerPositions(ScrollingNodeID, const LayoutRect& viewportRect, ScrollingLayerPositionAction);
+
private:
void setRootStateNode(Ref<ScrollingStateFrameScrollingNode>&&);
void addNode(ScrollingStateNode&);
@@ -87,9 +89,11 @@
void removeNodeAndAllDescendants(ScrollingStateNode*);
- void recursiveNodeWillBeRemoved(ScrollingStateNode* currNode);
+ void recursiveNodeWillBeRemoved(ScrollingStateNode*);
void willRemoveNode(ScrollingStateNode*);
+ void reconcileLayerPositionsRecursive(ScrollingStateNode&, const LayoutRect& viewportRect, ScrollingLayerPositionAction);
+
AsyncScrollingCoordinator* m_scrollingCoordinator;
// Contains all the nodes we know about (those in the m_rootStateNode tree, and in m_unparentedNodes subtrees).
StateNodeMap m_stateNodeMap;
