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..660d849fbfcccd025c72c435620a5b23df76fbf4 100644
--- a/components/enhanced_bookmarks/enhanced_bookmark_model.cc
+++ b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
@@ -22,10 +22,8 @@ const char* kIdKey = "stars.id";
 const char* kImageDataKey = "stars.imageData";
 const char* kNoteKey = "stars.note";
 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 +60,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);
@@ -91,7 +85,6 @@ EnhancedBookmarkModel::~EnhancedBookmarkModel() {
 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 +93,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 +104,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 +113,15 @@ 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();

[noyau1, 2014/09/15 08:08:02]

If existing bookmarks do not have IDs yet, this will break all the logic in the
services doing cluster and search. Those needs to be modified to avoid these
IDs.

[Mark, 2014/09/15 17:18:56]

We will create ids server-side for clustering and search.  The only time we
really need an id instantaneously created is when we are calling the "suggest"
functionality.  Outside of that, the id generation can wait until the sync
server sees the data (since that is when the clustering and search functionality
will actually be useful).  The big change we are doing in the JS code is to
allow for a server-side ids and client-side ids to be separate so we no longer
need the tight coupling.

This is how we are eliminating most of our data corruption issues. 

[noyau (Ping after 24h), 2014/09/16 07:33:04]

Yes, yes I understand that. All I'm saying is that there is a need to add a
if(!id.empty()) before adding those objects to the map stars_id/bookmarkNodes in
bookmark_server_service.

   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;
-}
-
 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,7 +166,7 @@ bool EnhancedBookmarkModel::SetOriginalImage(const BookmarkNode* node,
 
   std::string encoded;
   base::Base64Encode(output, &encoded);
-  SetMetaInfo(node, kImageDataKey, encoded, true);
+  SetMetaInfo(node, kImageDataKey, encoded);
   // Ensure that the bookmark has a stars.id, to trigger the server processing.
   GetRemoteId(node);
   return true;
@@ -253,8 +230,7 @@ void EnhancedBookmarkModel::SetVersionSuffix(
 
 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 +245,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);
 }