Made the bookmarks extension APIs aware of managed bookmarks.

The mutating APIs now fail with a new error if they're used to modify
managed bookmarks.

BUG=49598
R=kalman@chromium.org, sky@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275385 0039d316-1c4b-4281-b951-d872f2087c98
parent 0db294c0
...@@ -69,6 +69,15 @@ bool ChromeBookmarkClient::IsDescendantOfManagedNode(const BookmarkNode* node) { ...@@ -69,6 +69,15 @@ bool ChromeBookmarkClient::IsDescendantOfManagedNode(const BookmarkNode* node) {
return node && node->HasAncestor(managed_node_); return node && node->HasAncestor(managed_node_);
} }
bool ChromeBookmarkClient::HasDescendantsOfManagedNode(
const std::vector<const BookmarkNode*>& list) {
for (size_t i = 0; i < list.size(); ++i) {
if (IsDescendantOfManagedNode(list[i]))
return true;
}
return false;
}
bool ChromeBookmarkClient::PreferTouchIcon() { bool ChromeBookmarkClient::PreferTouchIcon() {
#if !defined(OS_IOS) #if !defined(OS_IOS)
return false; return false;
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_ #ifndef CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_
#define CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_ #define CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_H_
#include <vector>
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/deferred_sequenced_task_runner.h" #include "base/deferred_sequenced_task_runner.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
...@@ -37,6 +39,10 @@ class ChromeBookmarkClient : public BookmarkClient, ...@@ -37,6 +39,10 @@ class ChromeBookmarkClient : public BookmarkClient,
// Returns true if the given node belongs to the managed bookmarks tree. // Returns true if the given node belongs to the managed bookmarks tree.
bool IsDescendantOfManagedNode(const BookmarkNode* node); bool IsDescendantOfManagedNode(const BookmarkNode* node);
// Returns true if there is at least one managed node in the |list|.
bool HasDescendantsOfManagedNode(
const std::vector<const BookmarkNode*>& list);
// BookmarkClient: // BookmarkClient:
virtual bool PreferTouchIcon() OVERRIDE; virtual bool PreferTouchIcon() OVERRIDE;
virtual base::CancelableTaskTracker::TaskId GetFaviconImageForURL( virtual base::CancelableTaskTracker::TaskId GetFaviconImageForURL(
......
...@@ -266,3 +266,19 @@ TEST_F(ChromeBookmarkClientTest, RemoveAllDoesntRemoveManaged) { ...@@ -266,3 +266,19 @@ TEST_F(ChromeBookmarkClientTest, RemoveAllDoesntRemoveManaged) {
EXPECT_EQ(0, model_->bookmark_bar_node()->child_count()); EXPECT_EQ(0, model_->bookmark_bar_node()->child_count());
Mock::VerifyAndClearExpectations(&observer_); Mock::VerifyAndClearExpectations(&observer_);
} }
TEST_F(ChromeBookmarkClientTest, HasDescendantsOfManagedNode) {
const BookmarkNode* user_node = model_->AddURL(model_->other_node(),
0,
base::ASCIIToUTF16("foo bar"),
GURL("http://www.google.com"));
const BookmarkNode* managed_node = client_->managed_node()->GetChild(0);
ASSERT_TRUE(managed_node);
std::vector<const BookmarkNode*> nodes;
EXPECT_FALSE(client_->HasDescendantsOfManagedNode(nodes));
nodes.push_back(user_node);
EXPECT_FALSE(client_->HasDescendantsOfManagedNode(nodes));
nodes.push_back(managed_node);
EXPECT_TRUE(client_->HasDescendantsOfManagedNode(nodes));
}
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/bookmarks/bookmark_stats.h" #include "chrome/browser/bookmarks/bookmark_stats.h"
#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
#include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h" #include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h"
#include "chrome/browser/extensions/api/bookmarks/bookmark_api_helpers.h" #include "chrome/browser/extensions/api/bookmarks/bookmark_api_helpers.h"
#include "chrome/browser/extensions/extension_web_ui.h" #include "chrome/browser/extensions/extension_web_ui.h"
...@@ -351,10 +352,15 @@ void BookmarkManagerPrivateDragEventRouter::ClearBookmarkNodeData() { ...@@ -351,10 +352,15 @@ void BookmarkManagerPrivateDragEventRouter::ClearBookmarkNodeData() {
bool ClipboardBookmarkManagerFunction::CopyOrCut(bool cut, bool ClipboardBookmarkManagerFunction::CopyOrCut(bool cut,
const std::vector<std::string>& id_list) { const std::vector<std::string>& id_list) {
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); ChromeBookmarkClient* client = GetChromeBookmarkClient();
std::vector<const BookmarkNode*> nodes; std::vector<const BookmarkNode*> nodes;
EXTENSION_FUNCTION_VALIDATE(GetNodesFromVector(model, id_list, &nodes)); EXTENSION_FUNCTION_VALIDATE(
bookmark_utils::CopyToClipboard(model, nodes, cut); GetNodesFromVector(client->model(), id_list, &nodes));
if (cut && client->HasDescendantsOfManagedNode(nodes)) {
error_ = bookmark_keys::kModifyManagedError;
return false;
}
bookmark_utils::CopyToClipboard(client->model(), nodes, cut);
return true; return true;
} }
...@@ -381,10 +387,8 @@ bool BookmarkManagerPrivatePasteFunction::RunOnReady() { ...@@ -381,10 +387,8 @@ bool BookmarkManagerPrivatePasteFunction::RunOnReady() {
EXTENSION_FUNCTION_VALIDATE(params); EXTENSION_FUNCTION_VALIDATE(params);
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile());
const BookmarkNode* parent_node = GetNodeFromString(model, params->parent_id); const BookmarkNode* parent_node = GetNodeFromString(model, params->parent_id);
if (!parent_node) { if (!CanBeModified(parent_node))
error_ = bookmark_keys::kNoParentError;
return false; return false;
}
bool can_paste = bookmark_utils::CanPasteFromClipboard(parent_node); bool can_paste = bookmark_utils::CanPasteFromClipboard(parent_node);
if (!can_paste) if (!can_paste)
return false; return false;
...@@ -413,13 +417,15 @@ bool BookmarkManagerPrivateCanPasteFunction::RunOnReady() { ...@@ -413,13 +417,15 @@ bool BookmarkManagerPrivateCanPasteFunction::RunOnReady() {
scoped_ptr<CanPaste::Params> params(CanPaste::Params::Create(*args_)); scoped_ptr<CanPaste::Params> params(CanPaste::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params); EXTENSION_FUNCTION_VALIDATE(params);
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); ChromeBookmarkClient* client = GetChromeBookmarkClient();
const BookmarkNode* parent_node = GetNodeFromString(model, params->parent_id); const BookmarkNode* parent_node =
GetNodeFromString(client->model(), params->parent_id);
if (!parent_node) { if (!parent_node) {
error_ = bookmark_keys::kNoParentError; error_ = bookmark_keys::kNoParentError;
return false; return false;
} }
bool can_paste = bookmark_utils::CanPasteFromClipboard(parent_node); bool can_paste = bookmark_utils::CanPasteFromClipboard(parent_node) &&
!client->IsDescendantOfManagedNode(parent_node);
SetResult(new base::FundamentalValue(can_paste)); SetResult(new base::FundamentalValue(can_paste));
return true; return true;
} }
...@@ -433,10 +439,8 @@ bool BookmarkManagerPrivateSortChildrenFunction::RunOnReady() { ...@@ -433,10 +439,8 @@ bool BookmarkManagerPrivateSortChildrenFunction::RunOnReady() {
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile());
const BookmarkNode* parent_node = GetNodeFromString(model, params->parent_id); const BookmarkNode* parent_node = GetNodeFromString(model, params->parent_id);
if (!parent_node) { if (!CanBeModified(parent_node))
error_ = bookmark_keys::kNoParentError;
return false; return false;
}
model->SortChildren(parent_node); model->SortChildren(parent_node);
return true; return true;
} }
...@@ -564,10 +568,8 @@ bool BookmarkManagerPrivateDropFunction::RunOnReady() { ...@@ -564,10 +568,8 @@ bool BookmarkManagerPrivateDropFunction::RunOnReady() {
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile());
const BookmarkNode* drop_parent = GetNodeFromString(model, params->parent_id); const BookmarkNode* drop_parent = GetNodeFromString(model, params->parent_id);
if (!drop_parent) { if (!CanBeModified(drop_parent))
error_ = bookmark_keys::kNoParentError;
return false; return false;
}
int drop_index; int drop_index;
if (params->index) if (params->index)
...@@ -730,15 +732,15 @@ bool BookmarkManagerPrivateRemoveTreesFunction::RunOnReady() { ...@@ -730,15 +732,15 @@ bool BookmarkManagerPrivateRemoveTreesFunction::RunOnReady() {
scoped_ptr<RemoveTrees::Params> params(RemoveTrees::Params::Create(*args_)); scoped_ptr<RemoveTrees::Params> params(RemoveTrees::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params); EXTENSION_FUNCTION_VALIDATE(params);
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); ChromeBookmarkClient* client = GetChromeBookmarkClient();
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
bookmarks::ScopedGroupBookmarkActions group_deletes(model); bookmarks::ScopedGroupBookmarkActions group_deletes(client->model());
#endif #endif
int64 id; int64 id;
for (size_t i = 0; i < params->id_list.size(); ++i) { for (size_t i = 0; i < params->id_list.size(); ++i) {
if (!GetBookmarkIdAsInt64(params->id_list[i], &id)) if (!GetBookmarkIdAsInt64(params->id_list[i], &id))
return false; return false;
if (!bookmark_api_helpers::RemoveNode(model, id, true, &error_)) if (!bookmark_api_helpers::RemoveNode(client, id, true, &error_))
return false; return false;
} }
......
...@@ -7,12 +7,15 @@ ...@@ -7,12 +7,15 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/prefs/pref_service.h" #include "base/prefs/pref_service.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
#include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/bookmarks/test/bookmark_test_helpers.h" #include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/user_prefs/user_prefs.h" #include "components/user_prefs/user_prefs.h"
...@@ -23,6 +26,25 @@ ...@@ -23,6 +26,25 @@
#define MAYBE_BookmarkManager BookmarkManager #define MAYBE_BookmarkManager BookmarkManager
#endif #endif
IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_BookmarkManager) { IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_BookmarkManager) {
// Add managed bookmarks.
Profile* profile = browser()->profile();
ChromeBookmarkClient* client =
BookmarkModelFactory::GetChromeBookmarkClientForProfile(profile);
BookmarkModel* model = client->model();
test::WaitForBookmarkModelToLoad(model);
base::ListValue list;
base::DictionaryValue* node = new base::DictionaryValue();
node->SetString("name", "Managed Bookmark");
node->SetString("url", "http://www.chromium.org");
list.Append(node);
node = new base::DictionaryValue();
node->SetString("name", "Managed Folder");
node->Set("children", new base::ListValue());
list.Append(node);
profile->GetPrefs()->Set(prefs::kManagedBookmarks, list);
ASSERT_EQ(2, client->managed_node()->child_count());
ASSERT_TRUE(RunComponentExtensionTest("bookmark_manager/standard")) ASSERT_TRUE(RunComponentExtensionTest("bookmark_manager/standard"))
<< message_; << message_;
} }
......
...@@ -20,6 +20,7 @@ const char kInvalidIndexError[] = "Index out of bounds."; ...@@ -20,6 +20,7 @@ const char kInvalidIndexError[] = "Index out of bounds.";
const char kInvalidUrlError[] = "Invalid URL."; const char kInvalidUrlError[] = "Invalid URL.";
const char kModifySpecialError[] = "Can't modify the root bookmark folders."; const char kModifySpecialError[] = "Can't modify the root bookmark folders.";
const char kEditBookmarksDisabled[] = "Bookmark editing is disabled."; const char kEditBookmarksDisabled[] = "Bookmark editing is disabled.";
const char kModifyManagedError[] = "Can't modify managed bookmarks.";
} // namespace bookmark_api_constants } // namespace bookmark_api_constants
} // namespace extensions } // namespace extensions
...@@ -24,6 +24,7 @@ extern const char kInvalidIndexError[]; ...@@ -24,6 +24,7 @@ extern const char kInvalidIndexError[];
extern const char kInvalidUrlError[]; extern const char kInvalidUrlError[];
extern const char kModifySpecialError[]; extern const char kModifySpecialError[];
extern const char kEditBookmarksDisabled[]; extern const char kEditBookmarksDisabled[];
extern const char kModifyManagedError[];
} // namespace bookmark_api_constants } // namespace bookmark_api_constants
} // namespace extensions } // namespace extensions
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
#include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h" #include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h"
#include "chrome/common/extensions/api/bookmarks.h" #include "chrome/common/extensions/api/bookmarks.h"
#include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_model.h"
...@@ -97,10 +98,11 @@ void AddNodeFoldersOnly(const BookmarkNode* node, ...@@ -97,10 +98,11 @@ void AddNodeFoldersOnly(const BookmarkNode* node,
return AddNodeHelper(node, nodes, recurse, true); return AddNodeHelper(node, nodes, recurse, true);
} }
bool RemoveNode(BookmarkModel* model, bool RemoveNode(ChromeBookmarkClient* client,
int64 id, int64 id,
bool recursive, bool recursive,
std::string* error) { std::string* error) {
BookmarkModel* model = client->model();
const BookmarkNode* node = GetBookmarkNodeByID(model, id); const BookmarkNode* node = GetBookmarkNodeByID(model, id);
if (!node) { if (!node) {
*error = keys::kNoNodeError; *error = keys::kNoNodeError;
...@@ -110,6 +112,10 @@ bool RemoveNode(BookmarkModel* model, ...@@ -110,6 +112,10 @@ bool RemoveNode(BookmarkModel* model,
*error = keys::kModifySpecialError; *error = keys::kModifySpecialError;
return false; return false;
} }
if (client->IsDescendantOfManagedNode(node)) {
*error = keys::kModifyManagedError;
return false;
}
if (node->is_folder() && !node->empty() && !recursive) { if (node->is_folder() && !node->empty() && !recursive) {
*error = keys::kFolderNotEmptyError; *error = keys::kFolderNotEmptyError;
return false; return false;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
class BookmarkModel; class BookmarkModel;
class BookmarkNode; class BookmarkNode;
class ChromeBookmarkClient;
// Helper functions. // Helper functions.
namespace extensions { namespace extensions {
...@@ -34,7 +35,7 @@ void AddNodeFoldersOnly(const BookmarkNode* node, ...@@ -34,7 +35,7 @@ void AddNodeFoldersOnly(const BookmarkNode* node,
api::bookmarks::BookmarkTreeNode> >* nodes, api::bookmarks::BookmarkTreeNode> >* nodes,
bool recurse); bool recurse);
bool RemoveNode(BookmarkModel* model, bool RemoveNode(ChromeBookmarkClient* client,
int64 id, int64 id,
bool recursive, bool recursive,
std::string* error); std::string* error);
......
...@@ -7,10 +7,14 @@ ...@@ -7,10 +7,14 @@
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
#include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h" #include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h"
#include "chrome/common/extensions/api/bookmarks.h" #include "chrome/common/extensions/api/bookmarks.h"
#include "chrome/test/base/testing_profile.h"
#include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/test/test_bookmark_client.h" #include "components/bookmarks/test/bookmark_test_helpers.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace extensions { namespace extensions {
...@@ -22,12 +26,19 @@ namespace bookmark_api_helpers { ...@@ -22,12 +26,19 @@ namespace bookmark_api_helpers {
class ExtensionBookmarksTest : public testing::Test { class ExtensionBookmarksTest : public testing::Test {
public: public:
ExtensionBookmarksTest() : client_(NULL), model_(NULL), folder_(NULL) {}
virtual void SetUp() OVERRIDE { virtual void SetUp() OVERRIDE {
model_ = client_.CreateModel(false); profile_.CreateBookmarkModel(false);
client_ =
BookmarkModelFactory::GetChromeBookmarkClientForProfile(&profile_);
model_ = client_->model();
test::WaitForBookmarkModelToLoad(model_);
model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("Digg"), model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("Digg"),
GURL("http://www.reddit.com")); GURL("http://www.reddit.com"));
model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("News"), model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("News"),
GURL("http://www.foxnews.com")); GURL("http://www.foxnews.com"));
folder_ = model_->AddFolder( folder_ = model_->AddFolder(
model_->other_node(), 0, base::ASCIIToUTF16("outer folder")); model_->other_node(), 0, base::ASCIIToUTF16("outer folder"));
model_->AddFolder(folder_, 0, base::ASCIIToUTF16("inner folder 1")); model_->AddFolder(folder_, 0, base::ASCIIToUTF16("inner folder 1"));
...@@ -38,10 +49,13 @@ class ExtensionBookmarksTest : public testing::Test { ...@@ -38,10 +49,13 @@ class ExtensionBookmarksTest : public testing::Test {
folder_, 0, base::ASCIIToUTF16("CNet"), GURL("http://cnet.com")); folder_, 0, base::ASCIIToUTF16("CNet"), GURL("http://cnet.com"));
} }
test::TestBookmarkClient client_; content::TestBrowserThreadBundle thread_bundle_;
scoped_ptr<BookmarkModel> model_; TestingProfile profile_;
ChromeBookmarkClient* client_;
BookmarkModel* model_;
const BookmarkNode* folder_; const BookmarkNode* folder_;
}; };
TEST_F(ExtensionBookmarksTest, GetFullTreeFromRoot) { TEST_F(ExtensionBookmarksTest, GetFullTreeFromRoot) {
scoped_ptr<BookmarkTreeNode> tree( scoped_ptr<BookmarkTreeNode> tree(
GetBookmarkTreeNode(model_->other_node(), GetBookmarkTreeNode(model_->other_node(),
...@@ -80,5 +94,42 @@ TEST_F(ExtensionBookmarksTest, GetSubtreeFoldersOnly) { ...@@ -80,5 +94,42 @@ TEST_F(ExtensionBookmarksTest, GetSubtreeFoldersOnly) {
ASSERT_EQ("inner folder 1", inner_folder->title); ASSERT_EQ("inner folder 1", inner_folder->title);
} }
TEST_F(ExtensionBookmarksTest, RemoveNodeInvalidId) {
int64 invalid_id = model_->next_node_id();
std::string error;
EXPECT_FALSE(RemoveNode(client_, invalid_id, true, &error));
EXPECT_EQ(keys::kNoNodeError, error);
}
TEST_F(ExtensionBookmarksTest, RemoveNodePermanent) {
std::string error;
EXPECT_FALSE(RemoveNode(client_, model_->other_node()->id(), true, &error));
EXPECT_EQ(keys::kModifySpecialError, error);
}
TEST_F(ExtensionBookmarksTest, RemoveNodeManaged) {
const BookmarkNode* managed_bookmark =
model_->AddURL(client_->managed_node(),
0,
base::ASCIIToUTF16("Chromium"),
GURL("http://www.chromium.org"));
std::string error;
EXPECT_FALSE(RemoveNode(client_, managed_bookmark->id(), true, &error));
EXPECT_EQ(keys::kModifyManagedError, error);
}
TEST_F(ExtensionBookmarksTest, RemoveNodeNotRecursive) {
std::string error;
EXPECT_FALSE(RemoveNode(client_, folder_->id(), false, &error));
EXPECT_EQ(keys::kFolderNotEmptyError, error);
}
TEST_F(ExtensionBookmarksTest, RemoveNodeRecursive) {
EXPECT_EQ(3, model_->other_node()->child_count());
std::string error;
EXPECT_TRUE(RemoveNode(client_, folder_->id(), true, &error));
EXPECT_EQ(2, model_->other_node()->child_count());
}
} // namespace bookmark_api_helpers } // namespace bookmark_api_helpers
} // namespace extensions } // namespace extensions
...@@ -2,8 +2,38 @@ ...@@ -2,8 +2,38 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/prefs/pref_service.h"
#include "base/values.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
#include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_node.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/bookmarks/test/bookmark_test_helpers.h"
IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Bookmarks) { IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Bookmarks) {
// Add test managed bookmarks to verify that the bookmarks API can read them
// and can't modify them.
Profile* profile = browser()->profile();
ChromeBookmarkClient* client =
BookmarkModelFactory::GetChromeBookmarkClientForProfile(profile);
BookmarkModel* model = client->model();
test::WaitForBookmarkModelToLoad(model);
base::ListValue list;
base::DictionaryValue* node = new base::DictionaryValue();
node->SetString("name", "Managed Bookmark");
node->SetString("url", "http://www.chromium.org");
list.Append(node);
node = new base::DictionaryValue();
node->SetString("name", "Managed Folder");
node->Set("children", new base::ListValue());
list.Append(node);
profile->GetPrefs()->Set(prefs::kManagedBookmarks, list);
ASSERT_EQ(2, client->managed_node()->child_count());
ASSERT_TRUE(RunExtensionTest("bookmarks")) << message_; ASSERT_TRUE(RunExtensionTest("bookmarks")) << message_;
} }
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/bookmarks/bookmark_html_writer.h" #include "chrome/browser/bookmarks/bookmark_html_writer.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h" #include "chrome/browser/extensions/api/bookmarks/bookmark_api_constants.h"
#include "chrome/browser/extensions/api/bookmarks/bookmark_api_helpers.h" #include "chrome/browser/extensions/api/bookmarks/bookmark_api_helpers.h"
...@@ -117,6 +118,10 @@ bool BookmarksFunction::RunAsync() { ...@@ -117,6 +118,10 @@ bool BookmarksFunction::RunAsync() {
return true; return true;
} }
ChromeBookmarkClient* BookmarksFunction::GetChromeBookmarkClient() {
return BookmarkModelFactory::GetChromeBookmarkClientForProfile(GetProfile());
}
bool BookmarksFunction::GetBookmarkIdAsInt64(const std::string& id_string, bool BookmarksFunction::GetBookmarkIdAsInt64(const std::string& id_string,
int64* id) { int64* id) {
if (base::StringToInt64(id_string, id)) if (base::StringToInt64(id_string, id))
...@@ -154,14 +159,8 @@ const BookmarkNode* BookmarksFunction::CreateBookmarkNode( ...@@ -154,14 +159,8 @@ const BookmarkNode* BookmarksFunction::CreateBookmarkNode(
return NULL; return NULL;
} }
const BookmarkNode* parent = GetBookmarkNodeByID(model, parentId); const BookmarkNode* parent = GetBookmarkNodeByID(model, parentId);
if (!parent) { if (!CanBeModified(parent))
error_ = keys::kNoParentError;
return NULL; return NULL;
}
if (parent->is_root()) { // Can't create children of the root.
error_ = keys::kModifySpecialError;
return NULL;
}
int index; int index;
if (!details.index.get()) { // Optional (defaults to end). if (!details.index.get()) { // Optional (defaults to end).
...@@ -211,6 +210,23 @@ bool BookmarksFunction::EditBookmarksEnabled() { ...@@ -211,6 +210,23 @@ bool BookmarksFunction::EditBookmarksEnabled() {
return false; return false;
} }
bool BookmarksFunction::CanBeModified(const BookmarkNode* node) {
if (!node) {
error_ = keys::kNoParentError;
return false;
}
if (node->is_root()) {
error_ = keys::kModifySpecialError;
return false;
}
ChromeBookmarkClient* client = GetChromeBookmarkClient();
if (client->IsDescendantOfManagedNode(node)) {
error_ = keys::kModifyManagedError;
return false;
}
return true;
}
void BookmarksFunction::BookmarkModelChanged() { void BookmarksFunction::BookmarkModelChanged() {
} }
...@@ -566,8 +582,8 @@ bool BookmarksRemoveFunction::RunOnReady() { ...@@ -566,8 +582,8 @@ bool BookmarksRemoveFunction::RunOnReady() {
if (name() == BookmarksRemoveTreeFunction::function_name()) if (name() == BookmarksRemoveTreeFunction::function_name())
recursive = true; recursive = true;
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); ChromeBookmarkClient* client = GetChromeBookmarkClient();
if (!bookmark_api_helpers::RemoveNode(model, id, recursive, &error_)) if (!bookmark_api_helpers::RemoveNode(client, id, recursive, &error_))
return false; return false;
return true; return true;
...@@ -630,15 +646,8 @@ bool BookmarksMoveFunction::RunOnReady() { ...@@ -630,15 +646,8 @@ bool BookmarksMoveFunction::RunOnReady() {
parent = GetBookmarkNodeByID(model, parentId); parent = GetBookmarkNodeByID(model, parentId);
} }
if (!parent) { if (!CanBeModified(parent) || !CanBeModified(node))
error_ = keys::kNoParentError;
// TODO(erikkay) return an error message.
return false;
}
if (parent == model->root_node()) {
error_ = keys::kModifySpecialError;
return false; return false;
}
int index; int index;
if (params->destination.index.get()) { // Optional (defaults to end). if (params->destination.index.get()) { // Optional (defaults to end).
...@@ -695,7 +704,7 @@ bool BookmarksUpdateFunction::RunOnReady() { ...@@ -695,7 +704,7 @@ bool BookmarksUpdateFunction::RunOnReady() {
} }
const BookmarkNode* node = GetBookmarkNodeFromId(params->id); const BookmarkNode* node = GetBookmarkNodeFromId(params->id);
if (!node) if (!CanBeModified(node))
return false; return false;
BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile()); BookmarkModel* model = BookmarkModelFactory::GetForProfile(GetProfile());
...@@ -896,7 +905,7 @@ void BookmarksMoveFunction::GetQuotaLimitHeuristics( ...@@ -896,7 +905,7 @@ void BookmarksMoveFunction::GetQuotaLimitHeuristics(
void BookmarksUpdateFunction::GetQuotaLimitHeuristics( void BookmarksUpdateFunction::GetQuotaLimitHeuristics(
QuotaLimitHeuristics* heuristics) const { QuotaLimitHeuristics* heuristics) const {
BookmarksQuotaLimitFactory::Build<BookmarksUpdateFunction>(heuristics); BookmarksQuotaLimitFactory::Build<BookmarksUpdateFunction>(heuristics);
}; }
void BookmarksCreateFunction::GetQuotaLimitHeuristics( void BookmarksCreateFunction::GetQuotaLimitHeuristics(
QuotaLimitHeuristics* heuristics) const { QuotaLimitHeuristics* heuristics) const {
......
...@@ -18,6 +18,8 @@ ...@@ -18,6 +18,8 @@
#include "extensions/browser/event_router.h" #include "extensions/browser/event_router.h"
#include "ui/shell_dialogs/select_file_dialog.h" #include "ui/shell_dialogs/select_file_dialog.h"
class ChromeBookmarkClient;
namespace base { namespace base {
class FilePath; class FilePath;
class ListValue; class ListValue;
...@@ -125,6 +127,9 @@ class BookmarksFunction : public ChromeAsyncExtensionFunction, ...@@ -125,6 +127,9 @@ class BookmarksFunction : public ChromeAsyncExtensionFunction,
// RunAsync semantic equivalent called when the bookmarks are ready. // RunAsync semantic equivalent called when the bookmarks are ready.
virtual bool RunOnReady() = 0; virtual bool RunOnReady() = 0;
// Helper to get the ChromeBookmarkClient.
ChromeBookmarkClient* GetChromeBookmarkClient();
// Helper to get the bookmark id as int64 from the given string id. // Helper to get the bookmark id as int64 from the given string id.
// Sets error_ to an error string if the given id string can't be parsed // Sets error_ to an error string if the given id string can't be parsed
// as an int64. In case of error, doesn't change id and returns false. // as an int64. In case of error, doesn't change id and returns false.
...@@ -146,6 +151,12 @@ class BookmarksFunction : public ChromeAsyncExtensionFunction, ...@@ -146,6 +151,12 @@ class BookmarksFunction : public ChromeAsyncExtensionFunction,
// error_ to the appropriate error string. // error_ to the appropriate error string.
bool EditBookmarksEnabled(); bool EditBookmarksEnabled();
// Helper that checks if |node| can be modified. Returns false if |node|
// is NULL, or a managed node, or the root node. In these cases the node
// can't be edited, can't have new child nodes appended, and its direct
// children can't be moved or reordered.
bool CanBeModified(const BookmarkNode* node);
private: private:
// BaseBookmarkModelObserver: // BaseBookmarkModelObserver:
virtual void BookmarkModelChanged() OVERRIDE; virtual void BookmarkModelChanged() OVERRIDE;
......
...@@ -9,6 +9,7 @@ const pass = chrome.test.callbackPass; ...@@ -9,6 +9,7 @@ const pass = chrome.test.callbackPass;
const fail = chrome.test.callbackFail; const fail = chrome.test.callbackFail;
const assertEq = chrome.test.assertEq; const assertEq = chrome.test.assertEq;
const assertTrue = chrome.test.assertTrue; const assertTrue = chrome.test.assertTrue;
const assertFalse = chrome.test.assertFalse;
const bookmarks = chrome.bookmarks; const bookmarks = chrome.bookmarks;
const bookmarkManager = chrome.bookmarkManagerPrivate; const bookmarkManager = chrome.bookmarkManagerPrivate;
var fooNode, fooNode2, barNode, gooNode, count, emptyFolder, emptyFolder2; var fooNode, fooNode2, barNode, gooNode, count, emptyFolder, emptyFolder2;
...@@ -294,6 +295,26 @@ var tests = [ ...@@ -294,6 +295,26 @@ var tests = [
})); }));
}, },
function clipboard6() {
// Verify that we can't cut managed folders.
bookmarks.getChildren('4', pass(function(result) {
assertEq(2, result.length);
const error = "Can't modify managed bookmarks.";
bookmarkManager.cut([ result[0].id ], fail(error));
// Copying is fine.
bookmarkManager.copy([ result[0].id ], pass());
// Pasting to a managed folder is not allowed.
assertTrue(result[1].url === undefined);
bookmarkManager.canPaste(result[1].id, pass(function(result) {
assertFalse(result, 'Should not be able to paste to managed folders.');
}));
bookmarkManager.paste(result[1].id, fail(error));
}));
},
function canEdit() { function canEdit() {
bookmarkManager.canEdit(pass(function(result) { bookmarkManager.canEdit(pass(function(result) {
assertTrue(result, 'Should be able to edit bookmarks'); assertTrue(result, 'Should be able to edit bookmarks');
......
...@@ -12,7 +12,13 @@ ...@@ -12,7 +12,13 @@
var expected = [ var expected = [
{"children": [ {"children": [
{children:[], id:"1", parentId:"0", index:0, title:"Bookmarks bar"}, {children:[], id:"1", parentId:"0", index:0, title:"Bookmarks bar"},
{children:[], id:"2", parentId:"0", index:1, title:"Other bookmarks"} {children:[], id:"2", parentId:"0", index:1, title:"Other bookmarks"},
{id:"4", parentId:"0", index:3, title:"Managed bookmarks", 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:[]}
]
}
], ],
id:"0", title:"" id:"0", title:""
} }
...@@ -130,6 +136,10 @@ chrome.test.runTests([ ...@@ -130,6 +136,10 @@ chrome.test.runTests([
chrome.bookmarks.get("1", pass(function(results) { chrome.bookmarks.get("1", pass(function(results) {
chrome.test.assertTrue(compareNode(results[0], expected[0].children[0])); chrome.test.assertTrue(compareNode(results[0], expected[0].children[0]));
})); }));
chrome.bookmarks.get("5", pass(function(results) {
chrome.test.assertTrue(compareNode(
results[0], expected[0].children[2].children[0]));
}));
chrome.bookmarks.get("42", fail("Can't find bookmark for id.")); chrome.bookmarks.get("42", fail("Can't find bookmark for id."));
}, },
...@@ -193,6 +203,12 @@ chrome.test.runTests([ ...@@ -193,6 +203,12 @@ chrome.test.runTests([
chrome.bookmarks.create(node, fail(error)); chrome.bookmarks.create(node, fail(error));
}, },
function createInManaged() {
const error = "Can't modify managed bookmarks.";
var node = {parentId:"4", title:"g404", url:"http://www.google.com/404"};
chrome.bookmarks.create(node, fail(error));
},
function createFolder() { function createFolder() {
var node = {parentId:"1", title:"foo bar"}; // folder var node = {parentId:"1", title:"foo bar"}; // folder
chrome.test.listenOnce(chrome.bookmarks.onCreated, function(id, created) { chrome.test.listenOnce(chrome.bookmarks.onCreated, function(id, created) {
...@@ -286,6 +302,23 @@ chrome.test.runTests([ ...@@ -286,6 +302,23 @@ chrome.test.runTests([
})); }));
}, },
function moveToManaged() {
var managed_node = expected[0].children[2];
chrome.test.assertEq("4", managed_node.id);
const error = "Can't modify managed bookmarks.";
chrome.bookmarks.move(node1.id, {parentId:managed_node.id}, fail(error));
verifyTreeIsExpected(pass());
},
function moveFromManaged() {
var managed_node = expected[0].children[2];
var moving_node = managed_node.children[0];
var other = expected[0].children[1];
const error = "Can't modify managed bookmarks.";
chrome.bookmarks.move(moving_node.id, {parentId:other.id}, fail(error));
verifyTreeIsExpected(pass());
},
function search() { function search() {
chrome.bookmarks.search("baz bar", pass(function(results) { chrome.bookmarks.search("baz bar", pass(function(results) {
// matches node1 & node3 // matches node1 & node3
...@@ -312,6 +345,11 @@ chrome.test.runTests([ ...@@ -312,6 +345,11 @@ chrome.test.runTests([
// Does not match any node since permanent nodes are stripped from search // Does not match any node since permanent nodes are stripped from search
chrome.test.assertEq(0, results.length); chrome.test.assertEq(0, results.length);
})); }));
chrome.bookmarks.search("Managed", pass(function(results) {
// Matches the Managed Bookmark and the Managed Folder but not the
// managed_node.
chrome.test.assertEq(2, results.length);
}));
}, },
function update() { function update() {
...@@ -353,6 +391,13 @@ chrome.test.runTests([ ...@@ -353,6 +391,13 @@ chrome.test.runTests([
})); }));
}, },
function updateManaged() {
var managed_node = expected[0].children[2];
var updating_node = managed_node.children[0];
const error = "Can't modify managed bookmarks.";
chrome.bookmarks.update(updating_node.id, {"title": "New"}, fail(error));
},
function remove() { function remove() {
var parentId = node1.parentId; var parentId = node1.parentId;
chrome.test.listenOnce(chrome.bookmarks.onRemoved, chrome.test.listenOnce(chrome.bookmarks.onRemoved,
...@@ -372,6 +417,13 @@ chrome.test.runTests([ ...@@ -372,6 +417,13 @@ chrome.test.runTests([
})); }));
}, },
function removeManaged() {
var managed_node = expected[0].children[2];
var removing_node = managed_node.children[0];
const error = "Can't modify managed bookmarks.";
chrome.bookmarks.remove(removing_node.id, fail(error));
},
function searchRemoved() { function searchRemoved() {
// Search for deleted node // Search for deleted node
chrome.bookmarks.search("baz bar", pass(function(results) { chrome.bookmarks.search("baz bar", pass(function(results) {
...@@ -396,6 +448,13 @@ chrome.test.runTests([ ...@@ -396,6 +448,13 @@ chrome.test.runTests([
})); }));
}, },
function removeManagedTree() {
var managed_node = expected[0].children[2];
var managed_folder = managed_node.children[1];
const error = "Can't modify managed bookmarks.";
chrome.bookmarks.removeTree(managed_folder.id, fail(error));
},
function searchRemovedTree() { function searchRemovedTree() {
// Search for deleted folder and enclosed node3 // Search for deleted folder and enclosed node3
chrome.bookmarks.search("foo bar", pass(function(results) { chrome.bookmarks.search("foo bar", pass(function(results) {
...@@ -493,7 +552,8 @@ chrome.test.runTests([ ...@@ -493,7 +552,8 @@ chrome.test.runTests([
chrome.test.assertTrue(failed, "Calling with 0 should fail"); chrome.test.assertTrue(failed, "Calling with 0 should fail");
chrome.bookmarks.getRecent(10000, pass(function(results) { chrome.bookmarks.getRecent(10000, pass(function(results) {
chrome.test.assertEq(3, results.length, // Should include the "Managed Bookmark".
chrome.test.assertEq(4, results.length,
"Should have gotten all recent bookmarks"); "Should have gotten all recent bookmarks");
})); }));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment