more bookmark tests, plus fix a couple of API bugs

BUG=19099,17288
TEST=browser_tests.exe --gtest_filter=ExtensionApiTest.Bookmarks

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

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

8 files changed, 160 insertions, 32 deletions

diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc
index 1edef1c..910684f 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module.cc
@@ -60,7 +60,8 @@ class ExtensionBookmarks {
         DictionaryValue* dict = GetNodeDictionary(child, true);
         children->Append(dict);
       }
-      dict->Set(keys::kChildrenKey, children);
+      if (node->is_folder())
+        dict->Set(keys::kChildrenKey, children);
     }
     return dict;
   }
@@ -515,22 +516,27 @@ bool MoveBookmarkFunction::RunImpl() {
   }
 
   model->Move(node, parent, index);
-  return true;
-}
 
-bool SetBookmarkTitleFunction::RunImpl() {
-  EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_DICTIONARY));
-  DictionaryValue* json = static_cast<DictionaryValue*>(args_);
+  DictionaryValue* ret = ExtensionBookmarks::GetNodeDictionary(node, false);
+  result_.reset(ret);
 
-  std::wstring title;
-  json->GetString(keys::kTitleKey, &title);  // Optional (empty is clear).
+  return true;
+}
 
-  BookmarkModel* model = profile()->GetBookmarkModel();
-  int64 id = 0;
+bool UpdateBookmarkFunction::RunImpl() {
+  EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_LIST));
+  const ListValue* args = static_cast<const ListValue*>(args_);
+  int64 id;
   std::string id_string;
-  EXTENSION_FUNCTION_VALIDATE(json->GetString(keys::kIdKey, &id_string));
+  EXTENSION_FUNCTION_VALIDATE(args->GetString(0, &id_string));
   if (!GetBookmarkIdAsInt64(id_string, &id))
     return false;
+  DictionaryValue* updates;
+  EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &updates));
+  std::wstring title;
+  updates->GetString(keys::kTitleKey, &title);  // Optional (empty is clear).
+
+  BookmarkModel* model = profile()->GetBookmarkModel();
   const BookmarkNode* node = model->GetNodeByID(id);
   if (!node) {
     error_ = keys::kNoNodeError;
@@ -543,5 +549,9 @@ bool SetBookmarkTitleFunction::RunImpl() {
     return false;
   }
   model->SetTitle(node, title);
+
+  DictionaryValue* ret = ExtensionBookmarks::GetNodeDictionary(node, false);
+  result_.reset(ret);
+
   return true;
 }
diff --git a/chrome/browser/extensions/extension_bookmarks_module.h b/chrome/browser/extensions/extension_bookmarks_module.h
index c9301eb..cb2cd6c 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.h
+++ b/chrome/browser/extensions/extension_bookmarks_module.h
@@ -108,7 +108,7 @@ class MoveBookmarkFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-class SetBookmarkTitleFunction : public BookmarksFunction {
+class UpdateBookmarkFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
diff --git a/chrome/browser/extensions/extension_bookmarks_module_constants.cc b/chrome/browser/extensions/extension_bookmarks_module_constants.cc
index ef4e3f4..769fe09 100644
--- a/chrome/browser/extensions/extension_bookmarks_module_constants.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module_constants.cc
@@ -42,6 +42,6 @@ const char kRemoveBookmarkFunction[] = "bookmarks.remove";
 const char kRemoveBookmarkTreeFunction[] = "bookmarks.removeTree";
 const char kCreateBookmarkFunction[] = "bookmarks.create";
 const char kMoveBookmarkFunction[] = "bookmarks.move";
-const char kSetBookmarkTitleFunction[] = "bookmarks.update";
+const char kUpdateBookmarkFunction[] = "bookmarks.update";
 
 }  // namespace extension_bookmarks_module_constants
diff --git a/chrome/browser/extensions/extension_bookmarks_module_constants.h b/chrome/browser/extensions/extension_bookmarks_module_constants.h
index 5f49b44..12d677d 100644
--- a/chrome/browser/extensions/extension_bookmarks_module_constants.h
+++ b/chrome/browser/extensions/extension_bookmarks_module_constants.h
@@ -48,7 +48,7 @@ extern const char kRemoveBookmarkFunction[];
 extern const char kRemoveBookmarkTreeFunction[];
 extern const char kCreateBookmarkFunction[];
 extern const char kMoveBookmarkFunction[];
-extern const char kSetBookmarkTitleFunction[];
+extern const char kUpdateBookmarkFunction[];
 
 };  // namespace extension_bookmarks_module_constants
 
diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc
index fbd329e..d0281c3 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.cc
+++ b/chrome/browser/extensions/extension_function_dispatcher.cc
@@ -133,8 +133,8 @@ void FactoryRegistry::ResetFunctions() {
       &NewExtensionFunction<CreateBookmarkFunction>;
   factories_[bookmarks::kMoveBookmarkFunction] =
       &NewExtensionFunction<MoveBookmarkFunction>;
-  factories_[bookmarks::kSetBookmarkTitleFunction] =
-      &NewExtensionFunction<SetBookmarkTitleFunction>;
+  factories_[bookmarks::kUpdateBookmarkFunction] =
+      &NewExtensionFunction<UpdateBookmarkFunction>;
 
   // Toolstrips.
   factories_[toolstrip::kExpandFunction] =
diff --git a/chrome/common/common_resources.grd b/chrome/common/common_resources.grd
index 9f422c7..6c07b07 100644
--- a/chrome/common/common_resources.grd
+++ b/chrome/common/common_resources.grd
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file. as1 -->
+without changes to the corresponding grd file. ek -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/common_resources.h" type="rc_header">
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index 33170c1..aca961d 100755
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -831,7 +831,14 @@
               "index": {"type": "integer", "minimum": 0, "optional": true}
             }
           },
-          {"type": "function", "name": "callback", "optional": true, "parameters": []}
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {"name": "result", "$ref": "BookmarkTreeNode" }
+            ]
+          }
         ]
       },
       {
diff --git a/chrome/test/data/extensions/api_test/bookmarks/test.js b/chrome/test/data/extensions/api_test/bookmarks/test.js
index 4325e18..28d1b2e 100644
--- a/chrome/test/data/extensions/api_test/bookmarks/test.js
+++ b/chrome/test/data/extensions/api_test/bookmarks/test.js
@@ -3,18 +3,26 @@
 
 var expected = [
   {"children": [
-    {"children": [], "id": "1", "parentId": "0", "index": 0, "title":"Bookmarks bar"},
-    {"children": [], "id": "2", "parentId": "0", "index": 1, "title":"Other bookmarks"}
+      {children:[], id:"1", parentId:"0", index:0, title:"Bookmarks bar"},
+      {children:[], id:"2", parentId:"0", index:1, title:"Other bookmarks"}
     ],
-   "id": "0", "title": ""
+   id:"0", title:""
   }
 ];
 
+// Some variables that are used across multiple tests.
+var node1 = {parentId:"1", title:"Foo bar baz",
+             url:"http://www.example.com/hello"};
+var node2 = {parentId:"1", title:"foo quux",
+             url:"http://www.example.com/bar"};
+var node3 = {parentId:"1", title:"bar baz",
+             url:"http://www.google.com/hello/quux"};
+
 var testCallback = chrome.test.testCallback;
 
 function compareNode(left, right) {
-  //console.log(JSON.stringify(left));
-  //console.log(JSON.stringify(right));
+  //chrome.test.log(JSON.stringify(left));
+  //chrome.test.log(JSON.stringify(right));
   // TODO(erikkay): do some comparison of dateAdded
   if (left.id != right.id)
     return "id mismatch: " + left.id + " != " + right.id;
@@ -28,8 +36,8 @@ function compareNode(left, right) {
 }
 
 function compareTrees(left, right) {
-  //console.log(JSON.stringify(left));
-  //console.log(JSON.stringify(right));
+  //chrome.test.log(JSON.stringify(left) || "<null>");
+  //chrome.test.log(JSON.stringify(right) || "<null>");
   if (left == null && right == null) {
     console.log("both left and right are NULL");
     return true;
@@ -56,7 +64,7 @@ chrome.test.runTests([
   function getTree() {
     chrome.bookmarks.getTree(testCallback(true, function(results) {
       chrome.test.assertTrue(compareTrees(results, expected),
-                 "getTree() result != expected");
+                             "getTree() result != expected");
       expected = results;
     }));
   },
@@ -70,28 +78,131 @@ chrome.test.runTests([
   function getArray() {
     chrome.bookmarks.get(["1", "2"], testCallback(true, function(results) {
       chrome.test.assertTrue(compareNode(results[0], expected[0].children[0]),
-                 "get() result != expected");
+                             "get() result != expected");
       chrome.test.assertTrue(compareNode(results[1], expected[0].children[1]),
-                 "get() result != expected");
+                             "get() result != expected");
     }));
   },
 
   function getChildren() {
     chrome.bookmarks.getChildren("0", testCallback(true, function(results) {
       chrome.test.assertTrue(compareNode(results[0], expected[0].children[0]),
-                 "getChildren() result != expected");
+                             "getChildren() result != expected");
       chrome.test.assertTrue(compareNode(results[1], expected[0].children[1]),
-                 "getChildren() result != expected");
+                             "getChildren() result != expected");
     }));
   },
 
   function create() {
-    var node = {parentId: "1", title:"google", url:"http://www.google.com/"};
+    var node = {parentId:"1", title:"google", url:"http://www.google.com/"};
     chrome.bookmarks.create(node, testCallback(true, function(results) {
       node.id = results.id;  // since we couldn't know this going in
       node.index = 0;
       chrome.test.assertTrue(compareNode(node, results),
-                 "created node != source");
+                             "created node != source");
+      expected[0].children[0].children.push(node);
+    }));
+  },
+  
+  function createFolder() {
+    var node = {parentId:"1", title:"foo bar"};  // folder
+    chrome.bookmarks.create(node, testCallback(true, function(results) {
+      node.id = results.id;  // since we couldn't know this going in
+      node.index = 1;
+      node.children = [];
+      chrome.test.assertTrue(compareNode(node, results),
+                             "created node != source");
+      expected[0].children[0].children.push(node);
+    }));
+  },
+  
+  function move_setup() {
+    chrome.bookmarks.create(node1, testCallback(false, function(results) {
+      node1.id = results.id;
+      node1.index = results.index;
+      expected[0].children[0].children.push(node1);
+    }));
+    chrome.bookmarks.create(node2, testCallback(false, function(results) {
+      node2.id = results.id;
+      node2.index = results.index;
+      expected[0].children[0].children.push(node2);
     }));
+    chrome.bookmarks.create(node3, testCallback(false, function(results) {
+      node3.id = results.id;
+      node3.index = results.index;
+      expected[0].children[0].children.push(node3);
+    }));
+    chrome.bookmarks.getTree(testCallback(true, function(results) {
+      chrome.test.assertTrue(compareTrees(expected, results),
+                             "getTree() result != expected");
+      expected = results;
+    }));
+  },
+  
+  function move() {
+    // Move node1, node2, and node3 from their current location (the bookmark
+    // bar) to be under the "other bookmarks" folder instead.
+    var other = expected[0].children[1];
+    //var folder = expected[0].children[0].children[1];
+    //console.log(JSON.stringify(node1));
+    chrome.bookmarks.move(node1.id, {parentId:other.id},
+                          testCallback(false, function(results) {
+      chrome.test.assertEq(results.parentId, other.id);
+      node1.parentId = results.parentId;
+    }));
+    chrome.bookmarks.move(node2.id, {parentId:other.id},
+                          testCallback(false, function(results) {
+      chrome.test.assertEq(results.parentId, other.id);
+      node3.parentId = results.parentId;
+    }));
+    // Insert node3 at index 1 rather than at the end.  This should result in
+    // an order of node1, node3, node2.
+    chrome.bookmarks.move(node3.id, {parentId:other.id, index:1},
+                          testCallback(true, function(results) {
+      chrome.test.assertEq(results.parentId, other.id);
+      chrome.test.assertEq(results.index, 1);
+      node3.parentId = results.parentId;
+      node3.index = 1;
+    }));
+  },
+  
+  function search() {
+    chrome.bookmarks.search("baz bar", testCallback(false, function(results) {
+      // matches node1 & node3
+      console.log(JSON.stringify(results));
+      chrome.test.assertEq(2, results.length);
+    }));
+    chrome.bookmarks.search("www hello", testCallback(false, function(results) {
+      // matches node1 & node3
+      chrome.test.assertEq(2, results.length);
+    }));
+    chrome.bookmarks.search("bar example",
+                            testCallback(false, function(results) {
+      // matches node2
+      chrome.test.assertEq(1, results.length);
+    }));
+    chrome.bookmarks.search("foo bar", testCallback(false, function(results) {
+      // matches node1
+      chrome.test.assertEq(1, results.length);
+    }));
+    chrome.bookmarks.search("quux", testCallback(true, function(results) {
+      // matches node2 & node3
+      chrome.test.assertEq(2, results.length);
+    }));
+  },
+  
+  function update() {
+    chrome.bookmarks.update(node1.id, {"title": "hello world"},
+                            testCallback(true, function(results) {
+      chrome.test.assertEq("hello world", results.title);
+    }));
+  },
+  
+  function remove() {
+    chrome.test.succeed();
+  },
+  
+  function removeTree() {
+    chrome.test.succeed();
   },
 ]);