Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Commit d4b754d

Browse files
author
[email protected]
committed
[Async overflow scrolling] Vertical scrolls over a horizontally scrollable overflow are captured
https://bugs.webkit.org/show_bug.cgi?id=210320 Reviewed by Tim Horton. Source/WebCore: This patch adds somewhat more sophisticated latching logic to the scrolling thread, which fixes the inability to vertically scroll if the mouse is over an enclosed horizontal scroller. ScrollingTree owns a ScrollingTreeLatchingController, which determines when to latch and clear latching. Latch clearing uses a 100ms delay (like main thread latching) so that a starting gesture soon after another gesture, whose initial x and y are small, goes to the same scroller as before. Scrolling tree latching works as follows. When we receive a scroll event which allows use of the latched node, and there is a latched node, send the event directly to that node. Otherwise, hit-test to find the most deeply nested scroll target. Traverse up the parent chain giving each node an opportunity to handle the event. If handled by a node, that node becomes the latched node. ScrollingTree no longer tracks the latched node itself. Tests: scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow.html * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * page/scrolling/ScrollingTree.cpp: (WebCore::ScrollingTree::shouldHandleWheelEventSynchronously): (WebCore::ScrollingTree::handleWheelEvent): (WebCore::ScrollingTree::commitTreeState): (WebCore::ScrollingTree::latchedNodeID const): (WebCore::ScrollingTree::clearLatchedNode): (WebCore::ScrollingTree::mainFrameScrollPosition const): (WebCore::ScrollingTree::scrollingTreeAsText): (WebCore::ScrollingTree::setOrClearLatchedNode): Deleted. (WebCore::ScrollingTree::latchedNode): Deleted. (WebCore::ScrollingTree::setLatchedNode): Deleted. * page/scrolling/ScrollingTree.h: (WebCore::ScrollingTree::hasLatchedNode const): Deleted. * page/scrolling/ScrollingTreeLatchingController.cpp: Added. (WebCore::ScrollingTreeLatchingController::receivedWheelEvent): (WebCore::ScrollingTreeLatchingController::latchedNodeForEvent const): (WebCore::ScrollingTreeLatchingController::latchedNodeID const): (WebCore::ScrollingTreeLatchingController::nodeDidHandleEvent): (WebCore::ScrollingTreeLatchingController::nodeWasRemoved): (WebCore::ScrollingTreeLatchingController::clearLatchedNode): * page/scrolling/ScrollingTreeLatchingController.h: Added. * page/scrolling/ScrollingTreeScrollingNode.cpp: (WebCore::ScrollingTreeScrollingNode::isLatchedNode const): (WebCore::ScrollingTreeScrollingNode::canScrollWithWheelEvent const): (WebCore::ScrollingTreeScrollingNode::scrollLimitReached const): * page/scrolling/ScrollingTreeScrollingNode.h: * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm: (WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent): * page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm: (WebCore::ScrollingTreeOverflowScrollingNodeMac::handleWheelEvent): LayoutTests: Tests for horizontal inside vertical overflow, test that hit-testing inside scrolled-to-limit horizontal scrollers still allows for swipe navigation, and test the latching timeout. * fast/scrolling/latching/iframe-latch-small-deltas-expected.txt: * fast/scrolling/latching/iframe-latch-small-deltas.html: * scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe-expected.txt: Added. * scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html: Added. * scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow-expected.txt: Added. * scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@259872 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 3f0d97d commit d4b754d

21 files changed

+552
-77
lines changed

‎LayoutTests/ChangeLog

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
2020-04-09 Simon Fraser <[email protected]>
2+
3+
[Async overflow scrolling] Vertical scrolls over a horizontally scrollable overflow are captured
4+
https://bugs.webkit.org/show_bug.cgi?id=210320
5+
6+
Reviewed by Tim Horton.
7+
8+
Tests for horizontal inside vertical overflow, test that hit-testing inside scrolled-to-limit horizontal scrollers
9+
still allows for swipe navigation, and test the latching timeout.
10+
11+
* fast/scrolling/latching/iframe-latch-small-deltas-expected.txt:
12+
* fast/scrolling/latching/iframe-latch-small-deltas.html:
13+
* scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe-expected.txt: Added.
14+
* scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html: Added.
15+
* scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow-expected.txt: Added.
16+
* scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow.html: Added.
17+
118
2020-04-10 Simon Fraser <[email protected]>
219

320
Hit-testing a WebTiledBackingLayer with an animation asserts

‎LayoutTests/fast/scrolling/latching/iframe-latch-small-deltas-expected.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ After wait
1313
PASS iframeTarget.contentWindow.pageYOffset is 500
1414
PASS window.pageYOffset is 200
1515
FAIL iframeTarget.contentWindow.pageYOffset should be 480. Was 500.
16-
FAIL window.pageYOffset should be 200. Was 180.
16+
PASS window.pageYOffset is 200
1717
PASS successfullyParsed is true
1818

1919
TEST COMPLETE

‎LayoutTests/fast/scrolling/latching/iframe-latch-small-deltas.html

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
shouldBe('iframeTarget.contentWindow.pageYOffset', '500');
5050
shouldBe('window.pageYOffset', '200');
5151

52-
eventSender.monitorWheelEvents();
5352
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
5453
eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 1, 'changed', 'none');
5554
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 1, 'changed', 'none');
@@ -62,7 +61,7 @@
6261
finishJSTest();
6362
}
6463

65-
function setupTopLevel()
64+
function setupTopLevel()
6665
{
6766
description("Tests that a second scroll with small x/y deltas uses latching from an earlier scroll.");
6867
setTimeout(scrollTest, 0);

‎LayoutTests/scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe-expected.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
Scrolls vertically
2+
Scrolls horizontally
3+
A back swipe, over a scrolled-to-left overflow, should start a navigation gesture.
4+
5+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
6+
7+
8+
Swipe to the left
9+
PASS minXOffset is 0
10+
PASS maxXOffset is -1000
11+
PASS successfullyParsed is true
12+
13+
TEST COMPLETE
14+

‎LayoutTests/scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
2+
<html>
3+
<head>
4+
<style>
5+
body {
6+
height: 2000px;
7+
}
8+
9+
.scroller {
10+
margin: 10px;
11+
width: 500px;
12+
height: 400px;
13+
overflow: auto;
14+
border: 2px solid black;
15+
}
16+
17+
.horizontal .scroller {
18+
width: 400px;
19+
height: 400px;
20+
margin: 10px;
21+
}
22+
23+
.filler {
24+
height: 200px;
25+
width: 100%;
26+
margin: 10px;
27+
background-color: silver;
28+
}
29+
30+
.horizontal .filler {
31+
height: 80%;
32+
width: 200%;
33+
background-color: silver;
34+
}
35+
</style>
36+
<script src="../../../resources/js-test-pre.js"></script>
37+
<script src="../../../resources/ui-helper.js"></script>
38+
39+
<script>
40+
jsTestIsAsync = true;
41+
42+
var minXOffset = 0;
43+
var maxXOffset = -1000;
44+
45+
async function onScrollCompletion(x, y)
46+
{
47+
eventSender.monitorWheelEvents();
48+
eventSender.mouseMoveTo(x, y);
49+
eventSender.mouseScrollByWithWheelAndMomentumPhases(1, 0, "began", "none");
50+
eventSender.mouseScrollByWithWheelAndMomentumPhases(10, 0, "changed", "none");
51+
eventSender.mouseScrollByWithWheelAndMomentumPhases(10, 0, "changed", "none");
52+
eventSender.mouseScrollByWithWheelAndMomentumPhases(10, 0, "changed", "none");
53+
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
54+
return UIHelper.waitForScrollCompletion();
55+
}
56+
57+
async function onSwipeCallback()
58+
{
59+
return new Promise(resolve => {
60+
testRunner.installDidBeginSwipeCallback(resolve);
61+
});
62+
}
63+
64+
async function scrollTest()
65+
{
66+
description('A back swipe, over a scrolled-to-left overflow, should start a navigation gesture.');
67+
68+
if (window.testRunner) {
69+
testRunner.setNavigationGesturesEnabled(true);
70+
}
71+
72+
history.pushState({ name: 'backstate' }, 'back');
73+
74+
window.addEventListener('scroll', () => {
75+
minXOffset = Math.min(minXOffset, window.pageXOffset);
76+
maxXOffset = Math.max(maxXOffset, window.pageXOffset);
77+
}, false);
78+
79+
if (!window.eventSender) {
80+
finishJSTest();
81+
return;
82+
}
83+
84+
const mouseLocationX = 150; // Over the horizontally scrollable overflow.
85+
const mouseLocationY = 300;
86+
87+
debug('Swipe to the left');
88+
89+
await Promise.all([onScrollCompletion(mouseLocationX, mouseLocationY), onSwipeCallback()]);
90+
91+
// Should not have received any scroll events.
92+
shouldBe('minXOffset', '0');
93+
shouldBe('maxXOffset', '-1000');
94+
95+
finishJSTest();
96+
}
97+
98+
window.addEventListener('load', () => {
99+
scrollTest();
100+
}, false);
101+
</script>
102+
</head>
103+
<body>
104+
<div id="outer" class="scroller">
105+
<div class="filler">Scrolls vertically</div>
106+
<div id="inner" class="horizontal scroller">
107+
<div class="filler">Scrolls horizontally</div>
108+
</div>
109+
<div class="filler"></div>
110+
</div>
111+
<div id="console"></div>
112+
<script src="../../../resources/js-test-post.js"></script>
113+
</body>
114+
</html>

‎LayoutTests/scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow-expected.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
Scrolls vertically
2+
Scrolls horizontally
3+
Initial state
4+
PASS outerScroller.scrollTop is 0
5+
PASS innerScroller.scrollTop is 0
6+
After scrolll
7+
PASS outerScroller.scrollTop is 200
8+
PASS innerScroller.scrollTop is 0
9+
PASS successfullyParsed is true
10+
11+
TEST COMPLETE
12+

‎LayoutTests/scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow.html

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
2+
<!DOCTYPE html>
3+
<html>
4+
<head>
5+
<style>
6+
body {
7+
height: 2000px;
8+
}
9+
10+
.scroller {
11+
margin: 10px;
12+
width: 500px;
13+
height: 400px;
14+
overflow: auto;
15+
border: 2px solid black;
16+
}
17+
18+
.horizontal .scroller {
19+
width: 400px;
20+
height: 400px;
21+
margin: 10px;
22+
}
23+
24+
.filler {
25+
height: 200px;
26+
width: 100%;
27+
margin: 10px;
28+
background-color: silver;
29+
}
30+
31+
.horizontal .filler {
32+
height: 80%;
33+
width: 200%;
34+
background-color: silver;
35+
}
36+
</style>
37+
<script src="../../../resources/js-test-pre.js"></script>
38+
<script>
39+
jsTestIsAsync = true;
40+
var outerScroller;
41+
var innerScroller;
42+
43+
function checkForScroll()
44+
{
45+
debug('After scrolll');
46+
shouldBe('outerScroller.scrollTop', '200');
47+
shouldBe('innerScroller.scrollTop', '0');
48+
49+
finishJSTest();
50+
}
51+
52+
function scrollTest()
53+
{
54+
outerScroller = document.getElementById('outer');
55+
innerScroller = document.getElementById('inner');
56+
57+
debug('Initial state');
58+
shouldBe('outerScroller.scrollTop', '0');
59+
shouldBe('innerScroller.scrollTop', '0');
60+
61+
if (!window.eventSender) {
62+
finishJSTest();
63+
return;
64+
}
65+
66+
eventSender.monitorWheelEvents();
67+
eventSender.mouseMoveTo(150, 300); // Over the horizontally scrollable overflow.
68+
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
69+
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "changed", "none");
70+
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "none", "continue");
71+
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "none", "end");
72+
eventSender.callAfterScrollingCompletes(checkForScroll);
73+
}
74+
75+
window.addEventListener('load', () => {
76+
setTimeout(scrollTest, 0);
77+
}, false);
78+
</script>
79+
</head>
80+
<body>
81+
<div id="outer" class="scroller">
82+
<div class="filler">Scrolls vertically</div>
83+
<div id="inner" class="horizontal scroller">
84+
<div class="filler">Scrolls horizontally</div>
85+
</div>
86+
<div class="filler"></div>
87+
</div>
88+
<div id="console"></div>
89+
<script src="../../../resources/js-test-post.js"></script>
90+
</body>
91+
</html>

‎Source/WebCore/ChangeLog

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,60 @@
1+
2020-04-09 Simon Fraser <[email protected]>
2+
3+
[Async overflow scrolling] Vertical scrolls over a horizontally scrollable overflow are captured
4+
https://bugs.webkit.org/show_bug.cgi?id=210320
5+
6+
Reviewed by Tim Horton.
7+
8+
This patch adds somewhat more sophisticated latching logic to the scrolling thread, which
9+
fixes the inability to vertically scroll if the mouse is over an enclosed horizontal scroller.
10+
11+
ScrollingTree owns a ScrollingTreeLatchingController, which determines when to latch and clear
12+
latching. Latch clearing uses a 100ms delay (like main thread latching) so that a starting gesture
13+
soon after another gesture, whose initial x and y are small, goes to the same scroller as before.
14+
15+
Scrolling tree latching works as follows. When we receive a scroll event which allows use of the
16+
latched node, and there is a latched node, send the event directly to that node. Otherwise,
17+
hit-test to find the most deeply nested scroll target. Traverse up the parent chain giving each node
18+
an opportunity to handle the event. If handled by a node, that node becomes the latched node.
19+
20+
ScrollingTree no longer tracks the latched node itself.
21+
22+
Tests: scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html
23+
scrollingcoordinator/mac/latching/horizontal-overflow-in-vertical-overflow.html
24+
25+
* Sources.txt:
26+
* WebCore.xcodeproj/project.pbxproj:
27+
* page/scrolling/ScrollingTree.cpp:
28+
(WebCore::ScrollingTree::shouldHandleWheelEventSynchronously):
29+
(WebCore::ScrollingTree::handleWheelEvent):
30+
(WebCore::ScrollingTree::commitTreeState):
31+
(WebCore::ScrollingTree::latchedNodeID const):
32+
(WebCore::ScrollingTree::clearLatchedNode):
33+
(WebCore::ScrollingTree::mainFrameScrollPosition const):
34+
(WebCore::ScrollingTree::scrollingTreeAsText):
35+
(WebCore::ScrollingTree::setOrClearLatchedNode): Deleted.
36+
(WebCore::ScrollingTree::latchedNode): Deleted.
37+
(WebCore::ScrollingTree::setLatchedNode): Deleted.
38+
* page/scrolling/ScrollingTree.h:
39+
(WebCore::ScrollingTree::hasLatchedNode const): Deleted.
40+
* page/scrolling/ScrollingTreeLatchingController.cpp: Added.
41+
(WebCore::ScrollingTreeLatchingController::receivedWheelEvent):
42+
(WebCore::ScrollingTreeLatchingController::latchedNodeForEvent const):
43+
(WebCore::ScrollingTreeLatchingController::latchedNodeID const):
44+
(WebCore::ScrollingTreeLatchingController::nodeDidHandleEvent):
45+
(WebCore::ScrollingTreeLatchingController::nodeWasRemoved):
46+
(WebCore::ScrollingTreeLatchingController::clearLatchedNode):
47+
* page/scrolling/ScrollingTreeLatchingController.h: Added.
48+
* page/scrolling/ScrollingTreeScrollingNode.cpp:
49+
(WebCore::ScrollingTreeScrollingNode::isLatchedNode const):
50+
(WebCore::ScrollingTreeScrollingNode::canScrollWithWheelEvent const):
51+
(WebCore::ScrollingTreeScrollingNode::scrollLimitReached const):
52+
* page/scrolling/ScrollingTreeScrollingNode.h:
53+
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
54+
(WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent):
55+
* page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm:
56+
(WebCore::ScrollingTreeOverflowScrollingNodeMac::handleWheelEvent):
57+
158
2020-04-10 Simon Fraser <[email protected]>
259

360
Hit-testing a WebTiledBackingLayer with an animation asserts

‎Source/WebCore/Headers.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,7 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS
905905
page/scrolling/ScrollingTree.h
906906
page/scrolling/ScrollingTreeFrameHostingNode.h
907907
page/scrolling/ScrollingTreeFrameScrollingNode.h
908+
page/scrolling/ScrollingTreeLatchingController.h
908909
page/scrolling/ScrollingTreeNode.h
909910
page/scrolling/ScrollingTreeOverflowScrollingNode.h
910911
page/scrolling/ScrollingTreeScrollingNode.h

‎Source/WebCore/Sources.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,6 +1735,7 @@ page/scrolling/ScrollingThread.cpp
17351735
page/scrolling/ScrollingTree.cpp
17361736
page/scrolling/ScrollingTreeFrameHostingNode.cpp
17371737
page/scrolling/ScrollingTreeFrameScrollingNode.cpp
1738+
page/scrolling/ScrollingTreeLatchingController.cpp
17381739
page/scrolling/ScrollingTreeNode.cpp
17391740
page/scrolling/ScrollingTreeOverflowScrollingNode.cpp
17401741
page/scrolling/ScrollingTreeScrollingNode.cpp

0 commit comments

Comments
 (0)