Replace 'descriptionStyles' in omnibox API with a simple xml

syntax. This should make it a bit easier to use.

BUG=64539
TEST=n/a

Review URL: http://codereview.chromium.org/5271009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67687 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 93 insertions, 87 deletions

diff --git a/chrome/browser/extensions/extension_omnibox_apitest.cc b/chrome/browser/extensions/extension_omnibox_apitest.cc
index defa068..854662d 100644
--- a/chrome/browser/extensions/extension_omnibox_apitest.cc
+++ b/chrome/browser/extensions/extension_omnibox_apitest.cc
@@ -147,7 +147,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, MAYBE_Basic) {
     EXPECT_EQ(ACMatchClassification::MATCH,
               result.match_at(1).contents_class[1].style);
 
-    EXPECT_EQ(description.find('>'),
+    EXPECT_EQ(description.find('>') + 1u,
               result.match_at(1).contents_class[2].offset);
     EXPECT_EQ(ACMatchClassification::NONE,
               result.match_at(1).contents_class[2].style);
@@ -157,7 +157,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, MAYBE_Basic) {
     EXPECT_EQ(ACMatchClassification::DIM,
               result.match_at(1).contents_class[3].style);
 
-    EXPECT_EQ(description.find(']'),
+    EXPECT_EQ(description.find(']') + 1u,
               result.match_at(1).contents_class[4].offset);
     EXPECT_EQ(ACMatchClassification::NONE,
               result.match_at(1).contents_class[4].style);
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index 4463a3c..1cb4a0e 100644
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -3941,20 +3941,7 @@
           "description": {
             "type": "string",
             "minLength": 1,
-            "description": "The text that is displayed in the URL dropdown. Can optionally be stylized by the descriptionStyles parameter."
-          },
-          "descriptionStyles": {
-            "type": "array",
-            "optional": true,
-            "description": "An array of style objects, created using styleUrl, styleMatch, or styleDim. A style applies to the region of text specified by the style's starting offset and length. If there are any overlapping regions of text covered by multiple styles, they will be added together (e.g. 'match' + 'dim' will display a dimmed match). Not all style combinations will be visually distinct (e.g. 'url' + 'dim' may look identical to 'url').",
-            "items": {
-              "type": "object",
-              "properties": {
-                "type": {"type": "string", "enum": ["url", "match", "dim"]},
-                "offset": {"type": "integer"},
-                "length": {"type": "integer", "optional": true}
-              }
-            }
+            "description": "The text that is displayed in the URL dropdown. Can contain XML-style markup for styling. The supported tags are 'url' (for a literal URL), 'match' (for highlighting text that matched what the user's query), and 'dim' (for dim helper text). The styles can be nested, eg. <dim><match>dimmed match</match></dim>."
           }
         }
       }
@@ -3971,7 +3958,8 @@
             "type": "array",
             "description": "Array of suggest results",
             "items": {
-              "$ref": "SuggestResult"
+              "type": "object",
+              "additionalProperties": { "type": "any" }
             }
           }
         ]
@@ -3986,53 +3974,14 @@
             "name": "suggestion",
             "description": "A partial SuggestResult object, without the 'content' parameter. See SuggestResult for a description of the parameters.",
             "properties": {
-	          "description": {
-	            "type": "string",
-	            "minLength": 1,
-                    "description": "The text to display in the default suggestion. The placeholder string '%s' can be included and will be replaced with the user's input."
-	          },
-	          "descriptionStyles": {
-	            "type": "array",
-	            "optional": true,
-	            "items": {
-	              "type": "object",
-	              "properties": {
-	                "type": {"type": "string", "enum": ["url", "match", "dim"]},
-	                "offset": {"type": "integer"},
-	                "length": {"type": "integer", "optional": true}
-	              }
-	            }
-	          }
+              "description": {
+                "type": "string",
+                "minLength": 1,
+                "description": "The text to display in the default suggestion. The placeholder string '%s' can be included and will be replaced with the user's input."
+              }
             }
           }
         ]
-      },
-      {
-        "name": "styleUrl",
-        "type": "function",
-        "description": "Constructor for the descriptionStyles parameter of the suggest callback. This style designates a region of text matching a URL or filename. A negative offset means the region will start at that many characters from the end of the description. If length is omitted, the region will apply for the rest of the description text.",
-        "parameters": [
-           {"type": "integer", "name": "offset"},
-           {"type": "integer", "name": "length", "minimum": 0, "optional": true}
-        ]
-      },
-      {
-        "name": "styleMatch",
-        "type": "function",
-        "description": "Constructor for the descriptionStyles parameter of the suggest callback. This style designates a region of text matching what the user typed. A negative offset means the region will start at that many characters from the end of the description. If length is omitted, the region will apply for the rest of the description text.",
-        "parameters": [
-           {"type": "integer", "name": "offset"},
-           {"type": "integer", "name": "length", "minimum": 0, "optional": true}
-        ]
-      },
-      {
-        "name": "styleDim",
-        "type": "function",
-        "description": "Constructor for the descriptionStyles parameter of the suggest callback. This style designates a region of dim helper text. A negative offset means the region will start at that many characters from the end of the description. If length is omitted, the region will apply for the rest of the description text.",
-        "parameters": [
-           {"type": "integer", "name": "offset"},
-           {"type": "integer", "name": "length", "minimum": 0, "optional": true}
-        ]
       }
     ],
     "events": [
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index 3eeb81d..53975f8 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -285,6 +285,64 @@ var chrome = chrome || {};
     };
   }
 
+  // Parses the xml syntax supported by omnibox suggestion results. Returns an
+  // object with two properties: 'description', which is just the text content,
+  // and 'descriptionStyles', which is an array of style objects in a format
+  // understood by the C++ backend.
+  function parseOmniboxDescription(input) {
+    var domParser = new DOMParser();
+
+    // The XML parser requires a single top-level element, but we want to
+    // support things like 'hello, <match>world</match>!'. So we wrap the
+    // provided text in generated root level element.
+    var root = domParser.parseFromString(
+        '<fragment>' + input + '</fragment>', 'text/xml');
+
+    // DOMParser has a terrible error reporting facility. Errors come out nested
+    // inside the returned document.
+    var error = root.querySelector('parsererror div');
+    if (error) {
+      throw new Error(error.textContent);
+    }
+
+    // Otherwise, it's valid, so build up the result.
+    var result = {
+      description: '',
+      descriptionStyles: []
+    };
+
+    // Recursively walk the tree.
+    (function(node) {
+      for (var i = 0, child; child = node.childNodes[i]; i++) {
+        // Append text nodes to our description.
+        if (child.nodeType == Node.TEXT_NODE) {
+          result.description += child.nodeValue;
+          continue;
+        }
+
+        // Process and descend into a subset of recognized tags.
+        if (child.nodeType == Node.ELEMENT_NODE &&
+            (child.nodeName == 'dim' || child.nodeName == 'match' ||
+             child.nodeName == 'url')) {
+          var style = {
+            'type': child.nodeName,
+            'offset': result.description.length
+          };
+          result.descriptionStyles.push(style);
+          arguments.callee(child);
+          style.length = result.description.length - style.offset;
+          continue;
+        }
+
+        // Descend into all other nodes, even if they are unrecognized, for
+        // forward compat.
+        arguments.callee(child);
+      }
+    })(root);
+
+    return result;
+  }
+
   function setupOmniboxEvents(extensionId) {
     chrome.omnibox.onInputChanged.dispatch =
         function(text, requestId) {
@@ -344,16 +402,11 @@ var chrome = chrome || {};
 
           module[functionDef.name] = (function() {
             var args = arguments;
-            if (this.updateArguments) {
-              // Functions whose signature has changed can define an
-              // |updateArguments| function to transform old argument lists
-              // into the new form, preserving compatibility.
-              // TODO(skerner): Once optional args can be omitted (crbug/29215),
-              // this mechanism will become unnecessary.  Consider removing it
-              // when crbug/29215 is fixed.
-              args = this.updateArguments.apply(this, args);
-            }
+            if (this.updateArgumentsPreValidate)
+              args = this.updateArgumentsPreValidate.apply(this, args);
             chromeHidden.validate(args, this.definition.parameters);
+            if (this.updateArgumentsPostValidate)
+              args = this.updateArgumentsPostValidate.apply(this, args);
 
             var retval;
             if (this.handleRequest) {
@@ -640,6 +693,12 @@ var chrome = chrome || {};
       return id;
     };
 
+    apiFunctions["omnibox.setDefaultSuggestion"].handleRequest =
+        function(details) {
+      var parseResult = parseOmniboxDescription(details.description);
+      sendRequest(this.name, [parseResult], this.definition.parameters);
+    };
+
     apiFunctions["contextMenus.create"].customCallback =
         function(name, request, response) {
       if (chrome.extension.lastError) {
@@ -684,7 +743,8 @@ var chrome = chrome || {};
       chromeHidden.contextMenus.handlers = {};
     };
 
-    apiFunctions["tabs.captureVisibleTab"].updateArguments = function() {
+    apiFunctions["tabs.captureVisibleTab"].updateArgumentsPreValidate =
+        function() {
       // Old signature:
       //    captureVisibleTab(int windowId, function callback);
       // New signature:
@@ -703,15 +763,16 @@ var chrome = chrome || {};
       return newArgs;
     };
 
-    apiFunctions["omnibox.styleUrl"].handleRequest = function(offset, length) {
-      return {type: "url", offset: offset, length: length};
-    };
-    apiFunctions["omnibox.styleMatch"].handleRequest =
-        function(offset, length) {
-      return {type: "match", offset: offset, length: length};
-    };
-    apiFunctions["omnibox.styleDim"].handleRequest = function(offset, length) {
-      return {type: "dim", offset: offset, length: length};
+    apiFunctions["omnibox.sendSuggestions"].updateArgumentsPostValidate =
+        function(requestId, userSuggestions) {
+      var suggestions = [];
+      for (var i = 0; i < userSuggestions.length; i++) {
+        var parseResult = parseOmniboxDescription(
+            userSuggestions[i].description);
+        parseResult.content = userSuggestions[i].content;
+        suggestions.push(parseResult);
+      }
+      return [requestId, suggestions];
     };
 
     if (chrome.test) {
diff --git a/chrome/test/data/extensions/api_test/omnibox/test.html b/chrome/test/data/extensions/api_test/omnibox/test.html
index d66b6e7..50e5d4f 100644
--- a/chrome/test/data/extensions/api_test/omnibox/test.html
+++ b/chrome/test/data/extensions/api_test/omnibox/test.html
@@ -5,16 +5,12 @@ chrome.omnibox.onInputChanged.addListener(
     if (text != "suggestio")
       return;
 
-    var desc = 'Description with style: <match>, [dim], (url till end)';  
+    var desc = 'Description with style: <match>&lt;match&gt;</match>, ' +
+               '<dim>[dim]</dim>, <url>(url till end)</url>';
     suggest([
       {
         content: text + "n1",
-        description: desc,
-        descriptionStyles: [
-          chrome.omnibox.styleMatch(desc.indexOf('<'), 6),
-          chrome.omnibox.styleDim(desc.indexOf('['), 4),
-          chrome.omnibox.styleUrl(desc.indexOf('(')),
-        ]
+        description: desc
       },
       {content: text + "n2", description: "description2"},
       {content: text + "n3", description: "description3"},