Only set remote id during url node creation.

components/enhanced_bookmarks/enhanced_bookmark_model.cc

Index: components/enhanced_bookmarks/enhanced_bookmark_model.cc
diff --git a/components/enhanced_bookmarks/enhanced_bookmark_model.cc b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
index a20e38bac31b494cd8b63e2ff329fd38d645477e..013f45753f28acddfa8c6e295c822ba79f16e0de 100644
--- a/components/enhanced_bookmarks/enhanced_bookmark_model.cc
+++ b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
@@ -9,10 +9,13 @@
 
 #include "base/base64.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/rand_util.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
+#include "components/enhanced_bookmarks/enhanced_bookmark_model_observer.h"
 #include "components/enhanced_bookmarks/proto/metadata.pb.h"
+#include "ui/base/models/tree_node_iterator.h"
 #include "url/gurl.h"
 
 namespace {
@@ -21,11 +24,10 @@ const char* kBookmarkBarId = "f_bookmarks_bar";
 const char* kIdKey = "stars.id";
 const char* kImageDataKey = "stars.imageData";
 const char* kNoteKey = "stars.note";
+const char* kOldIdKey = "stars.oldId";
 const char* kPageDataKey = "stars.pageData";
-const char* kUserEditKey = "stars.userEdit";
 const char* kVersionKey = "stars.version";
 
-const char* kFolderPrefix = "ebf_";
 const char* kBookmarkPrefix = "ebc_";
 
 // Helper method for working with bookmark metainfo.
@@ -62,13 +64,9 @@ bool PopulateImageData(const image::collections::ImageData_ImageInfo& info,
 
 // Generate a random remote id, with a prefix that depends on whether the node
 // is a folder or a bookmark.
-std::string GenerateRemoteId(bool is_folder) {
+std::string GenerateRemoteId() {
   std::stringstream random_id;
-  // Add prefix depending on whether the node is a folder or not.
-  if (is_folder)
-    random_id << kFolderPrefix;
-  else
-    random_id << kBookmarkPrefix;
+  random_id << kBookmarkPrefix;
 
   // Generate 32 digit hex string random suffix.
   random_id << std::hex << std::setfill('0') << std::setw(16);
@@ -81,17 +79,43 @@ namespace enhanced_bookmarks {
 
 EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model,
                                              const std::string& version)
-    : bookmark_model_(bookmark_model), version_(version) {
+    : bookmark_model_(bookmark_model),
+      loaded_(false),
+      weak_ptr_factory_(this),
+      version_(version) {
+  bookmark_model_->AddObserver(this);
+  if (bookmark_model_->loaded()) {
+    InitializeIdMap();
+    loaded_ = true;
+  }
 }
 
 EnhancedBookmarkModel::~EnhancedBookmarkModel() {
 }
 
+void EnhancedBookmarkModel::ShutDown() {
+  FOR_EACH_OBSERVER(EnhancedBookmarkModelObserver,
+                    observers_,
+                    EnhancedBookmarkModelShuttingDown());
+  weak_ptr_factory_.InvalidateWeakPtrs();
+  bookmark_model_->RemoveObserver(this);
+  bookmark_model_ = NULL;
+}
+
+void EnhancedBookmarkModel::AddObserver(
+    EnhancedBookmarkModelObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void EnhancedBookmarkModel::RemoveObserver(
+    EnhancedBookmarkModelObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 // Moves |node| to |new_parent| and inserts it at the given |index|.
 void EnhancedBookmarkModel::Move(const BookmarkNode* node,
                                  const BookmarkNode* new_parent,
                                  int index) {
-  // TODO(rfevang): Update meta info placement fields.
   bookmark_model_->Move(node, new_parent, index);
 }
 
@@ -100,12 +124,7 @@ const BookmarkNode* EnhancedBookmarkModel::AddFolder(
     const BookmarkNode* parent,
     int index,
     const base::string16& title) {
-  BookmarkNode::MetaInfoMap meta_info;
-  meta_info[kIdKey] = GenerateRemoteId(true);
-
-  // TODO(rfevang): Set meta info placement fields.
-  return bookmark_model_->AddFolderWithMetaInfo(
-      parent, index, title, &meta_info);
+  return bookmark_model_->AddFolder(parent, index, title);
 }
 
 // Adds a url at the specified position.
@@ -116,9 +135,7 @@ const BookmarkNode* EnhancedBookmarkModel::AddURL(
     const GURL& url,
     const base::Time& creation_time) {
   BookmarkNode::MetaInfoMap meta_info;
-  meta_info[kIdKey] = GenerateRemoteId(false);
-
-  // TODO(rfevang): Set meta info placement fields.
+  meta_info[kIdKey] = GenerateRemoteId();
   return bookmark_model_->AddURLWithCreationTimeAndMetaInfo(
       parent, index, title, url, creation_time, &meta_info);
 }
@@ -127,24 +144,23 @@ std::string EnhancedBookmarkModel::GetRemoteId(const BookmarkNode* node) {
   if (node == bookmark_model_->bookmark_bar_node())
     return kBookmarkBarId;
 
-  // Permanent nodes other than the bookmarks bar don't have ids.
-  DCHECK(!bookmark_model_->is_permanent_node(node));
-
   std::string id;
-  if (!node->GetMetaInfo(kIdKey, &id) || id.empty())
-    return SetRemoteId(node);
+  if (!node->GetMetaInfo(kIdKey, &id))
+    return std::string();
   return id;
 }
 
-std::string EnhancedBookmarkModel::SetRemoteId(const BookmarkNode* node) {
-  std::string remote_id = GenerateRemoteId(node->is_folder());
-  SetMetaInfo(node, kIdKey, remote_id, false);
-  return remote_id;
+const BookmarkNode* EnhancedBookmarkModel::BookmarkForRemoteId(
+    const std::string& remote_id) {
+  IdToNodeMap::iterator it = id_map_.find(remote_id);
+  if (it != id_map_.end())
+    return it->second;
+  return NULL;
 }
 
 void EnhancedBookmarkModel::SetDescription(const BookmarkNode* node,
                                            const std::string& description) {
-  SetMetaInfo(node, kNoteKey, description, true);
+  SetMetaInfo(node, kNoteKey, description);
 }
 
 std::string EnhancedBookmarkModel::GetDescription(const BookmarkNode* node) {
@@ -189,9 +205,7 @@ bool EnhancedBookmarkModel::SetOriginalImage(const BookmarkNode* node,
 
   std::string encoded;
   base::Base64Encode(output, &encoded);
-  SetMetaInfo(node, kImageDataKey, encoded, true);
-  // Ensure that the bookmark has a stars.id, to trigger the server processing.
-  GetRemoteId(node);
+  SetMetaInfo(node, kImageDataKey, encoded);
   return true;
 }
 
@@ -251,10 +265,120 @@ void EnhancedBookmarkModel::SetVersionSuffix(
   version_suffix_ = version_suffix;
 }
 
+void EnhancedBookmarkModel::BookmarkModelChanged() {
+}
+
+void EnhancedBookmarkModel::BookmarkModelLoaded(BookmarkModel* model,
+                                                bool ids_reassigned) {
+  InitializeIdMap();
+  FOR_EACH_OBSERVER(
+      EnhancedBookmarkModelObserver, observers_, EnhancedBookmarkModelLoaded());
+}
+
+void EnhancedBookmarkModel::BookmarkNodeAdded(BookmarkModel* model,
+                                              const BookmarkNode* parent,
+                                              int index) {
+  const BookmarkNode* node = parent->GetChild(index);
+  AddToIdMap(node);
+  ScheduleResetDuplicateRemoteIds();
+  FOR_EACH_OBSERVER(
+      EnhancedBookmarkModelObserver, observers_, EnhancedBookmarkAdded(node));
+}
+
+void EnhancedBookmarkModel::BookmarkNodeRemoved(
+    BookmarkModel* model,
+    const BookmarkNode* parent,
+    int old_index,
+    const BookmarkNode* node,
+    const std::set<GURL>& removed_urls) {
+  std::string remote_id = GetRemoteId(node);
+  id_map_.erase(remote_id);
+  FOR_EACH_OBSERVER(
+      EnhancedBookmarkModelObserver, observers_, EnhancedBookmarkRemoved(node));
+}
+
+void EnhancedBookmarkModel::OnWillChangeBookmarkMetaInfo(
+    BookmarkModel* model,
+    const BookmarkNode* node) {
+  prev_remote_id_ = GetRemoteId(node);
+}
+
+void EnhancedBookmarkModel::BookmarkMetaInfoChanged(BookmarkModel* model,
+                                                    const BookmarkNode* node) {
+  std::string remote_id = GetRemoteId(node);
+  if (remote_id != prev_remote_id_) {
+    id_map_.erase(prev_remote_id_);
+    if (!remote_id.empty()) {
+      AddToIdMap(node);
+      ScheduleResetDuplicateRemoteIds();
+    }
+    FOR_EACH_OBSERVER(
+        EnhancedBookmarkModelObserver,
+        observers_,
+        EnhancedBookmarkRemoteIdChanged(node, prev_remote_id_, remote_id));
+  }
+}
+
+void EnhancedBookmarkModel::BookmarkAllUserNodesRemoved(
+    BookmarkModel* model,
+    const std::set<GURL>& removed_urls) {
+  id_map_.clear();
+  // Re-initialize so non-user nodes with remote ids are present in the map.
+  InitializeIdMap();
+  FOR_EACH_OBSERVER(EnhancedBookmarkModelObserver,
+                    observers_,
+                    EnhancedBookmarkAllUserNodesRemoved());
+}
+
+void EnhancedBookmarkModel::InitializeIdMap() {
+  ui::TreeNodeIterator<const BookmarkNode> iterator(
+      bookmark_model_->root_node());
+  while (iterator.has_next()) {
+    AddToIdMap(iterator.Next());
+  }
+  ScheduleResetDuplicateRemoteIds();
+}
+
+void EnhancedBookmarkModel::AddToIdMap(const BookmarkNode* node) {
+  std::string remote_id = GetRemoteId(node);
+  if (remote_id.empty())
+    return;
+
+  // Try to insert the node.
+  std::pair<IdToNodeMap::iterator, bool> result =
+      id_map_.insert(make_pair(remote_id, node));
+  if (!result.second) {
+    // Some node already had the same remote id, so add both nodes to the
+    // to-be-reset set.
+    nodes_to_reset_[result.first->second] = remote_id;
+    nodes_to_reset_[node] = remote_id;
+  }
+}
+
+void EnhancedBookmarkModel::ScheduleResetDuplicateRemoteIds() {
+  if (!nodes_to_reset_.empty()) {
+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&EnhancedBookmarkModel::ResetDuplicateRemoteIds,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void EnhancedBookmarkModel::ResetDuplicateRemoteIds() {
+  for (NodeToIdMap::iterator it = nodes_to_reset_.begin();
+       it != nodes_to_reset_.end();
+       ++it) {
+    BookmarkNode::MetaInfoMap meta_info;
+    meta_info[kIdKey] = "";
+    meta_info[kOldIdKey] = it->second;
+    SetMultipleMetaInfo(it->first, meta_info);
+  }
+  nodes_to_reset_.clear();
+}
+
 void EnhancedBookmarkModel::SetMetaInfo(const BookmarkNode* node,
                                         const std::string& field,
-                                        const std::string& value,
-                                        bool user_edit) {
+                                        const std::string& value) {
   DCHECK(!bookmark_model_->is_permanent_node(node));
 
   BookmarkNode::MetaInfoMap meta_info;
@@ -269,7 +393,6 @@ void EnhancedBookmarkModel::SetMetaInfo(const BookmarkNode* node,
 
   meta_info[field] = value;
   meta_info[kVersionKey] = GetVersionString();
-  meta_info[kUserEditKey] = user_edit ? "true" : "false";
   bookmark_model_->SetNodeMetaInfoMap(node, meta_info);
 }
 
@@ -279,6 +402,37 @@ std::string EnhancedBookmarkModel::GetVersionString() {
   return version_ + '/' + version_suffix_;
 }
 
+void EnhancedBookmarkModel::SetMultipleMetaInfo(
+    const BookmarkNode* node,
+    BookmarkNode::MetaInfoMap meta_info) {
+  DCHECK(!bookmark_model_->is_permanent_node(node));
+
+  // Don't update anything if every value is already set correctly.
+  if (node->GetMetaInfoMap()) {
+    bool changed = false;
+    const BookmarkNode::MetaInfoMap* old_meta_info = node->GetMetaInfoMap();
+    for (BookmarkNode::MetaInfoMap::iterator it = meta_info.begin();
+         it != meta_info.end();
+         ++it) {
+      BookmarkNode::MetaInfoMap::const_iterator old_field =
+          old_meta_info->find(it->first);
+      if (old_field == old_meta_info->end() ||
+          old_field->second != it->second) {
+        changed = true;
+        break;
+      }
+    }
+    if (!changed)
+      return;
+
+    // Fill in the values that aren't changing
+    meta_info.insert(old_meta_info->begin(), old_meta_info->end());
+  }
+
+  meta_info[kVersionKey] = GetVersionString();
+  bookmark_model_->SetNodeMetaInfoMap(node, meta_info);
+}
+
 bool EnhancedBookmarkModel::SetAllImages(const BookmarkNode* node,
                                          const GURL& image_url,
                                          int image_width,