Skip to content

GitLab

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

Fix keyboard scrolling crash 

This crash was occurring because we added an element to the scroll chain
that didn't have a ScrollableArea. This occurs on an iframe because we
promote the iframe to be the effective root scroller (due to it filling
the viewport). However, iframe elements don't have a ScrollableArea so
this would crash when we bubble a logical scroll.

The condition for always adding the root scroller is incorrect. We want
to always add the _global_ root scroller, not the effective. The global
is where the apply scroll callback is registered which is what correctly
does pinch-zoom panning and overscroll glow. The global root scroller is
guaranteed to always have a ScrollableArea.

Bug: 904247
Change-Id: I14080a74ff3aa5a85347076e74edbc53b2fb9cfc
Reviewed-on: https://chromium-review.googlesource.com/c/1340610Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609378}
parent e19076e0
Hide whitespace changes
Inline Side-by-side
Showing with 58 additions and 4 deletions
third_party/blink/renderer/core/exported/web_frame_test.cc
View file @ 8bb6cda9
......@@ -11524,6 +11524,60 @@ TEST_F(WebFrameSimTest, FindInPageSelectNextMatch) {
<< visual_viewport.VisibleRectInDocument().ToString() << "]";
}
// Test bubbling a document (End key) scroll from an inner iframe. This test
// passes if it does not crash. https://crbug.com/904247.
TEST_F(WebFrameSimTest, ScrollToEndBubblingCrash) {
WebView().Resize(WebSize(500, 300));
WebView().GetPage()->GetSettings().SetScrollAnimatorEnabled(false);
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body, html {
width: 100%;
height: 100%;
margin: 0;
}
#frame {
width: 100%;
height: 100%;
border: 0;
}
</style>
<iframe id="frame" srcdoc="
<!DOCTYPE html>
<style>html {height: 300%;}</style>
"></iframe>
)HTML");
Compositor().BeginFrame();
RunPendingTasks();
// Focus the iframe.
WebView().AdvanceFocus(false);
WebKeyboardEvent key_event(WebInputEvent::kRawKeyDown,
WebInputEvent::kNoModifiers,
WebInputEvent::GetStaticTimeStampForTests());
key_event.windows_key_code = VKEY_END;
// Scroll the iframe to the end.
key_event.SetType(WebInputEvent::kRawKeyDown);
WebView().HandleInputEvent(WebCoalescedInputEvent(key_event));
key_event.SetType(WebInputEvent::kKeyUp);
WebView().HandleInputEvent(WebCoalescedInputEvent(key_event));
Compositor().BeginFrame();
// End key should now bubble from the iframe up to the main viewport.
key_event.SetType(WebInputEvent::kRawKeyDown);
WebView().HandleInputEvent(WebCoalescedInputEvent(key_event));
key_event.SetType(WebInputEvent::kKeyUp);
WebView().HandleInputEvent(WebCoalescedInputEvent(key_event));
}
// Basic smoke test of the paint path used by the Android disambiguation popup.
TEST_F(WebFrameSimTest, DisambiguationPopupPixelTest) {
WebView().Resize(WebSize(400, 600));
......
third_party/blink/renderer/core/input/scroll_manager.cc
View file @ 8bb6cda9
......@@ -187,10 +187,10 @@ bool ScrollManager::CanScroll(const ScrollState& scroll_state,
if (!current_node.GetLayoutBox())
return false;
// We need to always add the root scroller in the main frame even if the
// viewport isn't scrollable since we can always pinch-zoom and scroll as
// well as for overscroll effects.
if (current_node.IsEffectiveRootScroller() && frame_->IsMainFrame())
// We need to always add the global root scroller even if it isn't scrollable
// since we can always pinch-zoom and scroll as well as for overscroll
// effects.
if (current_node.GetLayoutBox()->IsGlobalRootScroller())
return true;
ScrollableArea* scrollable_area =
......
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