Commit 64b3b76a authored by Rahul Arakeri's avatar Rahul Arakeri Committed by Commit Bot

Fix for a GSB getting sent without a GSE.

This CL fixes an issue where a GSB was getting sent without closing an
existing scroll. There were some workarounds in place since a known bug
(crbug.com/979408) prevented us from using mouseUpAt. Since that bug is
fixed, the API mouseUpAt can now be used to send a GSE.

Bug: 979408
Change-Id: I571c714b5612f2190828ad0d0b60f790a832bcbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2045213Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#741115}
parent 9f8d40e6
...@@ -2674,13 +2674,7 @@ crbug.com/552085 external/wpt/css/css-cascade/important-prop.html [ Failure ] ...@@ -2674,13 +2674,7 @@ crbug.com/552085 external/wpt/css/css-cascade/important-prop.html [ Failure ]
crbug.com/953847 [ Mac ] fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ] crbug.com/953847 [ Mac ] fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ]
crbug.com/953847 [ Mac ] virtual/threaded-prefer-compositing/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ] crbug.com/953847 [ Mac ] virtual/threaded-prefer-compositing/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ]
crbug.com/953847 [ Mac ] virtual/scroll_customization/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ] crbug.com/953847 [ Mac ] virtual/scroll_customization/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ]
# This one too but is covered below in the CRASH expectation. crbug.com/953847 [ Mac ] virtual/compositor_threaded_scrollbar_scrolling/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ]
#crbug.com/953847 [ Mac ] virtual/compositor_threaded_scrollbar_scrolling/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Failure Timeout ]
# These cause a violation of a DCHECK in CC by sending a GestureScrollBegin
# without closing an existing one.
crbug.com/979408 virtual/compositor_threaded_scrollbar_scrolling/fast/scrolling/scrollbars/mouse-autoscrolling-on-scrollbar.html [ Crash Failure Timeout ]
crbug.com/979408 virtual/compositor_threaded_scrollbar_scrolling/fast/scrolling/scrollbars/mouse-scrolling-on-div-scrollbar-thumb.html [ Pass Crash Timeout ]
# Some control characters still not visible # Some control characters still not visible
crbug.com/893490 [ Mac ] external/wpt/css/css-text/white-space/control-chars-001.html [ Failure ] crbug.com/893490 [ Mac ] external/wpt/css/css-text/white-space/control-chars-001.html [ Failure ]
......
...@@ -52,15 +52,13 @@ ...@@ -52,15 +52,13 @@
await waitForCompositorCommit(); await waitForCompositorCommit();
scroller.scrollTop = 0; scroller.scrollTop = 0;
// TODO(arakeri): Split the mousePressOn API calls to mouseDownAt, waitFor
// and mouseUpAt once crbug.com/979408 is fixed.
const down_arrow_x = scrollerRect.right - BUTTON_WIDTH / 2; const down_arrow_x = scrollerRect.right - BUTTON_WIDTH / 2;
const down_arrow_y = scrollerRect.bottom - SCROLL_CORNER - BUTTON_WIDTH / 2; const down_arrow_y = scrollerRect.bottom - SCROLL_CORNER - BUTTON_WIDTH / 2;
await mousePressOn(down_arrow_x, down_arrow_y, PRESS_DURATION);
var err = `Autoscroll down failed (scroller.scrollTop = ${scroller.scrollTop})`;
// Verify that autoscroll happened. await mouseDownAt(down_arrow_x, down_arrow_y);
assert_greater_than(scroller.scrollTop, SCROLL_DELTA, err); await waitUntil(() => { return scroller.scrollTop > SCROLL_DELTA; },
`scroller.scrollTop = ${scroller.scrollTop} never went beyond ${SCROLL_DELTA}`);
await mouseUpAt(down_arrow_x, down_arrow_y);
// Since autoscroll for arrows happens at 800 px per second, verify that the // Since autoscroll for arrows happens at 800 px per second, verify that the
// scrollTop has not reached the end. // scrollTop has not reached the end.
...@@ -68,25 +66,21 @@ ...@@ -68,25 +66,21 @@
await waitForCompositorCommit(); await waitForCompositorCommit();
const current_offset = scroller.scrollTop; const current_offset = scroller.scrollTop;
err = `scroller.scrollTop = ${scroller.scrollTop} current_offset = ${current_offset}`; await conditionHolds(() => { return scroller.scrollTop == current_offset; },
await conditionHolds(() => { return scroller.scrollTop == current_offset; }, err); `scroller.scrollTop = ${scroller.scrollTop} current_offset = ${current_offset}`);
},"Test autoscroll down and autoscroll stop."); },"Test arrow autoscroll down and autoscroll stop.");
promise_test (async () => { promise_test (async () => {
await waitForCompositorCommit(); await waitForCompositorCommit();
scroller.scrollTop = 0; scroller.scrollTop = 0;
// TODO(arakeri): Split the mousePressOn API calls to mouseDownAt, waitFor
// and mouseUpAt once crbug.com/979408 is fixed. In its current state, the
// test will ensure that track autoscroll happens successfully and that the
// autoscroll aborts when thumb reaches the pointer.
const trackscroll_x = scrollerRect.right - BUTTON_WIDTH / 2; const trackscroll_x = scrollerRect.right - BUTTON_WIDTH / 2;
const trackscroll_y = scrollerRect.bottom - SCROLL_CORNER - BUTTON_WIDTH; const trackscroll_y = scrollerRect.bottom - SCROLL_CORNER - BUTTON_WIDTH;
await mousePressOn(trackscroll_x, trackscroll_y, PRESS_DURATION);
var err = `Autoscroll down failed (scroller.scrollTop = ${scroller.scrollTop})`;
// Verify that track autoscroll happened. await mouseDownAt(trackscroll_x, trackscroll_y);
assert_greater_than(scroller.scrollTop, SCROLL_DELTA, err); await waitUntil(() => { return scroller.scrollTop > SCROLL_DELTA; },
`scroller.scrollTop = ${scroller.scrollTop} never went beyond ${SCROLL_DELTA}`);
await mouseUpAt(trackscroll_x, trackscroll_y);
// Verify that the track autoscroll actually stops as expected. Since track // Verify that the track autoscroll actually stops as expected. Since track
// autoscroll in this particular case is 1480 px/sec (i.e 74 * 20), holding the // autoscroll in this particular case is 1480 px/sec (i.e 74 * 20), holding the
...@@ -96,15 +90,14 @@ ...@@ -96,15 +90,14 @@
assert_less_than(scroller.scrollTop, 800, "Track autosroll did not end."); assert_less_than(scroller.scrollTop, 800, "Track autosroll did not end.");
const current_offset = scroller.scrollTop; const current_offset = scroller.scrollTop;
err = `scroller.scrollTop = ${scroller.scrollTop} current_offset = ${current_offset}`; await conditionHolds(() => { return scroller.scrollTop == current_offset; },
await conditionHolds(() => { return scroller.scrollTop == current_offset; }, err); `scroller.scrollTop = ${scroller.scrollTop} current_offset = ${current_offset}`);
},"Test track autoscroll down and autoscroll stop."); },"Test track autoscroll down and autoscroll stop.");
promise_test (async () => { promise_test (async () => {
await waitForCompositorCommit(); await waitForCompositorCommit();
scroller.scrollTop = MAX_SCROLLER_OFFSET; scroller.scrollTop = MAX_SCROLLER_OFFSET;
// Schedule a scroller height increment 500ms out and immediately initiate autoscroll.
const content = document.getElementById("divContent"); const content = document.getElementById("divContent");
const originalDivHeight = content.clientHeight; const originalDivHeight = content.clientHeight;
const extendedDivHeight = originalDivHeight + 500; const extendedDivHeight = originalDivHeight + 500;
...@@ -115,16 +108,17 @@ ...@@ -115,16 +108,17 @@
const down_arrow_x = scrollerRect.right - BUTTON_WIDTH / 2; const down_arrow_x = scrollerRect.right - BUTTON_WIDTH / 2;
const down_arrow_y = scrollerRect.bottom - SCROLL_CORNER - BUTTON_WIDTH / 2; const down_arrow_y = scrollerRect.bottom - SCROLL_CORNER - BUTTON_WIDTH / 2;
// Keep the mouse pressed for 1000ms. Before this call completes, the previously // Keep the mouse pressed. The previously scheduled scroller height increment kicks in
// scheduled scroller height increment kicks in. At this point, the autoscrolling // and at this point, the autoscrolling is expected to take place. This should prove
// is expected to take place. This should prove that scrolling occured *after* the // that scrolling occured *after* the scroller length was extended.
// scroller length was extended (as long as the pointer was kept pressed). await mouseDownAt(down_arrow_x, down_arrow_y);
await mousePressOn(down_arrow_x, down_arrow_y, PRESS_DURATION);
var err = `Infinite autoscroll down failed (scroller.scrollTop = ${scroller.scrollTop})`;
// Verify that autoscroll took place beyond the old bounds. If there is a regression here, // Verify that autoscroll took place beyond the old bounds. If there is a regression here,
// the scroller.scrollTop would've stayed at MAX_SCROLLER_OFFSET. // the scroller.scrollTop would've stayed at MAX_SCROLLER_OFFSET.
assert_greater_than(scroller.scrollTop, MAX_SCROLLER_OFFSET, err); await waitUntil(() => { return scroller.scrollTop > MAX_SCROLLER_OFFSET; },
`Infinite autoscroll down failed (scroller.scrollTop = ${scroller.scrollTop})`);
await mouseUpAt(down_arrow_x, down_arrow_y);
// Reset the scroller dimensions. // Reset the scroller dimensions.
content.setAttribute("style","height:" + originalDivHeight + "px"); content.setAttribute("style","height:" + originalDivHeight + "px");
...@@ -167,10 +161,6 @@ ...@@ -167,10 +161,6 @@
await waitFor(() => { return scroller.scrollTop >= current_scrolltop; }, await waitFor(() => { return scroller.scrollTop >= current_scrolltop; },
`Animation did not restart [scroller.scrollTop = ${scroller.scrollTop}]`); `Animation did not restart [scroller.scrollTop = ${scroller.scrollTop}]`);
// TODO(arakeri): This was supposed to be mouseUpAt (because the mouse should be released at await mouseUpAt(down_arrow_x, down_arrow_y);
// end of the test). However, due to crbug.com/979408, the mouseUpAt API can't be used reliably.
// Once the bug is fixed, the line below needs to be updated. The point of using mouseClickOn
// for now is to ensure that we get a pointerup at the end of the test.
await mouseClickOn(down_arrow_x, down_arrow_y);
},"Test autoscroll play/pause when pointer moves in and out of arrow bounds."); },"Test autoscroll play/pause when pointer moves in and out of arrow bounds.");
</script> </script>
...@@ -65,6 +65,8 @@ window.onload = () => { ...@@ -65,6 +65,8 @@ window.onload = () => {
await mouseMoveTo(x, y); await mouseMoveTo(x, y);
assert_equals(standardDivFast.scrollTop, 0, "Vertical thumb drag beyond the track and back should not cause a scroll."); assert_equals(standardDivFast.scrollTop, 0, "Vertical thumb drag beyond the track and back should not cause a scroll.");
await mouseUpAt(x, y);
}, "Test thumb drags beyond scrollbar track."); }, "Test thumb drags beyond scrollbar track.");
promise_test (async () => { promise_test (async () => {
......
...@@ -53,10 +53,6 @@ window.onload = () => { ...@@ -53,10 +53,6 @@ window.onload = () => {
// Since the horizontal scrolling is essentially the same codepath as vertical, // Since the horizontal scrolling is essentially the same codepath as vertical,
// this need not be tested in the interest of making the test run faster. // this need not be tested in the interest of making the test run faster.
// TODO(sahir.vellani): Performing mouse interactions beyond this point
// without calling resetScrollOffset may lead to test failures on linux.
// (probably due to crbug.com/979408)
}, "Test mouse drags in increments on non-custom composited root scrollbar thumb."); }, "Test mouse drags in increments on non-custom composited root scrollbar thumb.");
} }
</script> </script>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment