autocomplete: Add AutocompleteMatch::MergeClassifications()

chrome/browser/autocomplete/autocomplete_match.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/autocomplete_match.h"
 
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
@@ skipping 215 matching lines @@
       (style | ACMatchClassification::MATCH) & ~ACMatchClassification::DIM));
 
   // Mark post-match portion of string (if any).
   const size_t after_match(match_location + match_length);
   if (after_match < overall_length) {
     classification->push_back(ACMatchClassification(after_match, style));
   }
 }
 
 // static
+AutocompleteMatch::ACMatchClassifications
+AutocompleteMatch::MergeClassifications(

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

Nit: Style guide is vague here; I usually suggest this line be indented 4

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

Done.

+    const ACMatchClassifications& classifications1,
+    const ACMatchClassifications& classifications2) {
+  ACMatchClassifications output;
+  for (ACMatchClassifications::const_iterator i = classifications1.begin(),

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

This is not quite safe.  If someone passes in an empty vector for
|classifications2| we'll deref garbage.

This function should get a guard like:

namespace {
bool IsTrivialClassification(const ACMatchClassifications& classifications) {
  return classifications.empty() ||
      ((classifications.size() == 1) &&
       (classifications.back().style == ACMatchClassification::NONE));
}
}

...

  if IsTrivialClassification(classifications1)
    return classifications2;
  if IsTrivialClassification(classifications2)
    return classifications1;
  ... (rest of function) ...

Then there should probably be some unittest cases for this.

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

Done.

+       j = classifications2.begin(); i != classifications1.end();) {
+    AutocompleteMatch::AddLastClassificationIfNecessary(&output,
+        std::max(i->offset, j->offset), i->style | j->style);
+    const size_t next_i_offset = (i + 1) == classifications1.end() ?
+        static_cast<size_t>(-1) : (i + 1)->offset;
+    const size_t next_j_offset = (j + 1) == classifications2.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;
+  }
+
+  return output;
+}
+
+// static
 std::string AutocompleteMatch::ClassificationsToString(
     const ACMatchClassifications& classifications) {
   std::string serialized_classifications;
   for (size_t i = 0; i < classifications.size(); ++i) {
     if (i)
       serialized_classifications += ',';
     serialized_classifications += base::IntToString(classifications[i].offset) +
         ',' + base::IntToString(classifications[i].style);
   }
   return serialized_classifications;
@@ skipping 156 matching lines @@
         << " is unsorted in relation to last offset of " << last_offset
         << ". Provider: " << (provider ? provider->name() : "None") << ".";
     DCHECK_LT(i->offset, text.length())
         << " Classification of [" << i->offset << "," << text.length()
         << "] is out of bounds for \"" << text << "\". Provider: "
         << (provider ? provider->name() : "None") << ".";
     last_offset = i->offset;
   }
 }
 #endif