Revert "Revert 84829 - Initial implementation of "Synced Bookmarks" folder."

chrome/browser/bookmarks/bookmark_model.cc

 // 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.
 
 #include "chrome/browser/bookmarks/bookmark_model.h"
 
 #include <algorithm>
 #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"
 #include "chrome/browser/bookmarks/bookmark_storage.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
 #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"
 #include "ui/base/l10n/l10n_util_collator.h"
 #include "ui/gfx/codec/png_codec.h"
 
 using base::Time;
 
 namespace {
 
@@ skipping 25 matching lines @@
   favicon_load_handle_ = 0;
   type_ = !url_.is_empty() ? URL : BOOKMARK_BAR;
   date_added_ = Time::Now();
 }
 
 void BookmarkNode::InvalidateFavicon() {
   loaded_favicon_ = false;
   favicon_ = SkBitmap();
 }
 
+bool BookmarkNode::IsVisible() const {

[sky, 2011/05/17 15:59:08]

Don't we want it visible if not empty too?

[Yaron, 2011/05/17 19:17:33]

I discussed this with Srikanth and the plan is to keep this as is for now
(namely, only make it visible if the flag is set).

[sky, 2011/05/17 20:00:05]

If we do that, couldn't that lead to weird states? For example, lets say you
enable the flag, bookmark google in the synced folder, unset the flag, relaunch
chrome, go to google, click the star, then the edit button. The sync folder
won't be shown, right? This scenario may cause logic problems in the editor and
other places.

[Yaron, 2011/05/17 21:38:37]

I think we'd be ok with some inconsistency with toggling the flag on and off,
since it's not really a user-supported option, but I am concerned about the
potential for a crash. Unsurprisingly, the situation you mentioned did crash so
I fixed it (see the diff in boomark_tree_model). Regardless, Srikanth said it's
fine to make it visible if it has children. I made the change and updated the
test.

[Yaron, 2011/05/17 21:40:13]

Err, it's in bookmark_editor_gtk.cc

+  if (type_ != BookmarkNode::SYNCED ||
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableSyncedBookmarksFolder)) {
+    return true;
+  }
+  return false;
+}
+
 void BookmarkNode::Reset(const history::StarredEntry& entry) {
   DCHECK(entry.type != history::StarredEntry::URL || entry.url == url_);
 
   favicon_ = SkBitmap();
   switch (entry.type) {
     case history::StarredEntry::URL:
       type_ = BookmarkNode::URL;
       break;
     case history::StarredEntry::USER_FOLDER:
       type_ = BookmarkNode::FOLDER;
       break;
     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;
     default:
       NOTREACHED();
   }
   date_added_ = entry.date_added;
   date_folder_modified_ = entry.date_folder_modified;
   set_title(entry.title);
 }
@@ skipping 29 matching lines @@
 
 }  // namespace
 
 BookmarkModel::BookmarkModel(Profile* profile)
     : profile_(profile),
       loaded_(false),
       file_changed_(false),
       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) {
   if (!profile_) {
     // Profile is null during testing.
     DoneLoading(CreateLoadDetails());
   }
 }
 
 BookmarkModel::~BookmarkModel() {
@@ skipping 115 matching lines @@
 }
 
 void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) {
   if (!node) {
     NOTREACHED();
     return;
   }
   if (node->GetTitle() == title)
     return;
 
-  if (node == bookmark_bar_node_ || node == other_node_) {
+  if (node == bookmark_bar_node_ || node == other_node_ ||

[sky, 2011/05/17 15:59:08]

is_permanent_node

[Yaron, 2011/05/17 19:17:33]

Done.

+      node == synced_node_) {
     NOTREACHED();
     return;
   }
 
   // The title index doesn't support changing the title, instead we remove then
   // add it back.
   index_->Remove(node);
   AsMutable(node)->set_title(title);
   index_->Add(node);
 
@@ skipping 288 matching lines @@
       details->ids_reassigned()) {
     // If bookmarks file changed externally, the IDs may have changed
     // externally. In that case, the decoder may have reassigned IDs to make
     // them unique. So when the file has changed externally, we should save the
     // bookmarks file to persist new IDs.
     if (store_.get())
       store_->ScheduleSave();
   }
   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_);
     // Update nodes_ordered_by_url_set_ from the nodes.
     PopulateNodesByURL(&root_);
   }
 
   loaded_ = true;
 
   loaded_signal_.Signal();
@@ skipping 125 matching lines @@
   entry.type = history::StarredEntry::BOOKMARK_BAR;
   return CreateRootNodeFromStarredEntry(entry);
 }
 
 BookmarkNode* BookmarkModel::CreateOtherBookmarksNode() {
   history::StarredEntry entry;
   entry.type = history::StarredEntry::OTHER;
   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));
   }
   return node;
 }
 
 void BookmarkModel::OnFaviconDataAvailable(
     FaviconService::Handle handle,
     history::FaviconData favicon) {
@@ skipping 79 matching lines @@
   return next_node_id_++;
 }
 
 void BookmarkModel::SetFileChanged() {
   file_changed_ = true;
 }
 
 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_);
 }