Commit 110ced38 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[LayoutNG] Fix legacy initiated abspos invalidation

Bug:

LayoutBlock::LayoutPositionedObject had a static position check whether
child reflow was necessary:
  if (.... ||
       (!IsLayoutNGBlockFlow() &&
        NeedsLayoutDueToStaticPosition(positioned_object))))
    layout_scope.SetChildNeedsLayout(positioned_object);
This check would skip reflow for all NGBlockFlow children.
This is incorrect. If child abspos layout was initiated by Legacy,
we should check static position.

Fix:
There was no way to tell in current code whether child abspos layout
was initiated by legacy.
Added a flag is_legacy_initiated_out_of_flow_layout_.
Flag was added to LayoutBlock per ikilpatrick recommendation.
Flag is set to true in LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout
Flag is set to false in NGOutOfFlowLayout.

With this fix, checkboxes on code review page are correct.

Bug: 863865
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ifc55ac03b76dd1aa2113181681bca1b5a2832fb2
Reviewed-on: https://chromium-review.googlesource.com/1190723
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586791}
parent 15cfbc0a
<!DOCTYPE html>
<title>Static position inside table cell</title>
<link rel="author" title="Aleks Totic" href="atotic@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#abs-non-replaced-width" title="10.3.7 Absolutely positioned, non-replaced elements">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div id="container" style="position:relative;">
<div id="changeMe" style="height:100px;"></div>
<div style="display:table-cell;">
<div style="position:absolute; width:100px; height:100px; background:green;"></div>
<div style="width:100px; height:100px; background:red;"></div>
</div>
</div>
<script>
document.body.offsetTop;
document.querySelector('#changeMe').style.height = "auto";
</script>
......@@ -107,7 +107,8 @@ LayoutBlock::LayoutBlock(ContainerNode* node)
descendants_with_floats_marked_for_layout_(false),
has_positioned_objects_(false),
has_percent_height_descendants_(false),
pagination_state_changed_(false) {
pagination_state_changed_(false),
is_legacy_initiated_out_of_flow_layout_(false) {
// LayoutBlockFlow calls setChildrenInline(true).
// By default, subclasses do not have inline children.
}
......@@ -802,11 +803,20 @@ void LayoutBlock::LayoutPositionedObject(LayoutBox* positioned_object,
return;
}
if (!positioned_object->NormalChildNeedsLayout() &&
(relayout_children || height_available_to_children_changed_ ||
(!IsLayoutNGBlockFlow() &&
NeedsLayoutDueToStaticPosition(positioned_object))))
layout_scope.SetChildNeedsLayout(positioned_object);
if (!positioned_object->NormalChildNeedsLayout()) {
bool update_child_needs_layout =
relayout_children || height_available_to_children_changed_;
if (!update_child_needs_layout) {
if (!positioned_object->IsLayoutNGObject() ||
ToLayoutBlock(positioned_object)
->IsLegacyInitiatedOutOfFlowLayout()) {
update_child_needs_layout |=
NeedsLayoutDueToStaticPosition(positioned_object);
}
}
if (update_child_needs_layout)
layout_scope.SetChildNeedsLayout(positioned_object);
}
LayoutUnit logical_top_estimate;
bool is_paginated = View()->GetLayoutState()->IsPaginated();
......
......@@ -357,6 +357,16 @@ class CORE_EXPORT LayoutBlock : public LayoutBox {
void MarkFixedPositionObjectForLayoutIfNeeded(LayoutObject* child,
SubtreeLayoutScope&);
public:
bool IsLegacyInitiatedOutOfFlowLayout() const {
return is_legacy_initiated_out_of_flow_layout_;
}
void SetIsLegacyInitiatedOutOfFlowLayout(bool b) {
is_legacy_initiated_out_of_flow_layout_ = b;
}
protected:
LayoutUnit MarginIntrinsicLogicalWidthForChild(const LayoutBox& child) const;
LayoutUnit BeforeMarginInLineDirection(LineDirectionMode) const;
......@@ -543,6 +553,10 @@ class CORE_EXPORT LayoutBlock : public LayoutBox {
// to, for all we know.
unsigned pagination_state_changed_ : 1;
// LayoutNG-only: This flag is true if an NG out of flow layout was
// initiated by Legacy positioning code.
unsigned is_legacy_initiated_out_of_flow_layout_ : 1;
// FIXME: This is temporary as we move code that accesses block flow
// member variables out of LayoutBlock and into LayoutBlockFlow.
friend class LayoutBlockFlow;
......
......@@ -271,6 +271,7 @@ void LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout() {
}
scoped_refptr<NGPhysicalFragment> child_fragment = fragment->Children()[0];
DCHECK_EQ(fragment->Children()[0]->GetLayoutObject(), this);
SetIsLegacyInitiatedOutOfFlowLayout(true);
}
} // namespace blink
......@@ -381,7 +381,10 @@ scoped_refptr<NGLayoutResult> NGOutOfFlowLayoutPart::LayoutDescendant(
layout_result = GenerateFragment(descendant.node, container_info,
block_estimate, node_position);
}
if (node.GetLayoutBox()->IsLayoutNGObject()) {
ToLayoutBlock(node.GetLayoutBox())
->SetIsLegacyInitiatedOutOfFlowLayout(false);
}
// Compute logical offset, NGAbsolutePhysicalPosition is calculated relative
// to the padding box so add back the container's borders.
NGBoxStrut inset = node_position.inset.ConvertToLogical(
......
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