sync: Enable typed tombstones commits

Modifies commit logic to include the model type of items in commit
requests.  The model type is implicitly specified by the presence of
a sub-message in the EntitySpecifics field.

Prior to this commit, only bookmarks would populate the sync entities on
commit.  The special case for bookmarks was introduced recently, in
r265587.

Unlike the bookmarks case, the non-bookmark items send up only enough
specifics to identify the model type.  The rest of the specifics are not
set.

This commit also adds some tests of the bookmark and non-bookmark
deletion request behaviors.

BUG=365752,373859

Review URL: https://codereview.chromium.org/298503002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271763 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 129 insertions, 1 deletions

diff --git a/sync/engine/commit_util.cc b/sync/engine/commit_util.cc
index 45bc2bb..36b7c06 100644
--- a/sync/engine/commit_util.cc
+++ b/sync/engine/commit_util.cc
@@ -182,6 +182,10 @@ void BuildCommitItem(
   // Deletion is final on the server, let's move things and then delete them.
   if (meta_entry.GetIsDel()) {
     sync_entry->set_deleted(true);
+
+    sync_pb::EntitySpecifics type_only_specifics;
+    AddDefaultFieldValue(meta_entry.GetModelType(),
+                         sync_entry->mutable_specifics());
   } else {
     SetEntrySpecifics(meta_entry, sync_entry);
   }
diff --git a/sync/engine/directory_commit_contribution_unittest.cc b/sync/engine/directory_commit_contribution_unittest.cc
index c38af19..475985c 100644
--- a/sync/engine/directory_commit_contribution_unittest.cc
+++ b/sync/engine/directory_commit_contribution_unittest.cc
@@ -25,6 +25,7 @@ class DirectoryCommitContributionTest : public ::testing::Test {
     syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, dir());
     CreateTypeRoot(&trans, dir(), PREFERENCES);
     CreateTypeRoot(&trans, dir(), EXTENSIONS);
+    CreateTypeRoot(&trans, dir(), BOOKMARKS);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -49,6 +50,32 @@ class DirectoryCommitContributionTest : public ::testing::Test {
     return entry.GetMetahandle();
   }
 
+  int64 CreateSyncedItem(syncable::WriteTransaction* trans,
+                         ModelType type,
+                         const std::string& tag) {
+    syncable::Entry parent_entry(
+        trans,
+        syncable::GET_BY_SERVER_TAG,
+        ModelTypeToRootTag(type));
+    syncable::MutableEntry entry(
+        trans,
+        syncable::CREATE,
+        type,
+        parent_entry.GetId(),
+        tag);
+
+    entry.PutId(syncable::Id::CreateFromServerId(
+        id_factory_.NewServerId().GetServerId()));
+    entry.PutBaseVersion(10);
+    entry.PutServerVersion(10);
+    entry.PutIsUnappliedUpdate(false);
+    entry.PutIsUnsynced(false);
+    entry.PutIsDel(false);
+    entry.PutServerIsDel(false);
+
+    return entry.GetMetahandle();
+  }
+
   void CreateSuccessfulCommitResponse(
       const sync_pb::SyncEntity& entity,
       sync_pb::CommitResponse::EntryResponse* response) {
@@ -177,6 +204,82 @@ TEST_F(DirectoryCommitContributionTest, PrepareCommit) {
   ext_cc->CleanUp();
 }
 
+// Check that deletion requests include a model type.
+// This was not always the case, but was implemented to allow us to loosen some
+// other restrictions in the protocol.
+TEST_F(DirectoryCommitContributionTest, DeletedItemsWithSpecifics) {
+  int64 pref1;
+  {
+    syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, dir());
+    pref1 = CreateSyncedItem(&trans, PREFERENCES, "pref1");
+    syncable::MutableEntry e1(&trans, syncable::GET_BY_HANDLE, pref1);
+    e1.PutIsDel(true);
+    e1.PutIsUnsynced(true);
+  }
+
+  DirectoryTypeDebugInfoEmitter emitter(PREFERENCES, &type_observers_);
+  scoped_ptr<DirectoryCommitContribution> pref_cc(
+      DirectoryCommitContribution::Build(dir(), PREFERENCES, 25, &emitter));
+  ASSERT_TRUE(pref_cc);
+
+  sync_pb::ClientToServerMessage message;
+  pref_cc->AddToCommitMessage(&message);
+
+  const sync_pb::CommitMessage& commit_message = message.commit();
+  ASSERT_EQ(1, commit_message.entries_size());
+  EXPECT_TRUE(
+      commit_message.entries(0).specifics().has_preference());
+
+  pref_cc->CleanUp();
+}
+
+// As ususal, bookmarks are special.  Bookmark deletion is special.
+// Deleted bookmarks include a valid "is folder" bit and their full specifics
+// (especially the meta info, which is what server really wants).
+TEST_F(DirectoryCommitContributionTest, DeletedBookmarksWithSpecifics) {
+  int64 bm1;
+  {
+    syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, dir());
+    bm1 = CreateSyncedItem(&trans, BOOKMARKS, "bm1");
+    syncable::MutableEntry e1(&trans, syncable::GET_BY_HANDLE, bm1);
+
+    e1.PutIsDir(true);
+    e1.PutServerIsDir(true);
+
+    sync_pb::EntitySpecifics specifics;
+    sync_pb::BookmarkSpecifics* bm_specifics = specifics.mutable_bookmark();
+    bm_specifics->set_url("http://www.chrome.com");
+    bm_specifics->set_title("Chrome");
+    sync_pb::MetaInfo* meta_info = bm_specifics->add_meta_info();
+    meta_info->set_key("K");
+    meta_info->set_value("V");
+    e1.PutSpecifics(specifics);
+
+    e1.PutIsDel(true);
+    e1.PutIsUnsynced(true);
+  }
+
+  DirectoryTypeDebugInfoEmitter emitter(BOOKMARKS, &type_observers_);
+  scoped_ptr<DirectoryCommitContribution> bm_cc(
+      DirectoryCommitContribution::Build(dir(), BOOKMARKS, 25, &emitter));
+  ASSERT_TRUE(bm_cc);
+
+  sync_pb::ClientToServerMessage message;
+  bm_cc->AddToCommitMessage(&message);
+
+  const sync_pb::CommitMessage& commit_message = message.commit();
+  ASSERT_EQ(1, commit_message.entries_size());
+
+  const sync_pb::SyncEntity& entity = commit_message.entries(0);
+  EXPECT_TRUE(entity.has_folder());
+  ASSERT_TRUE(entity.specifics().has_bookmark());
+  ASSERT_EQ(1, entity.specifics().bookmark().meta_info_size());
+  EXPECT_EQ("K", entity.specifics().bookmark().meta_info(0).key());
+  EXPECT_EQ("V", entity.specifics().bookmark().meta_info(0).value());
+
+  bm_cc->CleanUp();
+}
+
 // Creates some unsynced items, pretends to commit them, and hands back a
 // specially crafted response to the syncer in order to test commit response
 // processing.  The response simulates a succesful commit scenario.
diff --git a/sync/protocol/sync.proto b/sync/protocol/sync.proto
index 1e9b66a..2768242 100644
--- a/sync/protocol/sync.proto
+++ b/sync/protocol/sync.proto
@@ -692,7 +692,7 @@ message DataTypeContext {
 
 message ClientToServerMessage {
   required string share = 1;
-  optional int32 protocol_version = 2 [default = 32];
+  optional int32 protocol_version = 2 [default = 33];
   enum Contents {
     COMMIT = 1;
     GET_UPDATES = 2;
diff --git a/sync/syncable/model_type_unittest.cc b/sync/syncable/model_type_unittest.cc
index 3ac9fd116..7bf50e5 100644
--- a/sync/syncable/model_type_unittest.cc
+++ b/sync/syncable/model_type_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/test/values_test_util.h"
 #include "base/values.h"
+#include "sync/protocol/sync.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
@@ -117,5 +118,25 @@ TEST_F(ModelTypeTest, ModelTypeSetFromString) {
       two.Equals(ModelTypeSetFromString(ModelTypeSetToString(two))));
 }
 
+TEST_F(ModelTypeTest, DefaultFieldValues) {
+  syncer::ModelTypeSet types = syncer::ProtocolTypes();
+  for (ModelTypeSet::Iterator it = types.First(); it.Good(); it.Inc()) {
+    SCOPED_TRACE(ModelTypeToString(it.Get()));
+
+    sync_pb::EntitySpecifics specifics;
+    syncer::AddDefaultFieldValue(it.Get(), &specifics);
+    EXPECT_TRUE(specifics.IsInitialized());
+
+    std::string tmp;
+    EXPECT_TRUE(specifics.SerializeToString(&tmp));
+
+    sync_pb::EntitySpecifics from_string;
+    EXPECT_TRUE(from_string.ParseFromString(tmp));
+    EXPECT_TRUE(from_string.IsInitialized());
+
+    EXPECT_EQ(it.Get(), syncer::GetModelTypeFromSpecifics(from_string));
+  }
+}
+
 }  // namespace
 }  // namespace syncer