bookmarks: Remove obsolete sync transaction versions

The USS implementation of bookmark sync launched with M77 and most code
has already been cleaned up.

Transaction versions embedded in BookmarkModel are no longer used and
can also be deleted.

Change-Id: Icfe2ab727edce293c73b7b3ae521df66a71d843a
Bug: 933756
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081426Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746457}

components/bookmarks/browser/bookmark_codec.cc

@@ -42,8 +42,6 @@ const char BookmarkCodec::kURLKey[] = "url";
 const char BookmarkCodec::kDateModifiedKey[] = "date_modified";
 const char BookmarkCodec::kChildrenKey[] = "children";
 const char BookmarkCodec::kMetaInfo[] = "meta_info";
-const char BookmarkCodec::kSyncTransactionVersion[] =
-    "sync_transaction_version";
 const char BookmarkCodec::kTypeURL[] = "url";
 const char BookmarkCodec::kTypeFolder[] = "folder";
 const char BookmarkCodec::kSyncMetadata[] = "sync_metadata";
@@ -55,9 +53,7 @@ BookmarkCodec::BookmarkCodec()
     : ids_reassigned_(false),
       guids_reassigned_(false),
       ids_valid_(true),
-      maximum_id_(0),
-      model_sync_transaction_version_(
-          BookmarkNode::kInvalidSyncTransactionVersion) {}
+      maximum_id_(0) {}
 
 BookmarkCodec::~BookmarkCodec() = default;
 
@@ -66,7 +62,6 @@ std::unique_ptr<base::Value> BookmarkCodec::Encode(
     const std::string& sync_metadata_str) {
   return Encode(model->bookmark_bar_node(), model->other_node(),
                 model->mobile_node(), model->root_node()->GetMetaInfoMap(),
-                model->root_node()->sync_transaction_version(),
                 sync_metadata_str);
 }
 
@@ -75,7 +70,6 @@ std::unique_ptr<base::Value> BookmarkCodec::Encode(
     const BookmarkNode* other_folder_node,
     const BookmarkNode* mobile_folder_node,
     const BookmarkNode::MetaInfoMap* model_meta_info_map,
-    int64_t sync_transaction_version,
     const std::string& sync_metadata_str) {
   ids_reassigned_ = false;
   guids_reassigned_ = false;
@@ -86,11 +80,6 @@ std::unique_ptr<base::Value> BookmarkCodec::Encode(
   roots->Set(kMobileBookmarkFolderNameKey, EncodeNode(mobile_folder_node));
   if (model_meta_info_map)
     roots->Set(kMetaInfo, EncodeMetaInfo(*model_meta_info_map));
-  if (sync_transaction_version !=
-      BookmarkNode::kInvalidSyncTransactionVersion) {
-    roots->SetString(kSyncTransactionVersion,
-                     base::NumberToString(sync_transaction_version));
-  }
   auto main = std::make_unique<base::DictionaryValue>();
   main->SetInteger(kVersionKey, kCurrentVersion);
   FinalizeChecksum();
@@ -167,11 +156,6 @@ std::unique_ptr<base::Value> BookmarkCodec::EncodeNode(
   const BookmarkNode::MetaInfoMap* meta_info_map = node->GetMetaInfoMap();
   if (meta_info_map)
     value->Set(kMetaInfo, EncodeMetaInfo(*meta_info_map));
-  if (node->sync_transaction_version() !=
-      BookmarkNode::kInvalidSyncTransactionVersion) {
-    value->SetString(kSyncTransactionVersion,
-                     base::NumberToString(node->sync_transaction_version()));
-  }
   return std::move(value);
 }
 
@@ -244,15 +228,7 @@ bool BookmarkCodec::DecodeHelper(BookmarkNode* bb_node,
       ReassignIDsHelper(mobile_folder_node);
   }
 
-  if (!DecodeMetaInfo(*roots_d_value, &model_meta_info_map_,
-                      &model_sync_transaction_version_))
-    return false;
-
-  std::string sync_transaction_version_str;
-  if (roots_d_value->GetString(kSyncTransactionVersion,
-                               &sync_transaction_version_str) &&
-      !base::StringToInt64(sync_transaction_version_str,
-                           &model_sync_transaction_version_))
+  if (!DecodeMetaInfo(*roots_d_value, &model_meta_info_map_))
     return false;
 
   std::string sync_metadata_str_base64;
@@ -415,28 +391,17 @@ bool BookmarkCodec::DecodeNode(const base::DictionaryValue& value,
   node->SetTitle(title);
   node->set_date_added(Time::FromInternalValue(internal_time));
 
-  int64_t sync_transaction_version = node->sync_transaction_version();
   BookmarkNode::MetaInfoMap meta_info_map;
-  if (!DecodeMetaInfo(value, &meta_info_map, &sync_transaction_version))
+  if (!DecodeMetaInfo(value, &meta_info_map))
     return false;
   node->SetMetaInfoMap(meta_info_map);
 
-  std::string sync_transaction_version_str;
-  if (value.GetString(kSyncTransactionVersion, &sync_transaction_version_str) &&
-      !base::StringToInt64(sync_transaction_version_str,
-                           &sync_transaction_version))
-    return false;
-
-  node->set_sync_transaction_version(sync_transaction_version);
-
   return true;
 }
 
 bool BookmarkCodec::DecodeMetaInfo(const base::DictionaryValue& value,
-                                   BookmarkNode::MetaInfoMap* meta_info_map,
-                                   int64_t* sync_transaction_version) {
+                                   BookmarkNode::MetaInfoMap* meta_info_map) {
   DCHECK(meta_info_map);
-  DCHECK(sync_transaction_version);
   meta_info_map->clear();
 
   const base::Value* meta_info;
@@ -464,18 +429,6 @@ bool BookmarkCodec::DecodeMetaInfo(const base::DictionaryValue& value,
     return false;
   DecodeMetaInfoHelper(*meta_info_dict, std::string(), meta_info_map);
 
-  // Previously sync transaction version was stored in the meta info field
-  // using this key. If the key is present when decoding, set the sync
-  // transaction version to its value, then delete the field.
-  if (deserialized_holder) {
-    const char kBookmarkTransactionVersionKey[] = "sync.transaction_version";
-    auto it = meta_info_map->find(kBookmarkTransactionVersionKey);
-    if (it != meta_info_map->end()) {
-      base::StringToInt64(it->second, sync_transaction_version);
-      meta_info_map->erase(it);
-    }
-  }
-
   return true;
 }
 

components/bookmarks/browser/bookmark_codec.h

@@ -51,7 +51,6 @@ class BookmarkCodec {
       const BookmarkNode* other_folder_node,
       const BookmarkNode* mobile_folder_node,
       const BookmarkNode::MetaInfoMap* model_meta_info_map,
-      int64_t sync_transaction_version,
       const std::string& sync_metadata_str);
 
   // Decodes the previously encoded value to the specified nodes as well as
@@ -81,11 +80,6 @@ class BookmarkCodec {
     return model_meta_info_map_;
   }
 
-  // Return the sync transaction version of the bookmark model root.
-  int64_t model_sync_transaction_version() const {
-    return model_sync_transaction_version_;
-  }
-
   // Returns whether the IDs were reassigned during decoding. Always returns
   // false after encoding.
   bool ids_reassigned() const { return ids_reassigned_; }
@@ -110,7 +104,6 @@ class BookmarkCodec {
   static const char kDateModifiedKey[];
   static const char kChildrenKey[];
   static const char kMetaInfo[];
-  static const char kSyncTransactionVersion[];
   // Allows the BookmarkClient to read and a write a string blob from the JSON
   // file. That string captures the bookmarks sync metadata.
   static const char kSyncMetadata[];
@@ -154,14 +147,10 @@ class BookmarkCodec {
                   BookmarkNode* parent,
                   BookmarkNode* node);
 
-  // Decodes the meta info from the supplied value. If the meta info contains
-  // a "sync.transaction_version" key, the value of that field will be stored
-  // in the sync_transaction_version variable, then deleted. This is for
-  // backward-compatibility reasons.
-  // meta_info_map and sync_transaction_version must not be NULL.
+  // Decodes the meta info from the supplied value. meta_info_map must not be
+  // nullptr.
   bool DecodeMetaInfo(const base::DictionaryValue& value,
-                      BookmarkNode::MetaInfoMap* meta_info_map,
-                      int64_t* sync_transaction_version);
+                      BookmarkNode::MetaInfoMap* meta_info_map);
 
   // Decodes the meta info from the supplied sub-node dictionary. The values
   // found will be inserted in meta_info_map with the given prefix added to the
@@ -221,9 +210,6 @@ class BookmarkCodec {
   // Meta info set on bookmark model root.
   BookmarkNode::MetaInfoMap model_meta_info_map_;
 
-  // Sync transaction version set on bookmark model root.
-  int64_t model_sync_transaction_version_;
-
   DISALLOW_COPY_AND_ASSIGN(BookmarkCodec);
 };
 

components/bookmarks/browser/bookmark_codec_unittest.cc

@@ -184,8 +184,6 @@ class BookmarkCodecTest : public testing::Test {
                                 sync_metadata_str);
     model->set_next_node_id(max_id);
     AsMutable(model->root_node())->SetMetaInfoMap(codec->model_meta_info_map());
-    AsMutable(model->root_node())
-        ->set_sync_transaction_version(codec->model_sync_transaction_version());
 
     return result;
   }
@@ -452,28 +450,6 @@ TEST_F(BookmarkCodecTest, EncodeAndDecodeMetaInfo) {
   EXPECT_FALSE(child->GetMetaInfo("other_key", &meta_value));
 }
 
-TEST_F(BookmarkCodecTest, EncodeAndDecodeSyncTransactionVersion) {
-  // Add sync transaction version and encode.
-  std::unique_ptr<BookmarkModel> model(CreateTestModel2());
-  model->SetNodeSyncTransactionVersion(model->root_node(), 1);
-  const BookmarkNode* bbn = model->bookmark_bar_node();
-  model->SetNodeSyncTransactionVersion(bbn->children()[1].get(), 42);
-
-  std::string checksum;
-  std::unique_ptr<base::Value> value =
-      EncodeHelper(model.get(), /*sync_metadata_str=*/std::string(), &checksum);
-  ASSERT_TRUE(value.get() != nullptr);
-
-  // Decode and verify.
-  model = DecodeHelper(*value, checksum, &checksum, false,
-                       /*sync_metadata_str=*/nullptr);
-  EXPECT_EQ(1, model->root_node()->sync_transaction_version());
-  bbn = model->bookmark_bar_node();
-  EXPECT_EQ(42, bbn->children()[1]->sync_transaction_version());
-  EXPECT_EQ(BookmarkNode::kInvalidSyncTransactionVersion,
-            bbn->children()[0]->sync_transaction_version());
-}
-
 // Verifies that we can still decode the old codec format after changing the
 // way meta info is stored.
 TEST_F(BookmarkCodecTest, CanDecodeMetaInfoAsString) {
@@ -490,20 +466,11 @@ TEST_F(BookmarkCodecTest, CanDecodeMetaInfoAsString) {
   ASSERT_TRUE(Decode(&decoder, *root.get(), model.get(),
                      /*sync_metadata_str=*/nullptr));
 
-  EXPECT_EQ(1, model->root_node()->sync_transaction_version());
   const BookmarkNode* bbn = model->bookmark_bar_node();
-  EXPECT_EQ(BookmarkNode::kInvalidSyncTransactionVersion,
-            bbn->children()[0]->sync_transaction_version());
-  EXPECT_EQ(42, bbn->children()[1]->sync_transaction_version());
 
-  const char kSyncTransactionVersionKey[] = "sync.transaction_version";
   const char kNormalKey[] = "key";
   const char kNestedKey[] = "nested.key";
   std::string meta_value;
-  EXPECT_FALSE(
-      model->root_node()->GetMetaInfo(kSyncTransactionVersionKey, &meta_value));
-  EXPECT_FALSE(
-      bbn->children()[1]->GetMetaInfo(kSyncTransactionVersionKey, &meta_value));
   EXPECT_TRUE(bbn->children()[0]->GetMetaInfo(kNormalKey, &meta_value));
   EXPECT_EQ("value", meta_value);
   EXPECT_TRUE(bbn->children()[1]->GetMetaInfo(kNormalKey, &meta_value));

components/bookmarks/browser/bookmark_model.cc

@@ -502,20 +502,6 @@ void BookmarkModel::AddNonClonedKey(const std::string& key) {
   non_cloned_keys_.insert(key);
 }
 
-void BookmarkModel::SetNodeSyncTransactionVersion(
-    const BookmarkNode* node,
-    int64_t sync_transaction_version) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(client_->CanSyncNode(node));
-
-  if (sync_transaction_version == node->sync_transaction_version())
-    return;
-
-  AsMutable(node)->set_sync_transaction_version(sync_transaction_version);
-  if (store_)
-    store_->ScheduleSave();
-}
-
 void BookmarkModel::OnFaviconsChanged(const std::set<GURL>& page_urls,
                                       const GURL& icon_url) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -895,8 +881,6 @@ void BookmarkModel::DoneLoading(std::unique_ptr<BookmarkLoadDetails> details) {
                    VisibilityComparator(client_.get()));
 
   root_->SetMetaInfoMap(details->model_meta_info_map());
-  root_->set_sync_transaction_version(
-      details->model_sync_transaction_version());
 
   loaded_ = true;
   client_->DecodeBookmarkSyncMetadata(

components/bookmarks/browser/bookmark_model.h

@@ -304,10 +304,6 @@ class BookmarkModel : public BookmarkUndoProvider,
     return non_cloned_keys_;
   }
 
-  // Sets the sync transaction version of |node|.
-  void SetNodeSyncTransactionVersion(const BookmarkNode* node,
-                                     int64_t sync_transaction_version);
-
   // Notify BookmarkModel that the favicons for the given page URLs (e.g.
   // http://www.google.com) and the given icon URL (e.g.
   // http://www.google.com/favicon.ico) have changed. It is valid to call

components/bookmarks/browser/bookmark_node.cc

@@ -46,7 +46,6 @@ std::string PermanentNodeTypeToGuid(BookmarkNode::Type type) {
 // BookmarkNode ---------------------------------------------------------------
 
 // static
-const int64_t BookmarkNode::kInvalidSyncTransactionVersion = -1;
 const char BookmarkNode::kRootNodeGuid[] =
     "00000000-0000-4000-a000-000000000001";
 const char BookmarkNode::kBookmarkBarNodeGuid[] =

components/bookmarks/browser/bookmark_node.h

@@ -44,7 +44,6 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode>, public TitledUrlNode {
 
   typedef std::map<std::string, std::string> MetaInfoMap;
 
-  static const int64_t kInvalidSyncTransactionVersion;
   static const char kRootNodeGuid[];
   static const char kBookmarkBarNodeGuid[];
   static const char kOtherBookmarksNodeGuid[];
@@ -125,11 +124,6 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode>, public TitledUrlNode {
   // Returns NULL if there are no values in the map.
   const MetaInfoMap* GetMetaInfoMap() const;
 
-  void set_sync_transaction_version(int64_t sync_transaction_version) {
-    sync_transaction_version_ = sync_transaction_version;
-  }
-  int64_t sync_transaction_version() const { return sync_transaction_version_; }
-
   // TitledUrlNode interface methods.
   const base::string16& GetTitledUrlNodeTitle() const override;
   const GURL& GetTitledUrlNodeUrl() const override;
@@ -218,9 +212,6 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode>, public TitledUrlNode {
   // A map that stores arbitrary meta information about the node.
   std::unique_ptr<MetaInfoMap> meta_info_map_;
 
-  // The sync transaction version.
-  int64_t sync_transaction_version_ = kInvalidSyncTransactionVersion;
-
   const bool is_permanent_node_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkNode);

components/bookmarks/browser/bookmark_storage.cc

@@ -141,8 +141,6 @@ void LoadBookmarks(const base::FilePath& path,
       details->set_ids_reassigned(codec.ids_reassigned());
       details->set_guids_reassigned(codec.guids_reassigned());
       details->set_model_meta_info_map(codec.model_meta_info_map());
-      details->set_model_sync_transaction_version(
-          codec.model_sync_transaction_version());
       UMA_HISTOGRAM_TIMES("Bookmarks.DecodeTime",
                           TimeTicks::Now() - start_time);
       int64_t size = 0;
@@ -202,9 +200,7 @@ void LoadBookmarks(const base::FilePath& path,
 
 BookmarkLoadDetails::BookmarkLoadDetails(BookmarkClient* client)
     : load_managed_node_callback_(client->GetLoadManagedNodeCallback()),
-      index_(std::make_unique<TitledUrlIndex>()),
-      model_sync_transaction_version_(
-          BookmarkNode::kInvalidSyncTransactionVersion) {
+      index_(std::make_unique<TitledUrlIndex>()) {
   // WARNING: do NOT add |client| as a member. Much of this code runs on another
   // thread, and |client_| is not thread safe, and/or may be destroyed before
   // this.

components/bookmarks/browser/bookmark_storage.h

@@ -70,13 +70,6 @@ class BookmarkLoadDetails {
     model_meta_info_map_ = meta_info_map;
   }
 
-  int64_t model_sync_transaction_version() const {
-    return model_sync_transaction_version_;
-  }
-  void set_model_sync_transaction_version(int64_t sync_transaction_version) {
-    model_sync_transaction_version_ = sync_transaction_version;
-  }
-
   // Max id of the nodes.
   void set_max_id(int64_t max_id) { max_id_ = max_id; }
   int64_t max_id() const { return max_id_; }
@@ -128,7 +121,6 @@ class BookmarkLoadDetails {
   LoadManagedNodeCallback load_managed_node_callback_;
   std::unique_ptr<TitledUrlIndex> index_;
   BookmarkNode::MetaInfoMap model_meta_info_map_;
-  int64_t model_sync_transaction_version_;
   int64_t max_id_ = 1;
   std::string computed_checksum_;
   std::string stored_checksum_;