Title: [241859] trunk
Revision
241859
Author
an...@apple.com
Date
2019-02-20 19:28:11 -0800 (Wed, 20 Feb 2019)

Log Message

Make programmatic frame scrolling work on iOS
https://bugs.webkit.org/show_bug.cgi?id=194886

Reviewed by Simon Fraser.

Source/WebKit:

* UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
(WebKit::ScrollingTreeScrollingNodeDelegateIOS::commitStateAfterChildren):

Don't move based on the layout scroll position. This just overrides the user scroll position.
Remove ScrolledContentsLayer checks, we only need to deal with the ScrollContainerLayer here.

(WebKit::ScrollingTreeScrollingNodeDelegateIOS::setScrollLayerPosition):

Scroll the UIScrollView correctly. This is called as a result of setting the RequestedScrollPosition property.
Remove scroll origin code, it doesn't look correct (and is untested).

LayoutTests:

Test by Frederic Wang.

* fast/scrolling/ios/programmatic-scroll-iframe-expected.html: Added.
* fast/scrolling/ios/programmatic-scroll-iframe.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (241858 => 241859)


--- trunk/LayoutTests/ChangeLog	2019-02-21 02:55:50 UTC (rev 241858)
+++ trunk/LayoutTests/ChangeLog	2019-02-21 03:28:11 UTC (rev 241859)
@@ -1,3 +1,15 @@
+2019-02-20  Antti Koivisto  <an...@apple.com>
+
+        Make programmatic frame scrolling work on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=194886
+
+        Reviewed by Simon Fraser.
+
+        Test by Frederic Wang.
+
+        * fast/scrolling/ios/programmatic-scroll-iframe-expected.html: Added.
+        * fast/scrolling/ios/programmatic-scroll-iframe.html: Added.
+
 2019-02-20  Dean Jackson  <d...@apple.com>
 
         Rotation animations sometimes use the wrong origin (affects apple.com)

Added: trunk/LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe-expected.html (0 => 241859)


--- trunk/LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe-expected.html	2019-02-21 03:28:11 UTC (rev 241859)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Programmatic scrolling of iframe</title>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+  </head>
+  <body>
+    <p>This test passes if you see a green rectangle.</p>
+    <div style="position: absolute; top: 3em; width: 300px; height: 400px; background: green;">
+    </div>
+  </body>
+</html>

Added: trunk/LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe.html (0 => 241859)


--- trunk/LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe.html	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/programmatic-scroll-iframe.html	2019-02-21 03:28:11 UTC (rev 241859)
@@ -0,0 +1,98 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Programmatic scrolling of iframe</title>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+        if (window.internals)
+            internals.settings.setAsyncFrameScrollingEnabled(true);
+
+        function runTest() {
+            // This checks scrolling to the location of the green square.
+            document.getElementById("doNotScroll").contentWindow.window.scrollTo(0, 0);
+            document.getElementById("maxScrollX").contentWindow.window.scrollTo(100, 0);
+            document.getElementById("maxScrollY").contentWindow.window.scrollTo(0, 100);
+            document.getElementById("maxScrollXY").contentWindow.window.scrollTo(100, 100);
+            document.getElementById("scrollMiddle").contentWindow.window.scrollTo(50, 50);
+
+            // This checks scrolling outside the limit of the frame.
+            document.getElementById("scrollBelowXLimit").contentWindow.window.scrollBy(-100, 0);
+            document.getElementById("scrollAboveXLimit").contentWindow.window.scrollBy(200, 0);
+            document.getElementById("scrollBelowYLimit").contentWindow.window.scrollBy(0, -100);
+            document.getElementById("scrollAboveYLimit").contentWindow.window.scrollBy(0, 200);
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+
+        var frameToLoadCount = 9;
+        function newFrameLoaded() {
+            frameToLoadCount--;
+            if (frameToLoadCount == 0)
+                runTest();
+        }
+     </script>
+    <style>
+        iframe {
+            float: left;
+            background: linear-gradient(135deg, red, orange);
+            border: 0;
+            height: 100px;
+            width: 100px;
+            overflow: none;
+        }
+    </style>
+  </head>
+  <body>
+    <p>This test passes if you see a green rectangle.</p>
+    <div style="position: absolute; top: 3em; width: 300px; height: 400px; background: green;">
+        <iframe id="doNotScroll" style="left: 0px; top: 0px;" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='position: absolute; width: 100px; height: 100px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="maxScrollX" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='left: 100px; position: absolute; width: 100px; height: 100px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="maxScrollY" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='top: 100px; position: absolute; width: 100px; height: 100px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="maxScrollXY" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='left: 100px; top: 100px; position: absolute; width: 100px; height: 100px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="scrollMiddle" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='left: 50px; top: 50px; position: absolute; width: 100px; height: 100px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="scrollBelowXLimit" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='position: absolute; width: 200px; height: 200px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="scrollAboveXLimit" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='position: absolute; width: 200px; height: 200px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="scrollBelowYLimit" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='position: absolute; width: 200px; height: 200px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+        <iframe id="scrollAboveYLimit" scrolling="yes" srcdoc="
+            <body style='margin: 0; width: 200px; height: 200px'>
+                <div style='position: absolute; width: 200px; height: 200px; background: green;'></div>
+            </body>" _onload_="newFrameLoaded()">
+        </iframe>
+    </div>
+  </body>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (241858 => 241859)


--- trunk/Source/WebKit/ChangeLog	2019-02-21 02:55:50 UTC (rev 241858)
+++ trunk/Source/WebKit/ChangeLog	2019-02-21 03:28:11 UTC (rev 241859)
@@ -1,3 +1,21 @@
+2019-02-20  Antti Koivisto  <an...@apple.com>
+
+        Make programmatic frame scrolling work on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=194886
+
+        Reviewed by Simon Fraser.
+
+        * UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
+        (WebKit::ScrollingTreeScrollingNodeDelegateIOS::commitStateAfterChildren):
+
+        Don't move based on the layout scroll position. This just overrides the user scroll position.
+        Remove ScrolledContentsLayer checks, we only need to deal with the ScrollContainerLayer here.
+
+        (WebKit::ScrollingTreeScrollingNodeDelegateIOS::setScrollLayerPosition):
+
+        Scroll the UIScrollView correctly. This is called as a result of setting the RequestedScrollPosition property.
+        Remove scroll origin code, it doesn't look correct (and is untested).
+
 2019-02-20  Chris Dumez  <cdu...@apple.com>
 
         [PSON] Make sure hung processes are not kept alive by suspended pages or process caching

Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm (241858 => 241859)


--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm	2019-02-21 02:55:50 UTC (rev 241858)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm	2019-02-21 03:28:11 UTC (rev 241859)
@@ -223,7 +223,6 @@
 {
     SetForScope<bool> updatingChange(m_updatingFromStateNode, true);
     if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer)
-        || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrolledContentsLayer)
         || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize)
         || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ReachableContentsSize)
         || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollPosition)
@@ -232,8 +231,7 @@
         UIScrollView *scrollView = (UIScrollView *)[scrollLayer() delegate];
         ASSERT([scrollView isKindOfClass:[UIScrollView self]]);
 
-        if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer)
-            || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrolledContentsLayer)) {
+        if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollContainerLayer)) {
             if (!m_scrollViewDelegate)
                 m_scrollViewDelegate = adoptNS([[WKScrollingNodeScrollViewDelegate alloc] initWithScrollingTreeNodeDelegate:this]);
 
@@ -247,12 +245,8 @@
             recomputeInsets = true;
         }
 
-        if ((scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollPosition)
-            || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollOrigin))
-            && ![m_scrollViewDelegate _isInUserInteraction]) {
-            scrollView.contentOffset = scrollingStateNode.scrollPosition() + scrollOrigin();
+        if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ScrollOrigin))
             recomputeInsets = true;
-        }
 
         if (recomputeInsets) {
             UIEdgeInsets insets = UIEdgeInsetsMake(0, 0, 0, 0);
@@ -297,7 +291,12 @@
 
 void ScrollingTreeScrollingNodeDelegateIOS::setScrollLayerPosition(const FloatPoint& scrollPosition)
 {
-    [m_scrollLayer setPosition:CGPointMake(scrollPosition.x() + scrollOrigin().x(), scrollPosition.y() + scrollOrigin().y())];
+    BEGIN_BLOCK_OBJC_EXCEPTIONS
+    UIScrollView *scrollView = (UIScrollView *)[scrollLayer() delegate];
+    ASSERT([scrollView isKindOfClass:[UIScrollView self]]);
+    [scrollView setContentOffset:scrollPosition];
+    END_BLOCK_OBJC_EXCEPTIONS
+
     updateChildNodesAfterScroll(scrollPosition);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to