Commit d7772802 authored by Daniel Libby's avatar Daniel Libby Committed by Commit Bot

Fix scrollbar targeting for missing LayoutBox

It is possible for the scrollable area that is targeted for a scrollbar
scroll to have a layout box, but the parent hosting iframe does not.
In these cases we should just return a null target since we should not
attempt to scroll something that will not be visible.

This is really hard to repro outside of using eventSender, since
getting code to run between the queuing of a GSB targeted at a
scrollable area and the GSB executing happens as the stack
unwinds, but fixing this will fix the clusterfuzz test case.


R=bokan@chromium.org

Bug: 1140886
Change-Id: I65675ac565bb5d860d2b1fd7209a58ff835d27bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511860
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823415}
parent 129b81b0
...@@ -2623,6 +2623,49 @@ TEST_F(EventHandlerSimTest, ElementTargetedGestureScrollIFrame) { ...@@ -2623,6 +2623,49 @@ TEST_F(EventHandlerSimTest, ElementTargetedGestureScrollIFrame) {
ASSERT_EQ(scrollable_area->ScrollOffsetInt().Height(), delta_y); ASSERT_EQ(scrollable_area->ScrollOffsetInt().Height(), delta_y);
} }
TEST_F(EventHandlerSimTest, ElementTargetedGestureScrollIFrameNoCrash) {
WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600));
SimRequest request_outer("https://example.com/test-outer.html", "text/html");
SimRequest request_inner("https://example.com/test-inner.html", "text/html");
LoadURL("https://example.com/test-outer.html");
request_outer.Complete(R"HTML(
<!DOCTYPE html>
<iframe id="iframe" src="test-inner.html"></iframe>
<div style="height:1000px"></div>
)HTML");
request_inner.Complete(R"HTML(
<!DOCTYPE html>
<div style="height:1000px"></div>
)HTML");
Compositor().BeginFrame();
auto* const iframe =
To<HTMLFrameElementBase>(GetDocument().getElementById("iframe"));
FrameView* child_frame_view =
iframe->GetLayoutEmbeddedContent()->ChildFrameView();
auto* local_child_frame_view = DynamicTo<LocalFrameView>(child_frame_view);
ScrollableArea* scrollable_area = local_child_frame_view->GetScrollableArea();
iframe->style()->setProperty(GetDocument().GetExecutionContext(), "display",
"none", String(), ASSERT_NO_EXCEPTION);
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kTest);
// Target the iframe scrollable area and make sure it scrolls when targeted
// with gestures.
constexpr float delta_y = 100;
WebGestureEvent gesture_scroll_begin{
WebInputEvent::Type::kGestureScrollBegin, WebInputEvent::kNoModifiers,
WebInputEvent::GetStaticTimeStampForTests()};
gesture_scroll_begin.SetFrameScale(1);
gesture_scroll_begin.data.scroll_begin.delta_x_hint = 0;
gesture_scroll_begin.data.scroll_begin.delta_y_hint = -delta_y;
gesture_scroll_begin.data.scroll_begin.scrollable_area_element_id =
scrollable_area->GetScrollElementId().GetStableId();
GetDocument().GetFrame()->GetEventHandler().HandleGestureEvent(
gesture_scroll_begin);
}
TEST_F(EventHandlerSimTest, ElementTargetedGestureScrollViewport) { TEST_F(EventHandlerSimTest, ElementTargetedGestureScrollViewport) {
WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600)); WebView().MainFrameViewWidget()->Resize(gfx::Size(800, 600));
// Set a page scale factor so that the VisualViewport will also scroll. // Set a page scale factor so that the VisualViewport will also scroll.
......
...@@ -1090,6 +1090,11 @@ Node* ScrollManager::NodeTargetForScrollableAreaElementId( ...@@ -1090,6 +1090,11 @@ Node* ScrollManager::NodeTargetForScrollableAreaElementId(
LocalFrame* current_frame = layout_box->GetDocument().GetFrame(); LocalFrame* current_frame = layout_box->GetDocument().GetFrame();
while (current_frame) { while (current_frame) {
HTMLFrameOwnerElement* owner = current_frame->GetDocument()->LocalOwner(); HTMLFrameOwnerElement* owner = current_frame->GetDocument()->LocalOwner();
// If the hosting element has no layout box, don't return it for targeting
// since there's nothing to scroll.
if (!owner->GetLayoutBox())
break;
LocalFrame* owner_frame = LocalFrame* owner_frame =
owner ? owner->GetDocument().GetFrame() : nullptr; owner ? owner->GetDocument().GetFrame() : nullptr;
if (owner_frame == frame_) { if (owner_frame == frame_) {
......
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