[LayoutNG] Only CSS boxes should return non-nullptr for GetLayoutObject().

Using the LayoutObject associated with other (non-CSS box) fragments
(such as lines and columns) is generally wrong. We already had guards
for lines, but not for columns. Had to add a couple of new methods to
the NGPhysicalFragment API.

Also inlined some of the NGPhysicalFragment methods that I changed.

One change:
There used to be a direct call to LayoutObject::IsAnonymous(). I replaced
this with NGPhysicalFragment::IsAnonymousBlock(), which calls
LayoutObject::IsAnonymousBlock() instead of IsAnonymous(). I assume this
is more correct (given the comment where it's used), if it matters at all.

Change-Id: I2fe133fc85c5888adbc303d0d392e65ae548908d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913410
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718089}

third_party/blink/renderer/core/layout/ng/ng_block_node.cc

@@ -932,8 +932,9 @@ void NGBlockNode::PlaceChildrenInFlowThread(
     const NGPhysicalBoxFragment& physical_fragment) {
   const NGBlockBreakToken* previous_break_token = nullptr;
   for (const auto& child : physical_fragment.Children()) {
-    if (child->GetLayoutObject() != box_) {
-      DCHECK(child->GetLayoutObject()->IsColumnSpanAll());
+    const LayoutObject* child_object = child->GetLayoutObject();
+    if (child_object && child_object != box_) {
+      DCHECK(child_object->IsColumnSpanAll());
       CopyChildFragmentPosition(To<NGPhysicalBoxFragment>(*child), child.offset,
                                 physical_fragment);
       continue;

third_party/blink/renderer/core/layout/ng/ng_outline_utils.cc

@@ -23,10 +23,11 @@ bool NGOutlineUtils::HasPaintedOutline(const ComputedStyle& style,
 
 bool NGOutlineUtils::ShouldPaintOutline(
     const NGPhysicalBoxFragment& physical_fragment) {
+  if (!physical_fragment.IsInlineBox())
+    return true;
   const LayoutObject* layout_object = physical_fragment.GetLayoutObject();
   DCHECK(layout_object);
-  if (!layout_object->IsLayoutInline())
-    return true;
+  DCHECK(layout_object->IsLayoutInline());
 
   // A |LayoutInline| can be split across multiple objects. The first fragment
   // produced should paint the outline for *all* fragments.

third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.cc

@@ -82,7 +82,7 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
               : kFragmentBox,
           builder->BoxType()),
       baselines_(builder->baselines_) {
-  DCHECK(GetLayoutObject() && GetLayoutObject()->IsBoxModelObject());
+  DCHECK(layout_object_->IsBoxModelObject());
   if (NGFragmentItemsBuilder* items_builder = builder->ItemsBuilder()) {
     has_fragment_items_ = true;
     NGFragmentItems* items =
@@ -123,6 +123,8 @@ NGPhysicalBoxFragment::CloneAsHiddenForPaint() const {
 }
 
 bool NGPhysicalBoxFragment::HasSelfPaintingLayer() const {
+  if (!IsCSSBox())
+    return false;
   SECURITY_DCHECK(GetLayoutObject() && GetLayoutObject()->IsBoxModelObject());
   return (static_cast<const LayoutBoxModelObject*>(GetLayoutObject()))
       ->HasSelfPaintingLayer();
@@ -212,9 +214,10 @@ void NGPhysicalBoxFragment::AddSelfOutlineRects(
   if (!NGOutlineUtils::ShouldPaintOutline(*this))
     return;
 
+  if (IsInlineBox()) {
     const LayoutObject* layout_object = GetLayoutObject();
     DCHECK(layout_object);
-  if (layout_object->IsLayoutInline()) {
+    DCHECK(layout_object->IsLayoutInline());
     Vector<PhysicalRect> blockflow_outline_rects =
         layout_object->OutlineRects(PhysicalOffset(), outline_type);
     // The rectangles returned are offset from the containing block. We need the
@@ -234,12 +237,11 @@ void NGPhysicalBoxFragment::AddSelfOutlineRects(
     }
     return;
   }
-  DCHECK(layout_object->IsBox());
 
   // For anonymous blocks, the children add outline rects.
-  if (!layout_object->IsAnonymous()) {
+  if (!IsAnonymousBlock())
     outline_rects->emplace_back(additional_offset, Size().ToLayoutSize());
-  }
+
   if (outline_type == NGOutlineType::kIncludeBlockVisualOverflow &&
       !HasOverflowClip() && !HasControlClip(*this)) {
     // Tricky code ahead: we pass a 0,0 additional_offset to

third_party/blink/renderer/core/layout/ng/ng_physical_container_fragment.cc

@@ -126,11 +126,11 @@ void NGPhysicalContainerFragment::AddOutlineRectsForDescendant(
           DynamicTo<NGPhysicalBoxFragment>(descendant.get())) {
     const LayoutObject* descendant_layout_object =
         descendant_box->GetLayoutObject();
-    DCHECK(descendant_layout_object);
 
     // TODO(layoutng): Explain this check. I assume we need it because layers
     // may have transforms and so we have to go through LocalToAncestorRects?
     if (descendant_box->HasLayer()) {
+      DCHECK(descendant_layout_object);
       Vector<PhysicalRect> layer_outline_rects;
       descendant_box->AddSelfOutlineRects(PhysicalOffset(), outline_type,
                                           &layer_outline_rects);
@@ -144,12 +144,13 @@ void NGPhysicalContainerFragment::AddOutlineRectsForDescendant(
       return;
     }
 
-    if (descendant_layout_object->IsBox()) {
+    if (!descendant_box->IsInlineBox()) {
       descendant_box->AddSelfOutlineRects(
           additional_offset + descendant.Offset(), outline_type, outline_rects);
       return;
     }
 
+    DCHECK(descendant_layout_object);
     DCHECK(descendant_layout_object->IsLayoutInline());
     const LayoutInline* descendant_layout_inline =
         ToLayoutInline(descendant_layout_object);

third_party/blink/renderer/core/layout/ng/ng_physical_fragment.cc

@@ -269,10 +269,6 @@ void NGPhysicalFragment::Destroy() const {
   }
 }
 
-Node* NGPhysicalFragment::GetNode() const {
-  return !IsLineBox() ? layout_object_->GetNode() : nullptr;
-}
-
 PaintLayer* NGPhysicalFragment::Layer() const {
   if (!HasLayer())
     return nullptr;
@@ -292,27 +288,17 @@ bool NGPhysicalFragment::HasSelfPaintingLayer() const {
       ->HasSelfPaintingLayer();
 }
 
-bool NGPhysicalFragment::HasOverflowClip() const {
-  return !IsLineBox() && layout_object_->HasOverflowClip();
-}
-
-bool NGPhysicalFragment::ShouldClipOverflow() const {
-  return !IsLineBox() && layout_object_->ShouldClipOverflow();
-}
-
 bool NGPhysicalFragment::IsBlockFlow() const {
   return !IsLineBox() && layout_object_->IsLayoutBlockFlow();
 }
 
-bool NGPhysicalFragment::IsListMarker() const {
-  return !IsLineBox() && layout_object_->IsLayoutNGListMarker();
-}
-
 bool NGPhysicalFragment::IsPlacedByLayoutNG() const {
   // TODO(kojii): Move this to a flag for |LayoutNGBlockFlow::UpdateBlockLayout|
   // to set.
   if (IsLineBox())
     return false;
+  if (IsColumnBox())
+    return true;
   const LayoutBlock* container = layout_object_->ContainingBlock();
   if (!container)
     return false;
@@ -396,8 +382,7 @@ void NGPhysicalFragment::CheckType() const {
 void NGPhysicalFragment::CheckCanUpdateInkOverflow() const {
   if (!GetLayoutObject())
     return;
-  const DocumentLifecycle& lifecycle =
-      GetLayoutObject()->GetDocument().Lifecycle();
+  const DocumentLifecycle& lifecycle = GetDocument().Lifecycle();
   DCHECK(lifecycle.GetState() >= DocumentLifecycle::kLayoutClean &&
          lifecycle.GetState() < DocumentLifecycle::kCompositingClean)
       << lifecycle.GetState();

third_party/blink/renderer/core/layout/ng/ng_physical_fragment.h

@@ -136,7 +136,12 @@ class CORE_EXPORT NGPhysicalFragment
   bool IsCSSBox() const { return !IsLineBox() && !IsColumnBox(); }
 
   bool IsBlockFlow() const;
-  bool IsListMarker() const;
+  bool IsAnonymousBlock() const {
+    return IsCSSBox() && layout_object_->IsAnonymousBlock();
+  }
+  bool IsListMarker() const {
+    return IsCSSBox() && layout_object_->IsLayoutNGListMarker();
+  }
 
   // Return true if this fragment is a container established by a fieldset
   // element. Such a fragment contains an optional rendered legend fragment and
@@ -186,7 +191,17 @@ class CORE_EXPORT NGPhysicalFragment
   const ComputedStyle& Style() const {
     return layout_object_->EffectiveStyle(StyleVariant());
   }
-  Node* GetNode() const;
+
+  const Document& GetDocument() const {
+    DCHECK(layout_object_);
+    return layout_object_->GetDocument();
+  }
+  Node* GetNode() const {
+    return IsCSSBox() ? layout_object_->GetNode() : nullptr;
+  }
+  Node* GeneratingNode() const {
+    return IsCSSBox() ? layout_object_->GeneratingNode() : nullptr;
+  }
 
   // Whether there is a PaintLayer associated with the fragment.
   bool HasLayer() const { return IsCSSBox() && layout_object_->HasLayer(); }
@@ -199,8 +214,13 @@ class CORE_EXPORT NGPhysicalFragment
 
   // True if overflow != 'visible', except for certain boxes that do not allow
   // overflow clip; i.e., AllowOverflowClip() returns false.
-  bool HasOverflowClip() const;
-  bool ShouldClipOverflow() const;
+  bool HasOverflowClip() const {
+    return IsCSSBox() && layout_object_->HasOverflowClip();
+  }
+
+  bool ShouldClipOverflow() const {
+    return IsCSSBox() && layout_object_->ShouldClipOverflow();
+  }
 
   // This fragment is hidden for paint purpose, but exists for querying layout
   // information. Used for `text-overflow: ellipsis`.
@@ -213,12 +233,12 @@ class CORE_EXPORT NGPhysicalFragment
   // returns |nullptr| for the historical reasons. TODO(kojii): We may change
   // this in future. Use |IsLineBox()| instead of testing this is |nullptr|.
   const LayoutObject* GetLayoutObject() const {
-    return !IsLineBox() ? layout_object_ : nullptr;
+    return IsCSSBox() ? layout_object_ : nullptr;
   }
   // TODO(kojii): We should not have mutable version at all, the use of this
   // function should be eliminiated over time.
   LayoutObject* GetMutableLayoutObject() const {
-    return !IsLineBox() ? layout_object_ : nullptr;
+    return IsCSSBox() ? layout_object_ : nullptr;
   }
 
   // |NGPhysicalFragment| may live longer than the corresponding |LayoutObject|.

third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h

@@ -215,9 +215,7 @@ inline NGBoxFragmentPainter::NGBoxFragmentPainter(
     const NGPhysicalBoxFragment& box,
     const NGPaintFragment* paint_fragment,
     NGInlineCursor* descendants)
-    : BoxPainterBase(&box.GetLayoutObject()->GetDocument(),
-                     box.Style(),
-                     box.GetLayoutObject()->GeneratingNode()),
+    : BoxPainterBase(&box.GetDocument(), box.Style(), box.GeneratingNode()),
       box_fragment_(box),
       paint_fragment_(paint_fragment),
       items_(box.Items()),
@@ -235,8 +233,9 @@ inline NGBoxFragmentPainter::NGBoxFragmentPainter(
       if (paint_fragment)
         DCHECK_EQ(&paint_fragment->PhysicalFragment(), &box);
     }
-  } else if (box.GetLayoutObject()->SlowFirstChild() &&
-             box.GetLayoutObject()->SlowFirstChild()->IsLayoutFlowThread()) {
+  } else if (box.IsColumnBox() ||
+             (box.GetLayoutObject()->SlowFirstChild() &&
+              box.GetLayoutObject()->SlowFirstChild()->IsLayoutFlowThread())) {
     // TODO(kojii): NGPaintFragment for multicol has non-inline children
     // (kColumnBox). Could this be regular box fragments?
   } else {