autocomplete: Add AutocompleteMatch::MergeClassifications()

chrome/browser/autocomplete/shortcuts_provider.cc

Index: chrome/browser/autocomplete/shortcuts_provider.cc
diff --git a/chrome/browser/autocomplete/shortcuts_provider.cc b/chrome/browser/autocomplete/shortcuts_provider.cc
index 5e320e7d26e4e6c87ffcb7a5b4340d9be49312a1..9c0f575ca5dce1de8da84202074d5a19be7f3920 100644
--- a/chrome/browser/autocomplete/shortcuts_provider.cc
+++ b/chrome/browser/autocomplete/shortcuts_provider.cc
@@ -296,23 +296,12 @@ ACMatchClassifications ShortcutsProvider::ClassifyAllMatchesInString(
 
   // Merge match-marking data with original classifications.
   if ((match_class.size() == 1) &&

[Peter Kasting, 2012/09/10 18:56:11]

This conditional can be entirely removed once the guard is added to
MergeClassifications() (see comments in autocomplete_match.cc).

[Daniel Erat, 2012/09/10 19:36:17]

I had to update ShortcutsProviderTest.ClassifyAllMatchesInString(); the
following test expected behavior that's changed:

  class ClassifyTest classify_test5((string16()), ACMatchClassifications());
  ACMatchClassifications spans_j = classify_test5.RunTest(ASCIIToUTF16("man"));
  ASSERT_EQ(0U, spans_j.size());

The comment above ACMatchClassification's definition says, "All text must be
associated with some kind of classification.  Even if a match has no distinct
segments, its vector should contain an entry at offset 0 with no flags."

Should MergeClassifications() enforce this?  Right now, if classifications1 is
"0,0" and classifications2 is empty, we'll return an empty vector.  Would you
prefer that I explicitly check the empty case in MergeClassifications() and
return a valid "0,0" vector instead of possibly returning an empty vector?

-      (match_class.back().style == ACMatchClassification::NONE))
+      (match_class.back().style == ACMatchClassification::NONE)) {
     return original_class;
-  ACMatchClassifications output;
-  for (ACMatchClassifications::const_iterator i = original_class.begin(),
-       j = match_class.begin(); i != original_class.end();) {
-    AutocompleteMatch::AddLastClassificationIfNecessary(&output,
-        std::max(i->offset, j->offset), i->style | j->style);
-    const size_t next_i_offset = (i + 1) == original_class.end() ?
-        static_cast<size_t>(-1) : (i + 1)->offset;
-    const size_t next_j_offset = (j + 1) == match_class.end() ?
-        static_cast<size_t>(-1) : (j + 1)->offset;
-    if (next_i_offset >= next_j_offset)
-      ++j;
-    if (next_j_offset >= next_i_offset)
-      ++i;
+  } else {
+    return AutocompleteMatch::MergeClassifications(original_class,
+        match_class);
   }
-  return output;
 }
 
 history::ShortcutsBackend::ShortcutMap::const_iterator