Commit deeb6ca4 authored by Matt Giuca's avatar Matt Giuca Committed by Commit Bot

Revert "Call PaintLayer::UpdateLayerPositionsAfterLayout only from layout roots."

This reverts commit b0caaa35.

Reason for revert: Suspect introduces flake (https://crbug.com/990225).

LayoutSubtreeRootList::RandomRoot is crashing in about 50% of builds since this landed.

Original change's description:
> 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/+/1728717
> Reviewed-by: Stefan Zager <szager@chromium.org>
> Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#683409}

TBR=szager@chromium.org,chrishtr@chromium.org,eae@chromium.org

Change-Id: I50a9d9861dae814d0c564e072df8a1c66a9b5898
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 462719, 970224, 990225
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1732809Reviewed-by: default avatarMatt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683469}
parent 75e9c9f1
...@@ -695,8 +695,6 @@ void LocalFrameView::PerformLayout(bool in_subtree_layout) { ...@@ -695,8 +695,6 @@ 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.
...@@ -766,7 +764,6 @@ void LocalFrameView::UpdateLayout() { ...@@ -766,7 +764,6 @@ 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);
...@@ -779,6 +776,8 @@ void LocalFrameView::UpdateLayout() { ...@@ -779,6 +776,8 @@ 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)
...@@ -874,8 +873,9 @@ void LocalFrameView::UpdateLayout() { ...@@ -874,8 +873,9 @@ void LocalFrameView::UpdateLayout() {
frame_timing_requests_dirty_ = true; frame_timing_requests_dirty_ = true;
if (!in_subtree_layout) // FIXME: Could find the common ancestor layer of all dirty subtrees and
GetLayoutView()->EnclosingLayer()->UpdateLayerPositionsAfterLayout(); // mark from there. crbug.com/462719
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