autocomplete: Add AutocompleteMatch::MergeClassifications()

chrome/browser/autocomplete/shortcuts_provider.cc

 // Copyright (c) 2012 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 "chrome/browser/autocomplete/shortcuts_provider.h"
 
 #include <algorithm>
 #include <cmath>
 #include <map>
 #include <vector>
@@ skipping 277 matching lines @@
               ACMatchClassification(word_end, ACMatchClassification::NONE));
         }
         last_position = word_end;
         break;
       }
     }
     last_position = std::max(last_position, next_character);
   }
 
   // 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
     ShortcutsProvider::FindFirstMatch(const string16& keyword,
                                       history::ShortcutsBackend* backend) {
   DCHECK(backend);
   history::ShortcutsBackend::ShortcutMap::const_iterator it =
       backend->shortcuts_map().lower_bound(keyword);
   // Lower bound not necessarily matches the keyword, check for item pointed by
   // the lower bound iterator to at least start with keyword.
@@ skipping 31 matching lines @@
   // (1.0 / each 5 additional hits), up to a maximum of 5x as long.
   const double kMaxDecaySpeedDivisor = 5.0;
   const double kNumUsesPerDecaySpeedDivisorIncrement = 5.0;
   double decay_divisor = std::min(kMaxDecaySpeedDivisor,
       (shortcut.number_of_hits + kNumUsesPerDecaySpeedDivisorIncrement - 1) /
       kNumUsesPerDecaySpeedDivisorIncrement);
 
   return static_cast<int>((base_score / exp(decay_exponent / decay_divisor)) +
       0.5);
 }