Added the "unmodifiable" property to chrome.bookmarks.BookmarkTreeNode.

This optional property indicates the reason why this BookmarkTreeNode can't
be modified by the user nor the extension. For now the only value defined is
"managed", indicating that this bookmark was configured by the system
administrator.

If the property is not present then the bookmark is modifiable (default).

BUG=49598
R=kalman@chromium.org

Review URL: https://codereview.chromium.org/320473002

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

chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc

@@ -621,10 +621,11 @@ bool BookmarkManagerPrivateGetSubtreeFunction::RunOnReady() {
   }
 
   std::vector<linked_ptr<api::bookmarks::BookmarkTreeNode> > nodes;
+  ChromeBookmarkClient* client = GetChromeBookmarkClient();
   if (params->folders_only)
-    bookmark_api_helpers::AddNodeFoldersOnly(node, &nodes, true);
+    bookmark_api_helpers::AddNodeFoldersOnly(client, node, &nodes, true);
   else
-    bookmark_api_helpers::AddNode(node, &nodes, true);
+    bookmark_api_helpers::AddNode(client, node, &nodes, true);
   results_ = GetSubtree::Results::Create(nodes);
   return true;
 }
@@ -653,7 +654,8 @@ bool BookmarkManagerPrivateCreateWithMetaInfoFunction::RunOnReady() {
     return false;
 
   scoped_ptr<api::bookmarks::BookmarkTreeNode> result_node(
-      bookmark_api_helpers::GetBookmarkTreeNode(node, false, false));
+      bookmark_api_helpers::GetBookmarkTreeNode(
+          GetChromeBookmarkClient(), node, false, false));
   results_ = CreateWithMetaInfo::Results::Create(*result_node);
 
   return true;

chrome/browser/extensions/api/bookmarks/bookmark_api_helpers.cc

@@ -24,12 +24,14 @@ namespace bookmark_api_helpers {
 
 namespace {
 
-void AddNodeHelper(const BookmarkNode* node,
+void AddNodeHelper(ChromeBookmarkClient* client,
+                   const BookmarkNode* node,
                    std::vector<linked_ptr<BookmarkTreeNode> >* nodes,
                    bool recurse,
                    bool only_folders) {
   if (node->IsVisible()) {
-    linked_ptr<BookmarkTreeNode> new_node(GetBookmarkTreeNode(node,
+    linked_ptr<BookmarkTreeNode> new_node(GetBookmarkTreeNode(client,
+                                                              node,
                                                               recurse,
                                                               only_folders));
     nodes->push_back(new_node);
@@ -38,7 +40,8 @@ void AddNodeHelper(const BookmarkNode* node,
 
 }  // namespace
 
-BookmarkTreeNode* GetBookmarkTreeNode(const BookmarkNode* node,
+BookmarkTreeNode* GetBookmarkTreeNode(ChromeBookmarkClient* client,
+                                      const BookmarkNode* node,
                                       bool recurse,
                                       bool only_folders) {
   BookmarkTreeNode* bookmark_tree_node = new BookmarkTreeNode;
@@ -70,13 +73,16 @@ BookmarkTreeNode* GetBookmarkTreeNode(const BookmarkNode* node,
         new double(floor(node->date_added().ToDoubleT() * 1000)));
   }
 
+  if (client->IsDescendantOfManagedNode(node))
+    bookmark_tree_node->unmodifiable = BookmarkTreeNode::UNMODIFIABLE_MANAGED;
+
   if (recurse && node->is_folder()) {
     std::vector<linked_ptr<BookmarkTreeNode> > children;
     for (int i = 0; i < node->child_count(); ++i) {
       const BookmarkNode* child = node->GetChild(i);
       if (child->IsVisible() && (!only_folders || child->is_folder())) {
         linked_ptr<BookmarkTreeNode> child_node(
-            GetBookmarkTreeNode(child, true, only_folders));
+            GetBookmarkTreeNode(client, child, true, only_folders));
         children.push_back(child_node);
       }
     }
@@ -86,16 +92,18 @@ BookmarkTreeNode* GetBookmarkTreeNode(const BookmarkNode* node,
   return bookmark_tree_node;
 }
 
-void AddNode(const BookmarkNode* node,
+void AddNode(ChromeBookmarkClient* client,
+             const BookmarkNode* node,
              std::vector<linked_ptr<BookmarkTreeNode> >* nodes,
              bool recurse) {
-  return AddNodeHelper(node, nodes, recurse, false);
+  return AddNodeHelper(client, node, nodes, recurse, false);
 }
 
-void AddNodeFoldersOnly(const BookmarkNode* node,
+void AddNodeFoldersOnly(ChromeBookmarkClient* client,
+                        const BookmarkNode* node,
                         std::vector<linked_ptr<BookmarkTreeNode> >* nodes,
                         bool recurse) {
-  return AddNodeHelper(node, nodes, recurse, true);
+  return AddNodeHelper(client, node, nodes, recurse, true);
 }
 
 bool RemoveNode(ChromeBookmarkClient* client,

chrome/browser/extensions/api/bookmarks/bookmark_api_helpers.h

@@ -21,16 +21,19 @@ namespace bookmark_api_helpers {
 
 // The returned value is owned by the caller.
 api::bookmarks::BookmarkTreeNode* GetBookmarkTreeNode(
+    ChromeBookmarkClient* client,
     const BookmarkNode* node,
     bool recurse,
     bool only_folders);
 
 // Add a JSON representation of |node| to the JSON |nodes|.
-void AddNode(const BookmarkNode* node,
+void AddNode(ChromeBookmarkClient* client,
+             const BookmarkNode* node,
              std::vector<linked_ptr<api::bookmarks::BookmarkTreeNode> >* nodes,
              bool recurse);
 
-void AddNodeFoldersOnly(const BookmarkNode* node,
+void AddNodeFoldersOnly(ChromeBookmarkClient* client,
+                        const BookmarkNode* node,
                         std::vector<linked_ptr<
                             api::bookmarks::BookmarkTreeNode> >* nodes,
                         bool recurse);

chrome/browser/extensions/api/bookmarks/bookmark_api_helpers_unittest.cc

@@ -26,7 +26,8 @@ namespace bookmark_api_helpers {
 
 class ExtensionBookmarksTest : public testing::Test {
  public:
-  ExtensionBookmarksTest() : client_(NULL), model_(NULL), folder_(NULL) {}
+  ExtensionBookmarksTest()
+      : client_(NULL), model_(NULL), node_(NULL), folder_(NULL) {}
 
   virtual void SetUp() OVERRIDE {
     profile_.CreateBookmarkModel(false);
@@ -35,8 +36,8 @@ class ExtensionBookmarksTest : public testing::Test {
     model_ = client_->model();
     test::WaitForBookmarkModelToLoad(model_);
 
-    model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("Digg"),
-                   GURL("http://www.reddit.com"));
+    node_ = model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("Digg"),
+                           GURL("http://www.reddit.com"));
     model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("News"),
                    GURL("http://www.foxnews.com"));
     folder_ = model_->AddFolder(
@@ -53,12 +54,14 @@ class ExtensionBookmarksTest : public testing::Test {
   TestingProfile profile_;
   ChromeBookmarkClient* client_;
   BookmarkModel* model_;
+  const BookmarkNode* node_;
   const BookmarkNode* folder_;
 };
 
 TEST_F(ExtensionBookmarksTest, GetFullTreeFromRoot) {
   scoped_ptr<BookmarkTreeNode> tree(
-      GetBookmarkTreeNode(model_->other_node(),
+      GetBookmarkTreeNode(client_,
+                          model_->other_node(),
                           true,    // Recurse.
                           false));  // Not only folders.
   ASSERT_EQ(3U, tree->children->size());
@@ -66,7 +69,8 @@ TEST_F(ExtensionBookmarksTest, GetFullTreeFromRoot) {
 
 TEST_F(ExtensionBookmarksTest, GetFoldersOnlyFromRoot) {
   scoped_ptr<BookmarkTreeNode> tree(
-      GetBookmarkTreeNode(model_->other_node(),
+      GetBookmarkTreeNode(client_,
+                          model_->other_node(),
                           true,   // Recurse.
                           true));  // Only folders.
   ASSERT_EQ(1U, tree->children->size());
@@ -74,7 +78,8 @@ TEST_F(ExtensionBookmarksTest, GetFoldersOnlyFromRoot) {
 
 TEST_F(ExtensionBookmarksTest, GetSubtree) {
   scoped_ptr<BookmarkTreeNode> tree(
-      GetBookmarkTreeNode(folder_,
+      GetBookmarkTreeNode(client_,
+                          folder_,
                           true,    // Recurse.
                           false));  // Not only folders.
   ASSERT_EQ(4U, tree->children->size());
@@ -85,7 +90,8 @@ TEST_F(ExtensionBookmarksTest, GetSubtree) {
 
 TEST_F(ExtensionBookmarksTest, GetSubtreeFoldersOnly) {
   scoped_ptr<BookmarkTreeNode> tree(
-      GetBookmarkTreeNode(folder_,
+      GetBookmarkTreeNode(client_,
+                          folder_,
                           true,   // Recurse.
                           true));  // Only folders.
   ASSERT_EQ(2U, tree->children->size());
@@ -94,6 +100,34 @@ TEST_F(ExtensionBookmarksTest, GetSubtreeFoldersOnly) {
   ASSERT_EQ("inner folder 1", inner_folder->title);
 }
 
+TEST_F(ExtensionBookmarksTest, GetModifiableNode) {
+  scoped_ptr<BookmarkTreeNode> tree(
+      GetBookmarkTreeNode(client_,
+                          node_,
+                          false,    // Recurse.
+                          false));  // Only folders.
+  EXPECT_EQ("Digg", tree->title);
+  ASSERT_TRUE(tree->url);
+  EXPECT_EQ("http://www.reddit.com/", *tree->url);
+  EXPECT_EQ(BookmarkTreeNode::UNMODIFIABLE_NONE, tree->unmodifiable);
+}
+
+TEST_F(ExtensionBookmarksTest, GetManagedNode) {
+  const BookmarkNode* managed_bookmark =
+      model_->AddURL(client_->managed_node(),
+                     0,
+                     base::ASCIIToUTF16("Chromium"),
+                     GURL("http://www.chromium.org/"));
+  scoped_ptr<BookmarkTreeNode> tree(
+      GetBookmarkTreeNode(client_,
+                          managed_bookmark,
+                          false,    // Recurse.
+                          false));  // Only folders.
+  EXPECT_EQ("Chromium", tree->title);
+  EXPECT_EQ("http://www.chromium.org/", *tree->url);
+  EXPECT_EQ(BookmarkTreeNode::UNMODIFIABLE_MANAGED, tree->unmodifiable);
+}
+
 TEST_F(ExtensionBookmarksTest, RemoveNodeInvalidId) {
   int64 invalid_id = model_->next_node_id();
   std::string error;

chrome/browser/extensions/api/bookmarks/bookmarks_api.cc

@@ -237,9 +237,11 @@ void BookmarksFunction::BookmarkModelLoaded(BookmarkModel* model,
   Release();  // Balanced in RunOnReady().
 }
 
-BookmarkEventRouter::BookmarkEventRouter(BrowserContext* context,
-                                         BookmarkModel* model)
-    : browser_context_(context), model_(model) {
+BookmarkEventRouter::BookmarkEventRouter(Profile* profile)
+    : browser_context_(profile),
+      model_(BookmarkModelFactory::GetForProfile(profile)),
+      client_(
+          BookmarkModelFactory::GetChromeBookmarkClientForProfile(profile)) {
   model_->AddObserver(this);
 }
 
@@ -291,7 +293,7 @@ void BookmarkEventRouter::BookmarkNodeAdded(BookmarkModel* model,
                                             int index) {
   const BookmarkNode* node = parent->GetChild(index);
   scoped_ptr<BookmarkTreeNode> tree_node(
-      bookmark_api_helpers::GetBookmarkTreeNode(node, false, false));
+      bookmark_api_helpers::GetBookmarkTreeNode(client_, node, false, false));
   DispatchEvent(bookmarks::OnCreated::kEventName,
                 bookmarks::OnCreated::Create(base::Int64ToString(node->id()),
                                              *tree_node));
@@ -399,10 +401,8 @@ BookmarksAPI::GetFactoryInstance() {
 }
 
 void BookmarksAPI::OnListenerAdded(const EventListenerInfo& details) {
-  bookmark_event_router_.reset(new BookmarkEventRouter(
-      browser_context_,
-      BookmarkModelFactory::GetForProfile(
-          Profile::FromBrowserContext(browser_context_))));
+  bookmark_event_router_.reset(
+      new BookmarkEventRouter(Profile::FromBrowserContext(browser_context_)));
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
@@ -412,6 +412,7 @@ bool BookmarksGetFunction::RunOnReady() {
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   std::vector<linked_ptr<BookmarkTreeNode> > nodes;
+  ChromeBookmarkClient* client = GetChromeBookmarkClient();
   if (params->id_or_id_list.as_strings) {
     std::vector<std::string>& ids = *params->id_or_id_list.as_strings;
     size_t count = ids.size();
@@ -420,14 +421,14 @@ bool BookmarksGetFunction::RunOnReady() {
       const BookmarkNode* node = GetBookmarkNodeFromId(ids[i]);
       if (!node)
         return false;
-      bookmark_api_helpers::AddNode(node, &nodes, false);
+      bookmark_api_helpers::AddNode(client, node, &nodes, false);
     }
   } else {
     const BookmarkNode* node =
         GetBookmarkNodeFromId(*params->id_or_id_list.as_string);
     if (!node)
       return false;
-    bookmark_api_helpers::AddNode(node, &nodes, false);
+    bookmark_api_helpers::AddNode(client, node, &nodes, false);
   }
 
   results_ = bookmarks::Get::Results::Create(nodes);
@@ -447,7 +448,8 @@ bool BookmarksGetChildrenFunction::RunOnReady() {
   int child_count = node->child_count();
   for (int i = 0; i < child_count; ++i) {
     const BookmarkNode* child = node->GetChild(i);
-    bookmark_api_helpers::AddNode(child, &nodes, false);
+    bookmark_api_helpers::AddNode(
+        GetChromeBookmarkClient(), child, &nodes, false);
   }
 
   results_ = bookmarks::GetChildren::Results::Create(nodes);
@@ -471,7 +473,8 @@ bool BookmarksGetRecentFunction::RunOnReady() {
   std::vector<const BookmarkNode*>::iterator i = nodes.begin();
   for (; i != nodes.end(); ++i) {
     const BookmarkNode* node = *i;
-    bookmark_api_helpers::AddNode(node, &tree_nodes, false);
+    bookmark_api_helpers::AddNode(
+        GetChromeBookmarkClient(), node, &tree_nodes, false);
   }
 
   results_ = bookmarks::GetRecent::Results::Create(tree_nodes);
@@ -482,7 +485,7 @@ bool BookmarksGetTreeFunction::RunOnReady() {
   std::vector<linked_ptr<BookmarkTreeNode> > nodes;
   const BookmarkNode* node =
       BookmarkModelFactory::GetForProfile(GetProfile())->root_node();
-  bookmark_api_helpers::AddNode(node, &nodes, true);
+  bookmark_api_helpers::AddNode(GetChromeBookmarkClient(), node, &nodes, true);
   results_ = bookmarks::GetTree::Results::Create(nodes);
   return true;
 }
@@ -497,7 +500,7 @@ bool BookmarksGetSubTreeFunction::RunOnReady() {
     return false;
 
   std::vector<linked_ptr<BookmarkTreeNode> > nodes;
-  bookmark_api_helpers::AddNode(node, &nodes, true);
+  bookmark_api_helpers::AddNode(GetChromeBookmarkClient(), node, &nodes, true);
   results_ = bookmarks::GetSubTree::Results::Create(nodes);
   return true;
 }
@@ -542,9 +545,10 @@ bool BookmarksSearchFunction::RunOnReady() {
   }
 
   std::vector<linked_ptr<BookmarkTreeNode> > tree_nodes;
+  ChromeBookmarkClient* client = GetChromeBookmarkClient();
   for (std::vector<const BookmarkNode*>::iterator node_iter = nodes.begin();
        node_iter != nodes.end(); ++node_iter) {
-    bookmark_api_helpers::AddNode(*node_iter, &tree_nodes, false);
+    bookmark_api_helpers::AddNode(client, *node_iter, &tree_nodes, false);
   }
 
   results_ = bookmarks::Search::Results::Create(tree_nodes);
@@ -602,8 +606,8 @@ bool BookmarksCreateFunction::RunOnReady() {
   if (!node)
     return false;
 
-  scoped_ptr<BookmarkTreeNode> ret(
-      bookmark_api_helpers::GetBookmarkTreeNode(node, false, false));
+  scoped_ptr<BookmarkTreeNode> ret(bookmark_api_helpers::GetBookmarkTreeNode(
+      GetChromeBookmarkClient(), node, false, false));
   results_ = bookmarks::Create::Results::Create(*ret);
 
   return true;
@@ -663,7 +667,8 @@ bool BookmarksMoveFunction::RunOnReady() {
   model->Move(node, parent, index);
 
   scoped_ptr<BookmarkTreeNode> tree_node(
-      bookmark_api_helpers::GetBookmarkTreeNode(node, false, false));
+      bookmark_api_helpers::GetBookmarkTreeNode(
+          GetChromeBookmarkClient(), node, false, false));
   results_ = bookmarks::Move::Results::Create(*tree_node);
 
   return true;
@@ -718,7 +723,8 @@ bool BookmarksUpdateFunction::RunOnReady() {
     model->SetURL(node, url);
 
   scoped_ptr<BookmarkTreeNode> tree_node(
-      bookmark_api_helpers::GetBookmarkTreeNode(node, false, false));
+      bookmark_api_helpers::GetBookmarkTreeNode(
+          GetChromeBookmarkClient(), node, false, false));
   results_ = bookmarks::Update::Results::Create(*tree_node);
   return true;
 }

chrome/browser/extensions/api/bookmarks/bookmarks_api.h

@@ -41,7 +41,7 @@ struct CreateDetails;
 // the extension system.
 class BookmarkEventRouter : public BookmarkModelObserver {
  public:
-  BookmarkEventRouter(content::BrowserContext* context, BookmarkModel* model);
+  explicit BookmarkEventRouter(Profile* profile);
   virtual ~BookmarkEventRouter();
 
   // BookmarkModelObserver:
@@ -80,6 +80,7 @@ class BookmarkEventRouter : public BookmarkModelObserver {
 
   content::BrowserContext* browser_context_;
   BookmarkModel* model_;
+  ChromeBookmarkClient* client_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkEventRouter);
 };

chrome/common/extensions/api/bookmarks.json

@@ -57,6 +57,12 @@
             "optional": true,
             "description": "When the contents of this folder last changed, in milliseconds since the epoch."
           },
+          "unmodifiable": {
+            "type": "string",
+            "enum": ["managed"],
+            "optional": true,
+            "description": "Indicates the reason why this node is unmodifiable. The <var>managed</var> value indicates that this node was configured by the system administrator. Omitted if the node can be modified by the user and the extension (default)."
+          },
           "children": {
             "type": "array",
             "optional": true,

chrome/test/data/extensions/api_test/bookmarks/test.js

@@ -13,10 +13,12 @@ var expected = [
   {"children": [
       {children:[], id:"1", parentId:"0", index:0, title:"Bookmarks bar"},
       {children:[], id:"2", parentId:"0", index:1, title:"Other bookmarks"},
-      {id:"4", parentId:"0", index:3, title:"Managed bookmarks", children:[
+      {id:"4", parentId:"0", index:3, title:"Managed bookmarks",
+       unmodifiable:"managed", children:[
           {id:"5", parentId:"4", index:0, title:"Managed Bookmark",
-           url: "http://www.chromium.org/"},
-          {id:"6", parentId:"4", index:1, title:"Managed Folder", children:[]}
+           url:"http://www.chromium.org/", unmodifiable:"managed"},
+          {id:"6", parentId:"4", index:1, title:"Managed Folder",
+           children:[], unmodifiable:"managed"}
         ]
       }
     ],
@@ -61,6 +63,10 @@ function compareNode(left, right) {
     return "url mismatch: " + left.url + " != " + right.url;
   if (left.index != right.index)
     return "index mismatch: " + left.index + " != " + right.index;
+  if (left.unmodifiable != right.unmodifiable) {
+    return "unmodifiable mismatch: " + left.unmodifiable +
+           " != " + right.unmodifiable;
+  }
   return true;
 }
 

tools/json_schema_compiler/cc_generator.py

@@ -146,11 +146,16 @@ class _Generator(object):
   def _GenerateInitializersAndBody(self, type_):
     items = []
     for prop in type_.properties.values():
-      if prop.optional:
-        continue
-
       t = prop.type_
-      if t.property_type == PropertyType.INTEGER:
+
+      real_t = self._type_helper.FollowRef(t)
+      if real_t.property_type == PropertyType.ENUM:
+        items.append('%s(%s)' % (
+            prop.unix_name,
+            self._type_helper.GetEnumNoneValue(t)))
+      elif prop.optional:
+        continue
+      elif t.property_type == PropertyType.INTEGER:
         items.append('%s(0)' % prop.unix_name)
       elif t.property_type == PropertyType.DOUBLE:
         items.append('%s(0.0)' % prop.unix_name)
@@ -160,12 +165,11 @@ class _Generator(object):
             t.property_type == PropertyType.ARRAY or
             t.property_type == PropertyType.BINARY or  # mapped to std::string
             t.property_type == PropertyType.CHOICES or
-            t.property_type == PropertyType.ENUM or
             t.property_type == PropertyType.OBJECT or
             t.property_type == PropertyType.FUNCTION or
             t.property_type == PropertyType.REF or
             t.property_type == PropertyType.STRING):
-        # TODO(miket): It would be nice to initialize CHOICES and ENUM, but we
+        # TODO(miket): It would be nice to initialize CHOICES, but we
         # don't presently have the semantics to indicate which one of a set
         # should be the default.
         continue

tools/json_schema_compiler/test/enums_unittest.cc

@@ -40,6 +40,11 @@ TEST(JsonSchemaCompilerEnumsTest, EnumsAsTypes) {
     EXPECT_TRUE(args.Equals(ReturnsEnumAsType::Results::Create(
         ENUMERATION_ONE).get()));
   }
+  {
+    HasEnumeration enumeration;
+    EXPECT_EQ(ENUMERATION_NONE, enumeration.enumeration);
+    EXPECT_EQ(ENUMERATION_NONE, enumeration.optional_enumeration);
+  }
   {
     HasEnumeration enumeration;
     base::DictionaryValue value;