Commit 35f8ccbb authored by David Bokan's avatar David Bokan Committed by Commit Bot

Re-process RootScroller when LayoutObject is replaced

The RootScroller selection process sets various invalidations and cached
bits on the LayoutObject of the effective and global root scrollers.

These bits are later used to special case properties for the root
scroller. In particular, whether we force generating and compositing a
scroll node for the root scroller.

However, currently, we didn't expect that the layout object might be
replaced without the root scroller being demoted. This can happen when
the layout tree is reattached, as happens when we enter
`document.designMode`. When we get to root scroller selection, the
element is unchanged so we assume there's nothing to be done, which
leaves the layout object in a bad state for paint property building.

The solution is simple, only skip root scroller selection if the element
is unchanged *and* the layout object has the cached bit set.

Bug: 1029129
Change-Id: Iff2845ea06b73349f3e980bffa6b216592cbfa4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1941065Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Auto-Submit: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720330}
parent e3b9df2a
......@@ -175,7 +175,10 @@ void RootScrollerController::RecomputeEffectiveRootScroller() {
}
}
if (effective_root_scroller_ == new_effective_root_scroller)
// Note, the layout object can be replaced during a rebuild. In that case,
// re-run process even if the element itself is the same.
if (effective_root_scroller_ == new_effective_root_scroller &&
effective_root_scroller_->IsEffectiveRootScroller())
return;
Node* old_effective_root_scroller = effective_root_scroller_;
......
......@@ -2087,6 +2087,52 @@ TEST_F(ImplicitRootScrollerSimTest, UseCounterPositiveAfterLoad) {
GetDocument().IsUseCounted(WebFeature::kActivatedImplicitRootScroller));
}
// Test that we correctly recompute the cached bits and thus the root scroller
// properties in the event of a layout tree reattachment which causes the
// LayoutObject to be disposed and replaced with a new one.
TEST_F(ImplicitRootScrollerSimTest, LayoutTreeReplaced) {
WebView().MainFrameWidget()->Resize(WebSize(800, 600));
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<style>
::-webkit-scrollbar {
}
#rootscroller {
width: 100%;
height: 100%;
overflow: auto;
position: absolute;
left: 0;
top: 0;
}
#spacer {
height: 20000px;
width: 10px;
}
</style>
<div id="rootscroller">
<div id="spacer"></div>
</div>
)HTML");
Compositor().BeginFrame();
Element* scroller = GetDocument().getElementById("rootscroller");
ASSERT_EQ(scroller,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
ASSERT_TRUE(scroller->GetLayoutObject()->IsEffectiveRootScroller());
ASSERT_TRUE(scroller->GetLayoutObject()->IsGlobalRootScroller());
// This will cause the layout tree to be rebuilt and reattached which creates
// new LayoutObjects. Ensure the bits are reapplied to the new layout
// objects after they're recreated.
GetDocument().setDesignMode("on");
Compositor().BeginFrame();
EXPECT_TRUE(scroller->GetLayoutObject()->IsEffectiveRootScroller());
EXPECT_TRUE(scroller->GetLayoutObject()->IsGlobalRootScroller());
}
// Tests that if we have multiple valid candidates for implicit promotion, we
// don't promote either.
TEST_F(ImplicitRootScrollerSimTest, DontPromoteWhenMultipleAreValid) {
......
......@@ -139,7 +139,10 @@ void TopDocumentRootScrollerController::UpdateGlobalRootScroller(
if (!viewport_apply_scroll_)
return;
if (new_global_root_scroller == global_root_scroller_)
// Note, the layout object can be replaced during a rebuild. In that case,
// re-run process even if the element itself is the same.
if (new_global_root_scroller == global_root_scroller_ &&
global_root_scroller_->GetLayoutObject()->IsGlobalRootScroller())
return;
ScrollableArea* target_scroller = GetScrollableArea(new_global_root_scroller);
......
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