Reland "Make NGOffsetMapping available for non-LayoutNG LayoutBlockFlow"

This is a reland of b43c6fca

The original CL had moved a call to ToString() to within
DCHECK, because the result is used only by DCHECK. But the
ToString() has a side effect that must be performed.

Because of the change, this CL breaks if DCHECK is not
enabled. Unfortunately, no build configs in trybots and
CQs have DCHECK disabled.

Reverted the change, and added TODO to clean it up.

Original change's description:
> Make NGOffsetMapping available for non-LayoutNG LayoutBlockFlow
>
> This patch allows computing NGOffsetMapping for legacy inline
> formatting context.
>
> For an experimental project "Invisible DOM"[1][2] to support
> the find-in-page feature, it is needed to compute the text
> content with whitespace collapsing applied, along with the
> mapping to the DOM offset. This patch adds an API to compute
> NGOffsetMapping even when the LayoutBlockFlow is not laid out
> by LayoutNG for that purpose.
>
> Note that the project is still in the early phase. We may
> revisit the design as it moves forward.
>
> [1] https://github.com/rakina/searchable-invisible-dom
> [2] https://www.chromestatus.com/feature/5105291213406208
>
> Bug: 636993, 873057
> Change-Id: I101b411960813a7b9b5c9c6e2db85d28737af882
> Reviewed-on: https://chromium-review.googlesource.com/c/1322184
> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
> Commit-Queue: Koji Ishii <kojii@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606292}

TBR=yoshin@chromium.org, eae@chromium.org, xiaochengh@chromium.org, rakina@chromium.org

Bug: 636993, 873057
Change-Id: I8a29073fb18a1349d758d37033e6b046a54d5beb
Reviewed-on: https://chromium-review.googlesource.com/c/1327582Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606786}

third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.cc

@@ -283,30 +283,72 @@ const NGOffsetMapping* NGInlineNode::ComputeOffsetMappingIfNeeded() {
 
   NGInlineNodeData* data = MutableData();
   if (!data->offset_mapping) {
-    // TODO(xiaochengh): ComputeOffsetMappingIfNeeded() discards the
-    // NGInlineItems and text content built by |builder|, because they are
-    // already there in NGInlineNodeData. For efficiency, we should make
-    // |builder| not construct items and text content.
-    Vector<NGInlineItem> items;
-    items.ReserveCapacity(EstimateInlineItemsCount(*GetLayoutBlockFlow()));
-    NGInlineItemsBuilderForOffsetMapping builder(&items);
-    builder.GetOffsetMappingBuilder().ReserveCapacity(
-        EstimateOffsetMappingItemsCount(*GetLayoutBlockFlow()));
-    const bool update_layout = false;
-    CollectInlinesInternal(GetLayoutBlockFlow(), &builder, nullptr,
-                           update_layout);
-    String text = builder.ToString();
+    DCHECK(!data->text_content.IsNull());
+    ComputeOffsetMapping(GetLayoutBlockFlow(), data);
+    DCHECK(data->offset_mapping);
+  }
+
+  return data->offset_mapping.get();
+}
+
+void NGInlineNode::ComputeOffsetMapping(LayoutBlockFlow* layout_block_flow,
+                                        NGInlineNodeData* data) {
+  DCHECK(!data->offset_mapping);
+  DCHECK(!layout_block_flow->GetDocument().NeedsLayoutTreeUpdate());
+
+  // TODO(xiaochengh): ComputeOffsetMappingIfNeeded() discards the
+  // NGInlineItems and text content built by |builder|, because they are
+  // already there in NGInlineNodeData. For efficiency, we should make
+  // |builder| not construct items and text content.
+  Vector<NGInlineItem> items;
+  items.ReserveCapacity(EstimateInlineItemsCount(*layout_block_flow));
+  NGInlineItemsBuilderForOffsetMapping builder(&items);
+  builder.GetOffsetMappingBuilder().ReserveCapacity(
+      EstimateOffsetMappingItemsCount(*layout_block_flow));
+  const bool update_layout = false;
+  CollectInlinesInternal(layout_block_flow, &builder, nullptr, update_layout);
+
+  // TODO(kojii): We need to call ToString() regardless the |text| is needed or
+  // not, because it includes finalizing steps. It should be separated, because
+  // it's not intuitive to require a call of ToString().
+  String text = builder.ToString();
+  if (data->text_content.IsNull())
+    data->text_content = text;
+  else
     DCHECK_EQ(data->text_content, text);
 
-    // TODO(xiaochengh): This doesn't compute offset mapping correctly when
-    // text-transform CSS property changes text length.
-    NGOffsetMappingBuilder& mapping_builder = builder.GetOffsetMappingBuilder();
-    mapping_builder.SetDestinationString(data->text_content);
-    data->offset_mapping =
-        std::make_unique<NGOffsetMapping>(mapping_builder.Build());
+  // TODO(xiaochengh): This doesn't compute offset mapping correctly when
+  // text-transform CSS property changes text length.
+  NGOffsetMappingBuilder& mapping_builder = builder.GetOffsetMappingBuilder();
+  mapping_builder.SetDestinationString(data->text_content);
+  data->offset_mapping =
+      std::make_unique<NGOffsetMapping>(mapping_builder.Build());
+  DCHECK(data->offset_mapping);
+}
+
+const NGOffsetMapping* NGInlineNode::GetOffsetMapping(
+    LayoutBlockFlow* layout_block_flow,
+    std::unique_ptr<NGOffsetMapping>* storage) {
+  DCHECK(!layout_block_flow->GetDocument().NeedsLayoutTreeUpdate());
+
+  // If |layout_block_flow| is LayoutNG, compute from |NGInlineNode|.
+  if (layout_block_flow->IsLayoutNGMixin()) {
+    NGInlineNode node(layout_block_flow);
+    if (node.IsPrepareLayoutFinished())
+      return node.ComputeOffsetMappingIfNeeded();
+
+    // When this is not laid out yet, compute each time it is requested.
+    // TODO(kojii): We could still keep the result for later uses but it would
+    // add more states. Reconsider if this turned out to be needed.
   }
 
-  return data->offset_mapping.get();
+  // If this is not LayoutNG, compute the offset mapping and store in |storage|.
+  // The caller is responsible to keep |storage| for the life cycle.
+  NGInlineNodeData data;
+  ComputeOffsetMapping(layout_block_flow, &data);
+  *storage = std::move(data.offset_mapping);
+  DCHECK(*storage);
+  return storage->get();
 }
 
 // Depth-first-scan of all LayoutInline and LayoutText nodes that make up this

third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h

@@ -70,6 +70,17 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode {
   // This funciton must be called with clean layout.
   const NGOffsetMapping* ComputeOffsetMappingIfNeeded();
 
+  // Get |NGOffsetMapping| for the |layout_block_flow|. If |layout_block_flow|
+  // is LayoutNG and it is already laid out, this function is the same as
+  // |ComputeOffsetMappingIfNeeded|. |storage| is not used in this case.
+  //
+  // Otherwise, this function computes |NGOffsetMapping| and store in |storage|
+  // as well as returning the pointer. The caller is responsible for keeping
+  // |storage| for the life cycle of the returned |NGOffsetMapping|.
+  static const NGOffsetMapping* GetOffsetMapping(
+      LayoutBlockFlow* layout_block_flow,
+      std::unique_ptr<NGOffsetMapping>* storage);
+
   bool IsBidiEnabled() const { return Data().is_bidi_enabled_; }
   TextDirection BaseDirection() const { return Data().BaseDirection(); }
 
@@ -121,6 +132,9 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode {
   }
   const NGInlineNodeData& EnsureData();
 
+  static void ComputeOffsetMapping(LayoutBlockFlow* layout_block_flow,
+                                   NGInlineNodeData* data);
+
   friend class NGLineBreakerTest;
   friend class NGInlineNodeLegacy;
 };

third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping_test.cc

@@ -1149,4 +1149,40 @@ TEST_F(NGOffsetMappingTest, TextOverflowEllipsis) {
   TEST_RANGE(mapping.GetRanges(), text, 0u, 1u);
 }
 
+// Test |GetOffsetMapping| which is available both for LayoutNG and for legacy.
+class NGOffsetMappingGetterTest : public RenderingTest,
+                                  public testing::WithParamInterface<bool>,
+                                  private ScopedLayoutNGForTest {
+ public:
+  NGOffsetMappingGetterTest() : ScopedLayoutNGForTest(GetParam()) {}
+};
+
+INSTANTIATE_TEST_CASE_P(NGOffsetMappingTest,
+                        NGOffsetMappingGetterTest,
+                        testing::Bool());
+
+TEST_P(NGOffsetMappingGetterTest, Get) {
+  SetBodyInnerHTML(R"HTML(
+    <div id=container>
+      Whitespaces   in this text   should be   collapsed.
+    </div>
+  )HTML");
+  LayoutBlockFlow* layout_block_flow =
+      ToLayoutBlockFlow(GetLayoutObjectByElementId("container"));
+  DCHECK(layout_block_flow->ChildrenInline());
+
+  // For the purpose of this test, ensure this is laid out by each layout
+  // engine.
+  DCHECK_EQ(layout_block_flow->IsLayoutNGMixin(), GetParam());
+
+  std::unique_ptr<NGOffsetMapping> storage;
+  const NGOffsetMapping* mapping =
+      NGInlineNode::GetOffsetMapping(layout_block_flow, &storage);
+  EXPECT_TRUE(mapping);
+  EXPECT_EQ(!storage, GetParam());  // |storage| is used only for legacy.
+
+  const String& text_content = mapping->GetText();
+  EXPECT_EQ(text_content, "Whitespaces in this text should be collapsed.");
+}
+
 }  // namespace blink