summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoryfriedman@chromium.org <yfriedman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-05-26 22:07:17 +0000
committeryfriedman@chromium.org <yfriedman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-05-26 22:07:17 +0000
commit6892e2e416250521ed0778206da0093388b99300 (patch)
tree84d80407cbfa15cec3214d6d2822e74143a79ce6
parent46fe10d6c3cc0433626007f93c2b7784a2889ada (diff)
downloadchromium_src-6892e2e416250521ed0778206da0093388b99300.zip
chromium_src-6892e2e416250521ed0778206da0093388b99300.tar.gz
chromium_src-6892e2e416250521ed0778206da0093388b99300.tar.bz2
Revert "Revert 84829 - Initial implementation of "Synced Bookmarks" folder."
Second attempt to land this change. Fixes memory leak (82186), deleted bookmarks (82273) and crash on add bookmarks in windows (82349). Original desscription: Mostly ensures that sycned bookmarks are correctly treated as an immutable folder. Some of the UI-bits maybe incomplete. For example, if enable-synced-bookmarks-folder is set, then synced bookmarks will appear in the bookmark manager page and some components of the UI but it's not on the bookmark bar or anything like that. This change also ensures that the synced bookmark folder matches a sync folder if one is available. BUG= TEST=test bookmark addition/moving around in combination with sync Review URL: http://codereview.chromium.org/7012005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86902 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/app/generated_resources.grd6
-rw-r--r--chrome/browser/bookmarks/bookmark_codec.cc49
-rw-r--r--chrome/browser/bookmarks/bookmark_codec.h12
-rw-r--r--chrome/browser/bookmarks/bookmark_codec_unittest.cc49
-rw-r--r--chrome/browser/bookmarks/bookmark_html_writer.cc23
-rw-r--r--chrome/browser/bookmarks/bookmark_html_writer_unittest.cc11
-rw-r--r--chrome/browser/bookmarks/bookmark_model.cc38
-rw-r--r--chrome/browser/bookmarks/bookmark_model.h25
-rw-r--r--chrome/browser/bookmarks/bookmark_model_test_utils.cc4
-rw-r--r--chrome/browser/bookmarks/bookmark_model_unittest.cc246
-rw-r--r--chrome/browser/bookmarks/bookmark_storage.cc5
-rw-r--r--chrome/browser/bookmarks/bookmark_storage.h6
-rw-r--r--chrome/browser/bookmarks/bookmark_utils.cc6
-rw-r--r--chrome/browser/bookmarks/recently_used_folders_combo_model.cc7
-rw-r--r--chrome/browser/extensions/extension_bookmark_helpers.cc13
-rw-r--r--chrome/browser/extensions/extension_bookmarks_module.cc8
-rw-r--r--chrome/browser/history/history_types.h5
-rw-r--r--chrome/browser/history/starred_url_database.cc13
-rw-r--r--chrome/browser/sync/engine/download_updates_command.cc6
-rw-r--r--chrome/browser/sync/glue/bookmark_change_processor.cc9
-rw-r--r--chrome/browser/sync/glue/bookmark_model_associator.cc52
-rw-r--r--chrome/browser/sync/profile_sync_service_bookmark_unittest.cc116
-rw-r--r--chrome/browser/sync/protocol/sync.proto3
-rw-r--r--chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm2
-rw-r--r--chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc19
-rw-r--r--chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc6
-rw-r--r--chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc7
-rw-r--r--chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc10
-rw-r--r--chrome/common/chrome_switches.cc3
-rw-r--r--chrome/common/chrome_switches.h1
-rw-r--r--chrome/test/data/bookmarks/model_without_sync.json48
31 files changed, 646 insertions, 162 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 564898d..d2487f6 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -5911,6 +5911,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_BOOMARK_BAR_FOLDER_NAME" desc="Name shown in the tree for the bookmarks bar folder">
Bookmarks bar
</message>
+ <message name="IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME" desc="Name shown in the tree for the synced bookmarks folder">
+ Synced bookmarks
+ </message>
<message name="IDS_BOOMARK_BAR_OTHER_FOLDER_NAME" desc="Name shown in the tree for the other bookmarks folder">
Other bookmarks
</message>
@@ -5919,6 +5922,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_BOOMARK_BAR_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the bookmarks bar folder">
Bookmarks Bar
</message>
+ <message name="IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the synced bookmarks folder">
+ Synced Bookmarks
+ </message>
<message name="IDS_BOOMARK_BAR_OTHER_FOLDER_NAME" desc="In Title Case: Name shown in the tree for the other bookmarks folder">
Other Bookmarks
</message>
diff --git a/chrome/browser/bookmarks/bookmark_codec.cc b/chrome/browser/bookmarks/bookmark_codec.cc
index dd145b3..56b0c25 100644
--- a/chrome/browser/bookmarks/bookmark_codec.cc
+++ b/chrome/browser/bookmarks/bookmark_codec.cc
@@ -19,6 +19,7 @@ using base::Time;
const char* BookmarkCodec::kRootsKey = "roots";
const char* BookmarkCodec::kRootFolderNameKey = "bookmark_bar";
const char* BookmarkCodec::kOtherBookmarkFolderNameKey = "other";
+const char* BookmarkCodec::kSyncedBookmarkFolderNameKey = "synced";
const char* BookmarkCodec::kVersionKey = "version";
const char* BookmarkCodec::kChecksumKey = "checksum";
const char* BookmarkCodec::kIdKey = "id";
@@ -43,16 +44,19 @@ BookmarkCodec::BookmarkCodec()
BookmarkCodec::~BookmarkCodec() {}
Value* BookmarkCodec::Encode(BookmarkModel* model) {
- return Encode(model->GetBookmarkBarNode(), model->other_node());
+ return Encode(model->GetBookmarkBarNode(), model->other_node(),
+ model->synced_node());
}
Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node,
- const BookmarkNode* other_folder_node) {
+ const BookmarkNode* other_folder_node,
+ const BookmarkNode* synced_folder_node) {
ids_reassigned_ = false;
InitializeChecksum();
DictionaryValue* roots = new DictionaryValue();
roots->Set(kRootFolderNameKey, EncodeNode(bookmark_bar_node));
roots->Set(kOtherBookmarkFolderNameKey, EncodeNode(other_folder_node));
+ roots->Set(kSyncedBookmarkFolderNameKey, EncodeNode(synced_folder_node));
DictionaryValue* main = new DictionaryValue();
main->SetInteger(kVersionKey, kCurrentVersion);
@@ -67,6 +71,7 @@ Value* BookmarkCodec::Encode(const BookmarkNode* bookmark_bar_node,
bool BookmarkCodec::Decode(BookmarkNode* bb_node,
BookmarkNode* other_folder_node,
+ BookmarkNode* synced_folder_node,
int64* max_id,
const Value& value) {
ids_.clear();
@@ -75,12 +80,13 @@ bool BookmarkCodec::Decode(BookmarkNode* bb_node,
maximum_id_ = 0;
stored_checksum_.clear();
InitializeChecksum();
- bool success = DecodeHelper(bb_node, other_folder_node, value);
+ bool success = DecodeHelper(bb_node, other_folder_node, synced_folder_node,
+ value);
FinalizeChecksum();
// If either the checksums differ or some IDs were missing/not unique,
// reassign IDs.
if (!ids_valid_ || computed_checksum() != stored_checksum())
- ReassignIDs(bb_node, other_folder_node);
+ ReassignIDs(bb_node, other_folder_node, synced_folder_node);
*max_id = maximum_id_ + 1;
return success;
}
@@ -115,6 +121,7 @@ Value* BookmarkCodec::EncodeNode(const BookmarkNode* node) {
bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node,
BookmarkNode* other_folder_node,
+ BookmarkNode* synced_folder_node,
const Value& value) {
if (value.GetType() != Value::TYPE_DICTIONARY)
return false; // Unexpected type.
@@ -147,21 +154,45 @@ bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node,
if (!roots_d_value->Get(kRootFolderNameKey, &root_folder_value) ||
root_folder_value->GetType() != Value::TYPE_DICTIONARY ||
!roots_d_value->Get(kOtherBookmarkFolderNameKey, &other_folder_value) ||
- other_folder_value->GetType() != Value::TYPE_DICTIONARY)
- return false; // Invalid type for root folder and/or other folder.
-
+ other_folder_value->GetType() != Value::TYPE_DICTIONARY) {
+ return false; // Invalid type for root folder and/or other
+ // folder.
+ }
DecodeNode(*static_cast<DictionaryValue*>(root_folder_value), NULL,
bb_node);
DecodeNode(*static_cast<DictionaryValue*>(other_folder_value), NULL,
other_folder_node);
+
+ // Fail silently if we can't deserialize synced bookmarks. We can't require
+ // them to exist in order to be backwards-compatible with older versions of
+ // chrome.
+ Value* synced_folder_value;
+ if (roots_d_value->Get(kSyncedBookmarkFolderNameKey, &synced_folder_value) &&
+ synced_folder_value->GetType() == Value::TYPE_DICTIONARY) {
+ DecodeNode(*static_cast<DictionaryValue*>(synced_folder_value), NULL,
+ synced_folder_node);
+ } else {
+ // If we didn't find the synced folder, we're almost guaranteed to have a
+ // duplicate id when we add the synced folder. Consequently, if we don't
+ // intend to reassign ids in the future (ids_valid_ is still true), then at
+ // least reassign the synced bookmarks to avoid it colliding with anything
+ // else.
+ if (ids_valid_) {
+ ReassignIDsHelper(synced_folder_node);
+ }
+ }
+
// Need to reset the type as decoding resets the type to FOLDER. Similarly
// we need to reset the title as the title is persisted and restored from
// the file.
bb_node->set_type(BookmarkNode::BOOKMARK_BAR);
other_folder_node->set_type(BookmarkNode::OTHER_NODE);
+ synced_folder_node->set_type(BookmarkNode::SYNCED);
bb_node->set_title(l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME));
other_folder_node->set_title(
l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_OTHER_FOLDER_NAME));
+ synced_folder_node->set_title(
+ l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME));
return true;
}
@@ -289,10 +320,12 @@ bool BookmarkCodec::DecodeNode(const DictionaryValue& value,
}
void BookmarkCodec::ReassignIDs(BookmarkNode* bb_node,
- BookmarkNode* other_node) {
+ BookmarkNode* other_node,
+ BookmarkNode* synced_node) {
maximum_id_ = 0;
ReassignIDsHelper(bb_node);
ReassignIDsHelper(other_node);
+ ReassignIDsHelper(synced_node);
ids_reassigned_ = true;
}
diff --git a/chrome/browser/bookmarks/bookmark_codec.h b/chrome/browser/bookmarks/bookmark_codec.h
index fe57d1f..2d29df8 100644
--- a/chrome/browser/bookmarks/bookmark_codec.h
+++ b/chrome/browser/bookmarks/bookmark_codec.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -46,7 +46,8 @@ class BookmarkCodec {
// This method is public for use by StarredURLDatabase in migrating the
// bookmarks out of the database.
Value* Encode(const BookmarkNode* bookmark_bar_node,
- const BookmarkNode* other_folder_node);
+ const BookmarkNode* other_folder_node,
+ const BookmarkNode* synced_folder_node);
// Decodes the previously encoded value to the specified nodes as well as
// setting |max_node_id| to the greatest node id. Returns true on success,
@@ -55,6 +56,7 @@ class BookmarkCodec {
// |max_node_id| is set to the max id of the nodes.
bool Decode(BookmarkNode* bb_node,
BookmarkNode* other_folder_node,
+ BookmarkNode* synced_folder_node,
int64* max_node_id,
const Value& value);
@@ -76,6 +78,7 @@ class BookmarkCodec {
static const char* kRootsKey;
static const char* kRootFolderNameKey;
static const char* kOtherBookmarkFolderNameKey;
+ static const char* kSyncedBookmarkFolderNameKey;
static const char* kVersionKey;
static const char* kChecksumKey;
static const char* kIdKey;
@@ -98,6 +101,7 @@ class BookmarkCodec {
// Helper to perform decoding.
bool DecodeHelper(BookmarkNode* bb_node,
BookmarkNode* other_folder_node,
+ BookmarkNode* synced_folder_node,
const Value& value);
// Decodes the children of the specified node. Returns true on success.
@@ -105,7 +109,9 @@ class BookmarkCodec {
BookmarkNode* parent);
// Reassigns bookmark IDs for all nodes.
- void ReassignIDs(BookmarkNode* bb_node, BookmarkNode* other_node);
+ void ReassignIDs(BookmarkNode* bb_node,
+ BookmarkNode* other_node,
+ BookmarkNode* synced_node);
// Helper to recursively reassign IDs.
void ReassignIDsHelper(BookmarkNode* node);
diff --git a/chrome/browser/bookmarks/bookmark_codec_unittest.cc b/chrome/browser/bookmarks/bookmark_codec_unittest.cc
index 09b9bd4..7358188 100644
--- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc
@@ -2,7 +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_path.h"
+#include "base/file_util.h"
#include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
#include "base/string_util.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
@@ -10,6 +13,8 @@
#include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/browser/bookmarks/bookmark_model_test_utils.h"
#include "chrome/browser/bookmarks/bookmark_utils.h"
+#include "chrome/common/chrome_paths.h"
+#include "content/common/json_value_serializer.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
@@ -111,6 +116,7 @@ class BookmarkCodecTest : public testing::Test {
int64 max_id;
bool result = codec->Decode(AsMutable(model->GetBookmarkBarNode()),
AsMutable(model->other_node()),
+ AsMutable(model->synced_node()),
&max_id, value);
model->set_next_node_id(max_id);
return result;
@@ -161,6 +167,7 @@ class BookmarkCodecTest : public testing::Test {
std::set<int64> assigned_ids;
CheckIDs(model->GetBookmarkBarNode(), &assigned_ids);
CheckIDs(model->other_node(), &assigned_ids);
+ CheckIDs(model->synced_node(), &assigned_ids);
}
};
@@ -287,3 +294,45 @@ TEST_F(BookmarkCodecTest, PersistIDsTest) {
&decoded_model2,
true);
}
+
+TEST_F(BookmarkCodecTest, CanDecodeModelWithoutSyncedBookmarks) {
+ FilePath test_data_directory;
+ ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory));
+ FilePath test_file = test_data_directory.AppendASCII(
+ "bookmarks/model_without_sync.json");
+ ASSERT_TRUE(file_util::PathExists(test_file));
+
+ JSONFileValueSerializer serializer(test_file);
+ scoped_ptr<Value> root(serializer.Deserialize(NULL, NULL));
+
+ BookmarkModel decoded_model(NULL);
+ BookmarkCodec decoder;
+ ASSERT_TRUE(Decode(&decoder, &decoded_model, *root.get()));
+ ExpectIDsUnique(&decoded_model);
+
+ const BookmarkNode* bbn = decoded_model.GetBookmarkBarNode();
+ ASSERT_EQ(1, bbn->child_count());
+
+ const BookmarkNode* child = bbn->GetChild(0);
+ EXPECT_EQ(BookmarkNode::FOLDER, child->type());
+ EXPECT_EQ(ASCIIToUTF16("Folder A"), child->GetTitle());
+ ASSERT_EQ(1, child->child_count());
+
+ child = child->GetChild(0);
+ EXPECT_EQ(BookmarkNode::URL, child->type());
+ EXPECT_EQ(ASCIIToUTF16("Bookmark Manager"), child->GetTitle());
+
+ const BookmarkNode* other = decoded_model.other_node();
+ ASSERT_EQ(1, other->child_count());
+
+ child = other->GetChild(0);
+ EXPECT_EQ(BookmarkNode::FOLDER, child->type());
+ EXPECT_EQ(ASCIIToUTF16("Folder B"), child->GetTitle());
+ ASSERT_EQ(1, child->child_count());
+
+ child = child->GetChild(0);
+ EXPECT_EQ(BookmarkNode::URL, child->type());
+ EXPECT_EQ(ASCIIToUTF16("Get started with Google Chrome"), child->GetTitle());
+
+ ASSERT_TRUE(decoded_model.synced_node() != NULL);
+}
diff --git a/chrome/browser/bookmarks/bookmark_html_writer.cc b/chrome/browser/bookmarks/bookmark_html_writer.cc
index faaec66..85c48ed 100644
--- a/chrome/browser/bookmarks/bookmark_html_writer.cc
+++ b/chrome/browser/bookmarks/bookmark_html_writer.cc
@@ -114,12 +114,16 @@ class Writer : public Task {
DictionaryValue* roots_d_value = static_cast<DictionaryValue*>(roots);
Value* root_folder_value;
Value* other_folder_value;
+ Value* synced_folder_value;
if (!roots_d_value->Get(BookmarkCodec::kRootFolderNameKey,
&root_folder_value) ||
root_folder_value->GetType() != Value::TYPE_DICTIONARY ||
!roots_d_value->Get(BookmarkCodec::kOtherBookmarkFolderNameKey,
&other_folder_value) ||
- other_folder_value->GetType() != Value::TYPE_DICTIONARY) {
+ other_folder_value->GetType() != Value::TYPE_DICTIONARY ||
+ !roots_d_value->Get(BookmarkCodec::kSyncedBookmarkFolderNameKey,
+ &synced_folder_value) ||
+ synced_folder_value->GetType() != Value::TYPE_DICTIONARY) {
NOTREACHED();
return; // Invalid type for root folder and/or other folder.
}
@@ -129,7 +133,9 @@ class Writer : public Task {
if (!WriteNode(*static_cast<DictionaryValue*>(root_folder_value),
BookmarkNode::BOOKMARK_BAR) ||
!WriteNode(*static_cast<DictionaryValue*>(other_folder_value),
- BookmarkNode::OTHER_NODE)) {
+ BookmarkNode::OTHER_NODE) ||
+ !WriteNode(*static_cast<DictionaryValue*>(synced_folder_value),
+ BookmarkNode::SYNCED)) {
return;
}
@@ -286,10 +292,11 @@ class Writer : public Task {
NOTREACHED();
return false;
}
- if (folder_type != BookmarkNode::OTHER_NODE) {
- // The other folder name is not written out. This gives the effect of
- // making the contents of the 'other folder' be a sibling to the bookmark
- // bar folder.
+ if (folder_type != BookmarkNode::OTHER_NODE &&
+ folder_type != BookmarkNode::SYNCED) {
+ // The other/synced folder name are not written out. This gives the effect
+ // of making the contents of the 'other folder' be a sibling to the
+ // bookmark bar folder.
if (!WriteIndent() ||
!Write(kFolderStart) ||
!WriteTime(date_added_string) ||
@@ -329,7 +336,8 @@ class Writer : public Task {
return false;
}
}
- if (folder_type != BookmarkNode::OTHER_NODE) {
+ if (folder_type != BookmarkNode::OTHER_NODE &&
+ folder_type != BookmarkNode::SYNCED) {
// Close out the folder.
DecrementIndent();
if (!WriteIndent() ||
@@ -383,6 +391,7 @@ BookmarkFaviconFetcher::~BookmarkFaviconFetcher() {
void BookmarkFaviconFetcher::ExportBookmarks() {
ExtractUrls(profile_->GetBookmarkModel()->GetBookmarkBarNode());
ExtractUrls(profile_->GetBookmarkModel()->other_node());
+ ExtractUrls(profile_->GetBookmarkModel()->synced_node());
if (!bookmark_urls_.empty()) {
FetchNextFavicon();
} else {
diff --git a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc
index 2f80465..3b24b3c 100644
--- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc
@@ -167,6 +167,8 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
// F3
// F4
// url1
+ // Synced
+ // url1
string16 f1_title = ASCIIToUTF16("F\"&;<1\"");
string16 f2_title = ASCIIToUTF16("F2");
string16 f3_title = ASCIIToUTF16("F 3");
@@ -204,6 +206,7 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
model->AddURLWithCreationTime(f4, 0, url1_title, url1, t1);
model->AddURLWithCreationTime(model->GetBookmarkBarNode(), 2, url4_title,
url4, t4);
+ model->AddURLWithCreationTime(model->synced_node(), 0, url1_title, url1, t1);
// Write to a temp file.
BookmarksObserver observer(&message_loop);
@@ -226,8 +229,8 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
NULL,
&favicons);
- // Check loaded favicon (url1 is represents by 3 separate bookmarks).
- EXPECT_EQ(3U, favicons.size());
+ // Check loaded favicon (url1 is represented by 4 separate bookmarks).
+ EXPECT_EQ(4U, favicons.size());
for (size_t i = 0; i < favicons.size(); i++) {
if (url1_favicon == favicons[i].favicon_url) {
EXPECT_EQ(1U, favicons[i].urls.size());
@@ -239,7 +242,7 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
}
// Verify we got back what we wrote.
- ASSERT_EQ(7U, parsed_bookmarks.size());
+ ASSERT_EQ(8U, parsed_bookmarks.size());
// Windows and ChromeOS builds use Sentence case.
string16 bookmark_folder_name =
l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME);
@@ -257,4 +260,6 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
string16(), string16(), string16());
AssertBookmarkEntryEquals(parsed_bookmarks[6], false, url1, url1_title, t1,
f3_title, f4_title, string16());
+ AssertBookmarkEntryEquals(parsed_bookmarks[7], false, url1, url1_title, t1,
+ string16(), string16(), string16());
}
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index a48ecdf..7f74bc9 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -8,6 +8,7 @@
#include <functional>
#include "base/callback.h"
+#include "base/command_line.h"
#include "base/memory/scoped_vector.h"
#include "build/build_config.h"
#include "chrome/browser/bookmarks/bookmark_index.h"
@@ -16,6 +17,7 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/history/history_notifications.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_switches.h"
#include "content/common/notification_service.h"
#include "grit/generated_resources.h"
#include "ui/base/l10n/l10n_util.h"
@@ -61,6 +63,18 @@ void BookmarkNode::InvalidateFavicon() {
favicon_ = SkBitmap();
}
+bool BookmarkNode::IsVisible() const {
+ // The synced bookmark folder is invisible if the flag isn't set and there are
+ // no bookmarks under it.
+ if (type_ != BookmarkNode::SYNCED ||
+ CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableSyncedBookmarksFolder) ||
+ child_count() > 0) {
+ return true;
+ }
+ return false;
+}
+
void BookmarkNode::Reset(const history::StarredEntry& entry) {
DCHECK(entry.type != history::StarredEntry::URL || entry.url == url_);
@@ -75,6 +89,9 @@ void BookmarkNode::Reset(const history::StarredEntry& entry) {
case history::StarredEntry::BOOKMARK_BAR:
type_ = BookmarkNode::BOOKMARK_BAR;
break;
+ case history::StarredEntry::SYNCED:
+ type_ = BookmarkNode::SYNCED;
+ break;
case history::StarredEntry::OTHER:
type_ = BookmarkNode::OTHER_NODE;
break;
@@ -124,6 +141,7 @@ BookmarkModel::BookmarkModel(Profile* profile)
root_(GURL()),
bookmark_bar_node_(NULL),
other_node_(NULL),
+ synced_node_(NULL),
next_node_id_(1),
observers_(ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY),
loaded_signal_(TRUE, FALSE) {
@@ -259,7 +277,7 @@ void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) {
if (node->GetTitle() == title)
return;
- if (node == bookmark_bar_node_ || node == other_node_) {
+ if (is_permanent_node(node)) {
NOTREACHED();
return;
}
@@ -568,12 +586,14 @@ void BookmarkModel::DoneLoading(
}
bookmark_bar_node_ = details->release_bb_node();
other_node_ = details->release_other_folder_node();
+ synced_node_ = details->release_synced_folder_node();
index_.reset(details->release_index());
// WARNING: order is important here, various places assume bookmark bar then
// other node.
root_.Add(bookmark_bar_node_, 0);
root_.Add(other_node_, 1);
+ root_.Add(synced_node_, 2);
{
base::AutoLock url_lock(url_lock_);
@@ -719,14 +739,24 @@ BookmarkNode* BookmarkModel::CreateOtherBookmarksNode() {
return CreateRootNodeFromStarredEntry(entry);
}
+BookmarkNode* BookmarkModel::CreateSyncedBookmarksNode() {
+ history::StarredEntry entry;
+ entry.type = history::StarredEntry::SYNCED;
+ return CreateRootNodeFromStarredEntry(entry);
+}
+
BookmarkNode* BookmarkModel::CreateRootNodeFromStarredEntry(
const history::StarredEntry& entry) {
DCHECK(entry.type == history::StarredEntry::BOOKMARK_BAR ||
- entry.type == history::StarredEntry::OTHER);
+ entry.type == history::StarredEntry::OTHER ||
+ entry.type == history::StarredEntry::SYNCED);
BookmarkNode* node = new BookmarkNode(generate_next_node_id(), GURL());
node->Reset(entry);
if (entry.type == history::StarredEntry::BOOKMARK_BAR) {
node->set_title(l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME));
+ } else if (entry.type == history::StarredEntry::SYNCED) {
+ node->set_title(l10n_util::GetStringUTF16(
+ IDS_BOOMARK_BAR_SYNCED_FOLDER_NAME));
} else {
node->set_title(
l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_OTHER_FOLDER_NAME));
@@ -826,6 +856,8 @@ void BookmarkModel::SetFileChanged() {
BookmarkLoadDetails* BookmarkModel::CreateLoadDetails() {
BookmarkNode* bb_node = CreateBookmarkNode();
BookmarkNode* other_folder_node = CreateOtherBookmarksNode();
+ BookmarkNode* synced_folder_node = CreateSyncedBookmarksNode();
return new BookmarkLoadDetails(
- bb_node, other_folder_node, new BookmarkIndex(profile()), next_node_id_);
+ bb_node, other_folder_node, synced_folder_node,
+ new BookmarkIndex(profile()), next_node_id_);
}
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index 0e4ada68..b3c8c8d 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -50,7 +50,8 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
URL,
FOLDER,
BOOKMARK_BAR,
- OTHER_NODE
+ OTHER_NODE,
+ SYNCED
};
// Creates a new node with the specified url and id of 0
explicit BookmarkNode(const GURL& url);
@@ -109,6 +110,15 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
bool is_favicon_loaded() const { return loaded_favicon_; }
void set_favicon_loaded(bool value) { loaded_favicon_ = value; }
+ // Accessor method for controlling the visibility of a bookmark node/sub-tree.
+ // Note that visibility is not propagated down the tree hierarchy so if a
+ // parent node is marked as invisible, a child node may return "Visible". This
+ // function is primarily useful when traversing the model to generate a UI
+ // representation but we may want to suppress some nodes.
+ // TODO(yfriedman): Remove this when enable-synced-bookmarks-folder is
+ // no longer a command line flag.
+ bool IsVisible() const;
+
HistoryService::Handle favicon_load_handle() const {
return favicon_load_handle_;
}
@@ -193,6 +203,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
// Returns the 'other' node. This is NULL until loaded.
const BookmarkNode* other_node() { return other_node_; }
+ // Returns the 'synced' node. This is NULL until loaded.
+ const BookmarkNode* synced_node() { return synced_node_; }
+
// Returns the parent the last node was added to. This never returns NULL
// (as long as the model is loaded).
const BookmarkNode* GetParentForNewNodes();
@@ -311,6 +324,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
bool is_bookmark_bar_node(const BookmarkNode* node) const {
return node == bookmark_bar_node_;
}
+ bool is_synced_bookmarks_node(const BookmarkNode* node) const {
+ return node == synced_node_;
+ }
bool is_other_bookmarks_node(const BookmarkNode* node) const {
return node == other_node_;
}
@@ -319,7 +335,8 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
bool is_permanent_node(const BookmarkNode* node) const {
return is_root(node) ||
is_bookmark_bar_node(node) ||
- is_other_bookmarks_node(node);
+ is_other_bookmarks_node(node) ||
+ is_synced_bookmarks_node(node);
}
// Sets the store to NULL, making it so the BookmarkModel does not persist
@@ -380,10 +397,11 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
// Returns true if the parent and index are valid.
bool IsValidIndex(const BookmarkNode* parent, int index, bool allow_end);
- // Creates the bookmark bar/other nodes. These call into
+ // Creates the bookmark bar/synced/other nodes. These call into
// CreateRootNodeFromStarredEntry.
BookmarkNode* CreateBookmarkNode();
BookmarkNode* CreateOtherBookmarksNode();
+ BookmarkNode* CreateSyncedBookmarksNode();
// Creates a root node (either the bookmark bar node or other node) from the
// specified starred entry.
@@ -439,6 +457,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
BookmarkNode* bookmark_bar_node_;
BookmarkNode* other_node_;
+ BookmarkNode* synced_node_;
// The maximum ID assigned to the bookmark nodes in the model.
int64 next_node_id_;
diff --git a/chrome/browser/bookmarks/bookmark_model_test_utils.cc b/chrome/browser/bookmarks/bookmark_model_test_utils.cc
index 7286a9e..c2894fb 100644
--- a/chrome/browser/bookmarks/bookmark_model_test_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_model_test_utils.cc
@@ -39,5 +39,7 @@ void BookmarkModelTestUtils::AssertModelsEqual(BookmarkModel* expected,
AssertNodesEqual(expected->other_node(),
actual->other_node(),
check_ids);
+ AssertNodesEqual(expected->synced_node(),
+ actual->synced_node(),
+ check_ids);
}
-
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index aec548e..f87445e 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -6,6 +6,7 @@
#include <string>
#include "base/base_paths.h"
+#include "base/command_line.h"
#include "base/file_util.h"
#include "base/hash_tables.h"
#include "base/path_service.h"
@@ -20,6 +21,7 @@
#include "chrome/browser/history/history_notifications.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
#include "chrome/test/model_test_utils.h"
#include "chrome/test/testing_browser_process_test.h"
#include "chrome/test/testing_profile.h"
@@ -83,11 +85,16 @@ class BookmarkModelTest : public TestingBrowserProcessTest,
int index2;
};
- BookmarkModelTest() : model(NULL) {
- model.AddObserver(this);
- ClearCounts();
+ BookmarkModelTest() :
+ model_(NULL),
+ original_command_line_(*CommandLine::ForCurrentProcess()) {
+ model_.AddObserver(this);
+ ClearCounts();
}
+ virtual void TearDown() {
+ *CommandLine::ForCurrentProcess() = original_command_line_;
+ }
void Loaded(BookmarkModel* model) OVERRIDE {
// We never load from the db, so that this should never get invoked.
@@ -153,7 +160,9 @@ class BookmarkModelTest : public TestingBrowserProcessTest,
ASSERT_EQ(reordered_count, reordered_count_);
}
- BookmarkModel model;
+ BookmarkModel model_;
+
+ CommandLine original_command_line_;
int moved_count;
@@ -169,25 +178,32 @@ class BookmarkModelTest : public TestingBrowserProcessTest,
};
TEST_F(BookmarkModelTest, InitialState) {
- const BookmarkNode* bb_node = model.GetBookmarkBarNode();
+ const BookmarkNode* bb_node = model_.GetBookmarkBarNode();
ASSERT_TRUE(bb_node != NULL);
EXPECT_EQ(0, bb_node->child_count());
EXPECT_EQ(BookmarkNode::BOOKMARK_BAR, bb_node->type());
- const BookmarkNode* other_node = model.other_node();
+ const BookmarkNode* other_node = model_.other_node();
ASSERT_TRUE(other_node != NULL);
EXPECT_EQ(0, other_node->child_count());
EXPECT_EQ(BookmarkNode::OTHER_NODE, other_node->type());
+ const BookmarkNode* synced_node = model_.synced_node();
+ ASSERT_TRUE(synced_node != NULL);
+ EXPECT_EQ(0, synced_node->child_count());
+ EXPECT_EQ(BookmarkNode::SYNCED, synced_node->type());
+
EXPECT_TRUE(bb_node->id() != other_node->id());
+ EXPECT_TRUE(bb_node->id() != synced_node->id());
+ EXPECT_TRUE(other_node->id() != synced_node->id());
}
TEST_F(BookmarkModelTest, AddURL) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
const string16 title(ASCIIToUTF16("foo"));
const GURL url("http://foo.com");
- const BookmarkNode* new_node = model.AddURL(root, 0, title, url);
+ const BookmarkNode* new_node = model_.AddURL(root, 0, title, url);
AssertObserverCount(1, 0, 0, 0, 0);
observer_details.AssertEquals(root, NULL, 0, -1);
@@ -195,17 +211,38 @@ TEST_F(BookmarkModelTest, AddURL) {
ASSERT_EQ(title, new_node->GetTitle());
ASSERT_TRUE(url == new_node->GetURL());
ASSERT_EQ(BookmarkNode::URL, new_node->type());
- ASSERT_TRUE(new_node == model.GetMostRecentlyAddedNodeForURL(url));
+ ASSERT_TRUE(new_node == model_.GetMostRecentlyAddedNodeForURL(url));
EXPECT_TRUE(new_node->id() != root->id() &&
- new_node->id() != model.other_node()->id());
+ new_node->id() != model_.other_node()->id() &&
+ new_node->id() != model_.synced_node()->id());
+}
+
+TEST_F(BookmarkModelTest, AddURLToSyncedBookmarks) {
+ const BookmarkNode* root = model_.synced_node();
+ const string16 title(ASCIIToUTF16("foo"));
+ const GURL url("http://foo.com");
+
+ const BookmarkNode* new_node = model_.AddURL(root, 0, title, url);
+ AssertObserverCount(1, 0, 0, 0, 0);
+ observer_details.AssertEquals(root, NULL, 0, -1);
+
+ ASSERT_EQ(1, root->child_count());
+ ASSERT_EQ(title, new_node->GetTitle());
+ ASSERT_TRUE(url == new_node->GetURL());
+ ASSERT_EQ(BookmarkNode::URL, new_node->type());
+ ASSERT_TRUE(new_node == model_.GetMostRecentlyAddedNodeForURL(url));
+
+ EXPECT_TRUE(new_node->id() != root->id() &&
+ new_node->id() != model_.other_node()->id() &&
+ new_node->id() != model_.synced_node()->id());
}
TEST_F(BookmarkModelTest, AddFolder) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
const string16 title(ASCIIToUTF16("foo"));
- const BookmarkNode* new_node = model.AddFolder(root, 0, title);
+ const BookmarkNode* new_node = model_.AddFolder(root, 0, title);
AssertObserverCount(1, 0, 0, 0, 0);
observer_details.AssertEquals(root, NULL, 0, -1);
@@ -214,93 +251,94 @@ TEST_F(BookmarkModelTest, AddFolder) {
ASSERT_EQ(BookmarkNode::FOLDER, new_node->type());
EXPECT_TRUE(new_node->id() != root->id() &&
- new_node->id() != model.other_node()->id());
+ new_node->id() != model_.other_node()->id() &&
+ new_node->id() != model_.synced_node()->id());
// Add another folder, just to make sure folder_ids are incremented correctly.
ClearCounts();
- model.AddFolder(root, 0, title);
+ model_.AddFolder(root, 0, title);
AssertObserverCount(1, 0, 0, 0, 0);
observer_details.AssertEquals(root, NULL, 0, -1);
}
TEST_F(BookmarkModelTest, RemoveURL) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
const string16 title(ASCIIToUTF16("foo"));
const GURL url("http://foo.com");
- model.AddURL(root, 0, title, url);
+ model_.AddURL(root, 0, title, url);
ClearCounts();
- model.Remove(root, 0);
+ model_.Remove(root, 0);
ASSERT_EQ(0, root->child_count());
AssertObserverCount(0, 0, 1, 0, 0);
observer_details.AssertEquals(root, NULL, 0, -1);
// Make sure there is no mapping for the URL.
- ASSERT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL);
+ ASSERT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == NULL);
}
TEST_F(BookmarkModelTest, RemoveFolder) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
- const BookmarkNode* folder = model.AddFolder(root, 0, ASCIIToUTF16("foo"));
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
+ const BookmarkNode* folder = model_.AddFolder(root, 0, ASCIIToUTF16("foo"));
ClearCounts();
// Add a URL as a child.
const string16 title(ASCIIToUTF16("foo"));
const GURL url("http://foo.com");
- model.AddURL(folder, 0, title, url);
+ model_.AddURL(folder, 0, title, url);
ClearCounts();
// Now remove the folder.
- model.Remove(root, 0);
+ model_.Remove(root, 0);
ASSERT_EQ(0, root->child_count());
AssertObserverCount(0, 0, 1, 0, 0);
observer_details.AssertEquals(root, NULL, 0, -1);
// Make sure there is no mapping for the URL.
- ASSERT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL);
+ ASSERT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == NULL);
}
TEST_F(BookmarkModelTest, SetTitle) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
string16 title(ASCIIToUTF16("foo"));
const GURL url("http://foo.com");
- const BookmarkNode* node = model.AddURL(root, 0, title, url);
+ const BookmarkNode* node = model_.AddURL(root, 0, title, url);
ClearCounts();
title = ASCIIToUTF16("foo2");
- model.SetTitle(node, title);
+ model_.SetTitle(node, title);
AssertObserverCount(0, 0, 0, 1, 0);
observer_details.AssertEquals(node, NULL, -1, -1);
EXPECT_EQ(title, node->GetTitle());
}
TEST_F(BookmarkModelTest, SetURL) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
const string16 title(ASCIIToUTF16("foo"));
GURL url("http://foo.com");
- const BookmarkNode* node = model.AddURL(root, 0, title, url);
+ const BookmarkNode* node = model_.AddURL(root, 0, title, url);
ClearCounts();
url = GURL("http://foo2.com");
- model.SetURL(node, url);
+ model_.SetURL(node, url);
AssertObserverCount(0, 0, 0, 1, 0);
observer_details.AssertEquals(node, NULL, -1, -1);
EXPECT_EQ(url, node->GetURL());
}
TEST_F(BookmarkModelTest, Move) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
const string16 title(ASCIIToUTF16("foo"));
const GURL url("http://foo.com");
- const BookmarkNode* node = model.AddURL(root, 0, title, url);
- const BookmarkNode* folder1 = model.AddFolder(root, 0, ASCIIToUTF16("foo"));
+ const BookmarkNode* node = model_.AddURL(root, 0, title, url);
+ const BookmarkNode* folder1 = model_.AddFolder(root, 0, ASCIIToUTF16("foo"));
ClearCounts();
- model.Move(node, folder1, 0);
+ model_.Move(node, folder1, 0);
AssertObserverCount(0, 1, 0, 0, 0);
observer_details.AssertEquals(root, folder1, 1, 0);
@@ -312,17 +350,17 @@ TEST_F(BookmarkModelTest, Move) {
// And remove the folder.
ClearCounts();
- model.Remove(root, 0);
+ model_.Remove(root, 0);
AssertObserverCount(0, 0, 1, 0, 0);
observer_details.AssertEquals(root, NULL, 0, -1);
- EXPECT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL);
+ EXPECT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == NULL);
EXPECT_EQ(0, root->child_count());
}
TEST_F(BookmarkModelTest, Copy) {
- const BookmarkNode* root = model.GetBookmarkBarNode();
+ const BookmarkNode* root = model_.GetBookmarkBarNode();
static const std::string model_string("a 1:[ b c ] d 2:[ e f g ] h ");
- model_test_utils::AddNodesFromModelString(model, root, model_string);
+ model_test_utils::AddNodesFromModelString(model_, root, model_string);
// Validate initial model.
std::string actualModelString = model_test_utils::ModelStringFromNode(root);
@@ -331,42 +369,42 @@ TEST_F(BookmarkModelTest, Copy) {
// Copy 'd' to be after '1:b': URL item from bar to folder.
const BookmarkNode* nodeToCopy = root->GetChild(2);
const BookmarkNode* destination = root->GetChild(1);
- model.Copy(nodeToCopy, destination, 1);
+ model_.Copy(nodeToCopy, destination, 1);
actualModelString = model_test_utils::ModelStringFromNode(root);
EXPECT_EQ("a 1:[ b d c ] d 2:[ e f g ] h ", actualModelString);
// Copy '1:d' to be after 'a': URL item from folder to bar.
const BookmarkNode* folder = root->GetChild(1);
nodeToCopy = folder->GetChild(1);
- model.Copy(nodeToCopy, root, 1);
+ model_.Copy(nodeToCopy, root, 1);
actualModelString = model_test_utils::ModelStringFromNode(root);
EXPECT_EQ("a d 1:[ b d c ] d 2:[ e f g ] h ", actualModelString);
// Copy '1' to be after '2:e': Folder from bar to folder.
nodeToCopy = root->GetChild(2);
destination = root->GetChild(4);
- model.Copy(nodeToCopy, destination, 1);
+ model_.Copy(nodeToCopy, destination, 1);
actualModelString = model_test_utils::ModelStringFromNode(root);
EXPECT_EQ("a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f g ] h ", actualModelString);
// Copy '2:1' to be after '2:f': Folder within same folder.
folder = root->GetChild(4);
nodeToCopy = folder->GetChild(1);
- model.Copy(nodeToCopy, folder, 3);
+ model_.Copy(nodeToCopy, folder, 3);
actualModelString = model_test_utils::ModelStringFromNode(root);
EXPECT_EQ("a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h ",
actualModelString);
// Copy first 'd' to be after 'h': URL item within the bar.
nodeToCopy = root->GetChild(1);
- model.Copy(nodeToCopy, root, 6);
+ model_.Copy(nodeToCopy, root, 6);
actualModelString = model_test_utils::ModelStringFromNode(root);
EXPECT_EQ("a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h d ",
actualModelString);
// Copy '2' to be after 'a': Folder within the bar.
nodeToCopy = root->GetChild(4);
- model.Copy(nodeToCopy, root, 1);
+ model_.Copy(nodeToCopy, root, 1);
actualModelString = model_test_utils::ModelStringFromNode(root);
EXPECT_EQ("a 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] d 1:[ b d c ] "
"d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h d ",
@@ -375,34 +413,45 @@ TEST_F(BookmarkModelTest, Copy) {
// Tests that adding a URL to a folder updates the last modified time.
TEST_F(BookmarkModelTest, ParentForNewNodes) {
- ASSERT_EQ(model.GetBookmarkBarNode(), model.GetParentForNewNodes());
+ ASSERT_EQ(model_.GetBookmarkBarNode(), model_.GetParentForNewNodes());
+
+ const string16 title(ASCIIToUTF16("foo"));
+ const GURL url("http://foo.com");
+
+ model_.AddURL(model_.other_node(), 0, title, url);
+ ASSERT_EQ(model_.other_node(), model_.GetParentForNewNodes());
+}
+
+// Tests that adding a URL to a folder updates the last modified time.
+TEST_F(BookmarkModelTest, ParentForNewSyncedNodes) {
+ ASSERT_EQ(model_.GetBookmarkBarNode(), model_.GetParentForNewNodes());
const string16 title(ASCIIToUTF16("foo"));
const GURL url("http://foo.com");
- model.AddURL(model.other_node(), 0, title, url);
- ASSERT_EQ(model.other_node(), model.GetParentForNewNodes());
+ model_.AddURL(model_.synced_node(), 0, title, url);
+ ASSERT_EQ(model_.synced_node(), model_.GetParentForNewNodes());
}
// Make sure recently modified stays in sync when adding a URL.
TEST_F(BookmarkModelTest, MostRecentlyModifiedFolders) {
// Add a folder.
- const BookmarkNode* folder = model.AddFolder(model.other_node(), 0,
- ASCIIToUTF16("foo"));
+ const BookmarkNode* folder = model_.AddFolder(model_.other_node(), 0,
+ ASCIIToUTF16("foo"));
// Add a URL to it.
- model.AddURL(folder, 0, ASCIIToUTF16("blah"), GURL("http://foo.com"));
+ model_.AddURL(folder, 0, ASCIIToUTF16("blah"), GURL("http://foo.com"));
// Make sure folder is in the most recently modified.
std::vector<const BookmarkNode*> most_recent_folders =
- bookmark_utils::GetMostRecentlyModifiedFolders(&model, 1);
+ bookmark_utils::GetMostRecentlyModifiedFolders(&model_, 1);
ASSERT_EQ(1U, most_recent_folders.size());
ASSERT_EQ(folder, most_recent_folders[0]);
// Nuke the folder and do another fetch, making sure folder isn't in the
// returned list.
- model.Remove(folder->parent(), 0);
+ model_.Remove(folder->parent(), 0);
most_recent_folders =
- bookmark_utils::GetMostRecentlyModifiedFolders(&model, 1);
+ bookmark_utils::GetMostRecentlyModifiedFolders(&model_, 1);
ASSERT_EQ(1U, most_recent_folders.size());
ASSERT_TRUE(most_recent_folders[0] != folder);
}
@@ -412,19 +461,19 @@ TEST_F(BookmarkModelTest, MostRecentlyAddedEntries) {
// Add a couple of nodes such that the following holds for the time of the
// nodes: n1 > n2 > n3 > n4.
Time base_time = Time::Now();
- BookmarkNode* n1 = AsMutable(model.AddURL(model.GetBookmarkBarNode(),
+ BookmarkNode* n1 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(),
0,
ASCIIToUTF16("blah"),
GURL("http://foo.com/0")));
- BookmarkNode* n2 = AsMutable(model.AddURL(model.GetBookmarkBarNode(),
+ BookmarkNode* n2 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(),
1,
ASCIIToUTF16("blah"),
GURL("http://foo.com/1")));
- BookmarkNode* n3 = AsMutable(model.AddURL(model.GetBookmarkBarNode(),
+ BookmarkNode* n3 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(),
2,
ASCIIToUTF16("blah"),
GURL("http://foo.com/2")));
- BookmarkNode* n4 = AsMutable(model.AddURL(model.GetBookmarkBarNode(),
+ BookmarkNode* n4 = AsMutable(model_.AddURL(model_.GetBookmarkBarNode(),
3,
ASCIIToUTF16("blah"),
GURL("http://foo.com/3")));
@@ -435,7 +484,7 @@ TEST_F(BookmarkModelTest, MostRecentlyAddedEntries) {
// Make sure order is honored.
std::vector<const BookmarkNode*> recently_added;
- bookmark_utils::GetMostRecentlyAddedEntries(&model, 2, &recently_added);
+ bookmark_utils::GetMostRecentlyAddedEntries(&model_, 2, &recently_added);
ASSERT_EQ(2U, recently_added.size());
ASSERT_TRUE(n1 == recently_added[0]);
ASSERT_TRUE(n2 == recently_added[1]);
@@ -443,7 +492,7 @@ TEST_F(BookmarkModelTest, MostRecentlyAddedEntries) {
// swap 1 and 2, then check again.
recently_added.clear();
SwapDateAdded(n1, n2);
- bookmark_utils::GetMostRecentlyAddedEntries(&model, 4, &recently_added);
+ bookmark_utils::GetMostRecentlyAddedEntries(&model_, 4, &recently_added);
ASSERT_EQ(4U, recently_added.size());
ASSERT_TRUE(n2 == recently_added[0]);
ASSERT_TRUE(n1 == recently_added[1]);
@@ -457,38 +506,38 @@ TEST_F(BookmarkModelTest, GetMostRecentlyAddedNodeForURL) {
// nodes: n1 > n2
Time base_time = Time::Now();
const GURL url("http://foo.com/0");
- BookmarkNode* n1 = AsMutable(model.AddURL(
- model.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url));
- BookmarkNode* n2 = AsMutable(model.AddURL(
- model.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url));
+ BookmarkNode* n1 = AsMutable(model_.AddURL(
+ model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url));
+ BookmarkNode* n2 = AsMutable(model_.AddURL(
+ model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url));
n1->set_date_added(base_time + TimeDelta::FromDays(4));
n2->set_date_added(base_time + TimeDelta::FromDays(3));
// Make sure order is honored.
- ASSERT_EQ(n1, model.GetMostRecentlyAddedNodeForURL(url));
+ ASSERT_EQ(n1, model_.GetMostRecentlyAddedNodeForURL(url));
// swap 1 and 2, then check again.
SwapDateAdded(n1, n2);
- ASSERT_EQ(n2, model.GetMostRecentlyAddedNodeForURL(url));
+ ASSERT_EQ(n2, model_.GetMostRecentlyAddedNodeForURL(url));
}
// Makes sure GetBookmarks removes duplicates.
TEST_F(BookmarkModelTest, GetBookmarksWithDups) {
const GURL url("http://foo.com/0");
- model.AddURL(model.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url);
- model.AddURL(model.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url);
+ model_.AddURL(model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url);
+ model_.AddURL(model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url);
std::vector<GURL> urls;
- model.GetBookmarks(&urls);
+ model_.GetBookmarks(&urls);
EXPECT_EQ(1U, urls.size());
ASSERT_TRUE(urls[0] == url);
}
TEST_F(BookmarkModelTest, HasBookmarks) {
const GURL url("http://foo.com/");
- model.AddURL(model.GetBookmarkBarNode(), 0, ASCIIToUTF16("bar"), url);
+ model_.AddURL(model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("bar"), url);
- EXPECT_TRUE(model.HasBookmarks());
+ EXPECT_TRUE(model_.HasBookmarks());
}
namespace {
@@ -528,8 +577,8 @@ class StarredListener : public NotificationObserver {
TEST_F(BookmarkModelTest, NotifyURLsStarred) {
StarredListener listener;
const GURL url("http://foo.com/0");
- const BookmarkNode* n1 = model.AddURL(
- model.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url);
+ const BookmarkNode* n1 = model_.AddURL(
+ model_.GetBookmarkBarNode(), 0, ASCIIToUTF16("blah"), url);
// Starred notification should be sent.
EXPECT_EQ(1, listener.notification_count_);
@@ -541,23 +590,23 @@ TEST_F(BookmarkModelTest, NotifyURLsStarred) {
// Add another bookmark for the same URL. This should not send any
// notification.
- const BookmarkNode* n2 = model.AddURL(
- model.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url);
+ const BookmarkNode* n2 = model_.AddURL(
+ model_.GetBookmarkBarNode(), 1, ASCIIToUTF16("blah"), url);
EXPECT_EQ(0, listener.notification_count_);
// Remove n2.
- model.Remove(n2->parent(), 1);
+ model_.Remove(n2->parent(), 1);
n2 = NULL;
// Shouldn't have received any notification as n1 still exists with the same
// URL.
EXPECT_EQ(0, listener.notification_count_);
- EXPECT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == n1);
+ EXPECT_TRUE(model_.GetMostRecentlyAddedNodeForURL(url) == n1);
// Remove n1.
- model.Remove(n1->parent(), 0);
+ model_.Remove(n1->parent(), 0);
// Now we should get the notification.
EXPECT_EQ(1, listener.notification_count_);
@@ -762,6 +811,8 @@ TEST_F(BookmarkModelTestWithProfile, CreateAndRestore) {
const std::string bbn_contents;
// Structure of the children of the other node.
const std::string other_contents;
+ // Structure of the children of the synced node.
+ const std::string synced_contents;
} data[] = {
// See PopulateNodeFromString for a description of these strings.
{ "", "" },
@@ -789,11 +840,16 @@ TEST_F(BookmarkModelTestWithProfile, CreateAndRestore) {
PopulateNodeFromString(data[i].other_contents, &other);
PopulateBookmarkNode(&other, bb_model_, bb_model_->other_node());
+ TestNode synced;
+ PopulateNodeFromString(data[i].synced_contents, &synced);
+ PopulateBookmarkNode(&synced, bb_model_, bb_model_->synced_node());
+
profile_->CreateBookmarkModel(false);
BlockTillBookmarkModelLoaded();
VerifyModelMatchesNode(&bbn, bb_model_->GetBookmarkBarNode());
VerifyModelMatchesNode(&other, bb_model_->other_node());
+ VerifyModelMatchesNode(&synced, bb_model_->synced_node());
VerifyNoDuplicateIDs(bb_model_);
}
}
@@ -863,6 +919,9 @@ class BookmarkModelTestWithProfile2 : public BookmarkModelTestWithProfile {
ASSERT_TRUE(child->GetURL() ==
GURL("http://www.google.com/intl/en/about.html"));
+ parent = bb_model_->synced_node();
+ ASSERT_EQ(0, parent->child_count());
+
ASSERT_TRUE(bb_model_->IsBookmarked(GURL("http://www.google.com")));
}
@@ -972,8 +1031,8 @@ TEST_F(BookmarkModelTest, Sort) {
// 'C' and 'a' are folders.
TestNode bbn;
PopulateNodeFromString("B [ a ] d [ a ]", &bbn);
- const BookmarkNode* parent = model.GetBookmarkBarNode();
- PopulateBookmarkNode(&bbn, &model, parent);
+ const BookmarkNode* parent = model_.GetBookmarkBarNode();
+ PopulateBookmarkNode(&bbn, &model_, parent);
BookmarkNode* child1 = AsMutable(parent->GetChild(1));
child1->set_title(ASCIIToUTF16("a"));
@@ -985,7 +1044,7 @@ TEST_F(BookmarkModelTest, Sort) {
ClearCounts();
// Sort the children of the bookmark bar node.
- model.SortChildren(parent);
+ model_.SortChildren(parent);
// Make sure we were notified.
AssertObserverCount(0, 0, 0, 0, 1);
@@ -997,3 +1056,32 @@ TEST_F(BookmarkModelTest, Sort) {
EXPECT_EQ(parent->GetChild(2)->GetTitle(), ASCIIToUTF16("B"));
EXPECT_EQ(parent->GetChild(3)->GetTitle(), ASCIIToUTF16("d"));
}
+
+TEST_F(BookmarkModelTest, NodeVisibility) {
+ EXPECT_TRUE(model_.GetBookmarkBarNode()->IsVisible());
+ EXPECT_TRUE(model_.other_node()->IsVisible());
+ // Sync node invisible by default
+ EXPECT_FALSE(model_.synced_node()->IsVisible());
+
+ // Arbitrary node should be visible
+ TestNode bbn;
+ PopulateNodeFromString("B", &bbn);
+ const BookmarkNode* parent = model_.GetBookmarkBarNode();
+ PopulateBookmarkNode(&bbn, &model_, parent);
+ EXPECT_TRUE(parent->GetChild(0)->IsVisible());
+}
+
+TEST_F(BookmarkModelTest, SyncNodeVisibileIfFlagSet) {
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnableSyncedBookmarksFolder);
+ EXPECT_TRUE(model_.synced_node()->IsVisible());
+}
+
+TEST_F(BookmarkModelTest, SyncNodeVisibileWithChildren) {
+ const BookmarkNode* root = model_.synced_node();
+ const string16 title(ASCIIToUTF16("foo"));
+ const GURL url("http://foo.com");
+
+ model_.AddURL(root, 0, title, url);
+ EXPECT_TRUE(model_.synced_node()->IsVisible());
+}
diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc
index 2bb6305..050ee115 100644
--- a/chrome/browser/bookmarks/bookmark_storage.cc
+++ b/chrome/browser/bookmarks/bookmark_storage.cc
@@ -69,7 +69,7 @@ class BookmarkStorage::LoadTask : public Task {
BookmarkCodec codec;
TimeTicks start_time = TimeTicks::Now();
codec.Decode(details_->bb_node(), details_->other_folder_node(),
- &max_node_id, *root.get());
+ details_->synced_folder_node(), &max_node_id, *root.get());
details_->set_max_id(std::max(max_node_id, details_->max_id()));
details_->set_computed_checksum(codec.computed_checksum());
details_->set_stored_checksum(codec.stored_checksum());
@@ -80,6 +80,7 @@ class BookmarkStorage::LoadTask : public Task {
start_time = TimeTicks::Now();
AddBookmarksToIndex(details_->bb_node());
AddBookmarksToIndex(details_->other_folder_node());
+ AddBookmarksToIndex(details_->synced_folder_node());
UMA_HISTOGRAM_TIMES("Bookmarks.CreateBookmarkIndexTime",
TimeTicks::Now() - start_time);
}
@@ -115,10 +116,12 @@ class BookmarkStorage::LoadTask : public Task {
BookmarkLoadDetails::BookmarkLoadDetails(BookmarkNode* bb_node,
BookmarkNode* other_folder_node,
+ BookmarkNode* synced_folder_node,
BookmarkIndex* index,
int64 max_id)
: bb_node_(bb_node),
other_folder_node_(other_folder_node),
+ synced_folder_node_(synced_folder_node),
index_(index),
max_id_(max_id),
ids_reassigned_(false) {
diff --git a/chrome/browser/bookmarks/bookmark_storage.h b/chrome/browser/bookmarks/bookmark_storage.h
index 05e1392..19ec586 100644
--- a/chrome/browser/bookmarks/bookmark_storage.h
+++ b/chrome/browser/bookmarks/bookmark_storage.h
@@ -30,12 +30,17 @@ class BookmarkLoadDetails {
public:
BookmarkLoadDetails(BookmarkNode* bb_node,
BookmarkNode* other_folder_node,
+ BookmarkNode* synced_folder_node,
BookmarkIndex* index,
int64 max_id);
~BookmarkLoadDetails();
BookmarkNode* bb_node() { return bb_node_.get(); }
BookmarkNode* release_bb_node() { return bb_node_.release(); }
+ BookmarkNode* synced_folder_node() { return synced_folder_node_.get(); }
+ BookmarkNode* release_synced_folder_node() {
+ return synced_folder_node_.release();
+ }
BookmarkNode* other_folder_node() { return other_folder_node_.get(); }
BookmarkNode* release_other_folder_node() {
return other_folder_node_.release();
@@ -66,6 +71,7 @@ class BookmarkLoadDetails {
private:
scoped_ptr<BookmarkNode> bb_node_;
scoped_ptr<BookmarkNode> other_folder_node_;
+ scoped_ptr<BookmarkNode> synced_folder_node_;
scoped_ptr<BookmarkIndex> index_;
int64 max_id_;
std::string computed_checksum_;
diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 8ad306d..8206cfd 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -479,6 +479,12 @@ std::vector<const BookmarkNode*> GetMostRecentlyModifiedFolders(
find(nodes.begin(), nodes.end(), model->other_node()) == nodes.end()) {
nodes.push_back(model->other_node());
}
+
+ if (nodes.size() < max_count && model->synced_node()->IsVisible() &&
+ find(nodes.begin(), nodes.end(),
+ model->synced_node()) == nodes.end()) {
+ nodes.push_back(model->synced_node());
+ }
}
return nodes;
}
diff --git a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc
index 3bc18e2..0bc1b7d 100644
--- a/chrome/browser/bookmarks/recently_used_folders_combo_model.cc
+++ b/chrome/browser/bookmarks/recently_used_folders_combo_model.cc
@@ -26,12 +26,12 @@ RecentlyUsedFoldersComboModel::RecentlyUsedFoldersComboModel(
// We special case the placement of these, so remove them from the list, then
// fix up the order.
RemoveNode(model->GetBookmarkBarNode());
+ RemoveNode(model->synced_node());
RemoveNode(model->other_node());
RemoveNode(node->parent());
// Make the parent the first item, unless it's the bookmark bar or other node.
- if (node->parent() != model->GetBookmarkBarNode() &&
- node->parent() != model->other_node()) {
+ if (!model->is_permanent_node(node)) {
nodes_.insert(nodes_.begin(), node->parent());
}
@@ -42,6 +42,9 @@ RecentlyUsedFoldersComboModel::RecentlyUsedFoldersComboModel(
// And put the bookmark bar and other nodes at the end of the list.
nodes_.push_back(model->GetBookmarkBarNode());
nodes_.push_back(model->other_node());
+ if (model->synced_node()->IsVisible()) {
+ nodes_.push_back(model->synced_node());
+ }
std::vector<const BookmarkNode*>::iterator it = std::find(nodes_.begin(),
nodes_.end(),
diff --git a/chrome/browser/extensions/extension_bookmark_helpers.cc b/chrome/browser/extensions/extension_bookmark_helpers.cc
index a66839c8..36b6a73 100644
--- a/chrome/browser/extensions/extension_bookmark_helpers.cc
+++ b/chrome/browser/extensions/extension_bookmark_helpers.cc
@@ -50,7 +50,7 @@ DictionaryValue* GetNodeDictionary(const BookmarkNode* node,
ListValue* children = new ListValue();
for (int i = 0; i < childCount; ++i) {
const BookmarkNode* child = node->GetChild(i);
- if (!only_folders || child->is_folder()) {
+ if (child->IsVisible() && (!only_folders || child->is_folder())) {
DictionaryValue* dict = GetNodeDictionary(child, true, only_folders);
children->Append(dict);
}
@@ -64,8 +64,10 @@ void AddNode(const BookmarkNode* node,
ListValue* list,
bool recurse,
bool only_folders) {
- DictionaryValue* dict = GetNodeDictionary(node, recurse, only_folders);
- list->Append(dict);
+ if (node->IsVisible()) {
+ DictionaryValue* dict = GetNodeDictionary(node, recurse, only_folders);
+ list->Append(dict);
+ }
}
// Add a JSON representation of |node| to the JSON |list|.
@@ -76,8 +78,8 @@ void AddNode(const BookmarkNode* node,
}
void AddNodeFoldersOnly(const BookmarkNode* node,
- ListValue* list,
- bool recurse) {
+ ListValue* list,
+ bool recurse) {
return AddNode(node, list, recurse, true);
}
@@ -92,6 +94,7 @@ bool RemoveNode(BookmarkModel* model,
}
if (node == model->root_node() ||
node == model->other_node() ||
+ node == model->synced_node() ||
node == model->GetBookmarkBarNode()) {
*error = keys::kModifySpecialError;
return false;
diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc
index 8c31a72..f17ab59 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module.cc
@@ -512,9 +512,7 @@ bool MoveBookmarkFunction::RunImpl() {
error_ = keys::kNoNodeError;
return false;
}
- if (node == model->root_node() ||
- node == model->other_node() ||
- node == model->GetBookmarkBarNode()) {
+ if (model->is_permanent_node(node)) {
error_ = keys::kModifySpecialError;
return false;
}
@@ -606,9 +604,7 @@ bool UpdateBookmarkFunction::RunImpl() {
error_ = keys::kNoNodeError;
return false;
}
- if (node == model->root_node() ||
- node == model->other_node() ||
- node == model->GetBookmarkBarNode()) {
+ if (model->is_permanent_node(node)) {
error_ = keys::kModifySpecialError;
return false;
}
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 6d4925b..4d7484c 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -291,7 +291,10 @@ struct StarredEntry {
USER_FOLDER,
// The "other bookmarks" folder that holds uncategorized bookmarks.
- OTHER
+ OTHER,
+
+ // The synced folder.
+ SYNCED,
};
StarredEntry();
diff --git a/chrome/browser/history/starred_url_database.cc b/chrome/browser/history/starred_url_database.cc
index 5336e07..0fdf3b1 100644
--- a/chrome/browser/history/starred_url_database.cc
+++ b/chrome/browser/history/starred_url_database.cc
@@ -24,7 +24,9 @@
// id Unique identifier (primary key) for the entry.
// type Type of entry, if 0 this corresponds to a URL, 1 for
// a system folder, 2 for a user created folder, 3 for
-// other.
+// other. Note that 4 (synced) will never appear in the
+// database because it was added after this storage was
+// deprecated.
// url_id ID of the url, only valid if type == 0
// group_id ID of the folder, only valid if type != 0. This id comes
// from the UI and is NOT the same as id.
@@ -545,6 +547,12 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) {
entry.type = history::StarredEntry::OTHER;
BookmarkNode other_node(0, GURL());
other_node.Reset(entry);
+ // NOTE(yfriedman): We don't do anything with the synced star node because it
+ // won't ever exist in the starred node DB. We only need to create it to pass
+ // to "encode".
+ entry.type = history::StarredEntry::SYNCED;
+ BookmarkNode synced_node(0, GURL());
+ synced_node.Reset(entry);
std::map<history::UIStarID, history::StarID> folder_id_to_id_map;
typedef std::map<history::StarID, BookmarkNode*> IDToNodeMap;
@@ -579,6 +587,7 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) {
i != entries.end(); ++i) {
if (!i->parent_folder_id) {
DCHECK(i->type == history::StarredEntry::BOOKMARK_BAR ||
+ i->type == history::StarredEntry::SYNCED ||
i->type == history::StarredEntry::OTHER);
// Only entries with no parent should be the bookmark bar and other
// bookmarks folders.
@@ -616,7 +625,7 @@ bool StarredURLDatabase::MigrateBookmarksToFileImpl(const FilePath& path) {
// Save to file.
BookmarkCodec encoder;
scoped_ptr<Value> encoded_bookmarks(
- encoder.Encode(&bookmark_bar_node, &other_node));
+ encoder.Encode(&bookmark_bar_node, &other_node, &synced_node));
std::string content;
base::JSONWriter::Write(encoded_bookmarks.get(), true, &content);
diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc
index e05da4e..7677989 100644
--- a/chrome/browser/sync/engine/download_updates_command.cc
+++ b/chrome/browser/sync/engine/download_updates_command.cc
@@ -6,11 +6,13 @@
#include <string>
+#include "base/command_line.h"
#include "chrome/browser/sync/engine/syncer.h"
#include "chrome/browser/sync/engine/syncer_proto_util.h"
#include "chrome/browser/sync/engine/syncproto.h"
#include "chrome/browser/sync/syncable/directory_manager.h"
#include "chrome/browser/sync/syncable/model_type_payload_map.h"
+#include "chrome/common/chrome_switches.h"
using syncable::ScopedDirLookup;
@@ -33,6 +35,10 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
ClientToServerMessage::GET_UPDATES);
GetUpdatesMessage* get_updates =
client_to_server_message.mutable_get_updates();
+ if (CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableSyncedBookmarksFolder)) {
+ get_updates->set_include_syncable_bookmarks(true);
+ }
ScopedDirLookup dir(session->context()->directory_manager(),
session->context()->account_name());
diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc
index ff19f61..bd0a82b 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.cc
+++ b/chrome/browser/sync/glue/bookmark_change_processor.cc
@@ -188,10 +188,10 @@ void BookmarkChangeProcessor::BookmarkNodeRemoved(BookmarkModel* model,
}
void BookmarkChangeProcessor::BookmarkNodeChanged(BookmarkModel* model,
- const BookmarkNode* node) {
+ const BookmarkNode* node) {
DCHECK(running());
// We shouldn't see changes to the top-level nodes.
- if (node == model->GetBookmarkBarNode() || node == model->other_node()) {
+ if (model->is_permanent_node(node)) {
NOTREACHED() << "Saw update to permanent node!";
return;
}
@@ -225,7 +225,7 @@ void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model,
DCHECK(running());
const BookmarkNode* child = new_parent->GetChild(new_index);
// We shouldn't see changes to the top-level nodes.
- if (child == model->GetBookmarkBarNode() || child == model->other_node()) {
+ if (model->is_permanent_node(child)) {
NOTREACHED() << "Saw update to permanent node!";
return;
}
@@ -385,7 +385,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel(
model_associator_->GetChromeNodeFromSyncId(changes[i].id);
// Ignore changes to the permanent top-level nodes. We only care about
// their children.
- if ((dst == model->GetBookmarkBarNode()) || (dst == model->other_node()))
+ if (model->is_permanent_node(dst))
continue;
if (changes[i].action ==
sync_api::SyncManager::ChangeRecord::ACTION_DELETE) {
@@ -393,7 +393,6 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel(
DCHECK(i == 0 || changes[i-1].action == changes[i].action);
// Children of a deleted node should not be deleted; they may be
// reparented by a later change record. Move them to a temporary place.
- DCHECK(dst) << "Could not find node to be deleted";
if (!dst) // Can't do anything if we can't find the chrome node.
continue;
const BookmarkNode* parent = dst->parent();
diff --git a/chrome/browser/sync/glue/bookmark_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc
index c839501..af7c752 100644
--- a/chrome/browser/sync/glue/bookmark_model_associator.cc
+++ b/chrome/browser/sync/glue/bookmark_model_associator.cc
@@ -6,6 +6,7 @@
#include <stack>
+#include "base/command_line.h"
#include "base/hash_tables.h"
#include "base/message_loop.h"
#include "base/task.h"
@@ -17,6 +18,7 @@
#include "chrome/browser/sync/syncable/autofill_migration.h"
#include "chrome/browser/sync/syncable/nigori_util.h"
#include "chrome/browser/sync/util/cryptographer.h"
+#include "chrome/common/chrome_switches.h"
#include "content/browser/browser_thread.h"
namespace browser_sync {
@@ -40,6 +42,7 @@ namespace browser_sync {
// TODO(ncarter): Pull these tags from an external protocol specification
// rather than hardcoding them here.
static const char kBookmarkBarTag[] = "bookmark_bar";
+static const char kSyncedBookmarksTag[] = "synced_bookmarks";
static const char kOtherBookmarksTag[] = "other_bookmarks";
// Bookmark comparer for map of bookmark nodes.
@@ -232,6 +235,7 @@ void BookmarkModelAssociator::Disassociate(int64 sync_id) {
bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
DCHECK(has_nodes);
*has_nodes = false;
+ bool has_synced_folder = true;
int64 bookmark_bar_sync_id;
if (!GetSyncIdForTaggedNode(kBookmarkBarTag, &bookmark_bar_sync_id)) {
return false;
@@ -240,6 +244,10 @@ bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
if (!GetSyncIdForTaggedNode(kOtherBookmarksTag, &other_bookmarks_sync_id)) {
return false;
}
+ int64 synced_bookmarks_sync_id;
+ if (!GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_sync_id)) {
+ has_synced_folder = false;
+ }
sync_api::ReadTransaction trans(user_share_);
@@ -253,10 +261,18 @@ bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
return false;
}
+ sync_api::ReadNode synced_bookmarks_node(&trans);
+ if (has_synced_folder &&
+ !synced_bookmarks_node.InitByIdLookup(synced_bookmarks_sync_id)) {
+ return false;
+ }
+
// Sync model has user created nodes if either one of the permanent nodes
// has children.
*has_nodes = bookmark_bar_node.GetFirstChildId() != sync_api::kInvalidId ||
- other_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId;
+ other_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId ||
+ (has_synced_folder &&
+ synced_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId);
return true;
}
@@ -346,14 +362,32 @@ bool BookmarkModelAssociator::BuildAssociations() {
<< "are running against an out-of-date server?";
return false;
}
+ if (!AssociateTaggedPermanentNode(bookmark_model_->synced_node(),
+ kSyncedBookmarksTag) &&
+ // We only need to ensure that the "synced bookmarks" folder exists on the
+ // server if the command line flag is set.
+ CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableSyncedBookmarksFolder)) {
+ LOG(ERROR) << "Server did not create top-level synced nodes. Possibly "
+ << "we are running against an out-of-date server?";
+ return false;
+ }
int64 bookmark_bar_sync_id = GetSyncIdFromChromeId(
bookmark_model_->GetBookmarkBarNode()->id());
- DCHECK(bookmark_bar_sync_id != sync_api::kInvalidId);
+ DCHECK_NE(bookmark_bar_sync_id, sync_api::kInvalidId);
int64 other_bookmarks_sync_id = GetSyncIdFromChromeId(
bookmark_model_->other_node()->id());
- DCHECK(other_bookmarks_sync_id != sync_api::kInvalidId);
+ DCHECK_NE(other_bookmarks_sync_id, sync_api::kInvalidId);
+ int64 synced_bookmarks_sync_id = GetSyncIdFromChromeId(
+ bookmark_model_->synced_node()->id());
+ if (CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableSyncedBookmarksFolder)) {
+ DCHECK_NE(synced_bookmarks_sync_id, sync_api::kInvalidId);
+ }
std::stack<int64> dfs_stack;
+ if (synced_bookmarks_sync_id != sync_api::kInvalidId)
+ dfs_stack.push(synced_bookmarks_sync_id);
dfs_stack.push(other_bookmarks_sync_id);
dfs_stack.push(bookmark_bar_sync_id);
@@ -486,14 +520,25 @@ bool BookmarkModelAssociator::LoadAssociations() {
// We should always be able to find the permanent nodes.
return false;
}
+ int64 synced_bookmarks_id = -1;
+ if (!GetSyncIdForTaggedNode(kSyncedBookmarksTag, &synced_bookmarks_id) &&
+ CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableSyncedBookmarksFolder)) {
+
+ // We should always be able to find the permanent nodes.
+ return false;
+ }
// Build a bookmark node ID index since we are going to repeatedly search for
// bookmark nodes by their IDs.
BookmarkNodeIdIndex id_index;
id_index.AddAll(bookmark_model_->GetBookmarkBarNode());
id_index.AddAll(bookmark_model_->other_node());
+ id_index.AddAll(bookmark_model_->synced_node());
std::stack<int64> dfs_stack;
+ if (synced_bookmarks_id != -1)
+ dfs_stack.push(synced_bookmarks_id);
dfs_stack.push(other_bookmarks_id);
dfs_stack.push(bookmark_bar_id);
@@ -522,6 +567,7 @@ bool BookmarkModelAssociator::LoadAssociations() {
// Don't try to call NodesMatch on permanent nodes like bookmark bar and
// other bookmarks. They are not expected to match.
if (node != bookmark_model_->GetBookmarkBarNode() &&
+ node != bookmark_model_->synced_node() &&
node != bookmark_model_->other_node() &&
!NodesMatch(node, &sync_parent))
return false;
diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
index ee82421..39e6d74 100644
--- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
@@ -9,6 +9,7 @@
#include <stack>
#include <vector>
+#include "base/command_line.h"
#include "base/file_path.h"
#include "base/file_util.h"
#include "base/memory/scoped_ptr.h"
@@ -23,6 +24,7 @@
#include "chrome/browser/sync/glue/bookmark_change_processor.h"
#include "chrome/browser/sync/glue/bookmark_model_associator.h"
#include "chrome/browser/sync/syncable/directory_manager.h"
+#include "chrome/common/chrome_switches.h"
#include "chrome/test/sync/engine/test_id_factory.h"
#include "chrome/test/sync/engine/test_user_share.h"
#include "chrome/test/testing_profile.h"
@@ -281,7 +283,8 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
ProfileSyncServiceBookmarkTest()
: ui_thread_(BrowserThread::UI, &message_loop_),
file_thread_(BrowserThread::FILE, &message_loop_),
- model_(NULL) {
+ model_(NULL),
+ original_command_line_(*CommandLine::ForCurrentProcess()) {
}
virtual ~ProfileSyncServiceBookmarkTest() {
@@ -291,10 +294,13 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
virtual void SetUp() {
test_user_share_.SetUp();
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnableSyncedBookmarksFolder);
}
virtual void TearDown() {
test_user_share_.TearDown();
+ *CommandLine::ForCurrentProcess() = original_command_line_;
}
// Load (or re-load) the bookmark model. |load| controls use of the
@@ -359,8 +365,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
sync_api::ReadNode gnode(trans);
ASSERT_TRUE(InitSyncNodeFromChromeNode(bnode, &gnode));
// Non-root node titles and parents must match.
- if (bnode != model_->GetBookmarkBarNode() &&
- bnode != model_->other_node()) {
+ if (!model_->is_permanent_node(bnode)) {
EXPECT_EQ(bnode->GetTitle(), WideToUTF16Hack(gnode.GetTitle()));
EXPECT_EQ(
model_associator_->GetChromeNodeFromSyncId(gnode.GetParentId()),
@@ -459,6 +464,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
const BookmarkNode* root = model_->root_node();
EXPECT_EQ(root->GetIndexOf(model_->GetBookmarkBarNode()), 0);
EXPECT_EQ(root->GetIndexOf(model_->other_node()), 1);
+ EXPECT_EQ(root->GetIndexOf(model_->synced_node()), 2);
std::stack<int64> stack;
stack.push(bookmark_bar_id());
@@ -481,6 +487,11 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
ExpectModelMatch(&trans);
}
+ int64 synced_bookmarks_id() {
+ return
+ model_associator_->GetSyncIdFromChromeId(model_->synced_node()->id());
+ }
+
int64 other_bookmarks_id() {
return
model_associator_->GetSyncIdFromChromeId(model_->other_node()->id());
@@ -503,6 +514,7 @@ class ProfileSyncServiceBookmarkTest : public testing::Test {
protected:
BookmarkModel* model_;
+ CommandLine original_command_line_;
TestUserShare test_user_share_;
scoped_ptr<BookmarkChangeProcessor> change_processor_;
StrictMock<MockUnrecoverableErrorHandler> mock_unrecoverable_error_handler_;
@@ -514,6 +526,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, InitialState) {
EXPECT_TRUE(other_bookmarks_id());
EXPECT_TRUE(bookmark_bar_id());
+ EXPECT_TRUE(synced_bookmarks_id());
ExpectModelMatch();
}
@@ -540,6 +553,11 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
folder, 1, ASCIIToUTF16("Airplanes"), GURL("http://www.easyjet.com/"));
ExpectSyncerNodeMatching(url2);
ExpectModelMatch();
+ // Test addition.
+ const BookmarkNode* synced_folder =
+ model_->AddFolder(model_->synced_node(), 0, ASCIIToUTF16("pie"));
+ ExpectSyncerNodeMatching(synced_folder);
+ ExpectModelMatch();
// Test modification.
model_->SetTitle(url2, ASCIIToUTF16("EasyJet"));
@@ -556,6 +574,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
ExpectModelMatch();
model_->Copy(url2, model_->GetBookmarkBarNode(), 0);
ExpectModelMatch();
+ model_->SetTitle(synced_folder, ASCIIToUTF16("strawberry"));
+ ExpectModelMatch();
// Test deletion.
// Delete a single item.
@@ -565,6 +585,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, BookmarkModelOperations) {
model_->Remove(folder2->parent(),
folder2->parent()->GetIndexOf(folder2));
ExpectModelMatch();
+ model_->Remove(model_->synced_node(), 0);
+ ExpectModelMatch();
}
TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
@@ -591,6 +613,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
"scrollbars=0,status=0,toolbar=0,width=300," \
"height=300,resizable');});");
adds.AddURL(L"", javascript_url, other_bookmarks_id(), 0);
+ int64 u6 = adds.AddURL(L"Sync1", "http://www.syncable.edu/",
+ synced_bookmarks_id(), 0);
std::vector<sync_api::SyncManager::ChangeRecord>::const_iterator it;
// The bookmark model shouldn't yet have seen any of the nodes of |adds|.
@@ -619,6 +643,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
// Then add u3 after f1.
int64 u3_old_parent = mods.ModifyPosition(u3, f2, f1);
+ std::wstring u6_old_title = mods.ModifyTitle(u6, L"Synced Folder A");
+
// Test that the property changes have not yet taken effect.
ExpectBrowserNodeTitle(u2, u2_old_title);
/* ExpectBrowserNodeURL(u2, u2_old_url); */
@@ -629,6 +655,8 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
ExpectBrowserNodeParent(u3, u3_old_parent);
+ ExpectBrowserNodeTitle(u6, u6_old_title);
+
// Apply the changes.
mods.ApplyPendingChanges(change_processor_.get());
@@ -641,6 +669,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
FakeServerChange dels(&trans);
dels.Delete(u2);
dels.Delete(u3);
+ dels.Delete(u6);
ExpectBrowserNodeKnown(u2);
ExpectBrowserNodeKnown(u3);
@@ -649,6 +678,7 @@ TEST_F(ProfileSyncServiceBookmarkTest, ServerChangeProcessing) {
ExpectBrowserNodeUnknown(u2);
ExpectBrowserNodeUnknown(u3);
+ ExpectBrowserNodeUnknown(u6);
ExpectModelMatch(&trans);
}
@@ -935,23 +965,31 @@ namespace {
// | |-- f2u3, http://www.f2u3.com/
// | +-- f2u1, http://www.f2u1.com/
// +-- Other bookmarks
-// |-- f3
-// | |-- f3u4, http://www.f3u4.com/
-// | |-- f3u2, http://www.f3u2.com/
-// | |-- f3u3, http://www.f3u3.com/
-// | +-- f3u1, http://www.f3u1.com/
-// |-- u4, http://www.u4.com/
-// |-- u3, http://www.u3.com/
-// --- f4
-// | |-- f4u1, http://www.f4u1.com/
-// | |-- f4u2, http://www.f4u2.com/
-// | |-- f4u3, http://www.f4u3.com/
-// | +-- f4u4, http://www.f4u4.com/
-// |-- dup
-// | +-- dupu1, http://www.dupu1.com/
-// +-- dup
-// +-- dupu2, http://www.dupu1.com/
-//
+// | |-- f3
+// | | |-- f3u4, http://www.f3u4.com/
+// | | |-- f3u2, http://www.f3u2.com/
+// | | |-- f3u3, http://www.f3u3.com/
+// | | +-- f3u1, http://www.f3u1.com/
+// | |-- u4, http://www.u4.com/
+// | |-- u3, http://www.u3.com/
+// | --- f4
+// | | |-- f4u1, http://www.f4u1.com/
+// | | |-- f4u2, http://www.f4u2.com/
+// | | |-- f4u3, http://www.f4u3.com/
+// | | +-- f4u4, http://www.f4u4.com/
+// | |-- dup
+// | | +-- dupu1, http://www.dupu1.com/
+// | +-- dup
+// | +-- dupu2, http://www.dupu1.com/
+// |
+// +-- Synced bookmarks
+// |-- f5
+// | |-- f5u1, http://www.f5u1.com/
+// |-- f6
+// | |-- f6u1, http://www.f6u1.com/
+// | |-- f6u2, http://www.f6u2.com/
+// +-- u5, http://www.u5.com/
+
static TestData kBookmarkBarChildren[] = {
{ L"u2", "http://www.u2.com/" },
{ L"f1", NULL },
@@ -998,6 +1036,20 @@ static TestData kDup2Children[] = {
{ L"dupu2", "http://www.dupu2.com/" },
};
+static TestData kSyncedBookmarkChildren[] = {
+ { L"f5", NULL },
+ { L"f6", NULL },
+ { L"u5", "http://www.u5.com/" },
+};
+static TestData kF5Children[] = {
+ { L"f5u1", "http://www.f5u1.com/" },
+ { L"f5u2", "http://www.f5u2.com/" },
+};
+static TestData kF6Children[] = {
+ { L"f6u1", "http://www.f6u1.com/" },
+ { L"f6u2", "http://www.f6u2.com/" },
+};
+
} // anonymous namespace.
void ProfileSyncServiceBookmarkTestWithData::PopulateFromTestData(
@@ -1065,6 +1117,17 @@ void ProfileSyncServiceBookmarkTestWithData::WriteTestDataToBookmarkModel() {
dup_node = other_bookmarks_node->GetChild(5);
PopulateFromTestData(dup_node, kDup2Children, arraysize(kDup2Children));
+ const BookmarkNode* synced_bookmarks_node = model_->synced_node();
+ PopulateFromTestData(synced_bookmarks_node,
+ kSyncedBookmarkChildren,
+ arraysize(kSyncedBookmarkChildren));
+
+ ASSERT_GE(synced_bookmarks_node->child_count(), 3);
+ const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0);
+ PopulateFromTestData(f5_node, kF5Children, arraysize(kF5Children));
+ const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1);
+ PopulateFromTestData(f6_node, kF6Children, arraysize(kF6Children));
+
ExpectBookmarkModelMatchesTestData();
}
@@ -1095,6 +1158,18 @@ void ProfileSyncServiceBookmarkTestWithData::
CompareWithTestData(dup_node, kDup1Children, arraysize(kDup1Children));
dup_node = other_bookmarks_node->GetChild(5);
CompareWithTestData(dup_node, kDup2Children, arraysize(kDup2Children));
+
+ const BookmarkNode* synced_bookmarks_node = model_->synced_node();
+ CompareWithTestData(synced_bookmarks_node,
+ kSyncedBookmarkChildren,
+ arraysize(kSyncedBookmarkChildren));
+
+ ASSERT_GE(synced_bookmarks_node->child_count(), 3);
+ const BookmarkNode* f5_node = synced_bookmarks_node->GetChild(0);
+ CompareWithTestData(f5_node, kF5Children, arraysize(kF5Children));
+ const BookmarkNode* f6_node = synced_bookmarks_node->GetChild(1);
+ CompareWithTestData(f6_node, kF6Children, arraysize(kF6Children));
+
}
// Tests persistence of the profile sync service by unloading the
@@ -1161,6 +1236,7 @@ TEST_F(ProfileSyncServiceBookmarkTestWithData, MergeWithEmptyBookmarkModel) {
LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
EXPECT_EQ(model_->GetBookmarkBarNode()->child_count(), 0);
EXPECT_EQ(model_->other_node()->child_count(), 0);
+ EXPECT_EQ(model_->synced_node()->child_count(), 0);
// Now restart the sync service. Starting it should populate the bookmark
// model -- test for consistency.
diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto
index 32a8833..be947ad 100644
--- a/chrome/browser/sync/protocol/sync.proto
+++ b/chrome/browser/sync/protocol/sync.proto
@@ -387,6 +387,9 @@ message GetUpdatesMessage {
// containing GetUpdatesStreamingResponse. These ClientToServerResponses are
// delimited by a length prefix, which is encoded as a varint.
optional bool streaming = 7 [default = false];
+
+ // Whether to request the syncable_bookmarks permanent item.
+ optional bool include_syncable_bookmarks = 1000 [default = false];
};
message AuthenticateMessage {
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
index 24bfc90..f5c4a3f 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
@@ -539,6 +539,7 @@ void RecordAppLaunch(Profile* profile, GURL url) {
// Don't allow edit/delete of the bar node, or of "Other Bookmarks"
if ((node == nil) ||
(node == bookmarkModel_->other_node()) ||
+ (node == bookmarkModel_->synced_node()) ||
(node == bookmarkModel_->GetBookmarkBarNode()))
return NO;
return YES;
@@ -800,6 +801,7 @@ void RecordAppLaunch(Profile* profile, GURL url) {
BookmarkNode::Type type = senderNode->type();
if (type == BookmarkNode::BOOKMARK_BAR ||
type == BookmarkNode::OTHER_NODE ||
+ type == BookmarkNode::SYNCED ||
type == BookmarkNode::FOLDER) {
parent = senderNode;
newIndex = parent->child_count();
diff --git a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc
index 318f246..2efa80a 100644
--- a/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc
+++ b/chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk_unittest.cc
@@ -71,6 +71,8 @@ class BookmarkEditorGtkTest : public testing::Test {
// oa
// OF1
// of1a
+ // synced node
+ // sa
void AddTestData() {
std::string test_base = base_path();
@@ -89,6 +91,10 @@ class BookmarkEditorGtkTest : public testing::Test {
const BookmarkNode* of1 =
model_->AddFolder(model_->other_node(), 1, ASCIIToUTF16("OF1"));
model_->AddURL(of1, 0, ASCIIToUTF16("of1a"), GURL(test_base + "of1a"));
+
+ // Children of the synced node.
+ model_->AddURL(model_->synced_node(), 0, ASCIIToUTF16("sa"),
+ GURL(test_base + "sa"));
}
};
@@ -98,15 +104,22 @@ TEST_F(BookmarkEditorGtkTest, ModelsMatch) {
BookmarkEditor::EditDetails(),
BookmarkEditor::SHOW_TREE);
- // The root should have two children, one for the bookmark bar node,
- // the other for the 'other bookmarks' folder.
+ // The root should have two or three children, one for the bookmark bar node,
+ // another for the 'other bookmarks' folder, and depending on the visib
GtkTreeModel* store = GTK_TREE_MODEL(editor.tree_store_);
GtkTreeIter toplevel;
ASSERT_TRUE(gtk_tree_model_get_iter_first(store, &toplevel));
GtkTreeIter bookmark_bar_node = toplevel;
ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel));
GtkTreeIter other_node = toplevel;
- ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel));
+ if (model_->synced_node()->IsVisible()) {
+ // If we have a synced node, then the iterator should find one element after
+ // "other bookmarks"
+ ASSERT_TRUE(gtk_tree_model_iter_next(store, &toplevel));
+ ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel));
+ } else {
+ ASSERT_FALSE(gtk_tree_model_iter_next(store, &toplevel));
+ }
// The bookmark bar should have 2 nodes: folder F1 and F2.
GtkTreeIter f1_iter;
diff --git a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc
index cc2284f..df1d016 100644
--- a/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc
+++ b/chrome/browser/ui/gtk/bookmarks/bookmark_tree_model.cc
@@ -104,8 +104,10 @@ void AddToTreeStore(BookmarkModel* model, int64 selected_id,
GtkTreeStore* store, GtkTreeIter* selected_iter) {
const BookmarkNode* root_node = model->root_node();
for (int i = 0; i < root_node->child_count(); ++i) {
- AddToTreeStoreAt(root_node->GetChild(i), selected_id, store, selected_iter,
- NULL);
+ const BookmarkNode* child = root_node->GetChild(i);
+ if (child->IsVisible()) {
+ AddToTreeStoreAt(child, selected_id, store, selected_iter, NULL);
+ }
}
}
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
index 4690ce0..9667acc 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
@@ -485,9 +485,12 @@ BookmarkEditorView::EditorNode* BookmarkEditorView::CreateRootNode() {
EditorNode* root_node = new EditorNode(std::wstring(), 0);
const BookmarkNode* bb_root_node = bb_model_->root_node();
CreateNodes(bb_root_node, root_node);
- DCHECK(root_node->child_count() == 2);
+ DCHECK(root_node->child_count() >= 2 && root_node->child_count() <= 3);
DCHECK(bb_root_node->GetChild(0)->type() == BookmarkNode::BOOKMARK_BAR);
DCHECK(bb_root_node->GetChild(1)->type() == BookmarkNode::OTHER_NODE);
+ if (root_node->child_count() == 3) {
+ DCHECK(bb_root_node->GetChild(2)->type() == BookmarkNode::SYNCED);
+ }
return root_node;
}
@@ -495,7 +498,7 @@ void BookmarkEditorView::CreateNodes(const BookmarkNode* bb_node,
BookmarkEditorView::EditorNode* b_node) {
for (int i = 0; i < bb_node->child_count(); ++i) {
const BookmarkNode* child_bb_node = bb_node->GetChild(i);
- if (child_bb_node->is_folder()) {
+ if (child_bb_node->IsVisible() && child_bb_node->is_folder()) {
EditorNode* new_b_node =
new EditorNode(WideToUTF16(child_bb_node->GetTitle()),
child_bb_node->id());
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
index 042dc12..b059708 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
@@ -132,9 +132,13 @@ TEST_F(BookmarkEditorViewTest, ModelsMatch) {
CreateEditor(profile_.get(), NULL, BookmarkEditor::EditDetails(),
BookmarkEditorView::SHOW_TREE);
BookmarkEditorView::EditorNode* editor_root = editor_tree_model()->GetRoot();
- // The root should have two children, one for the bookmark bar node,
- // the other for the 'other bookmarks' folder.
- ASSERT_EQ(2, editor_root->child_count());
+ // The root should have two or three children: bookmark bar, other bookmarks
+ // and conditionally synced bookmarks.
+ if (model_->synced_node()->IsVisible()) {
+ ASSERT_EQ(3, editor_root->child_count());
+ } else {
+ ASSERT_EQ(2, editor_root->child_count());
+ }
BookmarkEditorView::EditorNode* bb_node = editor_root->GetChild(0);
// The root should have 2 nodes: folder F1 and F2.
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 059f378..16765cd 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -514,6 +514,9 @@ const char kEnableTabGroupsContextMenu[] = "enable-tab-groups-context-menu";
// Enable syncing browser typed urls.
const char kEnableSyncTypedUrls[] = "enable-sync-typed-urls";
+// Enable syncing browser typed urls.
+const char kEnableSyncedBookmarksFolder[] = "enable-synced-bookmarks-folder";
+
// Enable use of experimental TCP sockets API for sending data in the
// SYN packet.
const char kEnableTcpFastOpen[] = "enable-tcp-fastopen";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index b8626ae..9d51607 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -153,6 +153,7 @@ extern const char kEnableSyncEncryption[];
extern const char kEnableSyncPreferences[];
extern const char kEnableSyncSessions[];
extern const char kEnableSyncTypedUrls[];
+extern const char kEnableSyncedBookmarksFolder[];
extern const char kEnableTabGroupsContextMenu[];
extern const char kEnableTcpFastOpen[];
extern const char kEnableTopSites[];
diff --git a/chrome/test/data/bookmarks/model_without_sync.json b/chrome/test/data/bookmarks/model_without_sync.json
new file mode 100644
index 0000000..517f834
--- /dev/null
+++ b/chrome/test/data/bookmarks/model_without_sync.json
@@ -0,0 +1,48 @@
+{
+ "checksum": "47c64556f92189712e9ba2ae0b485d62",
+ "roots": {
+ "bookmark_bar": {
+ "children": [ {
+ "children": [ {
+ "date_added": "12949698950422234",
+ "id": "5",
+ "name": "Bookmark Manager",
+ "type": "url",
+ "url": "chrome-extension://eemcgdkfndhakfknompkggombfjjjeno/main.html#3"
+ } ],
+ "date_added": "12949698938004572",
+ "date_modified": "12949698958396466",
+ "id": "3",
+ "name": "Folder A",
+ "type": "folder"
+ } ],
+ "date_added": "0",
+ "date_modified": "0",
+ "id": "1",
+ "name": "Bookmarks Bar",
+ "type": "folder"
+ },
+ "other": {
+ "children": [ {
+ "children": [ {
+ "date_added": "12949698958396466",
+ "id": "6",
+ "name": "Get started with Google Chrome",
+ "type": "url",
+ "url": "http://tools.google.com/chrome/intl/en/welcome.html"
+ } ],
+ "date_added": "12949698944107898",
+ "date_modified": "12949698961627573",
+ "id": "4",
+ "name": "Folder B",
+ "type": "folder"
+ } ],
+ "date_added": "0",
+ "date_modified": "0",
+ "id": "2",
+ "name": "Other Bookmarks",
+ "type": "folder"
+ }
+ },
+ "version": 1
+}