Convert most two client sync_integration_tests to use FakeServer

This CL converts most two client sync_integration_tests to use the C++
FakeServer. The excluded tests cannot be converted yet because they
require notifications/triggering that is not implemented yet. No test
cases in two_client_apps_sync_test.cc have been converted yet due to
some observed flakiness in local runs.

In order to support the existing two client tests, many fixes to
FakeServer were made. The notable changes are:
1) Synchronization is added so that multiple clients can safely
   interact with the server.
2) A record of created PermanentEntity types is saved so that these
   entities are only created once (on first client connection).
3) The Nigori entity is now able to be mutated so additional clients
   can update its specifics.
4) UniqueClientEntity items can now be mutated. This is necessary for
   dictionary sync.
5) UniqueClientEntity items can now have parent IDs. This is necessary
   for device info sync and other types.
6) DeleteChildren has been fixed so that it no longer creates
   tombstones at the same time as searching for children. It now
   creates tombstones in a separate  pass.

BUG=323265

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

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

6 files changed, 139 insertions, 21 deletions

diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc
index d44814e..3827a85 100644
--- a/sync/test/fake_server/fake_server.cc
+++ b/sync/test/fake_server/fake_server.cc
@@ -16,6 +16,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/synchronization/lock.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_status_code.h"
 #include "sync/internal_api/public/base/model_type.h"
@@ -28,6 +29,8 @@
 using std::string;
 using std::vector;
 
+using base::AutoLock;
+using syncer::GetModelType;
 using syncer::ModelType;
 
 // The default birthday value.
@@ -94,13 +97,14 @@ class UpdateSieve {
   }
 
   // Returns the data type IDs of types being synced for the first time.
-  vector<ModelType> GetFirstTimeTypes() const {
+  vector<ModelType> GetFirstTimeTypes(
+      syncer::ModelTypeSet created_permanent_entity_types) const {
     vector<ModelType> types;
 
     ModelTypeToVersionMap::const_iterator it;
     for (it = request_from_version_.begin(); it != request_from_version_.end();
          ++it) {
-      if (it->second == 0)
+      if (it->second == 0 && !created_permanent_entity_types.Has(it->first))
         types.push_back(it->first);
     }
 
@@ -172,6 +176,11 @@ bool FakeServer::CreateDefaultPermanentItems(
     }
 
     ModelType model_type = *it;
+    if (created_permanent_entity_types_.Has(model_type)) {
+      // Do not create an entity for the type if it has already been created.
+      continue;
+    }
+
     FakeServerEntity* top_level_entity =
         PermanentEntity::CreateTopLevel(model_type);
     if (top_level_entity == NULL) {
@@ -200,6 +209,8 @@ bool FakeServer::CreateDefaultPermanentItems(
       }
       SaveEntity(other_bookmarks_entity);
     }
+
+    created_permanent_entity_types_.Put(model_type);
   }
 
   return true;
@@ -229,6 +240,8 @@ void FakeServer::SaveEntity(FakeServerEntity* entity) {
 int FakeServer::HandleCommand(const string& request,
                               int* response_code,
                               string* response) {
+  AutoLock lock(lock_);
+
   sync_pb::ClientToServerMessage message;
   bool parsed = message.ParseFromString(request);
   DCHECK(parsed);
@@ -269,7 +282,9 @@ bool FakeServer::HandleGetUpdatesRequest(
   response->set_changes_remaining(0);
 
   scoped_ptr<UpdateSieve> sieve = UpdateSieve::Create(get_updates);
-  if (!CreateDefaultPermanentItems(sieve->GetFirstTimeTypes())) {
+  vector<ModelType> first_time_types = sieve->GetFirstTimeTypes(
+      created_permanent_entity_types_);
+  if (!CreateDefaultPermanentItems(first_time_types)) {
     return false;
   }
   if (get_updates.create_mobile_bookmarks_folder() &&
@@ -325,8 +340,20 @@ bool FakeServer::CommitEntity(
     if (!DeleteChildren(client_entity.id_string())) {
       return false;
     }
+  } else if (GetModelType(client_entity) == syncer::NIGORI) {
+    // NIGORI is the only permanent item type that should be updated by the
+    // client.
+    entity = PermanentEntity::CreateUpdatedNigoriEntity(
+        client_entity,
+        entities_[client_entity.id_string()]);
   } else if (client_entity.has_client_defined_unique_tag()) {
-    entity = UniqueClientEntity::Create(client_entity);
+    if (entities_.find(client_entity.id_string()) != entities_.end()) {
+      entity = UniqueClientEntity::CreateUpdatedVersion(
+          client_entity,
+          entities_[client_entity.id_string()]);
+    } else {
+      entity = UniqueClientEntity::CreateNew(client_entity);
+    }
   } else {
     string parent_id = client_entity.parent_id_string();
     if (client_to_server_ids->find(parent_id) !=
@@ -388,16 +415,24 @@ bool FakeServer::IsChild(const string& id, const string& potential_parent_id) {
 }
 
 bool FakeServer::DeleteChildren(const string& id) {
+  vector<string> child_ids;
   for (EntityMap::iterator it = entities_.begin(); it != entities_.end();
        ++it) {
     if (IsChild(it->first, id)) {
-      FakeServerEntity* tombstone = TombstoneEntity::Create(it->first);
-      if (tombstone == NULL) {
-        return false;
-      }
-      SaveEntity(tombstone);
+      child_ids.push_back(it->first);
+    }
+  }
+
+  for (vector<string>::iterator it = child_ids.begin(); it != child_ids.end();
+       ++it) {
+    FakeServerEntity* tombstone = TombstoneEntity::Create(*it);
+    if (tombstone == NULL) {
+      LOG(WARNING) << "Tombstone creation failed for entity with ID " << *it;
+      return false;
     }
+    SaveEntity(tombstone);
   }
+
   return true;
 }
 
diff --git a/sync/test/fake_server/fake_server.h b/sync/test/fake_server/fake_server.h
index 511bcae..a13cc98 100644
--- a/sync/test/fake_server/fake_server.h
+++ b/sync/test/fake_server/fake_server.h
@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/test/fake_server/fake_server_entity.h"
@@ -74,6 +75,14 @@ class FakeServer {
   // |id|. A tombstone is not created for the entity itself.
   bool DeleteChildren(const std::string& id);
 
+  // The lock used to ensure that only one client is communicating with server
+  // at any given time.
+  //
+  // It is probably preferable to have FakeServer operate on its own thread and
+  // communicate with it via PostTask, but clients would still need to wait for
+  // requests to finish before proceeding.
+  base::Lock lock_;
+
   // This is the last version number assigned to an entity. The next entity will
   // have a version number of version_ + 1.
   int64 version_;
@@ -87,6 +96,11 @@ class FakeServer {
 
   // All Keystore keys known to the server.
   std::vector<std::string> keystore_keys_;
+
+  // All ModelTypes for which permanent entities have been created. These types
+  // are kept track of so that permanent entities are not recreated for new
+  // clients.
+  syncer::ModelTypeSet created_permanent_entity_types_;
 };
 
 }  // namespace fake_server
diff --git a/sync/test/fake_server/permanent_entity.cc b/sync/test/fake_server/permanent_entity.cc
index 2aa371a..480e380 100644
--- a/sync/test/fake_server/permanent_entity.cc
+++ b/sync/test/fake_server/permanent_entity.cc
@@ -39,7 +39,14 @@ FakeServerEntity* PermanentEntity::Create(const ModelType& model_type,
 
   string id = FakeServerEntity::CreateId(model_type, server_tag);
   string parent_id = FakeServerEntity::CreateId(model_type, parent_server_tag);
-  return new PermanentEntity(id, model_type, name, parent_id, server_tag);
+  sync_pb::EntitySpecifics entity_specifics;
+  AddDefaultFieldValue(model_type, &entity_specifics);
+  return new PermanentEntity(id,
+                             model_type,
+                             name,
+                             parent_id,
+                             server_tag,
+                             entity_specifics);
 }
 
 // static
@@ -48,17 +55,43 @@ FakeServerEntity* PermanentEntity::CreateTopLevel(
   string server_tag = syncer::ModelTypeToRootTag(model_type);
   string name = syncer::ModelTypeToString(model_type);
   string id = FakeServerEntity::CreateId(model_type, server_tag);
-  return new PermanentEntity(id, model_type, name, kRootParentTag, server_tag);
+  sync_pb::EntitySpecifics entity_specifics;
+  AddDefaultFieldValue(model_type, &entity_specifics);
+  return new PermanentEntity(id,
+                             model_type,
+                             name,
+                             kRootParentTag,
+                             server_tag,
+                             entity_specifics);
+}
+
+// static
+FakeServerEntity* PermanentEntity::CreateUpdatedNigoriEntity(
+    const sync_pb::SyncEntity& client_entity,
+    FakeServerEntity* current_server_entity) {
+  ModelType model_type = current_server_entity->GetModelType();
+  if (model_type != syncer::NIGORI) {
+    return NULL;
+  }
+
+  return new PermanentEntity(current_server_entity->GetId(),
+                             model_type,
+                             current_server_entity->GetName(),
+                             current_server_entity->GetParentId(),
+                             syncer::ModelTypeToRootTag(model_type),
+                             client_entity.specifics());
 }
 
 PermanentEntity::PermanentEntity(const string& id,
                                  const ModelType& model_type,
                                  const string& name,
                                  const string& parent_id,
-                                 const string& server_defined_unique_tag)
+                                 const string& server_defined_unique_tag,
+                                 const sync_pb::EntitySpecifics& specifics)
     : FakeServerEntity(id, model_type, 0, name),
       server_defined_unique_tag_(server_defined_unique_tag),
-      parent_id_(parent_id) { }
+      parent_id_(parent_id),
+      specifics_(specifics) { }
 
 string PermanentEntity::GetParentId() const {
   return parent_id_;
@@ -69,7 +102,7 @@ sync_pb::SyncEntity* PermanentEntity::SerializeAsProto() {
   FakeServerEntity::SerializeBaseProtoFields(sync_entity);
 
   sync_pb::EntitySpecifics* specifics = sync_entity->mutable_specifics();
-  AddDefaultFieldValue(FakeServerEntity::GetModelType(), specifics);
+  specifics->CopyFrom(specifics_);
 
   sync_entity->set_parent_id_string(parent_id_);
   sync_entity->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 3579720..ea84cfc 100644
--- a/sync/test/fake_server/permanent_entity.h
+++ b/sync/test/fake_server/permanent_entity.h
@@ -26,10 +26,17 @@ class PermanentEntity : public FakeServerEntity {
                                   const std::string& name,
                                   const std::string& parent_server_tag);
 
-  // Factory function for a top level BookmarkEntity. Top level means that the
+  // Factory function for a top level PermanentEntity. Top level means that the
   // entity's parent is the root entity (no PermanentEntity exists for root).
   static FakeServerEntity* CreateTopLevel(const syncer::ModelType& model_type);
 
+  // Factory function for creating an updated version of a PermanentEntity.
+  // This function should only be called for the Nigori entity and will return
+  // NULL if any other type is specified.
+  static FakeServerEntity* CreateUpdatedNigoriEntity(
+      const sync_pb::SyncEntity& client_entity,
+      FakeServerEntity* current_server_entity);
+
   // FakeServerEntity implementation.
   virtual std::string GetParentId() const OVERRIDE;
   virtual sync_pb::SyncEntity* SerializeAsProto() OVERRIDE;
@@ -41,11 +48,13 @@ class PermanentEntity : public FakeServerEntity {
                   const syncer::ModelType& model_type,
                   const std::string& name,
                   const std::string& parent_id,
-                  const std::string& server_defined_unique_tag);
+                  const std::string& server_defined_unique_tag,
+                  const sync_pb::EntitySpecifics& entity_specifics);
 
   // 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/unique_client_entity.cc b/sync/test/fake_server/unique_client_entity.cc
index 06fc082..693b24a 100644
--- a/sync/test/fake_server/unique_client_entity.cc
+++ b/sync/test/fake_server/unique_client_entity.cc
@@ -21,7 +21,7 @@ namespace fake_server {
 UniqueClientEntity::~UniqueClientEntity() { }
 
 // static
-FakeServerEntity* UniqueClientEntity::Create(
+FakeServerEntity* UniqueClientEntity::CreateNew(
     const sync_pb::SyncEntity& client_entity) {
   DCHECK(client_entity.has_client_defined_unique_tag());
   DCHECK(!client_entity.folder());
@@ -35,6 +35,22 @@ FakeServerEntity* UniqueClientEntity::Create(
                                 model_type,
                                 client_entity.version(),
                                 client_entity.name(),
+                                client_entity.parent_id_string(),
+                                client_entity.client_defined_unique_tag(),
+                                client_entity.specifics(),
+                                client_entity.ctime(),
+                                client_entity.mtime());
+}
+
+// static
+FakeServerEntity* UniqueClientEntity::CreateUpdatedVersion(
+    const sync_pb::SyncEntity& client_entity,
+    FakeServerEntity* current_server_entity) {
+  return new UniqueClientEntity(client_entity.id_string(),
+                                current_server_entity->GetModelType(),
+                                client_entity.version(),
+                                client_entity.name(),
+                                client_entity.parent_id_string(),
                                 client_entity.client_defined_unique_tag(),
                                 client_entity.specifics(),
                                 client_entity.ctime(),
@@ -46,20 +62,22 @@ UniqueClientEntity::UniqueClientEntity(
     const ModelType& model_type,
     int64 version,
     const string& name,
+    const string& parent_id,
     const string& client_defined_unique_tag,
     const sync_pb::EntitySpecifics& specifics,
     int64 creation_time,
     int64 last_modified_time)
     : FakeServerEntity(id, model_type, version, name),
+      parent_id_(parent_id),
       client_defined_unique_tag_(client_defined_unique_tag),
       specifics_(specifics),
       creation_time_(creation_time),
       last_modified_time_(last_modified_time) { }
 
 string UniqueClientEntity::GetParentId() const {
-  // Return a dummy value that cannot be a real parent ID. This type should
-  // never have parents.
-  return string();
+  // The parent ID for this type of entity should always be its ModelType's
+  // root node.
+  return parent_id_;
 }
 
 sync_pb::SyncEntity* UniqueClientEntity::SerializeAsProto() {
@@ -69,6 +87,7 @@ sync_pb::SyncEntity* UniqueClientEntity::SerializeAsProto() {
   sync_pb::EntitySpecifics* specifics = sync_entity->mutable_specifics();
   specifics->CopyFrom(specifics_);
 
+  sync_entity->set_parent_id_string(parent_id_);
   sync_entity->set_client_defined_unique_tag(client_defined_unique_tag_);
   sync_entity->set_ctime(creation_time_);
   sync_entity->set_mtime(last_modified_time_);
diff --git a/sync/test/fake_server/unique_client_entity.h b/sync/test/fake_server/unique_client_entity.h
index 6069222..6e876e3 100644
--- a/sync/test/fake_server/unique_client_entity.h
+++ b/sync/test/fake_server/unique_client_entity.h
@@ -20,7 +20,13 @@ class UniqueClientEntity : public FakeServerEntity {
   virtual ~UniqueClientEntity();
 
   // Factory function for UniqueClientEntity.
-  static FakeServerEntity* Create(const sync_pb::SyncEntity& client_entity);
+  static FakeServerEntity* CreateNew(const sync_pb::SyncEntity& client_entity);
+
+  // Factory function for creating a new version of an existing
+  // UniqueClientEntity.
+  static FakeServerEntity* CreateUpdatedVersion(
+      const sync_pb::SyncEntity& client_entity,
+      FakeServerEntity* current_server_entity);
 
   // FakeServerEntity implementation.
   virtual std::string GetParentId() const OVERRIDE;
@@ -33,12 +39,14 @@ class UniqueClientEntity : public FakeServerEntity {
                      const syncer::ModelType& model_type,
                      int64 version,
                      const std::string& name,
+                     const std::string& parent_id,
                      const std::string& client_defined_unique_tag,
                      const sync_pb::EntitySpecifics& specifics,
                      int64 creation_time,
                      int64 last_modified_time);
 
   // All member values have equivalent fields in SyncEntity.
+  std::string parent_id_;
   std::string client_defined_unique_tag_;
   sync_pb::EntitySpecifics specifics_;
   int64 creation_time_;