UI-process hit-testing needs to know about containing block relationships
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 19:54:25 +0000 (19:54 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 19:54:25 +0000 (19:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195845
<rdar://problem/48949633>

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/scrolling/ios/overflow-scroll-overlap-5.html

* page/scrolling/ScrollingTree.h:
* page/scrolling/ScrollingTreeScrollingNode.h:
* page/scrolling/cocoa/ScrollingTreePositionedNode.h:
(WebCore::ScrollingTreePositionedNode::layer const):

Source/WebKit:

Test: fast/scrolling/ios/overflow-scroll-overlap-5.html

When an overflow scroller contains a positioned element the element may not be on a descendant layer of the scroller,
yet should move along with it. This needs to be taken into account in UI-side hit testing.

* UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h:
(WebKit::RemoteLayerTreeNode::nonAncestorScrollContainerIDs const):
(WebKit::RemoteLayerTreeNode::addNonAncestorScrollContainerID):
(WebKit::RemoteLayerTreeNode::clearNonAncestorScrollContainerIDs):

Maintain non-ancestor scrolling relationships for layers.

* UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
(WebKit::RemoteScrollingCoordinatorProxy::commitScrollingTreeState):
(WebKit::RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations):
* UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
* UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
(WebKit::isScrolledBy):

Helper to figure out who scrolls who.

(-[UIView _web_findDescendantViewAtPoint:withEvent:]):
* UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:
(WebKit::RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations):

After commit, pull the non-ancestor scrolling relationships from the scrolling tree and update the layer tree.

LayoutTests:

* fast/scrolling/ios/overflow-scroll-overlap-5-expected.txt: Added.
* fast/scrolling/ios/overflow-scroll-overlap-5.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243316 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-5-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-5.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTree.h
Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h
Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp
Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm

index 7bc7e36..fce6680 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-21  Antti Koivisto  <antti@apple.com>
+
+        UI-process hit-testing needs to know about containing block relationships
+        https://bugs.webkit.org/show_bug.cgi?id=195845
+        <rdar://problem/48949633>
+
+        Reviewed by Simon Fraser.
+
+        * fast/scrolling/ios/overflow-scroll-overlap-5-expected.txt: Added.
+        * fast/scrolling/ios/overflow-scroll-overlap-5.html: Added.
+
 2019-03-21  Shawn Roberts  <sroberts@apple.com>
 
         Unreviewed, rebaseline test after failure in 243211.
diff --git a/LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-5-expected.txt b/LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-5-expected.txt
new file mode 100644 (file)
index 0000000..c270b04
--- /dev/null
@@ -0,0 +1,11 @@
+Test that scrollable areas with content that has complex containing block relationship are correctly targeted.
+
+case 1: Scrollable 1 
+case 2: Scrollable 2 
+case 3: Scrollable 3 
+case 4: Scrollable 4 
+case 5: Scrollable 5 
+case 6: Scrollable 7 
+case 7: Scrollable 10 
+case 8: Scrollable 11 
+
diff --git a/LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-5.html b/LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-5.html
new file mode 100644 (file)
index 0000000..f13f9d3
--- /dev/null
@@ -0,0 +1,133 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<script src="../../../resources/ui-helper.js"></script>
+<script src="../resources/overflow-scroll-overlap.js"></script>
+<style>
+.case {
+    width: 200px;
+    height: 200px;
+    display: inline-block;
+    position: relative;
+}
+.scrollcontent {
+    width: 500px;
+    height: 500px;
+    background: green;
+}
+
+.overflowscroll {
+    overflow: scroll;
+    height: 100px;
+    width: 100px;
+    position: absolute;
+    border: 2px solid black;
+}
+.overlapping {
+    position:absolute;
+    left: 25px;
+    top: 25px;
+    width: 100px;
+    height: 100px;
+    background: red;
+}
+.clip {
+    position:absolute;
+    width: 100px;
+    height: 100px;
+    overflow:hidden;
+}
+.large {
+    width: 3000px;
+    height: 150px;
+}
+#log {
+    position:relative;
+    white-space: pre;
+}
+</style>
+</head>
+<body onload="runTest()">
+<p>
+Test that scrollable areas with content that has complex containing block relationship are correctly targeted.
+</p>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent"></div>
+        <div class="overlapping" style="position:absolute;"></div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent"></div>
+        <div class="overlapping" style="top:-475px; position:relative;"></div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent"></div>
+        <div class="overlapping" style="position:absolute;">
+            <div class="overlapping" style="position:static; transform:translate3d(-50px, -5px, 0px)"></div>
+        </div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent"></div>
+        <div class="overlapping" style="top:-475px; position:relative;">
+            <div class="overlapping" style="position:static; transform:translate3d(-50px, -5px, 0px)"></div>
+        </div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="overflowscroll" style="left:25px; top:25px; position:absolute;">
+            <div class="scrollcontent"></div>
+        </div>
+        <div class="scrollcontent"></div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent"></div>
+        <div class="overflowscroll" style="left:60px; top:60px; position:absolute;">
+            <div class="scrollcontent"></div>
+        </div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent" style="position:absolute;"></div>
+        <div class="overflowscroll" style="position:absolute; left:25px;">
+            <div class="scrollcontent"></div>
+            <div class="overlapping" style="position:absolute;">
+                <div class="overlapping" style="position:static; transform:translate3d(-50px, -5px, 0px)"></div>
+            </div>
+        </div>
+    </div>
+</div>
+
+<div class="case">
+    <div class="overflowscroll target">
+        <div class="scrollcontent" style="position:absolute;"></div>
+        <div class="overflowscroll" style="position:absolute; left:60px;">
+            <div class="scrollcontent"></div>
+            <div class="overlapping" style="position:absolute;">
+                <div class="overlapping" style="position:static; transform:translate3d(-50px, -5px, 0px)"></div>
+            </div>
+        </div>
+    </div>
+</div>
+
+<div id=log></div>
+
+</body>
+</html>
index 81a1987..e15116c 100644 (file)
@@ -1,3 +1,18 @@
+2019-03-21  Antti Koivisto  <antti@apple.com>
+
+        UI-process hit-testing needs to know about containing block relationships
+        https://bugs.webkit.org/show_bug.cgi?id=195845
+        <rdar://problem/48949633>
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/scrolling/ios/overflow-scroll-overlap-5.html
+
+        * page/scrolling/ScrollingTree.h:
+        * page/scrolling/ScrollingTreeScrollingNode.h:
+        * page/scrolling/cocoa/ScrollingTreePositionedNode.h:
+        (WebCore::ScrollingTreePositionedNode::layer const):
+
 2019-03-21  Zalan Bujtas  <zalan@apple.com>
 
         [ContentChangeObserver] Cleanup reset state.
index e7202f6..b7be6f2 100644 (file)
@@ -73,7 +73,7 @@ public:
 
     virtual Ref<ScrollingTreeNode> createScrollingTreeNode(ScrollingNodeType, ScrollingNodeID) = 0;
     
-    ScrollingTreeNode* nodeForID(ScrollingNodeID) const;
+    WEBCORE_EXPORT ScrollingTreeNode* nodeForID(ScrollingNodeID) const;
 
     // Called after a scrolling tree node has handled a scroll and updated its layers.
     // Updates FrameView/RenderLayer scrolling state and GraphicsLayers.
index 40c0a88..7f04dea 100644 (file)
@@ -81,6 +81,11 @@ public:
     bool scrollLimitReached(const PlatformWheelEvent&) const;
     ScrollingTreeScrollingNode* scrollingNodeForPoint(LayoutPoint) const override;
 
+#if PLATFORM(COCOA)
+    CALayer *scrollContainerLayer() const { return m_scrollContainerLayer.get(); }
+    CALayer *scrolledContentsLayer() const { return m_scrolledContentsLayer.get(); }
+#endif
+
 protected:
     ScrollingTreeScrollingNode(ScrollingTree&, ScrollingNodeType, ScrollingNodeID);
 
@@ -120,11 +125,6 @@ protected:
 
     bool expectsWheelEventTestTrigger() const { return m_expectsWheelEventTestTrigger; }
 
-#if PLATFORM(COCOA)
-    CALayer *scrollContainerLayer() const { return m_scrollContainerLayer.get(); }
-    CALayer *scrolledContentsLayer() const { return m_scrolledContentsLayer.get(); }
-#endif
-
     LayoutPoint parentToLocalPoint(LayoutPoint) const override;
     LayoutPoint localToContentsPoint(LayoutPoint) const override;
 
index 2f77b7c..1a50660 100644 (file)
@@ -41,6 +41,8 @@ public:
 
     virtual ~ScrollingTreePositionedNode();
 
+    CALayer *layer() const { return m_layer.get(); }
+
 private:
     ScrollingTreePositionedNode(ScrollingTree&, ScrollingNodeID);
 
index df9348d..246f993 100644 (file)
@@ -1,3 +1,38 @@
+2019-03-21  Antti Koivisto  <antti@apple.com>
+
+        UI-process hit-testing needs to know about containing block relationships
+        https://bugs.webkit.org/show_bug.cgi?id=195845
+        <rdar://problem/48949633>
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/scrolling/ios/overflow-scroll-overlap-5.html
+
+        When an overflow scroller contains a positioned element the element may not be on a descendant layer of the scroller,
+        yet should move along with it. This needs to be taken into account in UI-side hit testing.
+
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h:
+        (WebKit::RemoteLayerTreeNode::nonAncestorScrollContainerIDs const):
+        (WebKit::RemoteLayerTreeNode::addNonAncestorScrollContainerID):
+        (WebKit::RemoteLayerTreeNode::clearNonAncestorScrollContainerIDs):
+
+        Maintain non-ancestor scrolling relationships for layers.
+
+        * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
+        (WebKit::RemoteScrollingCoordinatorProxy::commitScrollingTreeState):
+        (WebKit::RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations):
+        * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
+        * UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
+        (WebKit::isScrolledBy):
+
+        Helper to figure out who scrolls who.
+
+        (-[UIView _web_findDescendantViewAtPoint:withEvent:]):
+        * UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:
+        (WebKit::RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations):
+
+        After commit, pull the non-ancestor scrolling relationships from the scrolling tree and update the layer tree.
+
 2019-03-21  Daniel Bates  <dabates@apple.com>
 
         [iOS] Inline -_ensureFormAccessoryView into -formAccessoryView and have -_updateAccessory ensure we have a form accessory
index 5380481..350098b 100644 (file)
@@ -27,6 +27,7 @@
 
 #include <WebCore/GraphicsLayer.h>
 #include <wtf/RetainPtr.h>
+#include <wtf/Vector.h>
 
 OBJC_CLASS CALayer;
 #if PLATFORM(IOS_FAMILY)
@@ -58,6 +59,11 @@ public:
     const WebCore::Region& eventRegion() const { return m_eventRegion; }
     void setEventRegion(const WebCore::Region&);
 
+    // If empty the layer is scrolled by an ancestor scroller.
+    const auto& nonAncestorScrollContainerIDs() const { return m_nonAncestorScrollLayerIDs; }
+    void addNonAncestorScrollContainerID(WebCore::GraphicsLayer::PlatformLayerID layerID) { m_nonAncestorScrollLayerIDs.append(layerID); }
+    void clearNonAncestorScrollContainerIDs() { m_nonAncestorScrollLayerIDs.clear(); }
+
     void detachFromParent();
 
     static WebCore::GraphicsLayer::PlatformLayerID layerID(CALayer *);
@@ -76,6 +82,7 @@ private:
 #endif
 
     WebCore::Region m_eventRegion;
+    Vector<WebCore::GraphicsLayer::PlatformLayerID> m_nonAncestorScrollLayerIDs;
 };
 
 }
index 3f86de0..6441850 100644 (file)
@@ -80,10 +80,9 @@ void RemoteScrollingCoordinatorProxy::commitScrollingTreeState(const RemoteScrol
 {
     m_requestedScrollInfo = &requestedScrollInfo;
 
-    // FIXME: There must be a better idiom for this.
-    std::unique_ptr<ScrollingStateTree> stateTree(const_cast<RemoteScrollingCoordinatorTransaction&>(transaction).scrollingStateTree().release());
+    auto stateTree = WTFMove(const_cast<RemoteScrollingCoordinatorTransaction&>(transaction).scrollingStateTree());
 
-    const RemoteLayerTreeHost* layerTreeHost = this->layerTreeHost();
+    auto* layerTreeHost = this->layerTreeHost();
     if (!layerTreeHost) {
         ASSERT_NOT_REACHED();
         return;
@@ -92,6 +91,8 @@ void RemoteScrollingCoordinatorProxy::commitScrollingTreeState(const RemoteScrol
     connectStateNodeLayers(*stateTree, *layerTreeHost);
     m_scrollingTree->commitTreeState(WTFMove(stateTree));
 
+    establishLayerTreeScrollingRelations(*layerTreeHost);
+
     m_requestedScrollInfo = nullptr;
 }
 
@@ -163,6 +164,11 @@ void RemoteScrollingCoordinatorProxy::connectStateNodeLayers(ScrollingStateTree&
         }
     }
 }
+
+void RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations(const RemoteLayerTreeHost&)
+{
+}
+
 #endif
 
 bool RemoteScrollingCoordinatorProxy::handleWheelEvent(const PlatformWheelEvent& event)
index 62c0238..c07a559 100644 (file)
@@ -30,6 +30,7 @@
 #include "MessageReceiver.h"
 #include "RemoteScrollingCoordinator.h"
 #include "RemoteScrollingTree.h"
+#include <WebCore/GraphicsLayer.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/RefPtr.h>
 
@@ -108,6 +109,8 @@ public:
 
 private:
     void connectStateNodeLayers(WebCore::ScrollingStateTree&, const RemoteLayerTreeHost&);
+    void establishLayerTreeScrollingRelations(const RemoteLayerTreeHost&);
+
 #if ENABLE(CSS_SCROLL_SNAP)
     bool shouldSnapForMainFrameScrolling(WebCore::ScrollEventAxis) const;
     float closestSnapOffsetForMainFrameScrolling(WebCore::ScrollEventAxis, float scrollDestination, float velocity, unsigned& closestIndex) const;
@@ -124,6 +127,7 @@ private:
     unsigned m_currentVerticalSnapPointIndex { 0 };
 #endif
     bool m_propagatesMainFrameScrolls;
+    HashSet<WebCore::GraphicsLayer::PlatformLayerID> m_layersWithNonAncestorScrollingRelations;
 };
 
 } // namespace WebKit
index 28fe543..5aae321 100644 (file)
@@ -65,6 +65,22 @@ static void collectDescendantViewsAtPoint(Vector<UIView *, 16>& viewsAtPoint, UI
     };
 }
 
+static bool isScrolledBy(WKChildScrollView* scrollView, UIView *hitView)
+{
+    auto scrollLayerID = RemoteLayerTreeNode::layerID(scrollView.layer);
+
+    for (UIView *view = hitView; view; view = [view superview]) {
+        if (view == scrollView)
+            return true;
+
+        auto* node = RemoteLayerTreeNode::forCALayer(view.layer);
+        if (node && scrollLayerID && node->nonAncestorScrollContainerIDs().contains(scrollLayerID))
+            return true;
+    }
+
+    return false;
+}
+
 }
 
 @interface UIView (WKHitTesting)
@@ -89,8 +105,7 @@ static void collectDescendantViewsAtPoint(Vector<UIView *, 16>& viewsAtPoint, UI
         }
 
         if ([view isKindOfClass:[WKChildScrollView class]]) {
-            // See if the deepest view hit is actually a child of the scrollview.
-            if ([viewsAtPoint.last() isDescendantOfView:view])
+            if (WebKit::isScrolledBy((WKChildScrollView *)view, viewsAtPoint.last()))
                 return view;
         }
     }
index 4bf4515..62f12ba 100644 (file)
@@ -43,6 +43,8 @@
 #import <WebCore/ScrollSnapOffsetsInfo.h>
 #import <WebCore/ScrollTypes.h>
 #import <WebCore/ScrollingTreeFrameScrollingNode.h>
+#import <WebCore/ScrollingTreeOverflowScrollingNode.h>
+#import <WebCore/ScrollingTreePositionedNode.h>
 #endif
 
 namespace WebKit {
@@ -117,6 +119,32 @@ void RemoteScrollingCoordinatorProxy::scrollingTreeNodeDidEndScroll()
     m_webPageProxy.scrollingNodeScrollDidEndScroll();
 }
 
+void RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations(const RemoteLayerTreeHost& remoteLayerTreeHost)
+{
+    for (auto layerID : m_layersWithNonAncestorScrollingRelations) {
+        if (auto* layerNode = remoteLayerTreeHost.nodeForID(layerID))
+            layerNode->clearNonAncestorScrollContainerIDs();
+    }
+    m_layersWithNonAncestorScrollingRelations.clear();
+
+    // Usually a scroll view scrolls its descendant layers. In some positioning cases it also controls non-descendants.
+    // To do overlap hit testing correctly we tell layers about such relations.
+
+    // FIXME: This doesn't contain ScrollPositioningBehavior::Stationary nodes. They will need to be handled too.
+    //        See https://bugs.webkit.org/show_bug.cgi?id=196100
+    for (auto& overflowAndPositionedNodeIDs : m_scrollingTree->overflowRelatedNodes()) {
+        auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(overflowAndPositionedNodeIDs.key));
+        for (auto positionedNodeID : overflowAndPositionedNodeIDs.value) {
+            auto* positionedNode = downcast<ScrollingTreePositionedNode>(m_scrollingTree->nodeForID(positionedNodeID));
+            auto* positionedLayerNode = RemoteLayerTreeNode::forCALayer(positionedNode->layer());
+
+            positionedLayerNode->addNonAncestorScrollContainerID(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
+
+            m_layersWithNonAncestorScrollingRelations.add(positionedLayerNode->layerID());
+        }
+    }
+}
+
 #if ENABLE(CSS_SCROLL_SNAP)
 void RemoteScrollingCoordinatorProxy::adjustTargetContentOffsetForSnapping(CGSize maxScrollOffsets, CGPoint velocity, CGFloat topInset, CGPoint* targetContentOffset)
 {