Commit 4f89fbec authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

DL: Make "objects needing layout" code be display-lock aware.

This patch changes the way we count how many objects need layout.
Specifically, this accounts for the fact that some elements are locked,
and thus we don't recurse down into them at layout time. This makes
the stat reporting in the performance tab more representative of the
actual layout.

R=fergal@chromium.org, chrishtr@chromium.org

Bug: 999486
Change-Id: Iceb4a2d59566bc11b425a22a851cb32d97c55ed4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899307Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713084}
parent 21d84ed0
......@@ -1543,4 +1543,73 @@ TEST_F(DisplayLockContextRenderingTest,
EXPECT_FALSE(spanner_placeholder_object->CanBeSelectionLeaf());
}
TEST_F(DisplayLockContextRenderingTest, ObjectsNeedingLayoutConsidersLocks) {
SetHtmlInnerHTML(R"HTML(
<div id=a>
<div id=b>
<div id=c></div>
<div id=d></div>
</div>
<div id=e>
<div id=f></div>
<div id=g></div>
</div>
</div>
)HTML");
// Dirty all of the leaf nodes.
auto dirty_all = [this]() {
GetDocument().getElementById("c")->GetLayoutObject()->SetNeedsLayout(
"test");
GetDocument().getElementById("d")->GetLayoutObject()->SetNeedsLayout(
"test");
GetDocument().getElementById("f")->GetLayoutObject()->SetNeedsLayout(
"test");
GetDocument().getElementById("g")->GetLayoutObject()->SetNeedsLayout(
"test");
};
unsigned dirty_count = 0;
unsigned total_count = 0;
bool is_subtree = false;
dirty_all();
GetDocument().View()->CountObjectsNeedingLayout(dirty_count, total_count,
is_subtree);
// 7 divs + body + html + layout view
EXPECT_EQ(dirty_count, 10u);
EXPECT_EQ(total_count, 10u);
GetDocument().getElementById("e")->setAttribute(
html_names::kRendersubtreeAttr, "invisible");
UpdateAllLifecyclePhasesForTest();
// Note that the dirty_all call propagate the dirty bit from the unlocked
// subtree all the way up to the layout view, so everything on the way up is
// dirtied.
dirty_all();
GetDocument().View()->CountObjectsNeedingLayout(dirty_count, total_count,
is_subtree);
// Element with 2 children is locked, and it itself isn't dirty (just the
// children are). So, 10 - 3 = 7
EXPECT_EQ(dirty_count, 7u);
// We still see the locked element, so the total is 8.
EXPECT_EQ(total_count, 8u);
GetDocument().getElementById("a")->setAttribute(
html_names::kRendersubtreeAttr, "invisible");
UpdateAllLifecyclePhasesForTest();
// Note that this dirty_all call is now not propagating the dirty bits at all,
// since they are stopped at the top level div.
dirty_all();
GetDocument().View()->CountObjectsNeedingLayout(dirty_count, total_count,
is_subtree);
// Top level element is locked and the dirty bits were not propagated, so we
// expect 0 dirty elements. The total should be 4 ('a' + body + html + layout
// view);
EXPECT_EQ(dirty_count, 0u);
EXPECT_EQ(total_count, 4u);
}
} // namespace blink
......@@ -23,10 +23,17 @@ void LayoutSubtreeRootList::CountObjectsNeedingLayoutInRoot(
const LayoutObject* object,
unsigned& needs_layout_objects,
unsigned& total_objects) {
for (const LayoutObject* o = object; o; o = o->NextInPreOrder(object)) {
for (const LayoutObject* o = object; o;) {
++total_objects;
if (o->NeedsLayout())
bool display_locked =
o->LayoutBlockedByDisplayLock(DisplayLockLifecycleTarget::kChildren);
if (o->SelfNeedsLayout() || (!display_locked && o->NeedsLayout()))
++needs_layout_objects;
if (display_locked)
o = o->NextInPreOrderAfterChildren(object);
else
o = o->NextInPreOrder(object);
}
}
......
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