[LayoutNG] Move kOldLayoutRoot out of NGPhysicalFragment's box types

We have a special value kOldLayoutRoot in NGPhysicalFragment::NGBoxType, which
indicates that the box fragment is the root of new layout. However, this
overwrites the actual box type of the fragment. For example, when we have a
box that's both an inline block and an old layout root, we lose the inline
block box type information when checking NGPhysicalFragment::BoxType().

This patch moves the old layout root information out from NGBoxType as a
separate bit flag in NGPhysicalFragment (and also NGFragmentBuilder), so that
we can get rid of the issue.

Bug: 
Change-Id: Icb98481ccafa6cb3226a01c6dab85c17dac22d96
Reviewed-on: https://chromium-review.googlesource.com/818630
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523306}

third_party/WebKit/Source/core/BUILD.gn

@@ -1689,6 +1689,7 @@ jumbo_source_set("unit_tests") {
     "layout/ng/ng_layout_test.h",
     "layout/ng/ng_length_utils_test.cc",
     "layout/ng/ng_out_of_flow_layout_part_test.cc",
+    "layout/ng/ng_physical_box_fragment_test.cc",
     "layout/ng/ng_relative_utils_test.cc",
     "layout/shapes/BoxShapeTest.cpp",
     "layout/svg/LayoutSVGForeignObjectTest.cpp",

third_party/WebKit/Source/core/layout/ng/ng_block_node.cc

@@ -568,7 +568,7 @@ scoped_refptr<NGLayoutResult> NGBlockNode::RunOldLayout(
   // TODO(kojii): Implement use_first_line_style.
   NGFragmentBuilder builder(*this, box_->Style(), writing_mode,
                             box_->StyleRef().Direction());
-  builder.SetBoxType(NGPhysicalFragment::NGBoxType::kOldLayoutRoot);
+  builder.SetIsOldLayoutRoot();
   builder.SetInlineSize(box_size.inline_size);
   builder.SetBlockSize(box_size.block_size);
 

third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.cc

@@ -26,6 +26,7 @@ NGFragmentBuilder::NGFragmentBuilder(NGLayoutInputNode node,
       node_(node),
       layout_object_(node.GetLayoutObject()),
       box_type_(NGPhysicalFragment::NGBoxType::kNormalBox),
+      is_old_layout_root_(false),
       did_break_(false) {}
 
 NGFragmentBuilder::NGFragmentBuilder(LayoutObject* layout_object,
@@ -36,6 +37,7 @@ NGFragmentBuilder::NGFragmentBuilder(LayoutObject* layout_object,
       node_(nullptr),
       layout_object_(layout_object),
       box_type_(NGPhysicalFragment::NGBoxType::kNormalBox),
+      is_old_layout_root_(false),
       did_break_(false) {}
 
 NGFragmentBuilder::~NGFragmentBuilder() {}
@@ -159,6 +161,11 @@ NGFragmentBuilder& NGFragmentBuilder::SetBoxType(
   return *this;
 }
 
+NGFragmentBuilder& NGFragmentBuilder::SetIsOldLayoutRoot() {
+  is_old_layout_root_ = true;
+  return *this;
+}
+
 void NGFragmentBuilder::AddBaseline(NGBaselineRequest request,
                                     LayoutUnit offset) {
 #if DCHECK_IS_ON()
@@ -198,7 +205,7 @@ scoped_refptr<NGLayoutResult> NGFragmentBuilder::ToBoxFragment() {
   scoped_refptr<NGPhysicalBoxFragment> fragment =
       base::AdoptRef(new NGPhysicalBoxFragment(
           layout_object_, Style(), physical_size, children_,
-          contents_visual_rect, baselines_, BoxType(),
+          contents_visual_rect, baselines_, BoxType(), is_old_layout_root_,
           border_edges_.ToPhysical(GetWritingMode()), std::move(break_token)));
 
   Vector<NGPositionedFloat> positioned_floats;

third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h

@@ -89,6 +89,7 @@ class CORE_EXPORT NGFragmentBuilder final : public NGContainerFragmentBuilder {
 
   NGPhysicalFragment::NGBoxType BoxType() const;
   NGFragmentBuilder& SetBoxType(NGPhysicalFragment::NGBoxType);
+  NGFragmentBuilder& SetIsOldLayoutRoot();
 
   bool DidBreak() const { return did_break_; }
 
@@ -138,6 +139,7 @@ class CORE_EXPORT NGFragmentBuilder final : public NGContainerFragmentBuilder {
   LayoutUnit intrinsic_block_size_;
 
   NGPhysicalFragment::NGBoxType box_type_;
+  bool is_old_layout_root_;
   bool did_break_;
   LayoutUnit used_block_size_;
 

third_party/WebKit/Source/core/layout/ng/ng_physical_box_fragment.cc

@@ -17,6 +17,7 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
     const NGPhysicalOffsetRect& contents_visual_rect,
     Vector<NGBaseline>& baselines,
     NGBoxType box_type,
+    bool is_old_layout_root,
     unsigned border_edges,  // NGBorderEdges::Physical
     scoped_refptr<NGBreakToken> break_token)
     : NGPhysicalContainerFragment(layout_object,
@@ -29,6 +30,7 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
       baselines_(std::move(baselines)) {
   DCHECK(baselines.IsEmpty());  // Ensure move semantics is used.
   box_type_ = box_type;
+  is_old_layout_root_ = is_old_layout_root;
   border_edge_ = border_edges;
 }
 
@@ -98,7 +100,8 @@ scoped_refptr<NGPhysicalFragment> NGPhysicalBoxFragment::CloneWithoutOffset()
   scoped_refptr<NGPhysicalFragment> physical_fragment =
       base::AdoptRef(new NGPhysicalBoxFragment(
           layout_object_, Style(), size_, children_copy, contents_visual_rect_,
-          baselines_copy, BoxType(), border_edge_, break_token_));
+          baselines_copy, BoxType(), is_old_layout_root_, border_edge_,
+          break_token_));
   return physical_fragment;
 }
 

third_party/WebKit/Source/core/layout/ng/ng_physical_box_fragment.h

@@ -22,6 +22,7 @@ class CORE_EXPORT NGPhysicalBoxFragment final
                         const NGPhysicalOffsetRect& contents_visual_rect,
                         Vector<NGBaseline>& baselines,
                         NGBoxType box_type,
+                        bool is_old_layout_root,
                         unsigned,  // NGBorderEdges::Physical
                         scoped_refptr<NGBreakToken> break_token = nullptr);
 

third_party/WebKit/Source/core/layout/ng/ng_physical_box_fragment_test.cc

+// Copyright 2017 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 "core/layout/ng/ng_physical_box_fragment.h"
+
+#include "core/layout/LayoutBlockFlow.h"
+#include "core/layout/ng/ng_layout_test.h"
+
+namespace blink {
+
+class NGPhysicalBoxFragmentTest : public NGLayoutTest,
+                                  private ScopedLayoutNGPaintFragmentsForTest {
+ public:
+  NGPhysicalBoxFragmentTest()
+      : NGLayoutTest(), ScopedLayoutNGPaintFragmentsForTest(true) {}
+
+  const NGPhysicalBoxFragment& GetBodyFragment() const {
+    return *ToLayoutBlockFlow(GetDocument().body()->GetLayoutObject())
+                ->CurrentFragment();
+  }
+};
+
+// TODO(layout-dev): Design more straightforward way to ensure old layout
+// instead of using |contenteditable|.
+
+// Tests that a normal old layout root box fragment has correct box type.
+TEST_F(NGPhysicalBoxFragmentTest, NormalOldLayoutRoot) {
+  SetBodyInnerHTML("<div contenteditable>X</div>");
+  const NGPhysicalFragment* fragment =
+      GetBodyFragment().Children().front().get();
+  ASSERT_TRUE(fragment);
+  EXPECT_TRUE(fragment->IsBox());
+  EXPECT_EQ(NGPhysicalFragment::kNormalBox, fragment->BoxType());
+  EXPECT_TRUE(fragment->IsOldLayoutRoot());
+  EXPECT_TRUE(fragment->IsBlockLayoutRoot());
+}
+
+// Tests that a float old layout root box fragment has correct box type.
+TEST_F(NGPhysicalBoxFragmentTest, FloatOldLayoutRoot) {
+  SetBodyInnerHTML("<span contenteditable style='float:left'>X</span>foo");
+  const NGPhysicalFragment* fragment =
+      GetBodyFragment().Children().front().get();
+  ASSERT_TRUE(fragment);
+  EXPECT_TRUE(fragment->IsBox());
+  EXPECT_EQ(NGPhysicalFragment::kFloating, fragment->BoxType());
+  EXPECT_TRUE(fragment->IsOldLayoutRoot());
+  EXPECT_TRUE(fragment->IsBlockLayoutRoot());
+}
+
+// Tests that an inline block old layout root box fragment has correct box type.
+TEST_F(NGPhysicalBoxFragmentTest, InlineBlockOldLayoutRoot) {
+  SetBodyInnerHTML(
+      "<span contenteditable style='display:inline-block'>X</span>foo");
+  const NGPhysicalContainerFragment* line_box =
+      ToNGPhysicalContainerFragment(GetBodyFragment().Children().front().get());
+  const NGPhysicalFragment* fragment = line_box->Children().front().get();
+  ASSERT_TRUE(fragment);
+  EXPECT_TRUE(fragment->IsBox());
+  EXPECT_EQ(NGPhysicalFragment::kInlineBlock, fragment->BoxType());
+  EXPECT_TRUE(fragment->IsOldLayoutRoot());
+  EXPECT_TRUE(fragment->IsBlockLayoutRoot());
+}
+
+// Tests that an out-of-flow positioned old layout root box fragment has correct
+// box type.
+TEST_F(NGPhysicalBoxFragmentTest, OutOfFlowPositionedOldLayoutRoot) {
+  SetBodyInnerHTML(
+      "<style>body {position: absolute}</style>"
+      "<div contenteditable style='position: absolute'>X</div>");
+  const NGPhysicalFragment* fragment =
+      GetBodyFragment().Children().front().get();
+  ASSERT_TRUE(fragment);
+  EXPECT_TRUE(fragment->IsBox());
+  EXPECT_EQ(NGPhysicalFragment::kOutOfFlowPositioned, fragment->BoxType());
+  EXPECT_TRUE(fragment->IsOldLayoutRoot());
+  EXPECT_TRUE(fragment->IsBlockLayoutRoot());
+}
+
+}  // namespace blink

third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.cc

@@ -41,23 +41,33 @@ bool AppendFragmentOffsetAndSize(const NGPhysicalFragment* fragment,
   return has_content;
 }
 
-String StringForBoxType(NGPhysicalFragment::NGBoxType box_type) {
+String StringForBoxType(NGPhysicalFragment::NGBoxType box_type,
+                        bool is_old_layout_root) {
+  StringBuilder result;
   switch (box_type) {
     case NGPhysicalFragment::NGBoxType::kNormalBox:
-      return String();
+      break;
     case NGPhysicalFragment::NGBoxType::kInlineBlock:
-      return "inline-block";
+      result.Append("inline-block");
+      break;
     case NGPhysicalFragment::NGBoxType::kFloating:
-      return "floating";
+      result.Append("floating");
+      break;
     case NGPhysicalFragment::NGBoxType::kOutOfFlowPositioned:
-      return "out-of-flow-positioned";
-    case NGPhysicalFragment::NGBoxType::kOldLayoutRoot:
-      return "old-layout-root";
+      result.Append("out-of-flow-positioned");
+      break;
     case NGPhysicalFragment::NGBoxType::kAnonymousBox:
-      return "anonymous";
+      result.Append("anonymous");
+      break;
+    default:
+      NOTREACHED();
   }
-  NOTREACHED();
-  return String();
+  if (is_old_layout_root) {
+    if (result.length())
+      result.Append(" ");
+    result.Append("old-layout-root");
+  }
+  return result.ToString();
 }
 
 void AppendFragmentToString(const NGPhysicalFragment* fragment,
@@ -73,7 +83,8 @@ void AppendFragmentToString(const NGPhysicalFragment* fragment,
   if (fragment->IsBox()) {
     if (flags & NGPhysicalFragment::DumpType) {
       builder->Append("Box");
-      String box_type = StringForBoxType(fragment->BoxType());
+      String box_type =
+          StringForBoxType(fragment->BoxType(), fragment->IsOldLayoutRoot());
       if (!box_type.IsEmpty()) {
         builder->Append(" (");
         builder->Append(box_type);
@@ -162,6 +173,7 @@ NGPhysicalFragment::NGPhysicalFragment(LayoutObject* layout_object,
       break_token_(std::move(break_token)),
       type_(type),
       box_type_(NGBoxType::kNormalBox),
+      is_old_layout_root_(false),
       is_placed_(false) {}
 
 // Keep the implementation of the destructor here, to avoid dependencies on
@@ -279,7 +291,7 @@ String NGPhysicalFragment::ToString() const {
       "Type: '%d' Size: '%s' Offset: '%s' Placed: '%d', BoxType: '%s'", Type(),
       Size().ToString().Ascii().data(),
       is_placed_ ? Offset().ToString().Ascii().data() : "no offset", IsPlaced(),
-      StringForBoxType(BoxType()).Ascii().data());
+      StringForBoxType(BoxType(), IsOldLayoutRoot()).Ascii().data());
 }
 
 String NGPhysicalFragment::DumpFragmentTree(DumpFlags flags,

third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.h

@@ -53,7 +53,6 @@ class CORE_EXPORT NGPhysicalFragment
     kInlineBlock,
     kFloating,
     kOutOfFlowPositioned,
-    kOldLayoutRoot,
     // When adding new values, make sure the bit size of |box_type_| is large
     // enough to store.
 
@@ -75,6 +74,8 @@ class CORE_EXPORT NGPhysicalFragment
 
   // Returns the box type of this fragment.
   NGBoxType BoxType() const { return static_cast<NGBoxType>(box_type_); }
+  // Returns whether the fragment is old layout root.
+  bool IsOldLayoutRoot() const { return is_old_layout_root_; }
   // An inline block is represented as a kFragmentBox.
   // TODO(eae): This isn't true for replaces elements at the moment.
   bool IsInlineBlock() const { return BoxType() == NGBoxType::kInlineBlock; }
@@ -86,10 +87,10 @@ class CORE_EXPORT NGPhysicalFragment
   // co-owned by other fragments.
   bool IsAnonymousBox() const { return BoxType() == NGBoxType::kAnonymousBox; }
   // A block sub-layout starts on this fragment. Inline blocks, floats, out of
-  // flow positioned objects are such examples. This may be false on NG/legacy
+  // flow positioned objects are such examples. This is also true on NG/legacy
   // boundary.
   bool IsBlockLayoutRoot() const {
-    return BoxType() >= NGBoxType::kMinimumBlockLayoutRoot;
+    return BoxType() >= NGBoxType::kMinimumBlockLayoutRoot || IsOldLayoutRoot();
   }
 
   // |Offset()| is reliable only when this fragment was placed by LayoutNG
@@ -178,6 +179,7 @@ class CORE_EXPORT NGPhysicalFragment
 
   unsigned type_ : 2;  // NGFragmentType
   unsigned box_type_ : 3;  // NGBoxType
+  unsigned is_old_layout_root_ : 1;
   unsigned is_placed_ : 1;
   unsigned border_edge_ : 4;  // NGBorderEdges::Physical