Refactor chrome.bookmarks API to use JSON schema compiler.

chrome/browser/bookmarks/bookmark_extension_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/bookmarks/bookmark_extension_api.h"
 
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/i18n/file_util_icu.h"
 #include "base/i18n/time_formatting.h"
@@ skipping 16 matching lines @@
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/extensions/extensions_quota_service.h"
 #include "chrome/browser/importer/importer_data_types.h"
 #include "chrome/browser/importer/importer_host.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/chrome_select_file_policy.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/api/bookmarks.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_service.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace keys = bookmark_extension_api_constants;
 
+namespace Create = extensions::api::bookmarks::Create;

[brettw, 2012/07/26 18:03:15]

These namespaces seem really weird to me. It's confusing later on when you
write, for example, Get::Params. I would never expect "Get" to be a namespace.
Do you have any ideas for making this more clear?

[mitchellwrosen, 2012/07/26 18:05:14]

It's the only way of using the JSON schema compiler I've ever seen. I agree that
it looks weird.

[brettw, 2012/07/27 20:05:10]

It wouldn't look so weird if the users of this code wrote bookmarks::Create and
bookmarks::Get since then it falls under my expectations of what namespaces look
like.

So what do you think about doing "namespace bookmarks =
extensions::api::bookmarks" and then using that below?

[mitchellwrosen, 2012/07/27 22:53:21]

Done.

+namespace Get = extensions::api::bookmarks::Get;
+namespace GetChildren = extensions::api::bookmarks::GetChildren;
+namespace GetRecent = extensions::api::bookmarks::GetRecent;
+namespace GetSubTree = extensions::api::bookmarks::GetSubTree;
+namespace GetTree = extensions::api::bookmarks::GetTree;
+namespace Move = extensions::api::bookmarks::Move;
+namespace Remove = extensions::api::bookmarks::Remove;
+namespace Search = extensions::api::bookmarks::Search;
+namespace Update = extensions::api::bookmarks::Update;
+
 using base::TimeDelta;
 using content::BrowserThread;
 using content::WebContents;
+using extensions::api::bookmarks::BookmarkTreeNode;
 
 typedef QuotaLimitHeuristic::Bucket Bucket;
 typedef QuotaLimitHeuristic::Config Config;
 typedef QuotaLimitHeuristic::BucketList BucketList;
 typedef ExtensionsQuotaService::TimedLimit TimedLimit;
 typedef ExtensionsQuotaService::SustainedLimit SustainedLimit;
 typedef QuotaLimitHeuristic::BucketMapper BucketMapper;
 
 namespace {
 
@@ skipping 128 matching lines @@
   base::JSONWriter::Write(&args, &json_args);
   DispatchEvent(model->profile(), keys::kOnBookmarkMoved, json_args);
 }
 
 void BookmarkExtensionEventRouter::BookmarkNodeAdded(BookmarkModel* model,
                                                      const BookmarkNode* parent,
                                                      int index) {
   ListValue args;
   const BookmarkNode* node = parent->GetChild(index);
   args.Append(new StringValue(base::Int64ToString(node->id())));
-  DictionaryValue* obj =
-      bookmark_extension_helpers::GetNodeDictionary(node, false, false);
-  args.Append(obj);
+  BookmarkTreeNode* obj =
+      bookmark_extension_helpers::GetBookmarkTreeNode(node, false, false);
+  args.Append(obj->ToValue().get());
 
   std::string json_args;
   base::JSONWriter::Write(&args, &json_args);
   DispatchEvent(model->profile(), keys::kOnBookmarkCreated, json_args);
 }
 
 void BookmarkExtensionEventRouter::BookmarkNodeRemoved(
     BookmarkModel* model,
     const BookmarkNode* parent,
     int index,
@@ skipping 73 matching lines @@
     BookmarkModel* model) {
   ListValue args;
   std::string json_args;
   base::JSONWriter::Write(&args, &json_args);
   DispatchEvent(model->profile(),
                 keys::kOnBookmarkImportEnded,
                 json_args);
 }
 
 bool GetBookmarksFunction::RunImpl() {
+  scoped_ptr<Get::Params> params(Get::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  std::vector<linked_ptr<BookmarkTreeNode> > nodes;
   BookmarkModel* model = profile()->GetBookmarkModel();
-  scoped_ptr<ListValue> json(new ListValue());
-  Value* arg0;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &arg0));
-  if (arg0->IsType(Value::TYPE_LIST)) {
-    const ListValue* ids = static_cast<const ListValue*>(arg0);
-    size_t count = ids->GetSize();
+  if (params->id_or_id_list_type == Get::Params::ID_OR_ID_LIST_ARRAY) {
+    std::vector<std::string>* ids = params->id_or_id_list_array.get();
+    size_t count = ids->size();
     EXTENSION_FUNCTION_VALIDATE(count > 0);
     for (size_t i = 0; i < count; ++i) {
       int64 id;
-      std::string id_string;
-      EXTENSION_FUNCTION_VALIDATE(ids->GetString(i, &id_string));
-      if (!GetBookmarkIdAsInt64(id_string, &id))
+      if (!GetBookmarkIdAsInt64(ids->at(i), &id))
         return false;
       const BookmarkNode* node = model->GetNodeByID(id);
       if (!node) {
         error_ = keys::kNoNodeError;
         return false;
       } else {
-        bookmark_extension_helpers::AddNode(node, json.get(), false);
+        bookmark_extension_helpers::AddNode(node, &nodes, false);
       }
     }
   } else {
     int64 id;
-    std::string id_string;
-    EXTENSION_FUNCTION_VALIDATE(arg0->GetAsString(&id_string));
-    if (!GetBookmarkIdAsInt64(id_string, &id))
+    if (!GetBookmarkIdAsInt64(*params->id_or_id_list_string, &id))
       return false;
     const BookmarkNode* node = model->GetNodeByID(id);
     if (!node) {
       error_ = keys::kNoNodeError;
       return false;
     }
-    bookmark_extension_helpers::AddNode(node, json.get(), false);
+    bookmark_extension_helpers::AddNode(node, &nodes, false);
   }
 
-  SetResult(json.release());
+  results_ = Get::Results::Create(nodes);
   return true;
 }
 
 bool GetBookmarkChildrenFunction::RunImpl() {
-  BookmarkModel* model = profile()->GetBookmarkModel();
+  scoped_ptr<GetChildren::Params> params(GetChildren::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
   int64 id;
-  std::string id_string;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &id_string));
-  if (!GetBookmarkIdAsInt64(id_string, &id))
+  if (!GetBookmarkIdAsInt64(params->id, &id))
     return false;
-  scoped_ptr<ListValue> json(new ListValue());
-  const BookmarkNode* node = model->GetNodeByID(id);
+
+  std::vector<linked_ptr<BookmarkTreeNode> > nodes;
+  const BookmarkNode* node = profile()->GetBookmarkModel()->GetNodeByID(id);
   if (!node) {
     error_ = keys::kNoNodeError;
     return false;
   }
   int child_count = node->child_count();
   for (int i = 0; i < child_count; ++i) {
     const BookmarkNode* child = node->GetChild(i);
-    bookmark_extension_helpers::AddNode(child, json.get(), false);
+    bookmark_extension_helpers::AddNode(child, &nodes, false);
   }
 
-  SetResult(json.release());
+  results_ = GetChildren::Results::Create(nodes);
   return true;
 }
 
 bool GetBookmarkRecentFunction::RunImpl() {
-  int number_of_items;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &number_of_items));
-  if (number_of_items < 1)
+  scoped_ptr<GetRecent::Params> params(GetRecent::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+  if (params->number_of_items < 1)
     return false;
 
-  BookmarkModel* model = profile()->GetBookmarkModel();
-  ListValue* json = new ListValue();
   std::vector<const BookmarkNode*> nodes;
-  bookmark_utils::GetMostRecentlyAddedEntries(model, number_of_items, &nodes);
+  bookmark_utils::GetMostRecentlyAddedEntries(profile()->GetBookmarkModel(),
+                                              params->number_of_items,
+                                              &nodes);
+
+  std::vector<linked_ptr<BookmarkTreeNode> > tree_nodes;
   std::vector<const BookmarkNode*>::iterator i = nodes.begin();
   for (; i != nodes.end(); ++i) {
     const BookmarkNode* node = *i;
-    bookmark_extension_helpers::AddNode(node, json, false);
+    bookmark_extension_helpers::AddNode(node, &tree_nodes, false);
   }
-  SetResult(json);
+
+  results_ = GetRecent::Results::Create(tree_nodes);
   return true;
 }
 
 bool GetBookmarkTreeFunction::RunImpl() {
-  BookmarkModel* model = profile()->GetBookmarkModel();
-  scoped_ptr<ListValue> json(new ListValue());
-  const BookmarkNode* node = model->root_node();
-  bookmark_extension_helpers::AddNode(node, json.get(), true);
-  SetResult(json.release());
+  std::vector<linked_ptr<BookmarkTreeNode> > nodes;
+  const BookmarkNode* node = profile()->GetBookmarkModel()->root_node();
+  bookmark_extension_helpers::AddNode(node, &nodes, true);
+  results_ = GetTree::Results::Create(nodes);
   return true;
 }
 
 bool GetBookmarkSubTreeFunction::RunImpl() {
-  BookmarkModel* model = profile()->GetBookmarkModel();
-  scoped_ptr<ListValue> json(new ListValue());
-  Value* arg0;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &arg0));
+  scoped_ptr<GetSubTree::Params> params(GetSubTree::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
   int64 id;
-  std::string id_string;
-  EXTENSION_FUNCTION_VALIDATE(arg0->GetAsString(&id_string));
-  if (!GetBookmarkIdAsInt64(id_string, &id))
+  if (!GetBookmarkIdAsInt64(params->id, &id))
     return false;
-  const BookmarkNode* node = model->GetNodeByID(id);
+
+  const BookmarkNode* node = profile()->GetBookmarkModel()->GetNodeByID(id);
   if (!node) {
     error_ = keys::kNoNodeError;
     return false;
   }
-  bookmark_extension_helpers::AddNode(node, json.get(), true);
-  SetResult(json.release());
+
+  std::vector<linked_ptr<BookmarkTreeNode> > nodes;
+  bookmark_extension_helpers::AddNode(node, &nodes, true);
+  results_ = GetSubTree::Results::Create(nodes);
   return true;
 }
 
 bool SearchBookmarksFunction::RunImpl() {
-  string16 query;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &query));
+  scoped_ptr<Search::Params> params(Search::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
 
-  BookmarkModel* model = profile()->GetBookmarkModel();
-  ListValue* json = new ListValue();
   std::string lang = profile()->GetPrefs()->GetString(prefs::kAcceptLanguages);
   std::vector<const BookmarkNode*> nodes;
-  bookmark_utils::GetBookmarksContainingText(model, query,
+  bookmark_utils::GetBookmarksContainingText(profile()->GetBookmarkModel(),
+                                             UTF8ToUTF16(params->query),
                                              std::numeric_limits<int>::max(),
-                                             lang, &nodes);
-  std::vector<const BookmarkNode*>::iterator i = nodes.begin();
-  for (; i != nodes.end(); ++i) {
-    const BookmarkNode* node = *i;
-    bookmark_extension_helpers::AddNode(node, json, false);
+                                             lang,
+                                             &nodes);
+
+  std::vector<linked_ptr<BookmarkTreeNode> > tree_nodes;
+  for (std::vector<const BookmarkNode*>::iterator node_iter = nodes.begin();
+       node_iter != nodes.end(); ++node_iter) {
+    bookmark_extension_helpers::AddNode(*node_iter, &tree_nodes, false);
   }
 
-  SetResult(json);
+  results_ = Search::Results::Create(tree_nodes);
   return true;
 }
 
 // static
 bool RemoveBookmarkFunction::ExtractIds(const ListValue* args,
                                         std::list<int64>* ids,
                                         bool* invalid_id) {
   std::string id_string;
   if (!args->GetString(0, &id_string))
     return false;
   int64 id;
   if (base::StringToInt64(id_string, &id))
     ids->push_back(id);
   else
     *invalid_id = true;
   return true;
 }
 
 bool RemoveBookmarkFunction::RunImpl() {
   if (!EditBookmarksEnabled())
     return false;
-  std::list<int64> ids;
-  bool invalid_id = false;
-  EXTENSION_FUNCTION_VALIDATE(ExtractIds(args_.get(), &ids, &invalid_id));
-  if (invalid_id) {
+
+  scoped_ptr<Remove::Params> params(Remove::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  int64 id;
+  if (!base::StringToInt64(params->id, &id)) {
     error_ = keys::kInvalidIdError;
     return false;
   }
+
   bool recursive = false;
   if (name() == RemoveTreeBookmarkFunction::function_name())
     recursive = true;
 
   BookmarkModel* model = profile()->GetBookmarkModel();
-  size_t count = ids.size();
-  EXTENSION_FUNCTION_VALIDATE(count > 0);
-  for (std::list<int64>::iterator it = ids.begin(); it != ids.end(); ++it) {
-    if (!bookmark_extension_helpers::RemoveNode(model, *it, recursive, &error_))
-      return false;
-  }
+  if (!bookmark_extension_helpers::RemoveNode(model, id, recursive, &error_))
+    return false;
+
   return true;
 }
 
 bool CreateBookmarkFunction::RunImpl() {
   if (!EditBookmarksEnabled())
     return false;
-  DictionaryValue* json;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &json));
-  EXTENSION_FUNCTION_VALIDATE(json != NULL);
+
+  scoped_ptr<Create::Params> params(Create::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
 
   BookmarkModel* model = profile()->GetBookmarkModel();
   int64 parentId;
-  if (!json->HasKey(keys::kParentIdKey)) {
+
+  if (!params->bookmark.parent_id.get()) {
     // Optional, default to "other bookmarks".
     parentId = model->other_node()->id();
   } else {
-    std::string parentId_string;
-    EXTENSION_FUNCTION_VALIDATE(json->GetString(keys::kParentIdKey,
-                                                &parentId_string));
-    if (!GetBookmarkIdAsInt64(parentId_string, &parentId))
+    if (!GetBookmarkIdAsInt64(*params->bookmark.parent_id, &parentId))
       return false;
   }
   const BookmarkNode* parent = model->GetNodeByID(parentId);
   if (!parent) {
     error_ = keys::kNoParentError;
     return false;
   }
   if (parent->is_root()) {  // Can't create children of the root.
     error_ = keys::kModifySpecialError;
     return false;
   }
 
   int index;
-  if (!json->HasKey(keys::kIndexKey)) {  // Optional (defaults to end).
+  if (!params->bookmark.index.get()) {  // Optional (defaults to end).
     index = parent->child_count();
   } else {
-    EXTENSION_FUNCTION_VALIDATE(json->GetInteger(keys::kIndexKey, &index));
+    index = *params->bookmark.index;
     if (index > parent->child_count() || index < 0) {
       error_ = keys::kInvalidIndexError;
       return false;
     }
   }
 
-  string16 title;
-  json->GetString(keys::kTitleKey, &title);  // Optional.
-  std::string url_string;
-  json->GetString(keys::kUrlKey, &url_string);  // Optional.
+  string16 title;  // Optional.
+  if (params->bookmark.title.get())
+    title = UTF8ToUTF16(*params->bookmark.title.get());
+
+  std::string url_string;  // Optional.
+  if (params->bookmark.url.get())
+    url_string = *params->bookmark.url.get();
+
   GURL url(url_string);
   if (!url.is_empty() && !url.is_valid()) {
     error_ = keys::kInvalidUrlError;
     return false;
   }
 
   const BookmarkNode* node;
   if (url_string.length())
     node = model->AddURL(parent, index, title, url);
   else
     node = model->AddFolder(parent, index, title);
   DCHECK(node);
   if (!node) {
     error_ = keys::kNoNodeError;
     return false;
   }
 
-  DictionaryValue* ret =
-      bookmark_extension_helpers::GetNodeDictionary(node, false, false);
-  SetResult(ret);
+  scoped_ptr<BookmarkTreeNode> ret(
+      bookmark_extension_helpers::GetBookmarkTreeNode(node, false, false));
+  results_ = Create::Results::Create(*ret);
 
   return true;
 }
 
 // static
 bool MoveBookmarkFunction::ExtractIds(const ListValue* args,
                                       std::list<int64>* ids,
                                       bool* invalid_id) {
   // For now, Move accepts ID parameters in the same way as an Update.
   return UpdateBookmarkFunction::ExtractIds(args, ids, invalid_id);
 }
 
 bool MoveBookmarkFunction::RunImpl() {
   if (!EditBookmarksEnabled())
     return false;
-  std::list<int64> ids;
-  bool invalid_id = false;
-  EXTENSION_FUNCTION_VALIDATE(ExtractIds(args_.get(), &ids, &invalid_id));
-  if (invalid_id) {
+
+  scoped_ptr<Move::Params> params(Move::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  int64 id;
+  if (!base::StringToInt64(params->id, &id)) {
     error_ = keys::kInvalidIdError;
     return false;
   }
-  EXTENSION_FUNCTION_VALIDATE(ids.size() == 1);
-
-  DictionaryValue* destination;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(1, &destination));
 
   BookmarkModel* model = profile()->GetBookmarkModel();
-  const BookmarkNode* node = model->GetNodeByID(ids.front());
+  const BookmarkNode* node = model->GetNodeByID(id);
   if (!node) {
     error_ = keys::kNoNodeError;
     return false;
   }
   if (model->is_permanent_node(node)) {
     error_ = keys::kModifySpecialError;
     return false;
   }
 
   const BookmarkNode* parent = NULL;
-  if (!destination->HasKey(keys::kParentIdKey)) {
+  if (!params->destination.parent_id.get()) {
     // Optional, defaults to current parent.
     parent = node->parent();
   } else {
-    std::string parentId_string;
-    EXTENSION_FUNCTION_VALIDATE(destination->GetString(keys::kParentIdKey,
-        &parentId_string));
     int64 parentId;
-    if (!GetBookmarkIdAsInt64(parentId_string, &parentId))
+    if (!GetBookmarkIdAsInt64(*params->destination.parent_id, &parentId))
       return false;
 
     parent = model->GetNodeByID(parentId);
   }
   if (!parent) {
     error_ = keys::kNoParentError;
     // TODO(erikkay) return an error message.
     return false;
   }
   if (parent == model->root_node()) {
     error_ = keys::kModifySpecialError;
     return false;
   }
 
   int index;
-  if (destination->HasKey(keys::kIndexKey)) {  // Optional (defaults to end).
-    EXTENSION_FUNCTION_VALIDATE(destination->GetInteger(keys::kIndexKey,
-                                                        &index));
+  if (params->destination.index.get()) {  // Optional (defaults to end).
+    index = *params->destination.index;
     if (index > parent->child_count() || index < 0) {
       error_ = keys::kInvalidIndexError;
       return false;
     }
   } else {
     index = parent->child_count();
   }
 
   model->Move(node, parent, index);
 
-  DictionaryValue* ret =
-      bookmark_extension_helpers::GetNodeDictionary(node, false, false);
-  SetResult(ret);
+  scoped_ptr<BookmarkTreeNode> tree_node(
+      bookmark_extension_helpers::GetBookmarkTreeNode(node, false, false));
+  results_ = Move::Results::Create(*tree_node);
 
   return true;
 }
 
 // static
 bool UpdateBookmarkFunction::ExtractIds(const ListValue* args,
                                         std::list<int64>* ids,
                                         bool* invalid_id) {
   // For now, Update accepts ID parameters in the same way as an Remove.
   return RemoveBookmarkFunction::ExtractIds(args, ids, invalid_id);
 }
 
 bool UpdateBookmarkFunction::RunImpl() {
   if (!EditBookmarksEnabled())
     return false;
-  std::list<int64> ids;
-  bool invalid_id = false;
-  EXTENSION_FUNCTION_VALIDATE(ExtractIds(args_.get(), &ids, &invalid_id));
-  if (invalid_id) {
+
+  scoped_ptr<Update::Params> params(Update::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  int64 id;
+  if (!base::StringToInt64(params->id, &id)) {
     error_ = keys::kInvalidIdError;
     return false;
   }
-  EXTENSION_FUNCTION_VALIDATE(ids.size() == 1);
 
-  DictionaryValue* updates;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(1, &updates));
+  BookmarkModel* model = profile()->GetBookmarkModel();
 
   // Optional but we need to distinguish non present from an empty title.
   string16 title;
-  const bool has_title = updates->GetString(keys::kTitleKey, &title);
+  bool has_title = false;
+  if (params->changes.title.get()) {
+    title = UTF8ToUTF16(*params->changes.title);
+    has_title = true;
+  }
 
   // Optional.
   std::string url_string;
-  updates->GetString(keys::kUrlKey, &url_string);
+  if (params->changes.url.get())
+    url_string = *params->changes.url;
   GURL url(url_string);
   if (!url_string.empty() && !url.is_valid()) {
     error_ = keys::kInvalidUrlError;
     return false;
   }
 
-  BookmarkModel* model = profile()->GetBookmarkModel();
-  const BookmarkNode* node = model->GetNodeByID(ids.front());
+  const BookmarkNode* node = model->GetNodeByID(id);
   if (!node) {
     error_ = keys::kNoNodeError;
     return false;
   }
   if (model->is_permanent_node(node)) {
     error_ = keys::kModifySpecialError;
     return false;
   }
   if (has_title)
     model->SetTitle(node, title);
   if (!url.is_empty())
     model->SetURL(node, url);
 
-  DictionaryValue* ret =
-      bookmark_extension_helpers::GetNodeDictionary(node, false, false);
-  SetResult(ret);
-
+  scoped_ptr<BookmarkTreeNode> tree_node(
+      bookmark_extension_helpers::GetBookmarkTreeNode(node, false, false));
+  results_ = Update::Results::Create(*tree_node);
   return true;
 }
 
 // Mapper superclass for BookmarkFunctions.
 template <typename BucketIdType>
 class BookmarkBucketMapper : public BucketMapper {
  public:
   virtual ~BookmarkBucketMapper() { STLDeleteValues(&buckets_); }
  protected:
   Bucket* GetBucket(const BucketIdType& id) {
@@ skipping 271 matching lines @@
                                            int index,
                                            void* params) {
 #if !defined(OS_ANDROID)
   // Android does not have support for the standard exporter.
   // TODO(jgreenwald): remove ifdef once extensions are no longer built on
   // Android.
   bookmark_html_writer::WriteBookmarks(profile(), path, NULL);
 #endif
   Release();  // Balanced in BookmarksIOFunction::SelectFile()
 }