Skip to content

Commit

Permalink
[LayoutNG] Add UnpositionedFloats to ConstraintSpace.
Browse files Browse the repository at this point in the history
Because of http://crbug.com/716930 and http://crrev.com/468470 we don't
place floats before an inline node anymore. This patch fixes that by
adding UnpositionedFloats to ConstraintSpace. So NGInlineLayoutAlgorithm
can position pending floats by itself.

TESTS=4 fixed, 2 broken
1) fast/block/float/canvas-with-floats-marked-for-layout.html is
   broken due to NOTREACHED in PaintController::ProcessNewItem.
   It may be a transient issue but I disabled it anyway.
2) external/wpt/css/CSS2/linebox/inline-formatting-context-009.xht
   It's a ref test. Verified manually, the failure is expected.

BUG=636993

Review-Url: https://codereview.chromium.org/2855333005
Cr-Commit-Position: refs/heads/master@{#469857}
  • Loading branch information
glebl authored and Commit bot committed May 6, 2017
1 parent 4c7e58d commit c9a170c
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 13 deletions.
6 changes: 2 additions & 4 deletions third_party/WebKit/LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,6 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-appl
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-applies-to-006.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-applies-to-007.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-applies-to-008.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-applies-to-009.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-applies-to-012.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-applies-to-015.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/clear-float-002.xht [ Skip ]
Expand Down Expand Up @@ -341,7 +340,6 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/float-repl
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/float-replaced-width-009.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/float-replaced-width-011.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floating-replaced-height-008.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-001.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-002.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-003.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-004.xht [ Skip ]
Expand Down Expand Up @@ -436,7 +434,6 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-wrap-top-
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-wrap-top-below-inline-002l.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-wrap-top-below-inline-002r.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-wrap-top-below-inline-003l.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-wrap-top-below-inline-003r.xht [ Skip ]

### external/wpt/css/CSS2/positioning
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/absolute-non-replaced-height-002.xht [ Skip ]
Expand Down Expand Up @@ -1033,7 +1030,6 @@ crbug.com/635619 virtual/layout_ng/fast/block/float/add-float-back-to-anonymous-
crbug.com/635619 virtual/layout_ng/fast/block/float/add-inline-between-floats-with-preceding-anonymous-box.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/add-inline-to-block-flow-and-ensure-layout-on-containers-of-removed-floats.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/add-inline-to-block-flow-with-block-children-that-do-not-need-anonymous-boxes.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/add-inlines-in-block-children-block.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/assert-when-moving-float.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-2.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/avoid-floats-when-negative-margin-top-3.html [ Skip ]
Expand All @@ -1049,6 +1045,7 @@ crbug.com/635619 virtual/layout_ng/fast/block/float/avoiding-float-centered.html
crbug.com/635619 virtual/layout_ng/fast/block/float/block-with-negative-margin-clears-float.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/br-with-clear-2.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/br-with-clear.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/canvas-with-floats-marked-for-layout.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/centered-float-avoidance-complexity.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/checkbox-and-radio-avoid-floats.html [ Skip ]
crbug.com/635619 virtual/layout_ng/fast/block/float/clamped-right-float.html [ Skip ]
Expand Down Expand Up @@ -1190,6 +1187,7 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatti
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-005.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-006.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-007.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-009.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-010b.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-011.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-012.xht [ Skip ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ NGInlineLayoutAlgorithm::NGInlineLayoutAlgorithm(
Initialize(break_token->ItemIndex(), break_token->TextOffset());
else
Initialize(0, 0);
container_builder_.MutableUnpositionedFloats() = space->UnpositionedFloats();
}

bool NGInlineLayoutAlgorithm::IsFirstLine() const {
Expand Down Expand Up @@ -250,6 +251,8 @@ bool NGInlineLayoutAlgorithm::CreateLineUpToLastBreakOpportunity() {
bfc_offset.block_offset += ConstraintSpace().MarginStrut().Sum();
MaybeUpdateFragmentBfcOffset(ConstraintSpace(), bfc_offset,
&container_builder_);
PositionPendingFloats(bfc_offset.block_offset, &container_builder_,
MutableConstraintSpace());
}

// Create a list of LineItemChunk from |start| and |last_break_opportunity|.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ RefPtr<NGLayoutResult> NGBlockLayoutAlgorithm::Layout() {
container_builder_.SetDirection(constraint_space_->Direction());
container_builder_.SetWritingMode(constraint_space_->WritingMode());
container_builder_.SetSize(size);
container_builder_.MutableUnpositionedFloats() =
constraint_space_->UnpositionedFloats();

NGBlockChildIterator child_iterator(Node()->FirstChild(), BreakToken());
NGBlockChildIterator::Entry entry = child_iterator.NextChild();
Expand Down Expand Up @@ -584,6 +586,11 @@ RefPtr<NGConstraintSpace> NGBlockLayoutAlgorithm::CreateConstraintSpaceForChild(
space_builder_.SetMarginStrut(child->IsFloating() ? NGMarginStrut()
: curr_margin_strut_);

if (!is_new_bfc) {
space_builder_.SetUnpositionedFloats(
container_builder_.MutableUnpositionedFloats());
}

if (child->IsInline()) {
// TODO(kojii): Setup space_builder_ appropriately for inline child.
space_builder_.SetClearanceOffset(ConstraintSpace().ClearanceOffset());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -986,15 +986,18 @@ TEST_F(NGBlockLayoutAlgorithmTest, PositionFloatFragments) {
auto* container_fragment =
ToNGPhysicalBoxFragment(body_fragment->Children()[0].Get());
ASSERT_EQ(1UL, container_fragment->Children().size());
ASSERT_EQ(4UL, container_fragment->PositionedFloats().size());
auto* regular_fragment =
ToNGPhysicalBoxFragment(container_fragment->Children()[0].Get());
ASSERT_EQ(2UL, container_fragment->PositionedFloats().size());
ASSERT_EQ(2UL, regular_fragment->PositionedFloats().size());

// ** Verify layout tree **
Element* left_float = GetDocument().getElementById("left-float");
// 8 = body's margin-top
int left_float_block_offset = 8;
EXPECT_EQ(left_float_block_offset, left_float->OffsetTop());
auto left_float_fragment =
container_fragment->PositionedFloats().at(0)->fragment;
regular_fragment->PositionedFloats().at(0)->fragment;
EXPECT_THAT(LayoutUnit(), left_float_fragment->TopOffset());

Element* left_wide_float = GetDocument().getElementById("left-wide-float");
Expand All @@ -1004,7 +1007,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, PositionFloatFragments) {
int left_wide_float_block_offset = 38;
EXPECT_EQ(left_wide_float_block_offset, left_wide_float->OffsetTop());
auto left_wide_float_fragment =
container_fragment->PositionedFloats().at(1)->fragment;
regular_fragment->PositionedFloats().at(1)->fragment;
// 30 = left-float's height.
EXPECT_THAT(LayoutUnit(30), left_wide_float_fragment->TopOffset());

Expand All @@ -1025,7 +1028,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, PositionFloatFragments) {
EXPECT_EQ(right_float_inline_offset, right_float->OffsetLeft());
EXPECT_EQ(right_float_block_offset, right_float->OffsetTop());
auto right_float_fragment =
container_fragment->PositionedFloats().at(2)->fragment;
container_fragment->PositionedFloats().at(0)->fragment;
// 60 = right_float_block_offset(68) - body's margin(8)
EXPECT_THAT(LayoutUnit(right_float_block_offset - 8),
right_float_fragment->TopOffset());
Expand All @@ -1045,7 +1048,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, PositionFloatFragments) {
EXPECT_EQ(left_float_with_margin_block_offset,
left_float_with_margin->OffsetTop());
auto left_float_with_margin_fragment =
container_fragment->PositionedFloats().at(3)->fragment;
container_fragment->PositionedFloats().at(1)->fragment;
// 70 = left_float_with_margin_block_offset(78) - body's margin(8)
EXPECT_THAT(LayoutUnit(left_float_with_margin_block_offset - 8),
left_float_with_margin_fragment->TopOffset());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ NGConstraintSpace::NGConstraintSpace(
const NGMarginStrut& margin_strut,
const NGLogicalOffset& bfc_offset,
const std::shared_ptr<NGExclusions>& exclusions,
Vector<RefPtr<NGFloatingObject>>& unpositioned_floats,
const WTF::Optional<LayoutUnit>& clearance_offset)
: available_size_(available_size),
percentage_resolution_size_(percentage_resolution_size),
Expand All @@ -50,7 +51,9 @@ NGConstraintSpace::NGConstraintSpace(
bfc_offset_(bfc_offset),
exclusions_(exclusions),
clearance_offset_(clearance_offset),
layout_opp_iter_(nullptr) {}
layout_opp_iter_(nullptr) {
unpositioned_floats_.swap(unpositioned_floats);
}

RefPtr<NGConstraintSpace> NGConstraintSpace::CreateFromLayoutObject(
const LayoutBox& box) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "core/layout/ng/geometry/ng_margin_strut.h"
#include "core/layout/ng/geometry/ng_physical_size.h"
#include "core/layout/ng/ng_exclusion.h"
#include "core/layout/ng/ng_floating_object.h"
#include "core/layout/ng/ng_layout_opportunity_iterator.h"
#include "core/layout/ng/ng_writing_mode.h"
#include "platform/heap/Handle.h"
Expand Down Expand Up @@ -124,6 +125,10 @@ class CORE_EXPORT NGConstraintSpace final

NGLogicalOffset BfcOffset() const { return bfc_offset_; }

Vector<RefPtr<NGFloatingObject>>& UnpositionedFloats() {
return unpositioned_floats_;
}

WTF::Optional<LayoutUnit> ClearanceOffset() const {
return clearance_offset_;
}
Expand All @@ -150,6 +155,7 @@ class CORE_EXPORT NGConstraintSpace final
const NGMarginStrut& margin_strut,
const NGLogicalOffset& bfc_offset,
const std::shared_ptr<NGExclusions>& exclusions,
Vector<RefPtr<NGFloatingObject>>& unpositioned_floats,
const WTF::Optional<LayoutUnit>& clearance_offset);

NGPhysicalSize InitialContainingBlockSize() const {
Expand Down Expand Up @@ -186,6 +192,7 @@ class CORE_EXPORT NGConstraintSpace final
const std::shared_ptr<NGExclusions> exclusions_;
WTF::Optional<LayoutUnit> clearance_offset_;
std::unique_ptr<NGLayoutOpportunityIterator> layout_opp_iter_;
Vector<RefPtr<NGFloatingObject>> unpositioned_floats_;
};

inline std::ostream& operator<<(std::ostream& stream,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ NGConstraintSpaceBuilder& NGConstraintSpaceBuilder::SetIsAnonymous(
return *this;
}

NGConstraintSpaceBuilder& NGConstraintSpaceBuilder::SetUnpositionedFloats(
Vector<RefPtr<NGFloatingObject>>& unpositioned_floats) {
unpositioned_floats_.swap(unpositioned_floats);
return *this;
}

RefPtr<NGConstraintSpace> NGConstraintSpaceBuilder::ToConstraintSpace(
NGWritingMode out_writing_mode) {
// Whether the child and the containing block are parallel to each other.
Expand Down Expand Up @@ -176,6 +182,8 @@ RefPtr<NGConstraintSpace> NGConstraintSpaceBuilder::ToConstraintSpace(
// Reset things that do not pass the Formatting Context boundary.
std::shared_ptr<NGExclusions> exclusions(
is_new_fc_ ? std::make_shared<NGExclusions>() : exclusions_);
if (is_new_fc_)
DCHECK(unpositioned_floats_.IsEmpty());
NGLogicalOffset bfc_offset = is_new_fc_ ? NGLogicalOffset() : bfc_offset_;
NGMarginStrut margin_strut = is_new_fc_ ? NGMarginStrut() : margin_strut_;
WTF::Optional<LayoutUnit> clearance_offset =
Expand All @@ -191,7 +199,8 @@ RefPtr<NGConstraintSpace> NGConstraintSpaceBuilder::ToConstraintSpace(
is_inline_direction_triggers_scrollbar_,
is_block_direction_triggers_scrollbar_,
static_cast<NGFragmentationType>(fragmentation_type_), is_new_fc_,
is_anonymous_, margin_strut, bfc_offset, exclusions, clearance_offset));
is_anonymous_, margin_strut, bfc_offset, exclusions,
unpositioned_floats_, clearance_offset));
}
return AdoptRef(new NGConstraintSpace(
out_writing_mode, static_cast<TextDirection>(text_direction_),
Expand All @@ -201,7 +210,8 @@ RefPtr<NGConstraintSpace> NGConstraintSpaceBuilder::ToConstraintSpace(
is_block_direction_triggers_scrollbar_,
is_inline_direction_triggers_scrollbar_,
static_cast<NGFragmentationType>(fragmentation_type_), is_new_fc_,
is_anonymous_, margin_strut, bfc_offset, exclusions, clearance_offset));
is_anonymous_, margin_strut, bfc_offset, exclusions, unpositioned_floats_,
clearance_offset));
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define NGConstraintSpaceBuilder_h

#include "core/layout/ng/ng_constraint_space.h"
#include "core/layout/ng/ng_floating_object.h"
#include "platform/wtf/Allocator.h"
#include "platform/wtf/Optional.h"

Expand Down Expand Up @@ -47,6 +48,9 @@ class CORE_EXPORT NGConstraintSpaceBuilder final {
NGConstraintSpaceBuilder& SetIsNewFormattingContext(bool is_new_fc);
NGConstraintSpaceBuilder& SetIsAnonymous(bool is_anonymous);

NGConstraintSpaceBuilder& SetUnpositionedFloats(
Vector<RefPtr<NGFloatingObject>>& unpositioned_floats);

NGConstraintSpaceBuilder& SetMarginStrut(const NGMarginStrut& margin_strut);

NGConstraintSpaceBuilder& SetBfcOffset(const NGLogicalOffset& offset);
Expand Down Expand Up @@ -87,6 +91,7 @@ class CORE_EXPORT NGConstraintSpaceBuilder final {
NGLogicalOffset bfc_offset_;
std::shared_ptr<NGExclusions> exclusions_;
WTF::Optional<LayoutUnit> clearance_offset_;
Vector<RefPtr<NGFloatingObject>> unpositioned_floats_;
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "core/layout/ng/geometry/ng_box_strut.h"
#include "core/layout/ng/geometry/ng_logical_size.h"
#include "core/layout/ng/ng_block_node.h"
#include "core/layout/ng/ng_constraint_space.h"
#include "core/layout/ng/ng_exclusion.h"
#include "core/layout/ng/ng_physical_fragment.h"
#include "core/style/ComputedStyle.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "core/layout/ng/ng_space_utils.h"

#include "core/layout/ng/ng_base_layout_algorithm_test.h"
#include "core/layout/ng/ng_block_node.h"
#include "core/style/ComputedStyle.h"

namespace blink {
Expand Down

0 comments on commit c9a170c

Please sign in to comment.