Skip to content

GitLab

T
tangled
Commit b2eff4ad authored by David Bokan's avatar David Bokan Committed by Commit Bot
Browse files
Options

Use RootScroller for NonFastScrollableRegions 

NonFastScrollableRegions are set on the FrameView's scrolling contents
layer in order to let the compositor know about scrollers that aren't
composited or need forwarding events to the main thread for some reason.

This logic breaks when a non-composited subscroller is placed inside
the root scroller. The root scroller is always composited and it becomes
the layout/outer viewport in the compositor. This means we never
actually check the frame view's scrolling layer for NFSRs since we stop
walking up the scroll tree once we've scrolled the viewport.

This CL fixes the issue by setting the regions on the root scroller's
scrolling contents layer, rather than the FrameView's. GetScrollableArea
on the root FrameView will return a RootFrameViewport which returns the
layout viewport's scrolling layer.

Bug: 954496
Change-Id: Ic6ee4273fe7d34a97c60c48f00d0ba4892295953
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1580219Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654700}
parent 1fd01016
Hide whitespace changes
Inline Side-by-side
Showing with 111 additions and 1 deletion
third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc
View file @ b2eff4ad
...@@ -184,7 +184,7 @@ void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) { ...@@ -184,7 +184,7 @@ void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) {
SetShouldHandleScrollGestureOnMainThreadRegion( SetShouldHandleScrollGestureOnMainThreadRegion(
main_thread_scrolling_region, main_thread_scrolling_region,
frame_view->LayoutViewport()->LayerForScrolling()); frame_view->GetScrollableArea()->LayerForScrolling());
// Fixed regions will be stored on the visual viewport's scroll layer. This // Fixed regions will be stored on the visual viewport's scroll layer. This
// is because a region for an area that's fixed to the layout viewport // is because a region for an area that's fixed to the layout viewport
...@@ -214,6 +214,8 @@ void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) { ...@@ -214,6 +214,8 @@ void ScrollingCoordinator::UpdateAfterPaint(LocalFrameView* frame_view) {
// TODO(pdr): This also takes over scroll animations if main thread // TODO(pdr): This also takes over scroll animations if main thread
// reasons are present. This needs to be implemented for // reasons are present. This needs to be implemented for
// BlinkGenPropertyTrees. // BlinkGenPropertyTrees.
// TODO(bokan): Does this work for scrollers other than FrameViews? If
// not, this will need to account for root scrollers.
SetShouldUpdateScrollLayerPositionOnMainThread( SetShouldUpdateScrollLayerPositionOnMainThread(
frame, frame_view->GetMainThreadScrollingReasons()); frame, frame_view->GetMainThreadScrollingReasons());
...@@ -716,6 +718,11 @@ void ScrollingCoordinator::SetShouldUpdateScrollLayerPositionOnMainThread( ...@@ -716,6 +718,11 @@ void ScrollingCoordinator::SetShouldUpdateScrollLayerPositionOnMainThread(
GraphicsLayer* visual_viewport_layer = visual_viewport.ScrollLayer(); GraphicsLayer* visual_viewport_layer = visual_viewport.ScrollLayer();
cc::Layer* visual_viewport_scroll_layer = cc::Layer* visual_viewport_scroll_layer =
GraphicsLayerToCcLayer(visual_viewport_layer); GraphicsLayerToCcLayer(visual_viewport_layer);
// TODO(bokan): It would probably make more sense to use the root scroller's
// layer here, but this code is only ever executed in !BGPT mode. With BGPT
// the MainThreadScrollingReasons are already stored on individual
// ScrollNodes. The CompositorAnimation hand-off should probably be
// generalized to work on non-FrameView scrollers though.
ScrollableArea* scrollable_area = frame->View()->LayoutViewport(); ScrollableArea* scrollable_area = frame->View()->LayoutViewport();
GraphicsLayer* layer = scrollable_area->LayerForScrolling(); GraphicsLayer* layer = scrollable_area->LayerForScrolling();
if (cc::Layer* scroll_layer = GraphicsLayerToCcLayer(layer)) { if (cc::Layer* scroll_layer = GraphicsLayerToCcLayer(layer)) {
...@@ -981,6 +988,8 @@ void ScrollingCoordinator::FrameViewRootLayerDidChange( ...@@ -981,6 +988,8 @@ void ScrollingCoordinator::FrameViewRootLayerDidChange(
bool ScrollingCoordinator::FrameScrollerIsDirty( bool ScrollingCoordinator::FrameScrollerIsDirty(
LocalFrameView* frame_view) const { LocalFrameView* frame_view) const {
DCHECK(frame_view); DCHECK(frame_view);
// TODO(bokan): This should probably be checking the root scroller in the
// FrameView, rather than the frame_view.
if (frame_view->FrameIsScrollableDidChange()) if (frame_view->FrameIsScrollableDidChange())
return true; return true;
......
third_party/blink/web_tests/fast/scrolling/non-composited-scroller-in-root-scroller.html 0 → 100644
View file @ b2eff4ad
<!DOCTYPE html>
<script src='../../resources/testharness.js'></script>
<script src='../../resources/testharnessreport.js'></script>
<script src='../../resources/gesture-util.js'></script>
<style>
body, html {
margin: 0;
width: 100%;
height: 100%;
}
iframe {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
border: 0;
}
pre {
/* Prevent test output from interfering with root scroller activation when
* running manually */
position: absolute;
}
</style>
<iframe id="rootscroller" srcdoc="
<!DOCTYPE html>
<style>
body {
height: 2000px;
}
#scroller {
position: absolute;
clip: rect(0px, 1000px, 500px, 0px);
overflow: auto;
height: 500px;
width: 500px;
background-color: salmon;
padding: 20px;
box-sizing: border-box;
text-align: center;
}
</style>
<div id='scroller'>
<p>
This scroller is positioned and has a clip region so should be
non-composited. It is contained in a viewport-sized IFRAME which should
be the root scroller.
</p>
<p>
Attempt to scroll here with wheel or touch. If the colored box scrolls,
the test passes. The IFRAME must not scroll.
</p>
<div style='height: 1600px'></div>
</div>">
</iframe>
<script>
window.onload = async () => {
const iframe = document.getElementById('rootscroller');
const scroller = iframe.contentDocument.getElementById('scroller');
promise_test(async () => {
await waitForCompositorCommit();
if (window.internals) {
assert_equals(window.internals.effectiveRootScroller(document),
iframe,
"IFrame must have been promoted to root scroller " +
"for this test to be effective");
}
assert_equals(iframe.scrollTop, 0, "IFRAME starts off unscrolled.");
assert_equals(scroller.scrollTop, 0, "Scroller starts off unscrolled.");
// Scroll over the colored scrolling DIV. Ensure it's the one that gets
// scrolled.
{
const delta = 1000;
const location = { x: 100, y: 100 };
await smoothScroll(delta,
location.x,
location.y,
GestureSourceType.TOUCH_INPUT,
'down',
SPEED_INSTANT);
// Make sure the scroll above went to the DIV and not the IFRAME.
assert_equals(iframe.contentWindow.scrollY, 0,
"|iframe| must not have scrolled.")
assert_greater_than(scroller.scrollTop, 150,
"|scroller| should have been scrolled.");
}
}, 'NonFastScrollableRegion inside root scroller.');
}
</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