[LayoutNG] Check first-line anonymous more correctly

When |LayoutBlockFlow| adds a child, it assumed all anonymous
|LayoutInline|s are anoymous wrappers for ::first-line, but
there are two other usage of anonymous |LayoutInline|.

This patch checks if it is really an anonymous |LayoutInline|
for ::first-line.

Bug: 962275
Change-Id: Ibbde37297e4636e4f0c51f0ed8249e3166639d76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610657
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659369}

third_party/blink/renderer/core/BUILD.gn

@@ -1993,6 +1993,7 @@ jumbo_source_set("unit_tests") {
     "layout/ng/inline/ng_offset_mapping_test.cc",
     "layout/ng/inline/ng_physical_line_box_fragment_test.cc",
     "layout/ng/inline/ng_physical_text_fragment_test.cc",
+    "layout/ng/list/layout_ng_list_item_test.cc",
     "layout/ng/ng_absolute_utils_test.cc",
     "layout/ng/ng_base_layout_algorithm_test.cc",
     "layout/ng/ng_base_layout_algorithm_test.h",

third_party/blink/renderer/core/layout/layout_block_flow.cc

@@ -3163,8 +3163,7 @@ void LayoutBlockFlow::AddChild(LayoutObject* new_child,
                         NeedsAnonymousInlineWrapper())) {
       LayoutObject* after_child =
           before_child ? before_child->PreviousSibling() : LastChild();
-      if (after_child && after_child->IsAnonymous() &&
-          after_child->IsLayoutInline()) {
+      if (after_child && after_child->IsFirstLineAnonymous()) {
         after_child->AddChild(new_child);
         return;
       }

third_party/blink/renderer/core/layout/layout_inline.cc

@@ -104,16 +104,14 @@ LayoutInline* LayoutInline::CreateAnonymous(Document* document) {
   return layout_inline;
 }
 
-bool LayoutInline::IsFirstLineAnonymous() const {
-  return false;
-}
-
 // A private class to distinguish anonymous inline box for ::first-line from
 // other inline boxes.
 class LayoutInlineForFirstLine : public LayoutInline {
  public:
   LayoutInlineForFirstLine(Element* element) : LayoutInline(element) {}
-  bool IsFirstLineAnonymous() const final { return true; }
+
+ protected:
+  bool VirtualIsFirstLineAnonymous() const final { return true; }
 };
 
 LayoutInline* LayoutInline::CreateAnonymousForFirstLine(Document* document) {

third_party/blink/renderer/core/layout/layout_inline.h

@@ -143,10 +143,6 @@ class CORE_EXPORT LayoutInline : public LayoutBoxModelObject {
     return ToElement(LayoutBoxModelObject::GetNode());
   }
 
-  // True if this is an anonymous inline box for ::first-line that wraps the
-  // whole inline formatting context.
-  virtual bool IsFirstLineAnonymous() const;
-
   LayoutUnit MarginLeft() const final;
   LayoutUnit MarginRight() const final;
   LayoutUnit MarginTop() const final;

third_party/blink/renderer/core/layout/layout_object.cc

@@ -338,6 +338,10 @@ bool LayoutObject::IsDescendantOf(const LayoutObject* obj) const {
   return false;
 }
 
+bool LayoutObject::VirtualIsFirstLineAnonymous() const {
+  return false;
+}
+
 bool LayoutObject::IsHR() const {
   return IsHTMLHRElement(GetNode());
 }

third_party/blink/renderer/core/layout/layout_object.h

@@ -876,6 +876,11 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
            !IsListMarker() && !IsLayoutFlowThread() &&
            !IsLayoutMultiColumnSet();
   }
+  // True if this is an anonymous inline box for ::first-line that wraps the
+  // whole inline formatting context.
+  bool IsFirstLineAnonymous() const {
+    return IsAnonymous() && VirtualIsFirstLineAnonymous();
+  }
   // If node has been split into continuations, it returns the first layout
   // object generated for the node.
   const LayoutObject* ContinuationRoot() const {
@@ -2399,6 +2404,11 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
   };
   virtual bool IsOfType(LayoutObjectType type) const { return false; }
 
+  // True if this is an anonymous inline box for ::first-line that wraps the
+  // whole inline formatting context. This is for subclasses to override.
+  // Callers should use |IsFirstLineAnonymous()|.
+  virtual bool VirtualIsFirstLineAnonymous() const;
+
   // Updates only the local style ptr of the object.  Does not update the state
   // of the object, and so only should be called when the style is known not to
   // have changed (or from SetStyle).

third_party/blink/renderer/core/layout/ng/list/layout_ng_inside_list_marker.h

@@ -20,6 +20,14 @@ class CORE_EXPORT LayoutNGInsideListMarker final : public LayoutInline {
 
   const char* GetName() const override { return "LayoutNGInsideListMarker"; }
 
+#if DCHECK_IS_ON()
+  void AddChild(LayoutObject* new_child, LayoutObject* before_child) override {
+    // List marker should have at most one child.
+    DCHECK(!FirstChild());
+    LayoutInline::AddChild(new_child, before_child);
+  }
+#endif
+
  private:
   bool IsOfType(LayoutObjectType) const override;
 };

third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.cc

@@ -293,6 +293,8 @@ void LayoutNGListItem::UpdateMarkerContentIfNeeded() {
   DCHECK(marker_);
 
   LayoutObject* child = marker_->SlowFirstChild();
+  // There should be at most one child.
+  DCHECK(!child || !child->SlowFirstChild());
   if (IsMarkerImage()) {
     StyleImage* list_style_image = StyleRef().ListStyleImage();
     if (child) {

third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item_test.cc

@@ -0,0 +1,50 @@
+// Copyright 2019 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/ng_base_layout_algorithm_test.h"
+
+#include "third_party/blink/renderer/core/dom/dom_token_list.h"
+#include "third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h"
+
+namespace blink {
+
+class LayoutNGListItemTest : public NGLayoutTest {};
+
+namespace {
+
+TEST_F(LayoutNGListItemTest, InsideWithFirstLine) {
+  SetBodyInnerHTML(R"HTML(
+    <!DOCTYPE html>
+    <style>
+    li {
+      list-style-position: inside;
+    }
+    .after::first-line {
+      background: yellow;
+    }
+    </style>
+    <div id=container>
+      <ul>
+        <li id=item>test</li>
+      </ul>
+    </div>
+  )HTML");
+
+  Element* container = GetElementById("container");
+  container->classList().Add("after");
+  GetDocument().UpdateStyleAndLayoutTree();
+
+  // The list-item should have a marker.
+  LayoutNGListItem* list_item =
+      ToLayoutNGListItem(GetLayoutObjectByElementId("item"));
+  LayoutObject* marker = list_item->Marker();
+  EXPECT_TRUE(marker);
+  // The marker should have only 1 child.
+  LayoutObject* marker_child = marker->SlowFirstChild();
+  EXPECT_TRUE(marker_child);
+  EXPECT_FALSE(marker_child->NextSibling());
+}
+
+}  // namespace
+}  // namespace blink