More changes to extension omnibox API for styles.

chrome/browser/extensions/extension_omnibox_api.cc

 // Copyright (c) 2010 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/extensions/extension_omnibox_api.h"
 
 #include "base/json/json_writer.h"
 #include "base/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_event_router.h"
@@ skipping 108 matching lines @@
 
   return true;
 }
 
 ExtensionOmniboxSuggestion::ExtensionOmniboxSuggestion() {}
 
 ExtensionOmniboxSuggestion::~ExtensionOmniboxSuggestion() {}
 
 bool ExtensionOmniboxSuggestion::ReadStylesFromValue(
     const ListValue& styles_value) {
-  // Start with a NONE style covering the entire range. As we iterate over the
-  // styles, bisect or overwrite the running style list with each new style.
   description_styles.clear();
-  description_styles.push_back(
-      ACMatchClassification(0, ACMatchClassification::NONE));
+
+  // Step 1: Build a vector of styles, 1 per character of description text.
+  std::vector<int> styles;
+  styles.resize(description.length());  // sets all styles to 0
 
   for (size_t i = 0; i < styles_value.GetSize(); ++i) {
     DictionaryValue* style;
     std::string type;
     int offset;
     int length;
     if (!styles_value.GetDictionary(i, &style))
       return false;
     if (!style->GetString(kDescriptionStylesType, &type))
       return false;
-    if (!style->GetInteger(kDescriptionStylesOffset, &offset))
+    if (!style->GetInteger(kDescriptionStylesOffset, &offset) || offset < 0)
       return false;
-    if (!style->GetInteger(kDescriptionStylesLength, &length))
-      return false;
+    if (!style->GetInteger(kDescriptionStylesLength, &length) || length < 0)

[asargent_no_longer_on_chrome, 2010/11/16 19:05:02]

It seems like passing a negative length value is likely to indicate a bug in the
calling extension's arithmetic. Would it make more sense to either return an
error or just ignore this style (ie treat it the same as length 0)? Your call.

On a related note, it might actually be useful to be able to send negative
values for the *offset* - in python I find it's often convenient to use negative
indices on strings and arrays to do things like "give me the last element in
this array" or "give me the last 5 characters from this string". 

[Matt Perry, 2010/11/16 22:26:44]

The negative checks here are just me being paranoid - the JSON schema should
prohibit negative values in the extension process.

If GetInteger fails, that means that length was omitted, which means "to the end
of the range".
That's a good idea. Using python-style ranges would probably be fairly intuitive
for developers.

+      length = description.length();
 
     int type_class =
         (type == "url") ? ACMatchClassification::URL :
         (type == "match") ? ACMatchClassification::MATCH :
         (type == "dim") ? ACMatchClassification::DIM : -1;
     if (type_class == -1)
       return false;
 
-    InsertNewStyle(type_class, offset, length);
+    for (int j = offset;
+         j < offset + length && j < static_cast<int>(styles.size()); ++j)
+      styles[j] |= type_class;
+  }
+
+  // Step 2: Convert the vector into continous runs of common styles.
+  for (size_t i = 0; i < styles.size(); ++i) {
+    if (i == 0 || styles[i] != styles[i-1])
+      description_styles.push_back(ACMatchClassification(i, styles[i]));
   }
 
   return true;
 }
 
-void ExtensionOmniboxSuggestion::InsertNewStyle(int type,
-                                                size_t offset,
-                                                size_t length) {
-  // Find the first and last existing styles that this new style overlaps. Those
-  // will have to be removed (if they completely overlap), or bisected.
-  size_t start = 0, end = 0;
-  while (end < description_styles.size()) {
-    if (start < description_styles.size() &&
-        description_styles[start].offset < offset)
-      ++start;
-    if (description_styles[end].offset <= offset + length) {
-      ++end;
-    } else {
-      break;
-    }
-  }
-
-  DCHECK_GT(end, 0u);
-  DCHECK(start == description_styles.size() ||
-         description_styles[start].offset >= offset);
-  DCHECK(end == description_styles.size() ||
-         description_styles[end].offset > offset + length);
-
-  // The last style in the overlapping range will come after our new style.
-  int last_style = description_styles[end - 1].style;
-
-  // Remove all overlapping styles.
-  if (start < end) {
-    description_styles.erase(description_styles.begin() + start,
-                             description_styles.begin() + end);
-  }
-
-  // Insert our new style.
-  description_styles.insert(description_styles.begin() + start,
-                            ACMatchClassification(offset, type));
-  if (offset + length < description.length()) {
-     description_styles.insert(description_styles.begin() + start + 1,
-                               ACMatchClassification(offset + length,
-                                                     last_style));
-  }
-}
-
 ExtensionOmniboxSuggestions::ExtensionOmniboxSuggestions() : request_id(0) {}
 
 ExtensionOmniboxSuggestions::~ExtensionOmniboxSuggestions() {}