Diff
Modified: trunk/LayoutTests/ChangeLog (246155 => 246156)
--- trunk/LayoutTests/ChangeLog 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/LayoutTests/ChangeLog 2019-06-06 16:39:34 UTC (rev 246156)
@@ -1,3 +1,14 @@
+2019-06-06 Antti Koivisto <[email protected]>
+
+ Position fixed is buggy with overflow:auto scrolling inside iframes
+ https://bugs.webkit.org/show_bug.cgi?id=154399
+ <rdar://problem/24742251>
+
+ Reviewed by Frederic Wang and Simon Fraser.
+
+ * scrollingcoordinator/ios/fixed-frame-overflow-swipe-expected.html: Added.
+ * scrollingcoordinator/ios/fixed-frame-overflow-swipe.html: Added.
+
2019-06-06 Antoine Quint <[email protected]>
[Pointer Events] Add support for chorded button interactions
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-frame-overflow-swipe-expected.html (0 => 246156)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-frame-overflow-swipe-expected.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-frame-overflow-swipe-expected.html 2019-06-06 16:39:34 UTC (rev 246156)
@@ -0,0 +1,86 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+ <meta name='viewport' content='initial-scale=1.0'>
+ <style>
+ .scroller {
+ margin: 10px;
+ height: 300px;
+ width: 300px;
+ border: 1px solid black;
+ overflow: scroll;
+ z-index: 0;
+ position: relative;
+ }
+
+ .fixed {
+ position: fixed;
+ top: 0px;
+ width: 200px;
+ height: 200px;
+ background-color: green;
+ }
+ </style>
+ <script src=''></script>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ async function doTest()
+ {
+ if (!window.testRunner)
+ return;
+
+ if (!testRunner.runUIScript)
+ return;
+
+ await UIHelper.ensurePresentationUpdate();
+ frame.contentDocument.querySelector('.scroller').scrollTo(0, 200);
+ await UIHelper.ensurePresentationUpdate();
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', doTest, false);
+ </script>
+</head>
+<body>
+ <iframe id='frame' width=300 height=400 srcdoc="<!DOCTYPE html>
+ <html>
+ <head>
+ <style>
+ .scroller {
+ margin: 10px;
+ height: 300px;
+ width: 300px;
+ border: 1px solid black;
+ overflow: scroll;
+ z-index: 0;
+ position: relative;
+ }
+
+ .fixed {
+ position: fixed;
+ bottom: 0px;
+ right: 0px;
+ width: 200px;
+ height: 200px;
+ background-color: green;
+ }
+ .spacer {
+ height: 1000px;
+ border: 2px solid blue;
+ }
+ </style>
+ </head>
+ <body>
+ <div class='scroller'>
+ <div class='fixed'></div>
+ <div class='spacer'></div>
+ </div>
+ </body>
+ </html>"
+ ></iframe>
+</body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-frame-overflow-swipe.html (0 => 246156)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-frame-overflow-swipe.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-frame-overflow-swipe.html 2019-06-06 16:39:34 UTC (rev 246156)
@@ -0,0 +1,84 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+ <meta name='viewport' content='initial-scale=1.0'>
+ <style>
+ .scroller {
+ margin: 10px;
+ height: 300px;
+ width: 300px;
+ border: 1px solid black;
+ overflow: scroll;
+ z-index: 0;
+ position: relative;
+ }
+
+ .fixed {
+ position: fixed;
+ top: 0px;
+ width: 200px;
+ height: 200px;
+ background-color: green;
+ }
+ </style>
+ <script src=''></script>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ async function doTest()
+ {
+ if (!window.testRunner)
+ return;
+
+ if (!testRunner.runUIScript)
+ return;
+
+ await touchAndDragFromPointToPoint(50, 200, 50, 50);
+ await liftUpAtPoint(50, 200);
+
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', doTest, false);
+ </script>
+</head>
+<body>
+ <iframe width=300 height=400 srcdoc="<!DOCTYPE html>
+ <html>
+ <head>
+ <style>
+ .scroller {
+ margin: 10px;
+ height: 300px;
+ width: 300px;
+ border: 1px solid black;
+ overflow: scroll;
+ z-index: 0;
+ position: relative;
+ }
+
+ .fixed {
+ position: fixed;
+ bottom: 0px;
+ right: 0px;
+ width: 200px;
+ height: 200px;
+ background-color: green;
+ }
+ .spacer {
+ height: 1000px;
+ border: 2px solid blue;
+ }
+ </style>
+ </head>
+ <body>
+ <div class='scroller'>
+ <div class='fixed'></div>
+ <div class='spacer'></div>
+ </div>
+ </body>
+ </html>"
+ ></iframe>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (246155 => 246156)
--- trunk/Source/WebCore/ChangeLog 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebCore/ChangeLog 2019-06-06 16:39:34 UTC (rev 246156)
@@ -1,3 +1,35 @@
+2019-06-06 Antti Koivisto <[email protected]>
+
+ Position fixed is buggy with overflow:auto scrolling inside iframes
+ https://bugs.webkit.org/show_bug.cgi?id=154399
+ <rdar://problem/24742251>
+
+ Reviewed by Frederic Wang and Simon Fraser.
+
+ Test: scrollingcoordinator/ios/fixed-frame-overflow-swipe.html
+
+ After layer tree commit we were calling mainFrameViewportChangedViaDelegatedScrolling (even if viewport did not change)
+ and expecting it to apply UI side scrolling deltas. However optimization prevents it from descending into subframes
+ and we fail to update those properly.
+
+ In reality we only need to to apply scrolling tree positiong after commit if there has been delegated scrolling after the last
+ one. Track this and do full update when needed.
+
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::applyLayerPositionsAfterCommit):
+
+ Add specific function for this. Don't do anything unless needed.
+
+ * page/scrolling/ScrollingTree.h:
+ (WebCore::ScrollingTree::didScrollByDelegatedScrolling):
+
+ Track if there has been any delegated scrolling.
+
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling):
+
+ We can now bail out if nothing changes since we no longer rely on this for post-commit updates.
+
2019-06-06 Zalan Bujtas <[email protected]>
[LFC][IFC] Layout and preferred width computation should both call placeInlineItems().
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (246155 => 246156)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2019-06-06 16:39:34 UTC (rev 246156)
@@ -254,6 +254,16 @@
node->commitStateAfterChildren(*stateNode);
}
+void ScrollingTree::applyLayerPositionsAfterCommit()
+{
+ // Scrolling tree needs to make adjustments only if the UI side positions have changed.
+ if (!m_wasScrolledByDelegatedScrollingSincePreviousCommit)
+ return;
+ m_wasScrolledByDelegatedScrollingSincePreviousCommit = false;
+
+ applyLayerPositions();
+}
+
void ScrollingTree::applyLayerPositions()
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (246155 => 246156)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2019-06-06 16:39:34 UTC (rev 246156)
@@ -71,6 +71,7 @@
WEBCORE_EXPORT virtual void commitTreeState(std::unique_ptr<ScrollingStateTree>);
WEBCORE_EXPORT virtual void applyLayerPositions();
+ WEBCORE_EXPORT void applyLayerPositionsAfterCommit();
virtual Ref<ScrollingTreeNode> createScrollingTreeNode(ScrollingNodeType, ScrollingNodeID) = 0;
@@ -84,9 +85,10 @@
virtual void scrollingTreeNodeRequestsScroll(ScrollingNodeID, const FloatPoint& /*scrollPosition*/, bool /*representsProgrammaticScroll*/) { }
// Delegated scrolling/zooming has caused the viewport to change, so update viewport-constrained layers
- // (but don't cause scroll events to be fired).
- WEBCORE_EXPORT virtual void mainFrameViewportChangedViaDelegatedScrolling(const FloatPoint& scrollPosition, const WebCore::FloatRect& layoutViewport, double scale);
+ WEBCORE_EXPORT void mainFrameViewportChangedViaDelegatedScrolling(const FloatPoint& scrollPosition, const WebCore::FloatRect& layoutViewport, double scale);
+ void didScrollByDelegatedScrolling() { m_wasScrolledByDelegatedScrollingSincePreviousCommit = true; }
+
void notifyRelatedNodesAfterScrollPositionChange(ScrollingTreeScrollingNode& changedNode);
virtual void reportSynchronousScrollingReasonsChanged(MonotonicTime, SynchronousScrollingReasons) { }
@@ -209,6 +211,7 @@
bool m_isHandlingProgrammaticScroll { false };
bool m_scrollingPerformanceLoggingEnabled { false };
bool m_asyncFrameOrOverflowScrollingEnabled { false };
+ bool m_wasScrolledByDelegatedScrollingSincePreviousCommit { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (246155 => 246156)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-06-06 16:39:34 UTC (rev 246156)
@@ -196,10 +196,9 @@
void ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport)
{
- // Even if position and overrideLayoutViewport haven't changed for this node, other nodes may have received new constraint data
- // via a commit, so the call to notifyRelatedNodesAfterScrollPositionChange() is necessary. We could avoid this if we knew that
- // no commits had happened.
bool scrollPositionChanged = !scrollPositionAndLayoutViewportMatch(position, overrideLayoutViewport);
+ if (!scrollPositionChanged)
+ return;
m_currentScrollPosition = adjustedScrollPosition(position, ScrollPositionClamp::None);
updateViewportForCurrentScrollPosition(overrideLayoutViewport);
@@ -207,9 +206,8 @@
repositionRelatedLayers();
scrollingTree().notifyRelatedNodesAfterScrollPositionChange(*this);
-
- if (scrollPositionChanged)
- scrollingTree().scrollingTreeNodeDidScroll(*this);
+ scrollingTree().scrollingTreeNodeDidScroll(*this);
+ scrollingTree().didScrollByDelegatedScrolling();
}
LayoutPoint ScrollingTreeScrollingNode::parentToLocalPoint(LayoutPoint point) const
Modified: trunk/Source/WebKit/ChangeLog (246155 => 246156)
--- trunk/Source/WebKit/ChangeLog 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebKit/ChangeLog 2019-06-06 16:39:34 UTC (rev 246156)
@@ -1,3 +1,25 @@
+2019-06-06 Antti Koivisto <[email protected]>
+
+ Position fixed is buggy with overflow:auto scrolling inside iframes
+ https://bugs.webkit.org/show_bug.cgi?id=154399
+ <rdar://problem/24742251>
+
+ Reviewed by Frederic Wang and Simon Fraser.
+
+ * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
+ (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
+
+ Remove viewportChangedViaDelegatedScrolling call as we were just relying on its side effect of (partially) applying
+ the scrolling tree. Instead call the new applyScrollingTreeLayerPositionsAfterCommit() unconditionally.
+ It only does work if there are local deltas to apply.
+
+ Local deltas will potentially need to be applied in non-fixed cases too and it is hard to reason about the conditions.
+
+ * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
+ (WebKit::RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositionsAfterCommit):
+ (WebKit::RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositions): Deleted.
+ * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
+
2019-06-06 Michael Catanzaro <[email protected]>
[WPE][GTK] Clean up use of initiatingPageID in implementation of webkit_uri_scheme_request_get_web_view()
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm (246155 => 246156)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm 2019-06-06 16:39:34 UTC (rev 246156)
@@ -217,15 +217,7 @@
m_webPageProxy.didCommitLayerTree(layerTreeTransaction);
#if ENABLE(ASYNC_SCROLLING)
- if (m_webPageProxy.scrollingCoordinatorProxy()->hasFixedOrSticky()) {
-#if PLATFORM(IOS_FAMILY)
- // If we got a new layer for a fixed or sticky node, its position from the WebProcess is probably stale. We need to re-run the "viewport" changed logic to udpate it with our UI-side state.
- FloatRect layoutViewport = m_webPageProxy.computeCustomFixedPositionRect(m_webPageProxy.unobscuredContentRect(), m_webPageProxy.unobscuredContentRectRespectingInputViewBounds(), m_webPageProxy.customFixedPositionRect(), m_webPageProxy.displayedContentScale(), FrameView::LayoutViewportConstraint::Unconstrained);
- m_webPageProxy.scrollingCoordinatorProxy()->viewportChangedViaDelegatedScrolling(m_webPageProxy.unobscuredContentRect().location(), layoutViewport, m_webPageProxy.displayedContentScale());
-#else
- m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositions();
-#endif
- }
+ m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositionsAfterCommit();
// Handle requested scroll position updates from the scrolling tree transaction after didCommitLayerTree()
// has updated the view size based on the content size.
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp (246155 => 246156)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp 2019-06-06 16:39:34 UTC (rev 246156)
@@ -192,9 +192,9 @@
m_scrollingTree->mainFrameViewportChangedViaDelegatedScrolling(scrollPosition, layoutViewport, scale);
}
-void RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositions()
+void RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositionsAfterCommit()
{
- m_scrollingTree->applyLayerPositions();
+ m_scrollingTree->applyLayerPositionsAfterCommit();
}
void RemoteScrollingCoordinatorProxy::currentSnapPointIndicesDidChange(WebCore::ScrollingNodeID nodeID, unsigned horizontal, unsigned vertical)
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h (246155 => 246156)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h 2019-06-06 16:34:45 UTC (rev 246155)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h 2019-06-06 16:39:34 UTC (rev 246156)
@@ -61,7 +61,7 @@
// Called externally when native views move around.
void viewportChangedViaDelegatedScrolling(const WebCore::FloatPoint& scrollPosition, const WebCore::FloatRect& layoutViewport, double scale);
- void applyScrollingTreeLayerPositions();
+ void applyScrollingTreeLayerPositionsAfterCommit();
void currentSnapPointIndicesDidChange(WebCore::ScrollingNodeID, unsigned horizontal, unsigned vertical);
