Commit b0caaa35 authored by Chris Harrelson's avatar Chris Harrelson Committed by Commit Bot

Call PaintLayer::UpdateLayerPositionsAfterLayout only from layout roots.

Previously, it always called it from the LayoutView, regardless of what
layout changed.

This will be more efficient in typical cases of non-root layouts. It
may be more inefficient in the following cases
a. Two layout roots are present, of which one is an ancestor of the other
(in this case, the new code will update the overlapping layout tree
portions twice).
b. Cases when LocalFrameview::UpdateLayout recurses, due to NeedsLayout
being true after the first one.

If we consider these cases sufficiently rare, then this CL is ok as-is.
Handling (a) and (b) is possible but is harder to implement.

Bug: 462719,970224
Change-Id: Ie65dc73a70bb9362dfa20ff9ad4070e3945d3c56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728717Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683409}
parent a7acf81c
...@@ -695,6 +695,8 @@ void LocalFrameView::PerformLayout(bool in_subtree_layout) { ...@@ -695,6 +695,8 @@ void LocalFrameView::PerformLayout(bool in_subtree_layout) {
continue; continue;
LayoutFromRootObject(*root); LayoutFromRootObject(*root);
root->PaintingLayer()->UpdateLayerPositionsAfterLayout();
// We need to ensure that we mark up all layoutObjects up to the // We need to ensure that we mark up all layoutObjects up to the
// LayoutView for paint invalidation. This simplifies our code as we // LayoutView for paint invalidation. This simplifies our code as we
// just always do a full tree walk. // just always do a full tree walk.
...@@ -764,6 +766,7 @@ void LocalFrameView::UpdateLayout() { ...@@ -764,6 +766,7 @@ void LocalFrameView::UpdateLayout() {
FontCachePurgePreventer font_cache_purge_preventer; FontCachePurgePreventer font_cache_purge_preventer;
StyleRetainScope style_retain_scope; StyleRetainScope style_retain_scope;
bool in_subtree_layout = IsSubtreeLayout();
{ {
base::AutoReset<bool> change_scheduling_enabled(&layout_scheduling_enabled_, base::AutoReset<bool> change_scheduling_enabled(&layout_scheduling_enabled_,
false); false);
...@@ -776,8 +779,6 @@ void LocalFrameView::UpdateLayout() { ...@@ -776,8 +779,6 @@ void LocalFrameView::UpdateLayout() {
ClearLayoutSubtreeRootsAndMarkContainingBlocks(); ClearLayoutSubtreeRootsAndMarkContainingBlocks();
GetLayoutView()->ClearHitTestCache(); GetLayoutView()->ClearHitTestCache();
bool in_subtree_layout = IsSubtreeLayout();
// TODO(crbug.com/460956): The notion of a single root for layout is no // TODO(crbug.com/460956): The notion of a single root for layout is no
// longer applicable. Remove or update this code. // longer applicable. Remove or update this code.
if (in_subtree_layout) if (in_subtree_layout)
...@@ -873,9 +874,8 @@ void LocalFrameView::UpdateLayout() { ...@@ -873,9 +874,8 @@ void LocalFrameView::UpdateLayout() {
frame_timing_requests_dirty_ = true; frame_timing_requests_dirty_ = true;
// FIXME: Could find the common ancestor layer of all dirty subtrees and if (!in_subtree_layout)
// mark from there. crbug.com/462719 GetLayoutView()->EnclosingLayer()->UpdateLayerPositionsAfterLayout();
GetLayoutView()->EnclosingLayer()->UpdateLayerPositionsAfterLayout();
TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID( TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
TRACE_DISABLED_BY_DEFAULT("blink.debug.layout.trees"), "LayoutTree", this, TRACE_DISABLED_BY_DEFAULT("blink.debug.layout.trees"), "LayoutTree", this,
......
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