Sync: add entity modification to FakeServer

This CL adds the ModifyEntity method to FakeServer. This method is used
to modify existing Sync data on the server-side in order to mimic the
commit of a different client. This CL also adds a desktop browser test,
DownloadModifiedBookmark, to make use of the feature.

Other changes in the CL:
1) Add two util methods to the Sync bookmarks test helper to count
   bookmarks by URL. This is needed because FakeServer::ModifyEntity
   does not currently handle top-level entity name changes.
2) Change the FakeServerEntity hierarchy so that its EntitySpecifics
   member is stored as a protected field in the base class. This is done
   so that the specifics can be modified as part of a FakeServer method.

BUG=365774

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

Cr-Commit-Position: refs/heads/master@{#336478}

11 files changed, 75 insertions, 29 deletions

diff --git a/sync/test/fake_server/bookmark_entity.cc b/sync/test/fake_server/bookmark_entity.cc
index 92b7847..e95efe7 100644
--- a/sync/test/fake_server/bookmark_entity.cc
+++ b/sync/test/fake_server/bookmark_entity.cc
@@ -98,11 +98,12 @@ BookmarkEntity::BookmarkEntity(
       originator_cache_guid_(originator_cache_guid),
       originator_client_item_id_(originator_client_item_id),
       unique_position_(unique_position),
-      specifics_(specifics),
       is_folder_(is_folder),
       parent_id_(parent_id),
       creation_time_(creation_time),
-      last_modified_time_(last_modified_time) { }
+      last_modified_time_(last_modified_time) {
+  SetSpecifics(specifics);
+}
 
 string BookmarkEntity::GetParentId() const {
   return parent_id_;
@@ -111,9 +112,6 @@ string BookmarkEntity::GetParentId() const {
 void BookmarkEntity::SerializeAsProto(sync_pb::SyncEntity* proto) {
   FakeServerEntity::SerializeBaseProtoFields(proto);
 
-  sync_pb::EntitySpecifics* specifics = proto->mutable_specifics();
-  specifics->CopyFrom(specifics_);
-
   proto->set_originator_cache_guid(originator_cache_guid_);
   proto->set_originator_client_item_id(originator_client_item_id_);
 
diff --git a/sync/test/fake_server/bookmark_entity.h b/sync/test/fake_server/bookmark_entity.h
index 5cbbdb2..4095382 100644
--- a/sync/test/fake_server/bookmark_entity.h
+++ b/sync/test/fake_server/bookmark_entity.h
@@ -59,7 +59,6 @@ class BookmarkEntity : public FakeServerEntity {
   std::string originator_cache_guid_;
   std::string originator_client_item_id_;
   sync_pb::UniquePosition unique_position_;
-  sync_pb::EntitySpecifics specifics_;
   bool is_folder_;
   std::string parent_id_;
   int64 creation_time_;
diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc
index 8307dec..80edc35 100644
--- a/sync/test/fake_server/fake_server.cc
+++ b/sync/test/fake_server/fake_server.cc
@@ -31,6 +31,7 @@ using std::string;
 using std::vector;
 
 using syncer::GetModelType;
+using syncer::GetModelTypeFromSpecifics;
 using syncer::ModelType;
 using syncer::ModelTypeSet;
 
@@ -220,9 +221,13 @@ bool FakeServer::CreateDefaultPermanentItems() {
   return true;
 }
 
+void FakeServer::UpdateEntityVersion(FakeServerEntity* entity) {
+  entity->SetVersion(++version_);
+}
+
 void FakeServer::SaveEntity(FakeServerEntity* entity) {
   delete entities_[entity->GetId()];
-  entity->SetVersion(++version_);
+  UpdateEntityVersion(entity);
   entities_[entity->GetId()] = entity;
 }
 
@@ -547,6 +552,23 @@ void FakeServer::InjectEntity(scoped_ptr<FakeServerEntity> entity) {
   SaveEntity(entity.release());
 }
 
+bool FakeServer::ModifyEntitySpecifics(
+    const std::string& id,
+    const sync_pb::EntitySpecifics& updated_specifics) {
+  if (entities_.find(id) == entities_.end()) {
+    return false;
+  }
+
+  FakeServerEntity* entity = entities_[id];
+  if (entity->GetModelType() != GetModelTypeFromSpecifics(updated_specifics)) {
+    return false;
+  }
+
+  entity->SetSpecifics(updated_specifics);
+  UpdateEntityVersion(entity);
+  return true;
+}
+
 bool FakeServer::SetNewStoreBirthday(const string& store_birthday) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (store_birthday_ == store_birthday)
diff --git a/sync/test/fake_server/fake_server.h b/sync/test/fake_server/fake_server.h
index f4374e7..74f58bd 100644
--- a/sync/test/fake_server/fake_server.h
+++ b/sync/test/fake_server/fake_server.h
@@ -68,6 +68,19 @@ class FakeServer {
   // result in successful server operations.
   void InjectEntity(scoped_ptr<FakeServerEntity> entity);
 
+  // Modifies the entity on the server with the given |id|. The entity's
+  // EntitySpecifics are replaced with |updated_specifics| and its version is
+  // updated. If the given |id| does not exist or the ModelType of
+  // |updated_specifics| does not match the entity, false is returned.
+  // Otherwise, true is returned to represent a successful modification.
+  //
+  // TODO(pvalenzuela): This method should support updating entity data beyond
+  // EntitySpecifics. For example, in the case of a bookmark, changing the
+  // BookmarkSpecifics title field currently does not modify the top-level
+  // entity's name field.
+  bool ModifyEntitySpecifics(const std::string& id,
+                             const sync_pb::EntitySpecifics& updated_specifics);
+
   // Sets a new store birthday so that tests may trigger a NOT_MY_BIRTHDAY
   // error. If |store_birthday| is the same as |store_birthday_|, false is
   // returned and this method has no effect.
@@ -176,6 +189,10 @@ class FakeServer {
   // Returns whether a triggered error should be sent for the request.
   bool ShouldSendTriggeredError() const;
 
+  // Updates the |entity| to a new version and increments the version counter
+  // that the server uses to assign versions.
+  void UpdateEntityVersion(FakeServerEntity* entity);
+
   // This is the last version number assigned to an entity. The next entity will
   // have a version number of version_ + 1.
   int64 version_;
diff --git a/sync/test/fake_server/fake_server_entity.cc b/sync/test/fake_server/fake_server_entity.cc
index f3fc604..60cb6b5 100644
--- a/sync/test/fake_server/fake_server_entity.cc
+++ b/sync/test/fake_server/fake_server_entity.cc
@@ -57,6 +57,11 @@ const std::string& FakeServerEntity::GetName() const {
   return name_;
 }
 
+void FakeServerEntity::SetSpecifics(
+    const sync_pb::EntitySpecifics& updated_specifics) {
+  specifics_ = updated_specifics;
+}
+
 // static
 string FakeServerEntity::CreateId(const ModelType& model_type,
                                   const string& inner_id) {
@@ -91,13 +96,16 @@ FakeServerEntity::FakeServerEntity(const string& id,
                                    const ModelType& model_type,
                                    int64 version,
                                    const string& name)
-      : model_type_(model_type),
-        id_(id),
+      : id_(id),
+        model_type_(model_type),
         version_(version),
         name_(name) {}
 
 void FakeServerEntity::SerializeBaseProtoFields(
     sync_pb::SyncEntity* sync_entity) {
+  sync_pb::EntitySpecifics* specifics = sync_entity->mutable_specifics();
+  specifics->CopyFrom(specifics_);
+
   // FakeServerEntity fields
   sync_entity->set_id_string(id_);
   sync_entity->set_version(version_);
diff --git a/sync/test/fake_server/fake_server_entity.h b/sync/test/fake_server/fake_server_entity.h
index 6c7b944..faf8347 100644
--- a/sync/test/fake_server/fake_server_entity.h
+++ b/sync/test/fake_server/fake_server_entity.h
@@ -36,6 +36,10 @@ class FakeServerEntity {
   void SetVersion(int64 version);
   const std::string& GetName() const;
 
+  // Replaces |specifics_| with |updated_specifics|. This method is meant to be
+  // used to mimic a client commit.
+  void SetSpecifics(const sync_pb::EntitySpecifics& updated_specifics);
+
   // Common data items needed by server
   virtual std::string GetParentId() const = 0;
   virtual void SerializeAsProto(sync_pb::SyncEntity* proto) = 0;
@@ -54,18 +58,21 @@ class FakeServerEntity {
 
   void SerializeBaseProtoFields(sync_pb::SyncEntity* sync_entity);
 
-  // The ModelType that categorizes this entity.
-  syncer::ModelType model_type_;
-
  private:
   // The entity's ID.
   std::string id_;
 
+  // The ModelType that categorizes this entity.
+  syncer::ModelType model_type_;
+
   // The version of this entity.
   int64 version_;
 
   // The name of the entity.
   std::string name_;
+
+  // The EntitySpecifics for the entity.
+  sync_pb::EntitySpecifics specifics_;
 };
 
 }  // namespace fake_server
diff --git a/sync/test/fake_server/permanent_entity.cc b/sync/test/fake_server/permanent_entity.cc
index da54439..00b1074 100644
--- a/sync/test/fake_server/permanent_entity.cc
+++ b/sync/test/fake_server/permanent_entity.cc
@@ -91,8 +91,9 @@ PermanentEntity::PermanentEntity(const string& id,
                                  const sync_pb::EntitySpecifics& specifics)
     : FakeServerEntity(id, model_type, 0, name),
       server_defined_unique_tag_(server_defined_unique_tag),
-      parent_id_(parent_id),
-      specifics_(specifics) { }
+      parent_id_(parent_id) {
+  SetSpecifics(specifics);
+}
 
 string PermanentEntity::GetParentId() const {
   return parent_id_;
@@ -101,9 +102,6 @@ string PermanentEntity::GetParentId() const {
 void PermanentEntity::SerializeAsProto(sync_pb::SyncEntity* proto) {
   FakeServerEntity::SerializeBaseProtoFields(proto);
 
-  sync_pb::EntitySpecifics* specifics = proto->mutable_specifics();
-  specifics->CopyFrom(specifics_);
-
   proto->set_parent_id_string(parent_id_);
   proto->set_server_defined_unique_tag(server_defined_unique_tag_);
 }
diff --git a/sync/test/fake_server/permanent_entity.h b/sync/test/fake_server/permanent_entity.h
index 253d70f..ee45cd7 100644
--- a/sync/test/fake_server/permanent_entity.h
+++ b/sync/test/fake_server/permanent_entity.h
@@ -53,7 +53,6 @@ class PermanentEntity : public FakeServerEntity {
   // All member values have equivalent fields in SyncEntity.
   std::string server_defined_unique_tag_;
   std::string parent_id_;
-  sync_pb::EntitySpecifics specifics_;
 };
 
 }  // namespace fake_server
diff --git a/sync/test/fake_server/tombstone_entity.cc b/sync/test/fake_server/tombstone_entity.cc
index 801690f..0a597197 100644
--- a/sync/test/fake_server/tombstone_entity.cc
+++ b/sync/test/fake_server/tombstone_entity.cc
@@ -26,7 +26,11 @@ FakeServerEntity* TombstoneEntity::Create(const string& id) {
 
 TombstoneEntity::TombstoneEntity(const string& id,
                                  const ModelType& model_type)
-    : FakeServerEntity(id, model_type, 0, string()) { }
+    : FakeServerEntity(id, model_type, 0, string()) {
+  sync_pb::EntitySpecifics specifics;
+  AddDefaultFieldValue(model_type, &specifics);
+  SetSpecifics(specifics);
+}
 
 string TombstoneEntity::GetParentId() const {
   return string();
@@ -34,9 +38,6 @@ string TombstoneEntity::GetParentId() const {
 
 void TombstoneEntity::SerializeAsProto(sync_pb::SyncEntity* proto) {
   FakeServerEntity::SerializeBaseProtoFields(proto);
-
-  sync_pb::EntitySpecifics* specifics = proto->mutable_specifics();
-  AddDefaultFieldValue(FakeServerEntity::GetModelType(), specifics);
 }
 
 bool TombstoneEntity::IsDeleted() const {
diff --git a/sync/test/fake_server/unique_client_entity.cc b/sync/test/fake_server/unique_client_entity.cc
index 5b80370..877893a 100644
--- a/sync/test/fake_server/unique_client_entity.cc
+++ b/sync/test/fake_server/unique_client_entity.cc
@@ -45,9 +45,10 @@ UniqueClientEntity::UniqueClientEntity(
     int64 last_modified_time)
     : FakeServerEntity(id, model_type, version, name),
       client_defined_unique_tag_(client_defined_unique_tag),
-      specifics_(specifics),
       creation_time_(creation_time),
-      last_modified_time_(last_modified_time) { }
+      last_modified_time_(last_modified_time) {
+  SetSpecifics(specifics);
+}
 
 UniqueClientEntity::~UniqueClientEntity() { }
 
@@ -98,15 +99,12 @@ std::string UniqueClientEntity::EffectiveIdForClientTaggedEntity(
 string UniqueClientEntity::GetParentId() const {
   // The parent ID for this type of entity should always be its ModelType's
   // root node.
-  return FakeServerEntity::GetTopLevelId(model_type_);
+  return FakeServerEntity::GetTopLevelId(GetModelType());
 }
 
 void UniqueClientEntity::SerializeAsProto(sync_pb::SyncEntity* proto) {
   FakeServerEntity::SerializeBaseProtoFields(proto);
 
-  sync_pb::EntitySpecifics* specifics = proto->mutable_specifics();
-  specifics->CopyFrom(specifics_);
-
   proto->set_parent_id_string(GetParentId());
   proto->set_client_defined_unique_tag(client_defined_unique_tag_);
   proto->set_ctime(creation_time_);
diff --git a/sync/test/fake_server/unique_client_entity.h b/sync/test/fake_server/unique_client_entity.h
index 7ad2e17..545d903 100644
--- a/sync/test/fake_server/unique_client_entity.h
+++ b/sync/test/fake_server/unique_client_entity.h
@@ -59,7 +59,6 @@ class UniqueClientEntity : public FakeServerEntity {
  private:
   // These member values have equivalent fields in SyncEntity.
   std::string client_defined_unique_tag_;
-  sync_pb::EntitySpecifics specifics_;
   int64 creation_time_;
   int64 last_modified_time_;
 };