Only set remote id during url node creation.

components/enhanced_bookmarks/enhanced_bookmark_model.cc

 // Copyright 2014 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 "components/enhanced_bookmarks/enhanced_bookmark_model.h"
 
 #include <iomanip>
 #include <sstream>
 
 #include "base/base64.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/enhanced_bookmarks/proto/metadata.pb.h"
+#include "ui/base/models/tree_node_iterator.h"
 #include "url/gurl.h"
 
 namespace {
 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.
 std::string DataForMetaInfoField(const BookmarkNode* node,
                                  const std::string& field) {
   std::string value;
   if (!node->GetMetaInfo(field, &value))
     return std::string();
 
   std::string decoded;
@@ skipping 16 matching lines @@
     return false;
 
   *out_url = url;
   *width = info.width();
   *height = info.height();
   return true;
 }
 
 // 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);
   random_id << base::RandUint64() << base::RandUint64();
   return random_id.str();
 }
 }  // namespace
 
 namespace enhanced_bookmarks {
 
 EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model,
                                              const std::string& version)
     : bookmark_model_(bookmark_model), version_(version) {
+  bookmark_model_->AddObserver(this);
+  if (bookmark_model_->loaded())
+    InitializeIdMap();
 }
 
 EnhancedBookmarkModel::~EnhancedBookmarkModel() {
 }
 
+void EnhancedBookmarkModel::ShutDown() {
+  bookmark_model_->RemoveObserver(this);
+  bookmark_model_ = NULL;
+}
+
 // 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);
 }
 
 // Adds a new folder node at the specified position.
 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.
 const BookmarkNode* EnhancedBookmarkModel::AddURL(
     const BookmarkNode* parent,
     int index,
     const base::string16& title,
     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);
 }
 
 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) {
   // First, look for a custom note set by the user.
   std::string description;
   if (node->GetMetaInfo(kNoteKey, &description) && !description.empty())
     return description;
 
   // If none are present, return the snippet.
   return GetSnippet(node);
@@ skipping 24 matching lines @@
   info->set_height(height);
   data.set_allocated_original_info(info.release());
 
   std::string output;
   bool result = data.SerializePartialToString(&output);
   if (!result)
     return false;
 
   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;
 }
 
 bool EnhancedBookmarkModel::GetOriginalImage(const BookmarkNode* node,
                                              GURL* url,
                                              int* width,
                                              int* height) {
   std::string decoded(DataForMetaInfoField(node, kImageDataKey));
   if (decoded == "")
     return false;
@@ skipping 39 matching lines @@
     return std::string();
 
   return data.snippet();
 }
 
 void EnhancedBookmarkModel::SetVersionSuffix(
     const std::string& version_suffix) {
   version_suffix_ = version_suffix;
 }
 
+void EnhancedBookmarkModel::BookmarkModelChanged() {
+}
+
+void EnhancedBookmarkModel::BookmarkModelLoaded(BookmarkModel* model,
+                                                bool ids_reassigned) {
+  InitializeIdMap();
+}
+
+void EnhancedBookmarkModel::BookmarkNodeAdded(BookmarkModel* model,
+                                              const BookmarkNode* parent,
+                                              int index) {
+  AddToIdMap(parent->GetChild(index));
+  ResetDuplicateRemoteIds();
+}
+
+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);
+}
+
+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);
+      ResetDuplicateRemoteIds();
+    }
+  }
+}
+
+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();
+}
+
+void EnhancedBookmarkModel::InitializeIdMap() {
+  ui::TreeNodeIterator<const BookmarkNode> iterator(
+      bookmark_model_->root_node());
+  while (iterator.has_next()) {
+    AddToIdMap(iterator.Next());
+  }
+  ResetDuplicateRemoteIds();
+}
+
+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;

[noyau (Ping after 24h), 2014/09/17 08:24:35]

Don't you need to remove the node you found in the map from the map as well?

[Rune Fevang, 2014/09/17 22:51:04]

It needs to stay there in case there are more duplicates (say 3), so all three
will be found. That's also why the resetting of duplicate ids is deferred until
the loading is completed instead of being done on detection. The nodes will be
removed from the id map when OnMetaInfoChanged is received.

[noyau (Ping after 24h), 2014/09/18 08:43:21]

Acknowledged.

+  }
+}
+
+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;
   const BookmarkNode::MetaInfoMap* old_meta_info = node->GetMetaInfoMap();
   if (old_meta_info)
     meta_info.insert(old_meta_info->begin(), old_meta_info->end());
 
   // Don't update anything if the value to set is already there.
   BookmarkNode::MetaInfoMap::iterator it = meta_info.find(field);
   if (it != meta_info.end() && it->second == value)
     return;
 
   meta_info[field] = value;
   meta_info[kVersionKey] = GetVersionString();
-  meta_info[kUserEditKey] = user_edit ? "true" : "false";
   bookmark_model_->SetNodeMetaInfoMap(node, meta_info);
 }
 
 std::string EnhancedBookmarkModel::GetVersionString() {
   if (version_suffix_.empty())
     return version_;
   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,
                                          int image_height,
                                          const GURL& thumbnail_url,
                                          int thumbnail_width,
                                          int thumbnail_height) {
   DCHECK(node->is_url());
   DCHECK(image_url.is_valid() || image_url.is_empty());
   DCHECK(thumbnail_url.is_valid() || thumbnail_url.is_empty());
@@ skipping 40 matching lines @@
   if (!result)
     return false;
 
   std::string encoded;
   base::Base64Encode(output, &encoded);
   bookmark_model_->SetNodeMetaInfo(node, kImageDataKey, encoded);
   return true;
 }
 
 }  // namespace enhanced_bookmarks