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

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);
   }

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.

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;

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