Skip to content

GitLab

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

[root-scroller] Remove check for layout phase 

This check existed because changing the root scroller used to require
synchronously calling UpdateLayoutAndStyle which would reach up and
dirty layout in an ancestor frame. This violated the lifecycle because
updating the root scroller can happen from the ancestor's layout.

Since we no longer need to recompute style (root scroller used to change
the background style), this is no longer needed. Test added to verify.

For a detailed description of how this could happen, see:
https://crrev.com/6f14d2bf37f56f73d1875b5c15298c70b77c4dac

Bug: 798719
Change-Id: I40cba8d8564d7365b4c7815bb6cfca7a1ad6d76c
Reviewed-on: https://chromium-review.googlesource.com/1044363Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556244}
parent 5bdc6c17
Hide whitespace changes
Inline Side-by-side
Showing with 53 additions and 8 deletions
third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc
View file @ 9dd61251
......@@ -308,14 +308,7 @@ void RootScrollerController::UpdateIFrameGeometryAndLayoutSize(
if (!child_view)
return;
// We can get here as a result of the "post layout resize" on the main frame.
// That happens from inside LocalFrameView::PerformLayout. Calling
// UpdateGeometry on the iframe causes it to layout which calls
// Document::UpdateStyleAndLayout. That tries to recurse up the hierarchy,
// reentering Layout on this Document. Thus, we avoid calling this here if
// we're in layout; it'll get called when this Document finishes laying out.
if (!document_->GetFrame()->View()->IsInPerformLayout())
child_view->UpdateGeometry();
child_view->UpdateGeometry();
if (&EffectiveRootScroller() == frame_owner)
child_view->SetLayoutSize(document_->GetFrame()->View()->GetLayoutSize());
......
third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
View file @ 9dd61251
......@@ -1373,6 +1373,58 @@ TEST_P(RootScrollerSimTest, RootScrollerDoesntAffectVisualViewport) {
EXPECT_EQ(120, frame->DomWindow()->visualViewport()->pageTop());
}
// Tests that we don't crash or violate lifecycle assumptions when we resize
// from within layout.
TEST_P(RootScrollerSimTest, ResizeFromResizeAfterLayout) {
WebView().GetSettings()->SetShrinksViewportContentToFit(true);
WebView().SetDefaultPageScaleLimits(0.25f, 5);
WebView().Resize(WebSize(800, 600));
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Start();
request.Write(R"HTML(
<!DOCTYPE html>
<style>
body, html {
width: 100%;
height: 100%;
margin: 0px;
}
#spacer {
width: 1000px;
height: 1000px;
}
#container {
width: 100%;
height: 100%;
border: 0;
}
</style>
<iframe id="container"
srcdoc="<!DOCTYPE html>
<style>html {height: 300%;}</style>">
</iframe>
)HTML");
Element* container = GetDocument().getElementById("container");
GetDocument().setRootScroller(container);
Compositor().BeginFrame();
RunPendingTasks();
ASSERT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
ASSERT_EQ(IntSize(800, 600), GetDocument().View()->Size());
request.Write(R"HTML(
<div style="width:2000px;height:1000px"></div>
)HTML");
request.Finish();
Compositor().BeginFrame();
ASSERT_EQ(IntSize(2000, 1500), GetDocument().View()->Size());
}
class ImplicitRootScrollerSimTest : public SimTest {
public:
ImplicitRootScrollerSimTest()
......
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