Reland "[LayoutNG] Introduce NGInlineFragmentTraversal in place of NGInlineFragmentIterator"

This is a reland of e62d46f6

The original change was reverted due to failures in the new unit tests, which assume
box fragment creation for inline elements <b>, <i> or <u>, which however is not true.

This reland ensures box fragment creation by adding border to those elements.

Original change's description:
> [LayoutNG] Introduce NGInlineFragmentTraversal in place of NGInlineFragmentIterator
>
> This patch introduces a utility class NGInlineFragmentTraversal, so that we can
> conveniently retrieve:
> - ancestors of a given physical fragment in a container
> - descendants of a given physical fragment
> - physical fragments in a container with the given layout object
>
> The utility class supersedes the existing NGInlineFragmentIterator class. Follow
> up patches will convert clients of NGInlineFragmentIterator to use the traversal
> class instead.
>
> Bug: 636993
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
> Change-Id: I7f3f553df18291df50f60075ab84294dd394d86c
> Reviewed-on: https://chromium-review.googlesource.com/807758
> Reviewed-by: Koji Ishii <kojii@chromium.org>
> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521758}

Tbr: eae@chromium.org, kojii@chromium.org, yoichio@chromium.org, yosin@chromium.org
Bug: 636993
Change-Id: I616a89ac0ca1360681e47bbc45317ffca5d99d90
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/809302Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521864}

third_party/WebKit/Source/core/BUILD.gn

@@ -1663,6 +1663,7 @@ jumbo_source_set("unit_tests") {
     "layout/ng/geometry/ng_physical_offset_rect_test.cc",
     "layout/ng/geometry/ng_physical_offset_test.cc",
     "layout/ng/geometry/ng_physical_rect_test.cc",
+    "layout/ng/inline/ng_inline_fragment_traversal_test.cc",
     "layout/ng/inline/ng_inline_items_builder_test.cc",
     "layout/ng/inline/ng_inline_layout_algorithm_test.cc",
     "layout/ng/inline/ng_inline_node_test.cc",

third_party/WebKit/Source/core/layout/ng/inline/ng_inline_fragment_iterator.cc

@@ -2,35 +2,33 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// TODO(xiaochengh): Rename this file into ng_inline_fragment_traversal.cc
+
 #include "core/layout/ng/inline/ng_inline_fragment_iterator.h"
 
 #include "core/layout/LayoutObject.h"
 #include "core/layout/ng/ng_physical_box_fragment.h"
-#include "platform/wtf/HashMap.h"
 
 namespace blink {
 
-NGInlineFragmentIterator::NGInlineFragmentIterator(
-    const NGPhysicalBoxFragment& box,
-    const LayoutObject* filter) {
-  DCHECK(filter);
+namespace {
 
-  CollectInlineFragments(box, {}, filter, &results_);
-}
+using Result = NGPhysicalFragmentWithOffset;
 
-// Create a map from a LayoutObject to a vector of PhysicalFragment and its
-// offset to the container box. This is done by collecting inline child
-// fragments of the container fragment, while accumulating the offset to the
-// container box.
-void NGInlineFragmentIterator::CollectInlineFragments(
-    const NGPhysicalContainerFragment& container,
+// Traverse the subtree of |container|, and collect the fragments satisfying
+// |filter| into the |results| vector. Guarantees to call |filter.AddOnEnter()|
+// for all fragments in preorder, and call |filter.RemoveOnExit()| on all
+// fragments in postorder. A fragment is collected if |AddOnEnter()| returns
+// true and |RemoveOnExit()| returns false on it.
+template <typename Filter, size_t inline_capacity>
+void CollectInlineFragments(const NGPhysicalContainerFragment& container,
                             NGPhysicalOffset offset_to_container_box,
-    const LayoutObject* filter,
-    Results* results) {
+                            Filter& filter,
+                            Vector<Result, inline_capacity>* results) {
   for (const auto& child : container.Children()) {
     NGPhysicalOffset child_offset = child->Offset() + offset_to_container_box;
 
-    if (filter == child->GetLayoutObject()) {
+    if (filter.AddOnEnter(child.get())) {
       results->push_back(
           NGPhysicalFragmentWithOffset{child.get(), child_offset});
     }
@@ -41,7 +39,136 @@ void NGInlineFragmentIterator::CollectInlineFragments(
       CollectInlineFragments(ToNGPhysicalContainerFragment(*child),
                              child_offset, filter, results);
     }
+
+    if (filter.RemoveOnExit(child.get())) {
+      DCHECK(results->size());
+      DCHECK_EQ(results->back().fragment, child.get());
+      results->pop_back();
+    }
+  }
+}
+
+// The filter for CollectInlineFragments() collecting all fragments traversed.
+class AddAllFilter {
+ public:
+  bool AddOnEnter(const NGPhysicalFragment*) const { return true; }
+  bool RemoveOnExit(const NGPhysicalFragment*) const { return false; }
+};
+
+// The filter for CollectInlineFragments() collecting fragments generated from
+// the given LayoutObject.
+class LayoutObjectFilter {
+ public:
+  explicit LayoutObjectFilter(const LayoutObject* layout_object)
+      : layout_object_(layout_object) {
+    DCHECK(layout_object);
+  }
+
+  bool AddOnEnter(const NGPhysicalFragment* fragment) const {
+    return fragment->GetLayoutObject() == layout_object_;
+  }
+  bool RemoveOnExit(const NGPhysicalFragment*) const { return false; }
+
+ private:
+  const LayoutObject* layout_object_;
+};
+
+// The filter for CollectInlineFragments() collecting inclusive ancestors of the
+// given fragment with the algorithm that, |fragment| is an ancestor of |target|
+// if and only if both of the following are true:
+// - |fragment| precedes |target| in preorder traversal
+// - |fragment| succeeds |target| in postorder traversal
+class InclusiveAncestorFilter {
+ public:
+  explicit InclusiveAncestorFilter(const NGPhysicalFragment& target)
+      : target_(&target) {}
+
+  bool AddOnEnter(const NGPhysicalFragment* fragment) {
+    if (fragment == target_)
+      has_entered_target_ = true;
+    ancestors_precede_in_preorder_.push_back(!has_entered_target_);
+    return true;
+  }
+
+  bool RemoveOnExit(const NGPhysicalFragment* fragment) {
+    if (fragment != target_) {
+      const bool precedes_in_preorder = ancestors_precede_in_preorder_.back();
+      ancestors_precede_in_preorder_.pop_back();
+      return !precedes_in_preorder || !has_exited_target_;
+    }
+    has_exited_target_ = true;
+    ancestors_precede_in_preorder_.pop_back();
+    return false;
   }
+
+ private:
+  const NGPhysicalFragment* target_;
+
+  bool has_entered_target_ = false;
+  bool has_exited_target_ = false;
+
+  // For each currently entered but not-yet-exited fragment, stores a boolean of
+  // whether it precedes |target_| in preorder.
+  Vector<bool> ancestors_precede_in_preorder_;
+};
+
+}  // namespace
+
+NGInlineFragmentIterator::NGInlineFragmentIterator(
+    const NGPhysicalBoxFragment& box,
+    const LayoutObject* layout_object) {
+  results_ = NGInlineFragmentTraversal::SelfFragmentsOf(box, layout_object);
+}
+
+// static
+Vector<Result, 1> NGInlineFragmentTraversal::SelfFragmentsOf(
+    const NGPhysicalContainerFragment& container,
+    const LayoutObject* layout_object) {
+  LayoutObjectFilter filter(layout_object);
+  Vector<Result, 1> results;
+  CollectInlineFragments(container, {}, filter, &results);
+  return results;
+}
+
+// static
+Vector<Result> NGInlineFragmentTraversal::DescendantsOf(
+    const NGPhysicalContainerFragment& container) {
+  AddAllFilter add_all;
+  Vector<Result> results;
+  CollectInlineFragments(container, {}, add_all, &results);
+  return results;
+}
+
+// static
+Vector<Result> NGInlineFragmentTraversal::InclusiveDescendantsOf(
+    const NGPhysicalFragment& root) {
+  Vector<Result> results =
+      root.IsContainer() ? DescendantsOf(ToNGPhysicalContainerFragment(root))
+                         : Vector<Result>();
+  results.push_front(Result{&root, {}});
+  return results;
+}
+
+// static
+Vector<Result> NGInlineFragmentTraversal::InclusiveAncestorsOf(
+    const NGPhysicalContainerFragment& container,
+    const NGPhysicalFragment& target) {
+  InclusiveAncestorFilter inclusive_ancestors_of(target);
+  Vector<Result> results;
+  CollectInlineFragments(container, {}, inclusive_ancestors_of, &results);
+  std::reverse(results.begin(), results.end());
+  return results;
+}
+
+// static
+Vector<Result> NGInlineFragmentTraversal::AncestorsOf(
+    const NGPhysicalContainerFragment& container,
+    const NGPhysicalFragment& target) {
+  Vector<Result> results = InclusiveAncestorsOf(container, target);
+  DCHECK(results.size());
+  DCHECK_EQ(results.front().fragment, &target);
+  results.erase(results.begin());
+  return results;
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/layout/ng/inline/ng_inline_fragment_iterator.h

@@ -5,6 +5,8 @@
 #ifndef NGInlineFragmentIterator_h
 #define NGInlineFragmentIterator_h
 
+// TODO(xiaochengh): Rename this file into ng_inline_fragment_traversal.h
+
 #include "core/CoreExport.h"
 #include "core/layout/ng/ng_physical_fragment.h"
 #include "platform/wtf/Allocator.h"
@@ -15,7 +17,43 @@ namespace blink {
 class LayoutObject;
 class NGPhysicalBoxFragment;
 class NGPhysicalContainerFragment;
-struct NGPhysicalOffset;
+
+// Utility class for traversing the physical fragment tree.
+class CORE_EXPORT NGInlineFragmentTraversal {
+  STATIC_ONLY(NGInlineFragmentTraversal);
+
+ public:
+  // Return list of ancestors from |target| to |container|. Offsets are relative
+  // to |container|.
+  static Vector<NGPhysicalFragmentWithOffset> AncestorsOf(
+      const NGPhysicalContainerFragment& container,
+      const NGPhysicalFragment& target);
+
+  // Return list inclusive ancestors from |target| to |container|. Offsets are
+  // relative to |container|.
+  static Vector<NGPhysicalFragmentWithOffset> InclusiveAncestorsOf(
+      const NGPhysicalContainerFragment& container,
+      const NGPhysicalFragment& target);
+
+  // Returns list of descendants in preorder. Offsets are relative to
+  // specified fragment.
+  static Vector<NGPhysicalFragmentWithOffset> DescendantsOf(
+      const NGPhysicalContainerFragment&);
+
+  // Returns list of inclusive descendants in preorder. Offsets are relative to
+  // specified fragment.
+  static Vector<NGPhysicalFragmentWithOffset> InclusiveDescendantsOf(
+      const NGPhysicalFragment&);
+
+  // Returns list of inline fragments produced from the specified LayoutObject.
+  // The search is restricted in the subtree of |container|.
+  static Vector<NGPhysicalFragmentWithOffset, 1> SelfFragmentsOf(
+      const NGPhysicalContainerFragment& container,
+      const LayoutObject* target);
+};
+
+// TODO(xiaochengh): Convert clients of NGInlineFragmentIterator to use
+// NGInlineFragmentTraversal::SelfFragmentsOf().
 
 // Iterate through inline descendant fragments.
 class CORE_EXPORT NGInlineFragmentIterator {
@@ -33,11 +71,6 @@ class CORE_EXPORT NGInlineFragmentIterator {
   Results::const_iterator end() const { return results_.end(); }
 
  private:
-  static void CollectInlineFragments(const NGPhysicalContainerFragment&,
-                                     NGPhysicalOffset offset_to_container_box,
-                                     const LayoutObject* filter,
-                                     Results*);
-
   Results results_;
 };
 

third_party/WebKit/Source/core/layout/ng/inline/ng_inline_fragment_traversal_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/inline/ng_inline_fragment_iterator.h"
+
+#include "core/layout/ng/inline/ng_physical_text_fragment.h"
+#include "core/layout/ng/ng_layout_test.h"
+#include "core/layout/ng/ng_physical_box_fragment.h"
+
+namespace blink {
+
+class NGInlineFragmentTraversalTest
+    : public NGLayoutTest,
+      private ScopedLayoutNGPaintFragmentsForTest {
+ public:
+  NGInlineFragmentTraversalTest()
+      : NGLayoutTest(), ScopedLayoutNGPaintFragmentsForTest(true) {}
+
+ protected:
+  const NGPhysicalBoxFragment& GetRootFragmentById(const char* id) const {
+    const Element* element = GetElementById(id);
+    DCHECK(element) << id;
+    const LayoutObject* layout_object = element->GetLayoutObject();
+    DCHECK(layout_object) << element;
+    DCHECK(layout_object->IsLayoutBlockFlow()) << element;
+    DCHECK(ToLayoutBlockFlow(layout_object)->CurrentFragment()) << element;
+    return *ToLayoutBlockFlow(layout_object)->CurrentFragment();
+  }
+
+  const NGPhysicalFragment& GetFragmentOfNode(
+      const NGPhysicalContainerFragment& container,
+      const Node* node) const {
+    const LayoutObject* layout_object = node->GetLayoutObject();
+    auto fragments =
+        NGInlineFragmentTraversal::SelfFragmentsOf(container, layout_object);
+    return *fragments.front().fragment;
+  }
+};
+
+#define EXPECT_NEXT_BOX(iter, id)                                           \
+  {                                                                         \
+    const auto& current = *iter++;                                          \
+    EXPECT_TRUE(current.fragment->IsBox()) << current.fragment->ToString(); \
+    EXPECT_EQ(GetLayoutObjectByElementId(id),                               \
+              current.fragment->GetLayoutObject());                         \
+  }
+
+#define EXPECT_NEXT_LINE_BOX(iter)             \
+  {                                            \
+    const auto& current = *iter++;             \
+    EXPECT_TRUE(current.fragment->IsLineBox()) \
+        << current.fragment->ToString();       \
+  }
+
+#define EXPECT_NEXT_TEXT(iter, content)                                      \
+  {                                                                          \
+    const auto& current = *iter++;                                           \
+    EXPECT_TRUE(current.fragment->IsText()) << current.fragment->ToString(); \
+    EXPECT_EQ(content, ToNGPhysicalTextFragment(current.fragment)->Text());  \
+  }
+
+TEST_F(NGInlineFragmentTraversalTest, DescendantsOf) {
+  SetBodyInnerHTML(
+      "<style>* { border: 1px solid}</style>"
+      "<div id=t>foo<b id=b>bar</b><br>baz</div>");
+  const auto descendants =
+      NGInlineFragmentTraversal::DescendantsOf(GetRootFragmentById("t"));
+  auto iter = descendants.begin();
+
+  EXPECT_NEXT_LINE_BOX(iter);
+  EXPECT_NEXT_TEXT(iter, "foo");
+  EXPECT_NEXT_BOX(iter, "b");
+  EXPECT_NEXT_TEXT(iter, "bar");
+  EXPECT_NEXT_TEXT(iter, "\n");
+  EXPECT_NEXT_LINE_BOX(iter);
+  EXPECT_NEXT_TEXT(iter, "baz");
+  EXPECT_EQ(iter, descendants.end());
+}
+
+TEST_F(NGInlineFragmentTraversalTest, InclusiveDescendantsOf) {
+  SetBodyInnerHTML(
+      "<style>* { border: 1px solid}</style>"
+      "<div id=t>foo<b id=b>bar</b><br>baz</div>");
+  auto descendants = NGInlineFragmentTraversal::InclusiveDescendantsOf(
+      GetRootFragmentById("t"));
+  auto iter = descendants.begin();
+
+  EXPECT_NEXT_BOX(iter, "t");
+  EXPECT_NEXT_LINE_BOX(iter);
+  EXPECT_NEXT_TEXT(iter, "foo");
+  EXPECT_NEXT_BOX(iter, "b");
+  EXPECT_NEXT_TEXT(iter, "bar");
+  EXPECT_NEXT_TEXT(iter, "\n");
+  EXPECT_NEXT_LINE_BOX(iter);
+  EXPECT_NEXT_TEXT(iter, "baz");
+  EXPECT_EQ(iter, descendants.end());
+}
+
+TEST_F(NGInlineFragmentTraversalTest, SelfFragmentsOf) {
+  SetBodyInnerHTML(
+      "<style>* { border: 1px solid}</style>"
+      "<div id=t>foo<b id=filter>bar<br>baz</b>bla</div>");
+  const auto descendants = NGInlineFragmentTraversal::SelfFragmentsOf(
+      GetRootFragmentById("t"), GetLayoutObjectByElementId("filter"));
+  auto iter = descendants.begin();
+
+  // <b> generates two box fragments since its content is in two lines.
+  EXPECT_NEXT_BOX(iter, "filter");
+  EXPECT_NEXT_BOX(iter, "filter");
+  EXPECT_EQ(iter, descendants.end());
+}
+
+TEST_F(NGInlineFragmentTraversalTest, AncestorsOf) {
+  SetBodyInnerHTML(
+      "<style>* { border: 1px solid}</style>"
+      "<div id=t>x"
+      "<b id=b>y<i id=i>z<u id=target>foo</u>z</i>y</b>"
+      "x</div>");
+  const NGPhysicalContainerFragment& root = GetRootFragmentById("t");
+  const NGPhysicalFragment& target =
+      GetFragmentOfNode(root, GetElementById("target")->firstChild());
+  auto ancestors = NGInlineFragmentTraversal::AncestorsOf(root, target);
+  auto iter = ancestors.begin();
+
+  EXPECT_NEXT_BOX(iter, "target");
+  EXPECT_NEXT_BOX(iter, "i");
+  EXPECT_NEXT_BOX(iter, "b");
+  EXPECT_NEXT_LINE_BOX(iter);
+  EXPECT_EQ(iter, ancestors.end());
+}
+
+TEST_F(NGInlineFragmentTraversalTest, InclusiveAncestorsOf) {
+  SetBodyInnerHTML(
+      "<style>* { border: 1px solid}</style>"
+      "<div id=t>x"
+      "<b id=b>y<i id=i>z<u id=target>foo</u>z</i>y</b>"
+      "x</div>");
+  const NGPhysicalContainerFragment& root = GetRootFragmentById("t");
+  const NGPhysicalFragment& target =
+      GetFragmentOfNode(root, GetElementById("target")->firstChild());
+  auto ancestors =
+      NGInlineFragmentTraversal::InclusiveAncestorsOf(root, target);
+  auto iter = ancestors.begin();
+
+  EXPECT_NEXT_TEXT(iter, "foo");
+  EXPECT_NEXT_BOX(iter, "target");
+  EXPECT_NEXT_BOX(iter, "i");
+  EXPECT_NEXT_BOX(iter, "b");
+  EXPECT_NEXT_LINE_BOX(iter);
+  EXPECT_EQ(iter, ancestors.end());
+}
+
+}  // namespace blink