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 "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* 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) {
 }
 
 EnhancedBookmarkModel::~EnhancedBookmarkModel() {
 }
 
 // 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();

[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) {
   // 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);
+  SetMetaInfo(node, kImageDataKey, encoded);
   // Ensure that the bookmark has a stars.id, to trigger the server processing.
   GetRemoteId(node);
   return true;
 }
 
 bool EnhancedBookmarkModel::GetOriginalImage(const BookmarkNode* node,
                                              GURL* url,
                                              int* width,
                                              int* height) {
   std::string decoded(DataForMetaInfoField(node, kImageDataKey));
@@ skipping 43 matching lines @@
   return data.snippet();
 }
 
 void EnhancedBookmarkModel::SetVersionSuffix(
     const std::string& version_suffix) {
   version_suffix_ = version_suffix;
 }
 
 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_;
 }
 
 bool EnhancedBookmarkModel::SetAllImages(const BookmarkNode* node,
@@ skipping 49 matching lines @@
   if (!result)
     return false;
 
   std::string encoded;
   base::Base64Encode(output, &encoded);
   bookmark_model_->SetNodeMetaInfo(node, kImageDataKey, encoded);
   return true;
 }
 
 }  // namespace enhanced_bookmarks