Make it so that chrome.bookmarks.update can update the URL of a bookmark.

This changes the API slightly. Previously, if someone called update with an empty object ({}) it would set the title to "". This was only documented in the code and not on the API page. Now that we support updating both the url and title it seems more reasonable to ignore a missing title.

BUG=34841
TEST=browser_tests.exe --gtest_filter=ExtensionApiTest.Bookmarks

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

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

5 files changed, 133 insertions, 18 deletions

diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc
index 33abc39..9385ff1 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module.cc
@@ -238,13 +238,15 @@ void ExtensionBookmarkEventRouter::BookmarkNodeChanged(
   ListValue args;
   args.Append(new StringValue(Int64ToString(node->id())));
 
-  // TODO(erikkay) The only two things that BookmarkModel sends this
-  // notification for are title and favicon.  Since we're currently ignoring
-  // favicon and since the notification doesn't say which one anyway, for now
-  // we only include title.  The ideal thing would be to change BookmarkModel
-  // to indicate what changed.
+  // TODO(erikkay) The only three things that BookmarkModel sends this
+  // notification for are title, url and favicon.  Since we're currently
+  // ignoring favicon and since the notification doesn't say which one anyway,
+  // for now we only include title and url.  The ideal thing would be to change
+  // BookmarkModel to indicate what changed.
   DictionaryValue* object_args = new DictionaryValue();
   object_args->SetString(keys::kTitleKey, node->GetTitle());
+  if (node->is_url())
+    object_args->SetString(keys::kUrlKey, node->GetURL().spec());
   args.Append(object_args);
 
   std::string json_args;
@@ -639,8 +641,22 @@ bool UpdateBookmarkFunction::RunImpl() {
   const ListValue* args = args_as_list();
   DictionaryValue* updates;
   EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &updates));
+
   std::wstring title;
-  updates->GetString(keys::kTitleKey, &title);  // Optional (empty is clear).
+  std::string url_string;
+
+  // Optional but we need to distinguish non present from an empty title.
+  const bool has_title = updates->GetString(keys::kTitleKey, &title);
+  updates->GetString(keys::kUrlKey, &url_string);  // Optional.
+
+  GURL url;
+  if (!url_string.empty()) {
+    url = GURL(url_string);
+
+    // If URL is present then it needs to be a non empty valid URL.
+    EXTENSION_FUNCTION_VALIDATE(!url.is_empty());
+    EXTENSION_FUNCTION_VALIDATE(url.is_valid());
+  }
 
   BookmarkModel* model = profile()->GetBookmarkModel();
   const BookmarkNode* node = model->GetNodeByID(ids.front());
@@ -654,7 +670,10 @@ bool UpdateBookmarkFunction::RunImpl() {
     error_ = keys::kModifySpecialError;
     return false;
   }
-  model->SetTitle(node, title);
+  if (has_title)
+    model->SetTitle(node, title);
+  if (!url.is_empty())
+    model->SetURL(node, url);
 
   DictionaryValue* ret = ExtensionBookmarks::GetNodeDictionary(node, false);
   result_.reset(ret);
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index 4e7b78e..5ade26f 100755
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -1515,14 +1515,15 @@
       {
         "name": "update",
         "type": "function",
-        "description": "Updates the properties of a bookmark or folder. Specify only the properties that you want to change; unspecified properties will be left unchanged.  <b>Note:</b> Currently, only 'title' is supported.",
+        "description": "Updates the properties of a bookmark or folder. Specify only the properties that you want to change; unspecified properties will be left unchanged.  <b>Note:</b> Currently, only 'title' and 'url' are supported.",
         "parameters": [
           {"type": "string", "name": "id"},
           {
             "type": "object",
             "name": "changes",
             "properties": {
-              "title": {"type": "string", "optional": true}
+              "title": {"type": "string", "optional": true},
+              "url": {"type": "string", "optional": true}
             }
           },
           {
@@ -1586,14 +1587,15 @@
       {
         "name": "onChanged",
         "type": "function",
-        "description": "Fired when a bookmark or folder changes.  <b>Note:</b> Currently, only title changes trigger this.",
+        "description": "Fired when a bookmark or folder changes.  <b>Note:</b> Currently, only title and url changes changes trigger this.",
         "parameters": [
           {"type": "string", "name": "id"},
           {
             "type": "object",
             "name": "changeInfo",
             "properties": {
-              "title": { "type": "string" }
+              "title": {"type": "string"},
+              "url": {"type": "string", "optional": true}
             }
           }
         ]
diff --git a/chrome/common/extensions/docs/bookmarks.html b/chrome/common/extensions/docs/bookmarks.html
index e129afd..313a8f2 100644
--- a/chrome/common/extensions/docs/bookmarks.html
+++ b/chrome/common/extensions/docs/bookmarks.html
@@ -2262,7 +2262,7 @@ For other examples and for help in viewing the source code, see
 
               <div class="description">
                 <p class="todo" style="display: none; ">Undocumented.</p>
-                <p>Updates the properties of a bookmark or folder. Specify only the properties that you want to change; unspecified properties will be left unchanged.  <b>Note:</b> Currently, only 'title' is supported.</p>
+                <p>Updates the properties of a bookmark or folder. Specify only the properties that you want to change; unspecified properties will be left unchanged.  <b>Note:</b> Currently, only 'title' and 'url' are supported.</p>
 
                 <!-- PARAMETERS -->
                 <h4>Parameters</h4>
@@ -2389,6 +2389,49 @@ For other examples and for help in viewing the source code, see
             </dl>
           </dd>
         </div>
+              </div><div>
+                <div>
+          <dt>
+            <var>url</var>
+              <em>
+
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional">optional</span>
+                    <span id="typeTemplate">
+                      <span style="display: none; ">
+                        <a> Type</a>
+                      </span>
+                      <span>
+                        <span style="display: none; ">
+                          array of <span><span></span></span>
+                        </span>
+                        <span>string</span>
+                      </span>
+                    </span>
+                  )
+                </div>
+
+              </em>
+          </dt>
+          <dd class="todo">
+            Undocumented.
+          </dd>
+          <dd style="display: none; ">
+            Description of this parameter from the json schema.
+          </dd>
+
+          <!-- OBJECT PROPERTIES -->
+          <dd style="display: none; ">
+            <dl>
+              <div>
+                <div>
+                </div>
+              </div>
+            </dl>
+          </dd>
+        </div>
               </div>
             </dl>
           </dd>
@@ -2535,7 +2578,7 @@ For other examples and for help in viewing the source code, see
 
               <div class="description">
                 <p class="todo" style="display: none; ">Undocumented.</p>
-                <p>Fired when a bookmark or folder changes.  <b>Note:</b> Currently, only title changes trigger this.</p>
+                <p>Fired when a bookmark or folder changes.  <b>Note:</b> Currently, only title and url changes changes trigger this.</p>
 
                 <!-- PARAMETERS -->
                 <h4>Parameters</h4>
@@ -2662,6 +2705,49 @@ For other examples and for help in viewing the source code, see
             </dl>
           </dd>
         </div>
+              </div><div>
+                <div>
+          <dt>
+            <var>url</var>
+              <em>
+
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional">optional</span>
+                    <span id="typeTemplate">
+                      <span style="display: none; ">
+                        <a> Type</a>
+                      </span>
+                      <span>
+                        <span style="display: none; ">
+                          array of <span><span></span></span>
+                        </span>
+                        <span>string</span>
+                      </span>
+                    </span>
+                  )
+                </div>
+
+              </em>
+          </dt>
+          <dd class="todo">
+            Undocumented.
+          </dd>
+          <dd style="display: none; ">
+            Description of this parameter from the json schema.
+          </dd>
+
+          <!-- OBJECT PROPERTIES -->
+          <dd style="display: none; ">
+            <dl>
+              <div>
+                <div>
+                </div>
+              </div>
+            </dl>
+          </dd>
+        </div>
               </div>
             </dl>
           </dd>
diff --git a/chrome/common/extensions/docs/tabs.html b/chrome/common/extensions/docs/tabs.html
index 82da463..0797d1e 100644
--- a/chrome/common/extensions/docs/tabs.html
+++ b/chrome/common/extensions/docs/tabs.html
@@ -1248,7 +1248,8 @@ For other examples and for help in viewing the source code, see
           <dd class="todo" style="display: none; ">
             Undocumented.
           </dd>
-          <dd>An ISO language code such as <code>en</code> or <code>fr</code>. For a complete list of languages supported by this method, see <a href='http://src.chromium.org/viewvc/chrome/trunk/src/third_party/cld/languages/internal/languages.cc'>kLanguageInfoTable</a>. The 2nd to 4th columns will be checked and the first non-NULL value will be returned except for Simplified Chinese for which zh-CN will be returned. For an unknown language, <code>und</code> will be returned.</dd>
+          <dd>An ISO language code such as <code>en</code> or <code>fr</code>. For a complete list of languages supported by this method, see <a href="http://src.chromium.org/viewvc/chrome/trunk/src/third_party/cld/languages/internal/languages.cc">kLanguageInfoTable</a>. The 2nd to 4th columns will be checked and the first non-NULL value will be returned except for Simplified Chinese for which zh-CN will be returned. For an unknown language, <code>und</code> will be returned.</dd>
+
           <!-- OBJECT PROPERTIES -->
           <dd style="display: none; ">
             <dl>
@@ -1273,7 +1274,7 @@ For other examples and for help in viewing the source code, see
               <div class="summary"><span style="display: none; ">void</span>
                   <!-- Note: intentionally longer 80 columns -->
                   <span>chrome.tabs.executeScript</span>(<span class="optional"><span style="display: none; ">, </span><span>integer</span>
-                      <var><span>tabId</span></var></span><span class="optional"><span>, </span><span>object</span>
+                      <var><span>tabId</span></var></span><span class="null"><span>, </span><span>object</span>
                       <var><span>details</span></var></span><span class="optional"><span>, </span><span>function</span>
                       <var><span>callback</span></var></span>)</div>
 
@@ -1336,7 +1337,7 @@ For other examples and for help in viewing the source code, see
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
-                    <span class="optional">optional</span>
+                    <span class="optional" style="display: none; ">optional</span>
                     <span id="typeTemplate">
                       <span style="display: none; ">
                         <a> Type</a>
@@ -2123,7 +2124,7 @@ For other examples and for help in viewing the source code, see
               <div class="summary"><span style="display: none; ">void</span>
                   <!-- Note: intentionally longer 80 columns -->
                   <span>chrome.tabs.insertCSS</span>(<span class="optional"><span style="display: none; ">, </span><span>integer</span>
-                      <var><span>tabId</span></var></span><span class="optional"><span>, </span><span>object</span>
+                      <var><span>tabId</span></var></span><span class="null"><span>, </span><span>object</span>
                       <var><span>details</span></var></span><span class="optional"><span>, </span><span>function</span>
                       <var><span>callback</span></var></span>)</div>
 
@@ -2186,7 +2187,7 @@ For other examples and for help in viewing the source code, see
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
-                    <span class="optional">optional</span>
+                    <span class="optional" style="display: none; ">optional</span>
                     <span id="typeTemplate">
                       <span style="display: none; ">
                         <a> Type</a>
diff --git a/chrome/test/data/extensions/api_test/bookmarks/test.js b/chrome/test/data/extensions/api_test/bookmarks/test.js
index d96f41f..e9c5742 100644
--- a/chrome/test/data/extensions/api_test/bookmarks/test.js
+++ b/chrome/test/data/extensions/api_test/bookmarks/test.js
@@ -268,6 +268,13 @@ chrome.test.runTests([
     chrome.bookmarks.update(node1.id, {"title": title}, pass(function(results) {
       chrome.test.assertEq(title, results.title);
     }));
+
+    var url = "http://example.com/hello"
+    chrome.bookmarks.update(node1.id, {"url": url}, pass(function(results) {
+      // Make sure that leaving out the title does not set the title to empty.
+      chrome.test.assertEq(title, results.title);
+      chrome.test.assertEq(url, results.url);
+    }));
   },
 
   function remove() {