[omnibox] [rich-autocompletion] Preserve cursor for split selections.

Split AC was added in CL crrev.com/c/2424985 . It allows e.g. the input
'a c' to match the text 'a [b ]c[ d]'

When generating the text selections from word matches, it removes empty
matches and merges adjacent matches. E.g. the input 'a b' produces the
selections 'a b[ c]' instead of 'a| |b[ c]'. This is desirable usually,
but not desirable for the last selection which is used to determine the
cursor position; merging or filtering the last selection incorrectly
alters the cursor.

This CL makes sure to preserve the last selection and, therefore, the
correct cursor. E.g. the input 'a c' produces selections 'a [b ]c|'
instead of 'a [b ]c'.

Bug: 1062446
Change-Id: Icfb13321d86454a8a2fbc89289fefda53287b770
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601046
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839170}

components/omnibox/browser/autocomplete_match.cc

@@ -1315,7 +1315,7 @@ bool AutocompleteMatch::TryRichAutocompletion(
     // |fill_into_edit| should already be set to |primary_text|.
     split_autocompletion = SplitAutocompletion(
         primary_text_lower,
-        InvertAndReverseRanges(primary_text_lower.length(), input_words));
+        TermMatchesToSelections(primary_text_lower.length(), input_words));
     allowed_to_be_default_match = true;
     RecordAdditionalInfo("autocompletion", "primary & split");
     return true;
@@ -1340,7 +1340,7 @@ bool AutocompleteMatch::TryRichAutocompletion(
     swapped_fill_into_edit = true;
     split_autocompletion = SplitAutocompletion(
         secondary_text_lower,
-        InvertAndReverseRanges(secondary_text_lower.length(), input_words));
+        TermMatchesToSelections(secondary_text_lower.length(), input_words));
     allowed_to_be_default_match = true;
     RecordAdditionalInfo("autocompletion", "secondary & split");
     return true;

components/omnibox/browser/inline_autocompletion_util.cc

@@ -58,22 +58,30 @@ std::vector<std::pair<size_t, size_t>> FindWordsSequentiallyAtWordbreak(
   return occurrences;
 }
 
-std::vector<gfx::Range> InvertAndReverseRanges(
+std::vector<gfx::Range> TermMatchesToSelections(
     size_t length,
-    std::vector<std::pair<size_t, size_t>> ranges) {
-  std::vector<gfx::Range> inverted;
+    std::vector<std::pair<size_t, size_t>> matches) {
+  std::vector<gfx::Range> selections;
   size_t cursor = length;
-  for (size_t i = ranges.size(); i-- != 0;) {
-    auto range = ranges[i];
-    // Skip empty ranges.
-    if (range.first == range.second)
+  for (size_t i = matches.size(); i-- != 0;) {
+    auto match = matches[i];
+    // Include the 1st iterated match as is. The 1st match determines the user's
+    // cursor, and skipping or merging the match would incorrectly move the
+    // user's cursor.
+    if (i == matches.size() - 1) {
+      selections.emplace_back(cursor, match.second);
+      cursor = match.first;
       continue;
-    // Merge adjacent ranges.
-    if (cursor != range.second)
-      inverted.emplace_back(cursor, range.second);
-    cursor = range.first;
+    }
+    // Skip empty matches.
+    if (match.first == match.second)
+      continue;
+    // Merge adjacent matches.
+    if (cursor != match.second)
+      selections.emplace_back(cursor, match.second);
+    cursor = match.first;
   }
   if (cursor != 0)
-    inverted.emplace_back(cursor, 0);
-  return inverted;
+    selections.emplace_back(cursor, 0);
+  return selections;
 }

components/omnibox/browser/inline_autocompletion_util.h

@@ -30,13 +30,14 @@ std::vector<std::pair<size_t, size_t>> FindWordsSequentiallyAtWordbreak(
     const base::string16& text,
     const base::string16& search);
 
-// Inverts and reverses |ranges| in a domain of [0, |length|). Ranges are
-// interpreted as {start, end}. E.g., if |length| is 10 and |ranges| are
-// {{2, 3} {5, 9}}, th |InvertRanges| will return {{10, 9}, {5, 3}, {2, 0}}.
-// Assumes |ranges| is in forward order; i.e. |ranges[i+1]| occurs after
-// |ranges[i]| and |ranges[i].second| after |ranges[i].first|.
-std::vector<gfx::Range> InvertAndReverseRanges(
+// Inverts and reverses term |matches| in a domain of [0, |length|) to determine
+// the selected non-matches. Ranges are interpreted as {start, end}. E.g., if
+// |length| is 10 and |matches| are {{2, 3} {5, 9}}, |InvertRanges| will return
+// {{10, 9}, {5, 3}, {2, 0}}. Assumes |matches| is in forward order; i.e.
+// |matches[i+1]| occurs after |matches[i]| and |matches[i].second| after
+// |matches[i].first|.
+std::vector<gfx::Range> TermMatchesToSelections(
     size_t length,
-    std::vector<std::pair<size_t, size_t>> ranges);
+    std::vector<std::pair<size_t, size_t>> matches);
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_INLINE_AUTOCOMPLETION_UTIL_H_

components/omnibox/browser/inline_autocompletion_util_unittest.cc

@@ -75,29 +75,33 @@ TEST(InlineAutocompletionUtilTest, FindWordsSequentiallyAtWordbreak) {
               testing::ElementsAre(pair{0, 1}, pair{3, 5}, pair{5, 6}));
 }
 
-TEST(InlineAutocompletionUtilTest, InvertAndReverseRanges) {
+TEST(InlineAutocompletionUtilTest, TermMatchesToSelections) {
   // Empty |ranges| in empty |length|.
-  EXPECT_THAT(InvertAndReverseRanges(0, {}), testing::ElementsAre());
+  EXPECT_THAT(TermMatchesToSelections(0, {}), testing::ElementsAre());
   // Empty |ranges| in non-empty |length|. 12345 -> [12345]
-  EXPECT_THAT(InvertAndReverseRanges(5, {}),
-              testing::ElementsAre(gfx::Range{5, 0}));
-  // Single empty range in |ranges|.  12|345 -> [12345]
-  EXPECT_THAT(InvertAndReverseRanges(5, {{2, 2}}),
+  EXPECT_THAT(TermMatchesToSelections(5, {}),
               testing::ElementsAre(gfx::Range{5, 0}));
+  // Single empty range in |ranges|.  12|345 -> [12][345]
+  EXPECT_THAT(TermMatchesToSelections(5, {{2, 2}}),
+              testing::ElementsAre(gfx::Range{5, 2}, gfx::Range{2, 0}));
+  // Multiple empty ranges in |ranges|.  1|2|3|4|5 -> [1234][5]
+  EXPECT_THAT(TermMatchesToSelections(5, {{1, 1}, {2, 2}, {3, 3}, {4, 4}}),
+              testing::ElementsAre(gfx::Range{5, 4}, gfx::Range{4, 0}));
   // Single range in |ranges|. 12[3]45 -> [12]3[45]
-  EXPECT_THAT(InvertAndReverseRanges(5, {{2, 3}}),
+  EXPECT_THAT(TermMatchesToSelections(5, {{2, 3}}),
               testing::ElementsAre(gfx::Range{5, 3}, gfx::Range{2, 0}));
-  // Single range in |ranges| spanning all of |length|. [12345] -> 12345
-  EXPECT_THAT(InvertAndReverseRanges(5, {{0, 5}}), testing::ElementsAre());
+  // Single range in |ranges| spanning all of |length|. [12345] -> 12345|
+  EXPECT_THAT(TermMatchesToSelections(5, {{0, 5}}),
+              testing::ElementsAre(gfx::Range{5, 5}));
   // Multiple ranges in |ranges|, including adjacent and empty ranges.
-  // 1[23][45]6[78]9 -> [1]2345[6]78[9]
-  EXPECT_THAT(InvertAndReverseRanges(9, {{1, 3}, {3, 5}, {6, 8}}),
+  // 1[23][45]6[7][8]9 -> [1]2345[6]78[9]
+  EXPECT_THAT(TermMatchesToSelections(9, {{1, 3}, {3, 5}, {6, 7}, {7, 8}}),
               testing::ElementsAre(gfx::Range{9, 8}, gfx::Range{6, 5},
                                    gfx::Range{1, 0}));
   // |ranges| ending at |length| and starting 0.
-  // [123][45]6[789] -> 12345[6]789
-  EXPECT_THAT(InvertAndReverseRanges(9, {{0, 3}, {3, 5}, {6, 9}}),
-              testing::ElementsAre(gfx::Range{6, 5}));
+  // [123][45]6[789] -> 12345[6]789|
+  EXPECT_THAT(TermMatchesToSelections(9, {{0, 3}, {3, 5}, {6, 9}}),
+              testing::ElementsAre(gfx::Range{9, 9}, gfx::Range{6, 5}));
 }
 
 }  // namespace