Commit 337a726c authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

Always relayout when an object gets or loses a PaintLayer.

This rids us of some crufty code, and also fixes bugs. The approach was also
broken for multicol, because we started UpdatePaginationRecursive() in the
middle of the tree without looking for a containing flow thread (pagination
layer). This would result in the new layer erroneously not becoming paginated.

Also had to update a unit test, to satisfy its requirement that the style change
won't trigger layout.

BUG=616596

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I50ed61a7174e360259b7b786bab01cf74616fc32
Reviewed-on: https://chromium-review.googlesource.com/542915
Commit-Queue: Morten Stenshorne <mstensho@opera.com>
Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488768}
parent 477a4b0a
...@@ -668,6 +668,7 @@ Bug(none) fast/multicol/composited-with-overflow-in-next-column.html [ Failure ] ...@@ -668,6 +668,7 @@ Bug(none) fast/multicol/composited-with-overflow-in-next-column.html [ Failure ]
Bug(none) fast/multicol/doubly-nested-with-top-padding-crossing-row-boundaries.html [ Failure ] Bug(none) fast/multicol/doubly-nested-with-top-padding-crossing-row-boundaries.html [ Failure ]
Bug(none) fast/multicol/relpos-inside-inline-block.html [ Failure ] Bug(none) fast/multicol/relpos-inside-inline-block.html [ Failure ]
Bug(none) fast/multicol/dynamic/abspos-multicol-with-spanner-becomes-spanner.html [ Failure ] Bug(none) fast/multicol/dynamic/abspos-multicol-with-spanner-becomes-spanner.html [ Failure ]
Bug(none) fast/multicol/dynamic/add-will-change-transform.html [ Failure ]
Bug(none) fast/multicol/dynamic/change-spanner-display.html [ Failure ] Bug(none) fast/multicol/dynamic/change-spanner-display.html [ Failure ]
Bug(none) fast/multicol/dynamic/change-spanner-parent-display.html [ Failure ] Bug(none) fast/multicol/dynamic/change-spanner-parent-display.html [ Failure ]
Bug(none) fast/multicol/dynamic/insert-block-among-text-in-anonymous-wrapper.html [ Failure ] Bug(none) fast/multicol/dynamic/insert-block-among-text-in-anonymous-wrapper.html [ Failure ]
......
<!DOCTYPE html>
<p>There should be a blue square below.</p>
<div style="width:60px; height:60px; background:blue;"></div>
<!DOCTYPE html>
<style>
.multicol {
width: 60px;
columns: 20px;
column-gap: 0;
column-fill: auto;
height: 60px;
}
.square {
width: 20px;
height: 20px;
background: blue;
}
</style>
<p>There should be a blue square below.</p>
<div class="multicol">
<div class="square"></div>
<div class="square"></div>
<div class="square"></div>
<div class="square"></div>
<div id="elm">
<div class="square"></div>
</div>
<div class="square"></div>
<div class="square"></div>
<div class="square"></div>
<div class="square"></div>
</div>
<script>
document.body.offsetTop;
var elm = document.getElementById("elm");
elm.style.willChange = "transform";
</script>
...@@ -88,28 +88,6 @@ LayoutBoxModelObject* FindFirstStickyBetween(LayoutObject* from, ...@@ -88,28 +88,6 @@ LayoutBoxModelObject* FindFirstStickyBetween(LayoutObject* from,
} }
} // namespace } // namespace
class FloatStateForStyleChange {
public:
static void SetWasFloating(LayoutBoxModelObject* box_model_object,
bool was_floating) {
was_floating_ = was_floating;
box_model_object_ = box_model_object;
}
static bool WasFloating(LayoutBoxModelObject* box_model_object) {
DCHECK_EQ(box_model_object, box_model_object_);
return was_floating_;
}
private:
// Used to store state between styleWillChange and styleDidChange
static bool was_floating_;
static LayoutBoxModelObject* box_model_object_;
};
bool FloatStateForStyleChange::was_floating_ = false;
LayoutBoxModelObject* FloatStateForStyleChange::box_model_object_ = nullptr;
// The HashMap for storing continuation pointers. // The HashMap for storing continuation pointers.
// The continuation chain is a singly linked list. As such, the HashMap's value // The continuation chain is a singly linked list. As such, the HashMap's value
// is the next pointer associated with the key. // is the next pointer associated with the key.
...@@ -284,8 +262,6 @@ void LayoutBoxModelObject::StyleWillChange(StyleDifference diff, ...@@ -284,8 +262,6 @@ void LayoutBoxModelObject::StyleWillChange(StyleDifference diff,
.InvalidatePaintIncludingNonCompositingDescendants(); .InvalidatePaintIncludingNonCompositingDescendants();
} }
FloatStateForStyleChange::SetWasFloating(this, IsFloating());
if (HasLayer() && diff.CssClipChanged()) if (HasLayer() && diff.CssClipChanged())
Layer()->ClearClipRects(); Layer()->ClearClipRects();
...@@ -298,8 +274,6 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff, ...@@ -298,8 +274,6 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
bool had_transform_related_property = HasTransformRelatedProperty(); bool had_transform_related_property = HasTransformRelatedProperty();
bool had_layer = HasLayer(); bool had_layer = HasLayer();
bool layer_was_self_painting = had_layer && Layer()->IsSelfPaintingLayer(); bool layer_was_self_painting = had_layer && Layer()->IsSelfPaintingLayer();
bool was_floating_before_style_changed =
FloatStateForStyleChange::WasFloating(this);
bool was_horizontal_writing_mode = IsHorizontalWritingMode(); bool was_horizontal_writing_mode = IsHorizontalWritingMode();
LayoutObject::StyleDidChange(diff, old_style); LayoutObject::StyleDidChange(diff, old_style);
...@@ -329,14 +303,15 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff, ...@@ -329,14 +303,15 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
PaintLayerType type = LayerTypeRequired(); PaintLayerType type = LayerTypeRequired();
if (type != kNoPaintLayer) { if (type != kNoPaintLayer) {
if (!Layer()) { if (!Layer()) {
if (was_floating_before_style_changed && IsFloating()) // In order to update this object properly, we need to lay it out again.
// However, if we have never laid it out, don't mark it for layout. If
// this is a new object, it may not yet have been inserted into the tree,
// and if we mark it for layout then, we risk upsetting the tree
// insertion machinery.
if (EverHadLayout())
SetChildNeedsLayout(); SetChildNeedsLayout();
CreateLayerAfterStyleChange(); CreateLayerAfterStyleChange();
if (Parent() && !NeedsLayout()) {
Layer()->UpdateSize();
// FIXME: We should call a specialized versions of this function.
Layer()->UpdateLayerPositionsAfterLayout();
}
} }
} else if (Layer() && Layer()->Parent()) { } else if (Layer() && Layer()->Parent()) {
PaintLayer* parent_layer = Layer()->Parent(); PaintLayer* parent_layer = Layer()->Parent();
...@@ -348,7 +323,7 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff, ...@@ -348,7 +323,7 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
Layer()->UpdateClipPath(old_style, StyleRef()); Layer()->UpdateClipPath(old_style, StyleRef());
// Calls DestroyLayer() which clears the layer. // Calls DestroyLayer() which clears the layer.
Layer()->RemoveOnlyThisLayerAfterStyleChange(); Layer()->RemoveOnlyThisLayerAfterStyleChange();
if (was_floating_before_style_changed && IsFloating()) if (EverHadLayout())
SetChildNeedsLayout(); SetChildNeedsLayout();
if (had_transform_related_property) { if (had_transform_related_property) {
SetNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation( SetNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation(
......
...@@ -49,6 +49,7 @@ TEST_P(PaintInvalidationTest, RecalcOverflowInvalidatesBackground) { ...@@ -49,6 +49,7 @@ TEST_P(PaintInvalidationTest, RecalcOverflowInvalidatesBackground) {
" margin: 0px;" " margin: 0px;"
" }" " }"
" #container {" " #container {"
" will-change: transform;"
" width: 100%;" " width: 100%;"
" height: 100%;" " height: 100%;"
" }" " }"
......
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