Commit 3d8da920 authored by David Grogan's avatar David Grogan Committed by Commit Bot

[LayoutNG] Make LayoutNGFlexibleBox inherit from LayoutBlock

It previously inherited from LayoutNGBlockFlow, which, aside from
violating is-a, prevented inline flex children from being blockified --
LayoutNGBlockFlow::AddChild does not call LayoutBlock::AddChild, which
is where the blockification happens.

Considered inheriting from LayoutFlexibleBox to get better baseline and
intrinsic size support, but those will have to be reimplemented in
LayoutNG anyway.

With this change, IsLayoutNGMixin() is even further insufficient to
check if a LayoutObject is implemented in NG. I modified some of the
checks to include IsLayoutNGFlexibleBox, but am not sure about the
accuracy.

mac-specific css3/flexbox/button-expected.png is because the underline
in the link is 1 pixel shorter.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I42811edeb53e8e2a80a64d5b30bf6627e0957b7d
Bug: 845235
Reviewed-on: https://chromium-review.googlesource.com/1062499
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561661}
parent 65122ec2
...@@ -371,6 +371,8 @@ blink_core_sources("layout") { ...@@ -371,6 +371,8 @@ blink_core_sources("layout") {
"ng/inline/ng_text_fragment_builder.h", "ng/inline/ng_text_fragment_builder.h",
"ng/layout_ng_block_flow.cc", "ng/layout_ng_block_flow.cc",
"ng/layout_ng_block_flow.h", "ng/layout_ng_block_flow.h",
"ng/layout_ng_flexible_box.cc",
"ng/layout_ng_flexible_box.h",
"ng/layout_ng_mixin.cc", "ng/layout_ng_mixin.cc",
"ng/layout_ng_mixin.h", "ng/layout_ng_mixin.h",
"ng/layout_ng_table_caption.cc", "ng/layout_ng_table_caption.cc",
......
...@@ -1492,6 +1492,8 @@ void LayoutFlexibleBox::LayoutLineItems(FlexLine* current_line, ...@@ -1492,6 +1492,8 @@ void LayoutFlexibleBox::LayoutLineItems(FlexLine* current_line,
// computeInnerFlexBaseSizeForChild. // computeInnerFlexBaseSizeForChild.
bool force_child_relayout = bool force_child_relayout =
relayout_children && !relaid_out_children_.Contains(child); relayout_children && !relaid_out_children_.Contains(child);
// TODO(dgrogan): Broaden the NG part of this check once NG types other
// than Mixin derivatives are cached.
if ((child->IsLayoutNGMixin() && if ((child->IsLayoutNGMixin() &&
ShouldForceLayoutForNGChild(ToLayoutBlockFlow(*child))) || ShouldForceLayoutForNGChild(ToLayoutBlockFlow(*child))) ||
(child->IsLayoutBlock() && (child->IsLayoutBlock() &&
......
...@@ -79,6 +79,7 @@ ...@@ -79,6 +79,7 @@
#include "third_party/blink/renderer/core/layout/layout_theme.h" #include "third_party/blink/renderer/core/layout/layout_theme.h"
#include "third_party/blink/renderer/core/layout/layout_view.h" #include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h" #include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_flexible_box.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_table_caption.h" #include "third_party/blink/renderer/core/layout/ng/layout_ng_table_caption.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_table_cell.h" #include "third_party/blink/renderer/core/layout/ng/layout_ng_table_cell.h"
#include "third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h" #include "third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h"
...@@ -278,8 +279,7 @@ LayoutObject* LayoutObject::CreateObject(Element* element, ...@@ -278,8 +279,7 @@ LayoutObject* LayoutObject::CreateObject(Element* element,
case EDisplay::kInlineFlex: case EDisplay::kInlineFlex:
if (RuntimeEnabledFeatures::LayoutNGFlexBoxEnabled() && if (RuntimeEnabledFeatures::LayoutNGFlexBoxEnabled() &&
ShouldUseNewLayout(style)) { ShouldUseNewLayout(style)) {
// TODO(dgrogan): Change this to new class LayoutNGFlex. return new LayoutNGFlexibleBox(element);
return new LayoutNGBlockFlow(element);
} }
return new LayoutFlexibleBox(element); return new LayoutFlexibleBox(element);
case EDisplay::kGrid: case EDisplay::kGrid:
......
...@@ -522,6 +522,9 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, ...@@ -522,6 +522,9 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
bool IsLayoutNGBlockFlow() const { bool IsLayoutNGBlockFlow() const {
return IsOfType(kLayoutObjectNGBlockFlow); return IsOfType(kLayoutObjectNGBlockFlow);
} }
bool IsLayoutNGFlexibleBox() const {
return IsOfType(kLayoutObjectNGFlexibleBox);
}
bool IsLayoutNGMixin() const { return IsOfType(kLayoutObjectNGMixin); } bool IsLayoutNGMixin() const { return IsOfType(kLayoutObjectNGMixin); }
bool IsLayoutNGListItem() const { return IsOfType(kLayoutObjectNGListItem); } bool IsLayoutNGListItem() const { return IsOfType(kLayoutObjectNGListItem); }
bool IsLayoutNGListMarker() const { bool IsLayoutNGListMarker() const {
...@@ -1989,6 +1992,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, ...@@ -1989,6 +1992,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
kLayoutObjectMedia, kLayoutObjectMedia,
kLayoutObjectMenuList, kLayoutObjectMenuList,
kLayoutObjectNGBlockFlow, kLayoutObjectNGBlockFlow,
kLayoutObjectNGFlexibleBox,
kLayoutObjectNGMixin, kLayoutObjectNGMixin,
kLayoutObjectNGListItem, kLayoutObjectNGListItem,
kLayoutObjectNGListMarker, kLayoutObjectNGListMarker,
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/core/layout/ng/layout_ng_flexible_box.h"
#include "third_party/blink/renderer/core/layout/layout_analyzer.h"
#include "third_party/blink/renderer/core/layout/ng/ng_block_node.h"
#include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
#include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
#include "third_party/blink/renderer/core/layout/ng/ng_out_of_flow_positioned_descendant.h"
#include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
namespace blink {
LayoutNGFlexibleBox::LayoutNGFlexibleBox(Element* element)
: LayoutBlock(element) {}
void LayoutNGFlexibleBox::UpdateBlockLayout(bool relayout_children) {
LayoutAnalyzer::BlockScope analyzer(*this);
scoped_refptr<NGConstraintSpace> constraint_space =
NGConstraintSpace::CreateFromLayoutObject(*this);
scoped_refptr<NGLayoutResult> result =
NGBlockNode(this).Layout(*constraint_space);
for (NGOutOfFlowPositionedDescendant descendant :
result->OutOfFlowPositionedDescendants())
descendant.node.UseOldOutOfFlowPositioning();
NGPhysicalBoxFragment* fragment =
ToNGPhysicalBoxFragment(result->PhysicalFragment().get());
// Pasted from layout_ng_block_flow. TODO(dgrogan): Factor a utility method.
const LayoutBlock* containing_block = ContainingBlock();
NGPhysicalOffset physical_offset;
if (containing_block) {
NGPhysicalSize containing_block_size(containing_block->Size().Width(),
containing_block->Size().Height());
NGLogicalOffset logical_offset(LogicalLeft(), LogicalTop());
physical_offset = logical_offset.ConvertToPhysical(
constraint_space->GetWritingMode(), constraint_space->Direction(),
containing_block_size, fragment->Size());
}
fragment->SetOffset(physical_offset);
}
} // namespace blink
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_LAYOUT_NG_FLEXIBLE_BOX_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_LAYOUT_NG_FLEXIBLE_BOX_H_
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/layout/layout_block_flow.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_mixin.h"
namespace blink {
class CORE_EXPORT LayoutNGFlexibleBox : public LayoutBlock {
public:
explicit LayoutNGFlexibleBox(Element*);
void UpdateBlockLayout(bool relayout_children) override;
bool IsFlexibleBox() const final { return true; }
const char* GetName() const override { return "LayoutNGFlexibleBox"; }
protected:
bool IsOfType(LayoutObjectType type) const override {
return type == kLayoutObjectNGFlexibleBox || LayoutBlock::IsOfType(type);
}
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_LAYOUT_NG_FLEXIBLE_BOX_H_
...@@ -14,7 +14,7 @@ namespace blink { ...@@ -14,7 +14,7 @@ namespace blink {
// LayoutMultiColumnSpannerPlaceholder. NG needs to skip these special // LayoutMultiColumnSpannerPlaceholder. NG needs to skip these special
// objects. The actual content is inside the flow thread. // objects. The actual content is inside the flow thread.
LayoutObject* GetLayoutObjectForFirstChildNode(LayoutBlockFlow* parent) { LayoutObject* GetLayoutObjectForFirstChildNode(LayoutBlock* parent) {
LayoutObject* child = parent->FirstChild(); LayoutObject* child = parent->FirstChild();
if (!child) if (!child)
return nullptr; return nullptr;
...@@ -37,7 +37,7 @@ LayoutObject* GetLayoutObjectForParentNode(LayoutObject* object) { ...@@ -37,7 +37,7 @@ LayoutObject* GetLayoutObjectForParentNode(LayoutObject* object) {
return parent; return parent;
} }
bool AreNGBlockFlowChildrenInline(const LayoutBlockFlow* block) { bool AreNGBlockFlowChildrenInline(const LayoutBlock* block) {
if (block->ChildrenInline()) if (block->ChildrenInline())
return true; return true;
if (const auto* first_child = block->FirstChild()) { if (const auto* first_child = block->FirstChild()) {
...@@ -48,14 +48,15 @@ bool AreNGBlockFlowChildrenInline(const LayoutBlockFlow* block) { ...@@ -48,14 +48,15 @@ bool AreNGBlockFlowChildrenInline(const LayoutBlockFlow* block) {
} }
bool IsManagedByLayoutNG(const LayoutObject& object) { bool IsManagedByLayoutNG(const LayoutObject& object) {
if (!object.IsLayoutNGMixin()) if (!object.IsLayoutNGMixin() && !object.IsLayoutNGFlexibleBox())
return false; return false;
const auto* containing_block = object.ContainingBlock(); const auto* containing_block = object.ContainingBlock();
if (!containing_block) if (!containing_block)
return false; return false;
if (containing_block->IsLayoutFlowThread()) if (containing_block->IsLayoutFlowThread())
containing_block = containing_block->ContainingBlock(); containing_block = containing_block->ContainingBlock();
return containing_block && containing_block->IsLayoutNGMixin(); return containing_block && (containing_block->IsLayoutNGMixin() ||
containing_block->IsLayoutNGFlexibleBox());
} }
} // namespace blink } // namespace blink
...@@ -7,13 +7,13 @@ ...@@ -7,13 +7,13 @@
namespace blink { namespace blink {
class LayoutBlockFlow; class LayoutBlock;
class LayoutObject; class LayoutObject;
// Return the layout object that should be the first child NGLayoutInputNode of // Return the layout object that should be the first child NGLayoutInputNode of
// |parent|. Normally this will just be the first layout object child, but there // |parent|. Normally this will just be the first layout object child, but there
// are certain layout objects that should be skipped for NG. // are certain layout objects that should be skipped for NG.
LayoutObject* GetLayoutObjectForFirstChildNode(LayoutBlockFlow*); LayoutObject* GetLayoutObjectForFirstChildNode(LayoutBlock*);
// Return the layout object that should be the parent NGLayoutInputNode of // Return the layout object that should be the parent NGLayoutInputNode of
// |object|. Normally this will just be the parent layout object, but there // |object|. Normally this will just be the parent layout object, but there
...@@ -22,7 +22,7 @@ LayoutObject* GetLayoutObjectForParentNode(LayoutObject*); ...@@ -22,7 +22,7 @@ LayoutObject* GetLayoutObjectForParentNode(LayoutObject*);
// Return true if the NGLayoutInputNode children of the NGLayoutInputNode // Return true if the NGLayoutInputNode children of the NGLayoutInputNode
// established by |block| will be inline; see LayoutObject::ChildrenInline(). // established by |block| will be inline; see LayoutObject::ChildrenInline().
bool AreNGBlockFlowChildrenInline(const LayoutBlockFlow*); bool AreNGBlockFlowChildrenInline(const LayoutBlock*);
// Return true if the layout object is a LayoutNG object that is managed by the // Return true if the layout object is a LayoutNG object that is managed by the
// LayoutNG engine (i.e. its containing block is a LayoutNG object as well). // LayoutNG engine (i.e. its containing block is a LayoutNG object as well).
......
...@@ -321,21 +321,23 @@ NGLayoutInputNode NGBlockNode::NextSibling() const { ...@@ -321,21 +321,23 @@ NGLayoutInputNode NGBlockNode::NextSibling() const {
} }
NGLayoutInputNode NGBlockNode::FirstChild() const { NGLayoutInputNode NGBlockNode::FirstChild() const {
auto* block = ToLayoutBlockFlow(box_); auto* block = ToLayoutBlock(box_);
auto* child = GetLayoutObjectForFirstChildNode(block); auto* child = GetLayoutObjectForFirstChildNode(block);
if (!child) if (!child)
return nullptr; return nullptr;
if (AreNGBlockFlowChildrenInline(block)) if (AreNGBlockFlowChildrenInline(block))
return NGInlineNode(block); return NGInlineNode(ToLayoutBlockFlow(block));
return NGBlockNode(ToLayoutBox(child)); return NGBlockNode(ToLayoutBox(child));
} }
bool NGBlockNode::CanUseNewLayout(const LayoutBox& box) { bool NGBlockNode::CanUseNewLayout(const LayoutBox& box) {
DCHECK(RuntimeEnabledFeatures::LayoutNGEnabled()); DCHECK(RuntimeEnabledFeatures::LayoutNGEnabled());
if (box.StyleRef().ForceLegacyLayout())
return false;
// When the style has |ForceLegacyLayout|, it's usually not LayoutNGMixin, // When the style has |ForceLegacyLayout|, it's usually not LayoutNGMixin,
// but anonymous block can be. // but anonymous block can be.
return box.IsLayoutNGMixin() && !box.StyleRef().ForceLegacyLayout(); return box.IsLayoutNGMixin() || box.IsLayoutNGFlexibleBox();
} }
bool NGBlockNode::CanUseNewLayout() const { bool NGBlockNode::CanUseNewLayout() const {
......
...@@ -121,12 +121,6 @@ scoped_refptr<NGLayoutResult> NGFlexLayoutAlgorithm::Layout() { ...@@ -121,12 +121,6 @@ scoped_refptr<NGLayoutResult> NGFlexLayoutAlgorithm::Layout() {
{flex_item.desired_location.X(), flex_item.desired_location.Y()}); {flex_item.desired_location.X(), flex_item.desired_location.Y()});
} }
} }
// TODO(dgrogan): This line is only needed because we erroneously tell the
// parent block layout algorithm that the flexbox doesn't create a new BFC, so
// a DCHECK is triggered. Remove this line after adding a LayoutNGFlexibleBox
// class and returning it from LayoutObject::CreateLayoutObject().
container_builder_.SetExclusionSpace(
std::make_unique<NGExclusionSpace>(ConstraintSpace().ExclusionSpace()));
return container_builder_.ToBoxFragment(); return container_builder_.ToBoxFragment();
} }
......
...@@ -289,7 +289,9 @@ bool NGPhysicalFragment::IsPlacedByLayoutNG() const { ...@@ -289,7 +289,9 @@ bool NGPhysicalFragment::IsPlacedByLayoutNG() const {
if (!layout_object_) if (!layout_object_)
return false; return false;
const LayoutBlock* container = layout_object_->ContainingBlock(); const LayoutBlock* container = layout_object_->ContainingBlock();
return container && container->IsLayoutNGMixin(); if (!container)
return false;
return container->IsLayoutNGMixin() || container->IsLayoutNGFlexibleBox();
} }
NGPixelSnappedPhysicalBoxStrut NGPhysicalFragment::BorderWidths() const { NGPixelSnappedPhysicalBoxStrut NGPhysicalFragment::BorderWidths() const {
......
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