More changes to extension omnibox API for styles.

chrome/browser/extensions/extension_omnibox_api.cc

Index: chrome/browser/extensions/extension_omnibox_api.cc
diff --git a/chrome/browser/extensions/extension_omnibox_api.cc b/chrome/browser/extensions/extension_omnibox_api.cc
index 705771f8e02b90141dd8c071adeaf6a57587ac31..61531c51abd1f62b219986630d14ed0238ac90bf 100644
--- a/chrome/browser/extensions/extension_omnibox_api.cc
+++ b/chrome/browser/extensions/extension_omnibox_api.cc
@@ -126,11 +126,11 @@ 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;
@@ -141,10 +141,10 @@ bool ExtensionOmniboxSuggestion::ReadStylesFromValue(
       return false;
     if (!style->GetString(kDescriptionStylesType, &type))
       return false;
-    if (!style->GetInteger(kDescriptionStylesOffset, &offset))
-      return false;
-    if (!style->GetInteger(kDescriptionStylesLength, &length))
+    if (!style->GetInteger(kDescriptionStylesOffset, &offset) || offset < 0)
       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 :
@@ -153,52 +153,18 @@ bool ExtensionOmniboxSuggestion::ReadStylesFromValue(
     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;
   }
 
-  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;
-    }
+  // 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]));
   }
 
-  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));
-  }
+  return true;
 }
 
 ExtensionOmniboxSuggestions::ExtensionOmniboxSuggestions() : request_id(0) {}