more extensions bookmarks changes:* add schema verification* add unit tests for schema* add a few new methods (getTree, getChildren)* add events

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

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

8 files changed, 468 insertions, 95 deletions

diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc
index aa720ca..3a175ed 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module.cc
@@ -4,9 +4,12 @@
 
 #include "chrome/browser/extensions/extension_bookmarks_module.h"
 
+#include "base/json_writer.h"
 #include "chrome/browser/bookmarks/bookmark_codec.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
+#include "chrome/browser/browser_list.h"
+#include "chrome/browser/extensions/extension_message_service.h"
 #include "chrome/browser/profile.h"
 
 namespace {
@@ -14,6 +17,8 @@ namespace {
 const wchar_t* kIdKey = L"id";
 const wchar_t* kIndexKey = L"index";
 const wchar_t* kParentIdKey = L"parentId";
+const wchar_t* kOldIndexKey = L"oldIndex";
+const wchar_t* kOldParentIdKey = L"oldParentId";
 const wchar_t* kUrlKey = L"url";
 const wchar_t* kTitleKey = L"title";
 const wchar_t* kChildrenIdsKey = L"childrenIds";
@@ -28,6 +33,13 @@ const char* kFolderNotEmptyError =
 const char* kInvalidIndexError = "Index out of bounds.";
 const char* kInvalidUrlError = "Invalid URL.";
 const char* kModifySpecialError = "Can't modify the root bookmark folders.";
+
+// events
+const char* kOnBookmarkAdded = "bookmark-added";
+const char* kOnBookmarkRemoved = "bookmark-removed";
+const char* kOnBookmarkChanged = "bookmark-changed";
+const char* kOnBookmarkMoved = "bookmark-moved";
+const char* kOnBookmarkChildrenReordered = "bookmark-children-reordered";
 };
 
 // Helper functions.
@@ -76,7 +88,140 @@ class ExtensionBookmarks {
   ExtensionBookmarks();
 };
 
-// TODO(erikkay): add a recursive version
+void BookmarksFunction::Run() {
+  // TODO(erikkay) temporary hack until adding an event listener can notify the
+  // browser.
+  ExtensionBookmarkEventRouter* event_router =
+      ExtensionBookmarkEventRouter::GetSingleton();
+  BookmarkModel* model = profile()->GetBookmarkModel();
+  event_router->Observe(model);
+  SyncExtensionFunction::Run();
+}
+
+// static
+ExtensionBookmarkEventRouter* ExtensionBookmarkEventRouter::GetSingleton() {
+  return Singleton<ExtensionBookmarkEventRouter>::get();
+}
+
+ExtensionBookmarkEventRouter::ExtensionBookmarkEventRouter() {
+}
+
+ExtensionBookmarkEventRouter::~ExtensionBookmarkEventRouter() {
+}
+
+void ExtensionBookmarkEventRouter::Observe(BookmarkModel* model) {
+  if (models_.find(model) == models_.end()) {
+    model->AddObserver(this);
+    models_.insert(model);
+  }
+}
+
+void ExtensionBookmarkEventRouter::DispatchEvent(Profile *profile,
+                                                 const char* event_name,
+                                                 const std::string json_args) {
+  ExtensionMessageService::GetInstance(profile->GetRequestContext())->
+      DispatchEventToRenderers(event_name, json_args);
+}
+
+void ExtensionBookmarkEventRouter::Loaded(BookmarkModel* model) {
+  // TODO(erikkay): Do we need an event here?  It seems unlikely that
+  // an extension would load before bookmarks loaded.
+}
+
+void ExtensionBookmarkEventRouter::BookmarkNodeMoved(BookmarkModel* model,
+                                                     BookmarkNode* old_parent,
+                                                     int old_index,
+                                                     BookmarkNode* new_parent,
+                                                     int new_index) {
+  ListValue args;
+  DictionaryValue* object_args = new DictionaryValue();
+  BookmarkNode* node = new_parent->GetChild(new_index);
+  object_args->SetInteger(kIdKey, node->id());
+  object_args->SetInteger(kParentIdKey, new_parent->id());
+  object_args->SetInteger(kIndexKey, new_index);
+  object_args->SetInteger(kOldParentIdKey, old_parent->id());
+  object_args->SetInteger(kOldIndexKey, old_index);
+  args.Append(object_args);
+
+  std::string json_args;
+  JSONWriter::Write(&args, false, &json_args);
+  DispatchEvent(model->profile(), kOnBookmarkMoved, json_args);
+}
+
+void ExtensionBookmarkEventRouter::BookmarkNodeAdded(BookmarkModel* model,
+                                                     BookmarkNode* parent,
+                                                     int index) {
+  ListValue args;
+  DictionaryValue* object_args = new DictionaryValue();
+  BookmarkNode* node = parent->GetChild(index);
+  object_args->SetInteger(kIdKey, node->id());
+  object_args->SetString(kTitleKey, node->GetTitle());
+  object_args->SetString(kUrlKey, node->GetURL().spec());
+  object_args->SetInteger(kParentIdKey, parent->id());
+  object_args->SetInteger(kIndexKey, index);
+  args.Append(object_args);
+
+  std::string json_args;
+  JSONWriter::Write(&args, false, &json_args);
+  DispatchEvent(model->profile(), kOnBookmarkAdded, json_args);
+}
+
+void ExtensionBookmarkEventRouter::BookmarkNodeRemoved(BookmarkModel* model,
+                                                       BookmarkNode* parent,
+                                                       int index) {
+  ListValue args;
+  DictionaryValue* object_args = new DictionaryValue();
+  object_args->SetInteger(kParentIdKey, parent->id());
+  object_args->SetInteger(kIndexKey, index);
+  args.Append(object_args);
+
+  std::string json_args;
+  JSONWriter::Write(&args, false, &json_args);
+  DispatchEvent(model->profile(), kOnBookmarkRemoved, json_args);
+}
+
+void ExtensionBookmarkEventRouter::BookmarkNodeChanged(BookmarkModel* model,
+                                                       BookmarkNode* node) {
+  ListValue args;
+  args.Append(new FundamentalValue(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.
+  DictionaryValue* object_args = new DictionaryValue();
+  object_args->SetString(kTitleKey, node->GetTitle());
+  args.Append(object_args);
+
+  std::string json_args;
+  JSONWriter::Write(&args, false, &json_args);
+  DispatchEvent(model->profile(), kOnBookmarkChanged, json_args);
+}
+
+void ExtensionBookmarkEventRouter::BookmarkNodeFavIconLoaded(
+    BookmarkModel* model, BookmarkNode* node) {
+  // TODO(erikkay) anything we should do here?
+}
+
+void ExtensionBookmarkEventRouter::BookmarkNodeChildrenReordered(
+    BookmarkModel* model, BookmarkNode* node) {
+  ListValue args;
+  args.Append(new FundamentalValue(node->id()));
+  int childCount = node->GetChildCount();
+  ListValue* children = new ListValue();
+  for (int i = 0; i < childCount; ++i) {
+    BookmarkNode* child = node->GetChild(i);
+    Value* child_id = new FundamentalValue(child->id());
+    children->Append(child_id);
+  }
+  args.Append(children);
+
+  std::string json_args;
+  JSONWriter::Write(&args, false, &json_args);
+  DispatchEvent(model->profile(), kOnBookmarkChildrenReordered, json_args);
+}
+
 bool GetBookmarksFunction::RunImpl() {
   // TODO(erikkay): the JSON schema doesn't support the TYPE_INTEGER
   // variant yet.
@@ -127,6 +272,35 @@ bool GetBookmarksFunction::RunImpl() {
   return true;
 }
 
+bool GetBookmarkChildrenFunction::RunImpl() {
+  int id;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&id));
+  BookmarkModel* model = profile()->GetBookmarkModel();
+  scoped_ptr<ListValue> json(new ListValue());
+  BookmarkNode* node = model->GetNodeByID(id);
+  if (!node) {
+    error_ = kNoNodeError;
+    return false;
+  }
+  int child_count = node->GetChildCount();
+  for (int i = 0; i < child_count; ++i) {
+    BookmarkNode* child = node->GetChild(i);
+    ExtensionBookmarks::AddNode(child, json.get(), false);
+  }
+
+  result_.reset(json.release());
+  return true;
+}
+
+bool GetBookmarkTreeFunction::RunImpl() {
+  BookmarkModel* model = profile()->GetBookmarkModel();
+  scoped_ptr<ListValue> json(new ListValue());
+  BookmarkNode* node = model->root_node();
+  ExtensionBookmarks::AddNode(node, json.get(), true);
+  result_.reset(json.release());
+  return true;
+}
+
 bool SearchBookmarksFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_STRING));
 
@@ -322,4 +496,3 @@ bool SetBookmarkTitleFunction::RunImpl() {
   model->SetTitle(node, title);
   return true;
 }
-
diff --git a/chrome/browser/extensions/extension_bookmarks_module.h b/chrome/browser/extensions/extension_bookmarks_module.h
index b7b38d8..def5440 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.h
+++ b/chrome/browser/extensions/extension_bookmarks_module.h
@@ -2,34 +2,105 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_BOOKMARKS_MODULE_H__
-#define CHROME_BROWSER_EXTENSIONS_EXTENSION_BOOKMARKS_MODULE_H__
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_BOOKMARKS_MODULE_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_BOOKMARKS_MODULE_H_
 
+#include <string>
+#include <vector>
+
+#include "base/singleton.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/extensions/extension_function.h"
 
-class GetBookmarksFunction : public SyncExtensionFunction {
+// Observes BookmarkModel and then routes the notifications as events to
+// the extension system.
+class ExtensionBookmarkEventRouter : public BookmarkModelObserver {
+ public:
+  static ExtensionBookmarkEventRouter* GetSingleton();
+  virtual ~ExtensionBookmarkEventRouter();
+
+  // Call this for each model to observe.  Safe to call multiple times per
+  // model.
+  void Observe(BookmarkModel* model);
+
+  // BookmarkModelObserver
+  virtual void Loaded(BookmarkModel* model);
+  virtual void BookmarkModelBeingDeleted(BookmarkModel* model) { }
+  virtual void BookmarkNodeMoved(BookmarkModel* model,
+                                 BookmarkNode* old_parent,
+                                 int old_index,
+                                 BookmarkNode* new_parent,
+                                 int new_index);
+  virtual void BookmarkNodeAdded(BookmarkModel* model,
+                                 BookmarkNode* parent,
+                                 int index);
+  virtual void BookmarkNodeRemoved(BookmarkModel* model,
+                                   BookmarkNode* parent,
+                                   int index);
+  virtual void BookmarkNodeRemoved(BookmarkModel* model,
+                                   BookmarkNode* parent,
+                                   int old_index,
+                                   BookmarkNode* node) {
+    BookmarkNodeRemoved(model, parent, old_index);
+  }
+  virtual void BookmarkNodeChanged(BookmarkModel* model,
+                                   BookmarkNode* node);
+  virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
+                                         BookmarkNode* node);
+  virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
+                                             BookmarkNode* node);
+
+ private:
+  ExtensionBookmarkEventRouter();
+  friend struct DefaultSingletonTraits<ExtensionBookmarkEventRouter>;
+
+  // Helper to actually dispatch an event to extension listeners.
+  void DispatchEvent(Profile* profile,
+                     const char* event_name,
+                     const std::string json_args);
+
+  // These are stored so that Observe can be called multiple times safely.
+  // This way the caller doesn't have to know whether it's already observing
+  // a particular model or not.  The pointers are not owned by this object.
+  std::set<BookmarkModel*> models_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionBookmarkEventRouter);
+};
+
+class BookmarksFunction : public SyncExtensionFunction {
+  virtual void Run();
+};
+
+class GetBookmarksFunction : public BookmarksFunction {
+  virtual bool RunImpl();
+};
+
+class GetBookmarkChildrenFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-class SearchBookmarksFunction : public SyncExtensionFunction {
+class GetBookmarkTreeFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-class RemoveBookmarkFunction : public SyncExtensionFunction {
+class SearchBookmarksFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-class CreateBookmarkFunction : public SyncExtensionFunction {
+class RemoveBookmarkFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-class MoveBookmarkFunction : public SyncExtensionFunction {
+class CreateBookmarkFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-class SetBookmarkTitleFunction : public SyncExtensionFunction {
+class MoveBookmarkFunction : public BookmarksFunction {
   virtual bool RunImpl();
 };
 
-#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_BOOKMARKS_MODULE_H__
+class SetBookmarkTitleFunction : public BookmarksFunction {
+  virtual bool RunImpl();
+};
 
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_BOOKMARKS_MODULE_H_
diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc
index 5ca347b..7e6099f 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.cc
+++ b/chrome/browser/extensions/extension_function_dispatcher.cc
@@ -63,6 +63,10 @@ FactoryRegistry::FactoryRegistry() {
 
   // Bookmarks
   factories_["GetBookmarks"] = &NewExtensionFunction<GetBookmarksFunction>;
+  factories_["GetBookmarkChildren"] =
+      &NewExtensionFunction<GetBookmarkChildrenFunction>;
+  factories_["GetBookmarkTree"] =
+      &NewExtensionFunction<GetBookmarkTreeFunction>;
   factories_["SearchBookmarks"] =
       &NewExtensionFunction<SearchBookmarksFunction>;
   factories_["RemoveBookmark"] = &NewExtensionFunction<RemoveBookmarkFunction>;
diff --git a/chrome/renderer/extensions/extension_api_client_unittest.cc b/chrome/renderer/extensions/extension_api_client_unittest.cc
index 87e4376..e635901 100755
--- a/chrome/renderer/extensions/extension_api_client_unittest.cc
+++ b/chrome/renderer/extensions/extension_api_client_unittest.cc
@@ -2,6 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "base/string_util.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/renderer/extensions/extension_process_bindings.h"
 #include "chrome/renderer/extensions/renderer_extension_bindings.h"
@@ -34,6 +38,23 @@ class ExtensionAPIClientTest : public RenderViewTest {
   void ExpectJsFail(const std::string& js, const std::string& message) {
     ExecuteJavaScript(js.c_str());
     EXPECT_EQ(message, GetConsoleMessage());
+    render_thread_.sink().ClearMessages();
+  }
+
+  void ExpectJsPass(const std::string& js,
+                    const std::string& function,
+                    const std::string& arg1) {
+    ExecuteJavaScript(js.c_str());
+    const IPC::Message* request_msg =
+      render_thread_.sink().GetUniqueMessageMatching(
+          ViewHostMsg_ExtensionRequest::ID);
+    ASSERT_EQ("", GetConsoleMessage()) << js;
+    ASSERT_TRUE(request_msg) << js;
+    ViewHostMsg_ExtensionRequest::Param params;
+    ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
+    ASSERT_EQ(function.c_str(), params.a) << js;
+    ASSERT_EQ(arg1.c_str(), params.b) << js;
+    render_thread_.sink().ClearMessages();
   }
 };
 
@@ -85,30 +106,15 @@ TEST_F(ExtensionAPIClientTest, GetTabsForWindow) {
   ExpectJsFail("chromium.tabs.getTabsForWindow(42, function(){});",
                "Uncaught Error: Too many arguments.");
 
-  ExecuteJavaScript("chromium.tabs.getTabsForWindow(function(){})");
-  const IPC::Message* request_msg =
-      render_thread_.sink().GetUniqueMessageMatching(
-          ViewHostMsg_ExtensionRequest::ID);
-  ASSERT_TRUE(request_msg);
-  ViewHostMsg_ExtensionRequest::Param params;
-  ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
-  ASSERT_EQ("GetTabsForWindow", params.a);
-  ASSERT_EQ("null", params.b);
+  ExpectJsPass("chromium.tabs.getTabsForWindow(function(){})",
+               "GetTabsForWindow", "null");
 }
 
 TEST_F(ExtensionAPIClientTest, GetTab) {
   ExpectJsFail("chromium.tabs.getTab(null, function(){});",
                "Uncaught Error: Parameter 0 is required.");
 
-  ExecuteJavaScript("chromium.tabs.getTab(42)");
-    const IPC::Message* request_msg =
-      render_thread_.sink().GetUniqueMessageMatching(
-          ViewHostMsg_ExtensionRequest::ID);
-  ASSERT_TRUE(request_msg);
-  ViewHostMsg_ExtensionRequest::Param params;
-  ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
-  ASSERT_EQ("GetTab", params.a);
-  ASSERT_EQ("42", params.b);
+  ExpectJsPass("chromium.tabs.getTab(42)", "GetTab", "42");
 }
 
 TEST_F(ExtensionAPIClientTest, CreateTab) {
@@ -122,21 +128,15 @@ TEST_F(ExtensionAPIClientTest, CreateTab) {
                "Uncaught Error: Invalid value for argument 0. Property "
                "'foo': Unexpected property.");
 
-  ExecuteJavaScript("chromium.tabs.createTab({"
-                    "  url:'http://www.google.com/',"
-                    "  selected:true,"
-                    "  windowId:4"
-                    "})");
-    const IPC::Message* request_msg =
-      render_thread_.sink().GetUniqueMessageMatching(
-          ViewHostMsg_ExtensionRequest::ID);
-  ASSERT_TRUE(request_msg);
-  ViewHostMsg_ExtensionRequest::Param params;
-  ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
-  ASSERT_EQ("CreateTab", params.a);
-  ASSERT_EQ("{\"url\":\"http://www.google.com/\","
-             "\"selected\":true,"
-             "\"windowId\":4}", params.b);
+  ExpectJsPass("chromium.tabs.createTab({"
+               "  url:'http://www.google.com/',"
+               "  selected:true,"
+               "  windowId:4"
+               "})",
+               "CreateTab",
+               "{\"url\":\"http://www.google.com/\","
+               "\"selected\":true,"
+               "\"windowId\":4}");
 }
 
 TEST_F(ExtensionAPIClientTest, UpdateTab) {
@@ -150,36 +150,90 @@ TEST_F(ExtensionAPIClientTest, UpdateTab) {
                "Uncaught Error: Invalid value for argument 0. Property "
                "'url': Expected 'string' but got 'integer'.");
 
-  ExecuteJavaScript("chromium.tabs.updateTab({"
-                    "  id:42,"
-                    "  url:'http://www.google.com/',"
-                    "  selected:true,"
-                    "  windowId:4"
-                    "})");
-    const IPC::Message* request_msg =
-      render_thread_.sink().GetUniqueMessageMatching(
-          ViewHostMsg_ExtensionRequest::ID);
-  ASSERT_TRUE(request_msg);
-  ViewHostMsg_ExtensionRequest::Param params;
-  ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
-  ASSERT_EQ("UpdateTab", params.a);
-  ASSERT_EQ("{\"id\":42,"
-             "\"url\":\"http://www.google.com/\","
-             "\"selected\":true,"
-             "\"windowId\":4}", params.b);
+  ExpectJsPass("chromium.tabs.updateTab({"
+               "  id:42,"
+               "  url:'http://www.google.com/',"
+               "  selected:true,"
+               "  windowId:4"
+               "})",
+               "UpdateTab",
+               "{\"id\":42,"
+               "\"url\":\"http://www.google.com/\","
+               "\"selected\":true,"
+               "\"windowId\":4}");
 }
 
 TEST_F(ExtensionAPIClientTest, RemoveTab) {
   ExpectJsFail("chromium.tabs.removeTab('foobar', function(){});",
                "Uncaught Error: Too many arguments.");
 
-  ExecuteJavaScript("chromium.tabs.removeTab(21)");
-    const IPC::Message* request_msg =
-      render_thread_.sink().GetUniqueMessageMatching(
-          ViewHostMsg_ExtensionRequest::ID);
-  ASSERT_TRUE(request_msg);
-  ViewHostMsg_ExtensionRequest::Param params;
-  ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
-  ASSERT_EQ("RemoveTab", params.a);
-  ASSERT_EQ("21", params.b);
+  ExpectJsPass("chromium.tabs.removeTab(21)", "RemoveTab", "21");
+}
+
+// Bookmark API tests
+// TODO(erikkay) add more variations here
+
+TEST_F(ExtensionAPIClientTest, CreateBookmark) {
+  ExpectJsFail(
+      "chromium.bookmarks.create({parentId:'x', title:0}, function(){})",
+      "Uncaught Error: Invalid value for argument 0. "
+      "Property 'parentId': Expected 'integer' but got 'string', "
+      "Property 'title': Expected 'string' but got 'integer'.");
+
+  ExpectJsPass(
+      "chromium.bookmarks.create({parentId:0, title:'x'}, function(){})",
+      "CreateBookmark",
+      "{\"parentId\":0,\"title\":\"x\"}");
+}
+
+TEST_F(ExtensionAPIClientTest, GetBookmarks) {
+  ExpectJsPass("chromium.bookmarks.get([], function(){});",
+               "GetBookmarks",
+               "[]");
+  ExpectJsPass("chromium.bookmarks.get([0,1,2,3], function(){});",
+               "GetBookmarks",
+               "[0,1,2,3]");
+  ExpectJsPass("chromium.bookmarks.get(null, function(){});",
+               "GetBookmarks",
+               "null");
+  ExpectJsFail("chromium.bookmarks.get({}, function(){});",
+               "Uncaught Error: Invalid value for argument 0. "
+               "Expected 'array' but got 'object'.");
+}
+
+TEST_F(ExtensionAPIClientTest, GetBookmarkChildren) {
+  ExpectJsPass("chromium.bookmarks.getChildren(42, function(){});",
+               "GetBookmarkChildren",
+               "42");
+}
+
+TEST_F(ExtensionAPIClientTest, GetBookmarkTree) {
+  ExpectJsPass("chromium.bookmarks.getTree(function(){});",
+               "GetBookmarkTree",
+               "null");
+}
+
+TEST_F(ExtensionAPIClientTest, SearchBookmarks) {
+  ExpectJsPass("chromium.bookmarks.search('hello',function(){});",
+               "SearchBookmarks",
+               "\"hello\"");
 }
+
+TEST_F(ExtensionAPIClientTest, RemoveBookmark) {
+  ExpectJsPass("chromium.bookmarks.remove({id:42});",
+               "RemoveBookmark",
+               "{\"id\":42}");
+}
+
+TEST_F(ExtensionAPIClientTest, MoveBookmark) {
+  ExpectJsPass("chromium.bookmarks.move({id:42,parentId:1,index:0});",
+               "MoveBookmark",
+               "{\"id\":42,\"parentId\":1,\"index\":0}");
+}
+
+TEST_F(ExtensionAPIClientTest, SetBookmarkTitle) {
+  ExpectJsPass("chromium.bookmarks.setTitle({id:42,title:'x'});",
+               "SetBookmarkTitle",
+               "{\"id\":42,\"title\":\"x\"}");
+}
+
diff --git a/chrome/renderer/renderer_resources.grd b/chrome/renderer/renderer_resources.grd
index c59e98c..cda253b 100755
--- a/chrome/renderer/renderer_resources.grd
+++ b/chrome/renderer/renderer_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.  -->
+without changes to the corresponding grd file. -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/renderer_resources.h" type="rc_header">
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index 75c7b54..1abb06f 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -19,6 +19,8 @@ var chromium;
   native function MoveTab();
   native function RemoveTab();
   native function GetBookmarks();
+  native function GetBookmarkChildren();
+  native function GetBookmarkTree();
   native function SearchBookmarks();
   native function RemoveBookmark();
   native function CreateBookmark();
@@ -238,35 +240,54 @@ var chromium;
   //----------------------------------------------------------------------------
 
   // Bookmarks
-  // TODO(erikkay): Call validate() in these functions.
   chromium.bookmarks = {};
 
   chromium.bookmarks.get = function(ids, callback) {
+    validate(arguments, arguments.callee.params);
     sendRequest(GetBookmarks, ids, callback);
   };
 
   chromium.bookmarks.get.params = [
     {
       type: "array",
-      items: {
-        type: chromium.types.pInt
-      },
-      minItems: 1,
+      items: chromium.types.pInt,
       optional: true
     },
-    chromium.types.optFun
+    chromium.types.fun
+  ];
+  
+  chromium.bookmarks.getChildren = function(id, callback) {
+    validate(arguments, arguments.callee.params);
+    sendRequest(GetBookmarkChildren, id, callback);
+  };
+
+  chromium.bookmarks.getChildren.params = [
+    chromium.types.pInt,
+    chromium.types.fun
+  ];
+  
+  chromium.bookmarks.getTree = function(callback) {
+    validate(arguments, arguments.callee.params);
+    sendRequest(GetBookmarkTree, null, callback);
+  };
+  
+  // TODO(erikkay): allow it to take an optional id as a starting point
+  chromium.bookmarks.getTree.params = [
+    chromium.types.fun
   ];
 
   chromium.bookmarks.search = function(query, callback) {
+    validate(arguments, arguments.callee.params);
     sendRequest(SearchBookmarks, query, callback);
   };
 
   chromium.bookmarks.search.params = [
-    chromium.types.string,
-    chromium.types.optFun
+    chromium.types.str,
+    chromium.types.fun
   ];
 
   chromium.bookmarks.remove = function(bookmark, callback) {
+    validate(arguments, arguments.callee.params);
     sendRequest(RemoveBookmark, bookmark, callback);
   };
 
@@ -275,13 +296,14 @@ var chromium;
       type: "object",
       properties: {
         id: chromium.types.pInt,
-        recursive: chromium.types.bool
+        recursive: chromium.types.optBool
       }
     },
     chromium.types.optFun
   ];
 
   chromium.bookmarks.create = function(bookmark, callback) {
+    validate(arguments, arguments.callee.params);
     sendRequest(CreateBookmark, bookmark, callback);
   };
 
@@ -291,14 +313,15 @@ var chromium;
       properties: {
         parentId: chromium.types.optPInt,
         index: chromium.types.optPInt,
-        title: chromium.types.optString,
-        url: chromium.types.optString,
+        title: chromium.types.optStr,
+        url: chromium.types.optStr,
       }
     },
     chromium.types.optFun
   ];
 
   chromium.bookmarks.move = function(obj, callback) {
+    validate(arguments, arguments.callee.params);
     sendRequest(MoveBookmark, obj, callback);
   };
 
@@ -315,6 +338,7 @@ var chromium;
   ];
 
   chromium.bookmarks.setTitle = function(bookmark, callback) {
+    validate(arguments, arguments.callee.params);
     sendRequest(SetBookmarkTitle, bookmark, callback);
   };
 
@@ -323,12 +347,32 @@ var chromium;
       type: "object",
       properties: {
         id: chromium.types.pInt,
-        title: chromium.types.optString
+        title: chromium.types.optStr
       }
     },
     chromium.types.optFun
   ];
+
+  // bookmark events
+
+  // Sends ({id, title, url, parentId, index})
+  chromium.bookmarks.onBookmarkAdded = new chromium.Event("bookmark-added");
+
+  // Sends ({parentId, index})
+  chromium.bookmarks.onBookmarkRemoved = new chromium.Event("bookmark-removed");
+
+  // Sends (id, object) where object has list of properties that have changed.
+  // Currently, this only ever includes 'title'.
+  chromium.bookmarks.onBookmarkChanged = new chromium.Event("bookmark-changed");
+
+  // Sends ({id, parentId, index, oldParentId, oldIndex})
+  chromium.bookmarks.onBookmarkMoved = new chromium.Event("bookmark-moved");
   
+  // Sends (id, [childrenIds])
+  chromium.bookmarks.onBookmarkChildrenReordered =
+      new chromium.Event("bookmark-children-reordered");
+
+
   //----------------------------------------------------------------------------
 
   // Self
diff --git a/chrome/test/data/extensions/bookmarks/bookmark_api.html b/chrome/test/data/extensions/bookmarks/bookmark_api.html
index ba8084e..4217ece 100644
--- a/chrome/test/data/extensions/bookmarks/bookmark_api.html
+++ b/chrome/test/data/extensions/bookmarks/bookmark_api.html
@@ -1,7 +1,6 @@
 <html>
 <link rel="stylesheet" type="text/css" href="extensions_toolstrip.css">
 <script>
-
 var dump = function(obj, indent) {
   if (indent === undefined)
     indent = "";
@@ -24,14 +23,20 @@ var dump = function(obj, indent) {
 }
 
 var testMoveBookmarks = function(event) {
+  testMoveBookmarks2(event);
+}
+
+var testMoveBookmarks2 = function(event) {
+  console.log(testMoveBookmarks2.caller.name);
   if (event.shiftKey) {
     // TODO - it would be nice to have a mechanism to do this built-in to a
     // context menu.
     window.location.reload();
     return;
   }
-  console.log("testMoveBookmarks");
+  console.log("testMoveBookmarks2");
   chromium.bookmarks.get([], function(root) {
+    console.log("1");
     chromium.bookmarks.get(root[0].childrenIds, function(root_children) {
       var bookmark_bar = root_children[0];  // bookmarks bar is always first
       chromium.bookmarks.get(bookmark_bar.childrenIds,
@@ -72,15 +77,6 @@ var testMoveBookmarks = function(event) {
 
 var dumpBookmarks = function(event) {
   window.open("bookmark_view.html");
-  /*
-    console.dir(results);
-    var win = window.open();
-    win.document.write("<html><body><pre>");
-    win.document.write(dump(results));
-    win.document.write("</pre></body></html>");
-    win.document.title = "Bookmarks";
-  });
-  */
 };
 </script>
 <body>
diff --git a/chrome/test/data/extensions/bookmarks/bookmark_view.html b/chrome/test/data/extensions/bookmarks/bookmark_view.html
index f038e97..5b9b064 100644
--- a/chrome/test/data/extensions/bookmarks/bookmark_view.html
+++ b/chrome/test/data/extensions/bookmarks/bookmark_view.html
@@ -17,9 +17,13 @@
   background-color: silver;
   border: 1px solid black;
 }
+
+.event-log {
+  font-family: monospace;
+}
+
 </style>
 <script>
-
 // XXX Hack: When you call window.open('chrome-extension://...'), the window is
 // first navigated to about:blank, and then to the final URL. This confuses the
 // code that sets up our v8 extensions, and we don't end up with them running.
@@ -28,6 +32,32 @@
 if (!chromium.bookmarks)
   location.reload();
 
+var logEvent = function(name, data) {
+  var log = document.getElementById("event-log");
+  log.innerHTML = name + "<br>" + log.innerHTML;
+  console.log("got event: " + name);
+}
+
+chromium.bookmarks.onBookmarkAdded.addListener(function(data) {
+  logEvent("onBookmarkAdded", data);
+});
+
+chromium.bookmarks.onBookmarkRemoved.addListener(function(data) {
+  logEvent("onBookmarkRemoved", data);
+});
+
+chromium.bookmarks.onBookmarkChanged.addListener(function(data) {
+  logEvent("onBookmarkChanged", data);
+});
+
+chromium.bookmarks.onBookmarkMoved.addListener(function(data) {
+  logEvent("onBookmarkMoved", data);
+});
+
+chromium.bookmarks.onBookmarkChildrenReordered.addListener(function(data) {
+  logEvent("onBookmarkChildrenReordered", data);
+});
+
 var prefix = "bookmark_";
 
 var toggleBookmark = function(event) {
@@ -109,4 +139,5 @@ var loadBookmarks = function() {
 <body onload="loadBookmarks()">
 <div id="container">
 </div>
+<div id="event-log"></div>
 </body>