Delete old sync proto field old_parent_id

The field hasn't been populated in a long time so it can be deleted from
the protobuf definition, and the old legacy code (not actually
exercised) deleted.

Change-Id: Iaf65d73e76841c2758679cfd26a93432a438b4ae
Bug: 923287
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2262992
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781803}

components/sync/engine_impl/commit_util.cc

@@ -123,18 +123,6 @@ void BuildCommitItem(const syncable::Entry& meta_entry,
     sync_entry->set_parent_id_string(SyncableIdToProto(new_parent_id));
   }
 
-  // If our parent has changed, send up the old one so the server
-  // can correctly deal with multiple parents.
-  // TODO(nick): With the server keeping track of the primary sync parent,
-  // it should not be necessary to provide the old_parent_id: the version
-  // number should suffice.
-  Id server_parent_id = meta_entry.GetServerParentId();
-  if (new_parent_id != server_parent_id && !server_parent_id.IsNull() &&
-      0 != meta_entry.GetBaseVersion() &&
-      syncable::CHANGES_VERSION != meta_entry.GetBaseVersion()) {
-    sync_entry->set_old_parent_id(SyncableIdToProto(server_parent_id));
-  }
-
   int64_t version = meta_entry.GetBaseVersion();
   if (syncable::CHANGES_VERSION == version || 0 == version) {
     // Undeletions are only supported for items that have a client tag.
@@ -296,8 +284,7 @@ void UpdateServerFieldsAfterCommit(
   // get the post-commit ID of the parent indicated by
   // committed_entry.parent_id_string(). That should be inferrable from the
   // information we have, but it's a bit convoluted to pull it out directly.
-  // Getting this right is important: SERVER_PARENT_ID gets fed back into
-  // old_parent_id during the next commit.
+  // Getting this right is important.
   local_entry->PutServerParentId(local_entry->GetParentId());
   local_entry->PutServerNonUniqueName(
       GetResultingPostCommitName(committed_entry, entry_response));

components/sync/engine_impl/syncer_unittest.cc

@@ -1316,38 +1316,6 @@ TEST_F(SyncerTest, TestClientCommandDuringCommit) {
   EXPECT_EQ(9, last_client_invalidation_hint_buffer_size_);
 }
 
-TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
-  syncable::Id folder_one_id = ids_.FromNumber(1);
-  syncable::Id folder_two_id = ids_.FromNumber(2);
-
-  mock_server_->AddUpdateDirectory(folder_one_id, TestIdFactory::root(),
-                                   "folder_one", 1, 1, foreign_cache_guid(),
-                                   "-1");
-  mock_server_->AddUpdateDirectory(folder_two_id, TestIdFactory::root(),
-                                   "folder_two", 1, 1, foreign_cache_guid(),
-                                   "-2");
-  EXPECT_TRUE(SyncShareNudge());
-  {
-    // A moved entry should send an "old parent."
-    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, GET_BY_ID, folder_one_id);
-    ASSERT_TRUE(entry.good());
-    entry.PutParentId(folder_two_id);
-    entry.PutIsUnsynced(true);
-    // A new entry should send no "old parent."
-    MutableEntry create(&trans, CREATE, BOOKMARKS, trans.root_id(),
-                        "new_folder");
-    create.PutIsUnsynced(true);
-    create.PutSpecifics(DefaultBookmarkSpecifics());
-  }
-  EXPECT_TRUE(SyncShareNudge());
-  const sync_pb::CommitMessage& commit = mock_server_->last_sent_commit();
-  ASSERT_EQ(2, commit.entries_size());
-  EXPECT_EQ("2", commit.entries(0).parent_id_string());
-  EXPECT_EQ("0", commit.entries(0).old_parent_id());
-  EXPECT_FALSE(commit.entries(1).has_old_parent_id());
-}
-
 TEST_F(SyncerTest, Test64BitVersionSupport) {
   int64_t really_big_int = std::numeric_limits<int64_t>::max() - 12;
   const string name("ringo's dang orang ran rings around my o-ring");

components/sync/protocol/proto_visitors.h

@@ -816,7 +816,6 @@ VISIT_PROTO_FIELDS(const sync_pb::SyncCycleCompletedEventInfo& proto) {
 VISIT_PROTO_FIELDS(const sync_pb::SyncEntity& proto) {
   VISIT(id_string);
   VISIT(parent_id_string);
-  VISIT(old_parent_id);
   VISIT(version);
   VISIT(mtime);
   VISIT(ctime);

components/sync/protocol/sync.proto

@@ -194,11 +194,8 @@ message SyncEntity {
   // Present in both GetUpdatesResponse and CommitMessage.
   optional string parent_id_string = 2;
 
-  // old_parent_id is only set in commits and indicates the old server
-  // parent(s) to remove. When omitted, the old parent is the same as
-  // the new.
-  // Present only in CommitMessage.
-  optional string old_parent_id = 3;
+  reserved 3;
+  reserved "old_parent_id";
 
   // The version of this item -- a monotonically increasing value that is
   // maintained by for each item.  If zero in a CommitMessage, the server

components/sync/test/engine/mock_connection_manager.cc

@@ -426,7 +426,6 @@ sync_pb::SyncEntity* MockConnectionManager::AddUpdateFromLastCommit() {
     sync_pb::SyncEntity* ent = GetUpdateResponse()->add_entries();
     ent->CopyFrom(last_sent_commit().entries(0));
     ent->clear_insert_after_item_id();
-    ent->clear_old_parent_id();
     ent->set_version(last_commit_response().entryresponse(0).version());
     ent->set_id_string(last_commit_response().entryresponse(0).id_string());