[SH-Blink] Fixes for link to text generation logic.

1. Reset range counters when new generation is requested.
2. Use ranges when selection starts and ends in the same block which has
   nested block

3. Fix crash when selection starts or ends with a non text node which
   has children. Crash is because using length of text content to find
   last position is incorrect when there are nested children. We should
   use Position::LastPositionInNode(*node) instead.

Bug: 1129650
Change-Id: I13e305f67c28aca7643903f96e258be8fbf8e9f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533123
Commit-Queue: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827375}

third_party/blink/renderer/core/editing/finder/find_buffer.cc

@@ -220,6 +220,11 @@ Node* FindBuffer::BackwardVisibleTextNode(Node& start_node) {
   return GetVisibleTextNode<BackwardDirection>(start_node);
 }
 
+bool FindBuffer::IsNodeBlockLevel(Node& node) {
+  const ComputedStyle* style = node.EnsureComputedStyle();
+  return style && !node.IsTextNode() && IsBlockLevel(style->Display());
+}
+
 FindBuffer::Results FindBuffer::FindMatches(const WebString& search_text,
                                             const blink::FindOptions options) {
   // We should return empty result if it's impossible to get a match (buffer is

third_party/blink/renderer/core/editing/finder/find_buffer.h

@@ -39,6 +39,9 @@ class CORE_EXPORT FindBuffer {
   static Node* ForwardVisibleTextNode(Node& start_node);
   static Node* BackwardVisibleTextNode(Node& start_node);
 
+  // Returns whether the given node is block level.
+  static bool IsNodeBlockLevel(Node& node);
+
   // A match result, containing the starting position of the match and
   // the length of the match.
   struct BufferMatchResult {

third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc

@@ -20,14 +20,6 @@ namespace blink {
 
 namespace {
 
-// Returns text within given positions of the node, skipping invisible children
-// and comments.
-String GetText(Node* node, int start_position, int end_position) {
-  auto range_start = Position(node, start_position);
-  auto range_end = Position(node, end_position);
-  return PlainText(EphemeralRange(range_start, range_end));
-}
-
 // Returns text content of the node, skipping invisible children and comments.
 String GetText(Node* node) {
   return PlainText(EphemeralRange::RangeOfContents(*node));
@@ -36,18 +28,24 @@ String GetText(Node* node) {
 // Returns true if text from beginning of |node| until |pos_offset| can be
 // considered empty. Otherwise, return false.
 bool IsFirstVisiblePosition(Node* node, unsigned pos_offset) {
-  return pos_offset == 0 ||
-         GetText(node, 0, pos_offset).StripWhiteSpace().IsEmpty();
+  auto range_start = Position::FirstPositionInNode(*node);
+  auto range_end = Position(node, pos_offset);
+  return pos_offset == 0 || PlainText(EphemeralRange(range_start, range_end))
+                                .StripWhiteSpace()
+                                .IsEmpty();
 }
 
 // Returns true if text from |pos_offset| until end of |node| can be considered
 // empty. Otherwise, return false.
 bool IsLastVisiblePosition(Node* node, unsigned pos_offset) {
+  auto range_start = Position(node, pos_offset);
+  auto range_end = Position::LastPositionInNode(*node);
   return pos_offset == node->textContent().length() ||
-         GetText(node, pos_offset, node->textContent().length())
+         PlainText(EphemeralRange(range_start, range_end))
              .StripWhiteSpace()
              .IsEmpty();
 }
+
 struct ForwadDirection {
   static Node* Next(const Node& node) { return FlatTreeTraversal::Next(node); }
   static Node* Next(const Node& node, const Node* stay_within) {
@@ -276,7 +274,10 @@ void TextFragmentSelectorGenerator::GenerateSelector(
   max_available_range_end_ = "";
   num_prefix_words_ = 0;
   num_suffix_words_ = 0;
+  num_range_start_words_ = 0;
+  num_range_end_words_ = 0;
   iteration_ = 0;
+  selector_ = nullptr;
 
   AdjustSelection();
   UMA_HISTOGRAM_COUNTS_1000(
@@ -404,17 +405,10 @@ void TextFragmentSelectorGenerator::GenerateExactSelector() {
   DCHECK_EQ(kExact, step_);
   DCHECK_EQ(kNeedsNewCandidate, state_);
   EphemeralRangeInFlatTree ephemeral_range(selection_range_);
-  Node* start_container =
-      ephemeral_range.StartPosition().ComputeContainerNode();
-  Node* end_container = ephemeral_range.EndPosition().ComputeContainerNode();
-
-  Node& start_first_block_ancestor =
-      FindBuffer::GetFirstBlockLevelAncestorInclusive(*start_container);
-  Node& end_first_block_ancestor =
-      FindBuffer::GetFirstBlockLevelAncestorInclusive(*end_container);
 
   // If not in same node, should use ranges.
-  if (!start_first_block_ancestor.isSameNode(&end_first_block_ancestor)) {
+  if (!IsInSameUninterruptedBlock(selection_range_->StartPosition(),
+                                  selection_range_->EndPosition())) {
     step_ = kRange;
     return;
   }
@@ -464,17 +458,10 @@ void TextFragmentSelectorGenerator::ExtendRangeSelector() {
       max_available_range_end_.IsEmpty()) {
     EphemeralRangeInFlatTree ephemeral_range(selection_range_);
 
-    Node& start_first_block_ancestor =
-        FindBuffer::GetFirstBlockLevelAncestorInclusive(
-            *ephemeral_range.StartPosition().ComputeContainerNode());
-    Node& end_first_block_ancestor =
-        FindBuffer::GetFirstBlockLevelAncestorInclusive(
-            *ephemeral_range.EndPosition().ComputeContainerNode());
-
     // If selection starts and ends in the same block, then split selected text
     // roughly in the middle.
-    // TODO(gayane): Should also check that there are no nested blocks.
-    if (start_first_block_ancestor.isSameNode(&end_first_block_ancestor)) {
+    if (IsInSameUninterruptedBlock(selection_range_->StartPosition(),
+                                   selection_range_->EndPosition())) {
       String selection_text = PlainText(ephemeral_range);
       selection_text.Ensure16Bit();
       int selection_length = selection_text.length();
@@ -618,4 +605,30 @@ String TextFragmentSelectorGenerator::GetNextTextBlock(
   auto range_end = Position(suffix_end, suffix_end->textContent().length());
   return PlainText(EphemeralRange(range_start, range_end)).StripWhiteSpace();
 }
+
+bool TextFragmentSelectorGenerator::IsInSameUninterruptedBlock(
+    const Position& start,
+    const Position& end) {
+  Node* start_node = start.ComputeContainerNode();
+  Node* end_node = end.ComputeContainerNode();
+
+  if (start_node->isSameNode(end_node))
+    return true;
+
+  Node& start_ancestor =
+      FindBuffer::GetFirstBlockLevelAncestorInclusive(*start_node);
+  Node& end_ancestor =
+      FindBuffer::GetFirstBlockLevelAncestorInclusive(*end_node);
+
+  if (!start_ancestor.isSameNode(&end_ancestor))
+    return false;
+
+  Node* node = start_node;
+  while (!node->isSameNode(end_node)) {
+    if (FindBuffer::IsNodeBlockLevel(*node))
+      return false;
+    node = FlatTreeTraversal::Next(*node);
+  }
+  return true;
+}
 }  // namespace blink

third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h

@@ -88,6 +88,10 @@ class CORE_EXPORT TextFragmentSelectorGenerator final
   String GetNextTextBlockForTesting(const Position& position) {
     return GetNextTextBlock(position);
   }
+  bool IsInSameUninterruptedBlockForTesting(const Position& start,
+                                            const Position& end) {
+    return IsInSameUninterruptedBlock(start, end);
+  }
 
   // Releases members if necessary.
   void ClearSelection();
@@ -128,6 +132,10 @@ class CORE_EXPORT TextFragmentSelectorGenerator final
   // boundaries.
   String GetNextTextBlock(const Position& position);
 
+  // Returns true if start and end positions are in the same block and there are
+  // no other blocks between them. Otherwise, returns false.
+  bool IsInSameUninterruptedBlock(const Position& start, const Position& end);
+
   void GenerateExactSelector();
   void ExtendRangeSelector();
   void ExtendContext();

third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator_test.cc

@@ -540,7 +540,7 @@ text text text text text text text text text text and last text",
                       .length());
 
   GenerateAndVerifySelector(second_selected_start, second_selected_end,
-                            "paragraph%20text%20text,and%20last%20text");
+                            "paragraph%20text,last%20text");
 }
 
 // When using all the selected text for the range is not enough for unique
@@ -678,7 +678,7 @@ TEST_F(TextFragmentSelectorGeneratorTest,
 }
 
 // Check the case when selections starts with an non text node.
-TEST_F(TextFragmentSelectorGeneratorTest, StartswithImage) {
+TEST_F(TextFragmentSelectorGeneratorTest, StartsWithImage) {
   SimRequest request("https://example.com/test.html", "text/html");
   LoadURL("https://example.com/test.html");
   request.Complete(R"HTML(
@@ -696,6 +696,27 @@ TEST_F(TextFragmentSelectorGeneratorTest, StartswithImage) {
   GenerateAndVerifySelector(start, end, "page-,First,-paragraph");
 }
 
+// Check the case when selections starts with an non text node.
+TEST_F(TextFragmentSelectorGeneratorTest, StartsWithBlockWithImage) {
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+    <!DOCTYPE html>
+    <div>Test page</div>
+    <div id="img_div">
+      <img id="img">
+    </div>
+    <p id='first'>First paragraph text that is longer  than 20 chars</p>
+  )HTML");
+  Node* img = GetDocument().getElementById("img_div");
+  Node* first_paragraph = GetDocument().getElementById("first")->firstChild();
+  const auto& start = Position(img, 0);
+  const auto& end = Position(first_paragraph, 5);
+  ASSERT_EQ("\nFirst", PlainText(EphemeralRange(start, end)));
+
+  GenerateAndVerifySelector(start, end, "page-,First,-paragraph");
+}
+
 // Check the case when selections ends with an non text node.
 TEST_F(TextFragmentSelectorGeneratorTest, EndswithImage) {
   SimRequest request("https://example.com/test.html", "text/html");
@@ -752,6 +773,24 @@ TEST_F(TextFragmentSelectorGeneratorTest, EndIsStartofNextBlock) {
   GenerateAndVerifySelector(start, end, "First%20paragraph,-Second");
 }
 
+// Checks that for short selection that have nested block element range selector
+// is used.
+TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode_Interrupted) {
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+    <!DOCTYPE html>
+    <div id='first'>First <div>block text</div> paragraph text</div>
+  )HTML");
+  Node* first_paragraph = GetDocument().getElementById("first")->firstChild();
+  const auto& start = Position(first_paragraph, 0);
+  const auto& end = Position(first_paragraph->nextSibling()->nextSibling(), 10);
+  ASSERT_EQ("First\nblock text\nparagraph",
+            PlainText(EphemeralRange(start, end)));
+
+  GenerateAndVerifySelector(start, end, "First,paragraph");
+}
+
 // Basic test case for |GetNextTextBlock|.
 TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock) {
   SimRequest request("https://example.com/test.html", "text/html");
@@ -1320,4 +1359,68 @@ TEST_F(TextFragmentSelectorGeneratorTest,
                                          ->GetNextTextBlockForTesting(end));
 }
 
+// Checks that selection in the same text node is considerered uninterrupted.
+TEST_F(TextFragmentSelectorGeneratorTest,
+       IsInSameUninterruptedBlock_OneTextNode) {
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+    <!DOCTYPE html>
+    <div id='first'>First paragraph text</div>
+  )HTML");
+  Node* first_paragraph = GetDocument().getElementById("first")->firstChild();
+  const auto& start = Position(first_paragraph, 0);
+  const auto& end = Position(first_paragraph, 15);
+  ASSERT_EQ("First paragraph", PlainText(EphemeralRange(start, end)));
+
+  EXPECT_TRUE(GetDocument()
+                  .GetFrame()
+                  ->GetTextFragmentSelectorGenerator()
+                  ->IsInSameUninterruptedBlockForTesting(start, end));
+}
+
+// Checks that selection in the same text node with nested non-block element is
+// considerered uninterrupted.
+TEST_F(TextFragmentSelectorGeneratorTest,
+       IsInSameUninterruptedBlock_NonBlockInterruption) {
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+    <!DOCTYPE html>
+    <div id='first'>First <i>styled text</i> paragraph text</div>
+  )HTML");
+  Node* first_paragraph = GetDocument().getElementById("first")->firstChild();
+  const auto& start = Position(first_paragraph, 0);
+  const auto& end = Position(first_paragraph->nextSibling()->nextSibling(), 10);
+  ASSERT_EQ("First styled text paragraph",
+            PlainText(EphemeralRange(start, end)));
+
+  EXPECT_TRUE(GetDocument()
+                  .GetFrame()
+                  ->GetTextFragmentSelectorGenerator()
+                  ->IsInSameUninterruptedBlockForTesting(start, end));
+}
+
+// Checks that selection in the same text node with nested block element is
+// considerered interrupted.
+TEST_F(TextFragmentSelectorGeneratorTest,
+       IsInSameUninterruptedBlock_BlockInterruption) {
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+    <!DOCTYPE html>
+    <div id='first'>First <div>block text</div> paragraph text</div>
+  )HTML");
+  Node* first_paragraph = GetDocument().getElementById("first")->firstChild();
+  const auto& start = Position(first_paragraph, 0);
+  const auto& end = Position(first_paragraph->nextSibling()->nextSibling(), 10);
+  ASSERT_EQ("First\nblock text\nparagraph",
+            PlainText(EphemeralRange(start, end)));
+
+  EXPECT_FALSE(GetDocument()
+                   .GetFrame()
+                   ->GetTextFragmentSelectorGenerator()
+                   ->IsInSameUninterruptedBlockForTesting(start, end));
+}
+
 }  // namespace blink