Revert 267422 because it broke compile on linux-clang buildbots.

The build failure:

sync/test/fake_server/fake_sync_server_http_handler.cc:51:61: error: too many arguments to function call, expected 2, have 3
                                                            &response);
                                                            ^~~~~~~~~
sync/test/fake_server/fake_server.h:44:3: note: 'HandleCommand' declared here
  void HandleCommand(const std::string& request,
  ^

> Use FakeServer-based invalidations for Sync tests
> 
> This CL creates a new InvalidationService implementation,
> FakeServerInvalidationService, to remove FakeServer-based tests'
> dependency on the Python XMPP server.
> 
> Another major change is that FakeServer's HandleCommand is now executed
> on the UI thread and locking has been removed from FakeServer itself.
> 
> BUG=323265
> 
> Review URL: https://codereview.chromium.org/234113002

TBR=pvalenzuela@chromium.org

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

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

11 files changed, 78 insertions, 361 deletions

diff --git a/chrome/browser/sync/test/integration/fake_server_invalidation_service.cc b/chrome/browser/sync/test/integration/fake_server_invalidation_service.cc
deleted file mode 100644
index c862519..0000000
--- a/chrome/browser/sync/test/integration/fake_server_invalidation_service.cc
+++ /dev/null
@@ -1,87 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/sync/test/integration/fake_server_invalidation_service.h"
-
-#include <string>
-
-#include "base/macros.h"
-#include "chrome/browser/invalidation/invalidation_service_util.h"
-#include "sync/internal_api/public/base/invalidation.h"
-#include "sync/internal_api/public/base/model_type.h"
-#include "sync/notifier/object_id_invalidation_map.h"
-#
-
-namespace fake_server {
-
-FakeServerInvalidationService::FakeServerInvalidationService()
-    : client_id_(invalidation::GenerateInvalidatorClientId()),
-      identity_provider_(&token_service_) {
-  invalidator_registrar_.UpdateInvalidatorState(syncer::INVALIDATIONS_ENABLED);
-}
-
-FakeServerInvalidationService::~FakeServerInvalidationService() {
-}
-
-// static
-KeyedService* FakeServerInvalidationService::Build(
-    content::BrowserContext* context) {
-  return new FakeServerInvalidationService();
-}
-
-void FakeServerInvalidationService::RegisterInvalidationHandler(
-      syncer::InvalidationHandler* handler) {
-  invalidator_registrar_.RegisterHandler(handler);
-}
-
-void FakeServerInvalidationService::UpdateRegisteredInvalidationIds(
-      syncer::InvalidationHandler* handler,
-      const syncer::ObjectIdSet& ids) {
-  invalidator_registrar_.UpdateRegisteredIds(handler, ids);
-}
-
-void FakeServerInvalidationService::UnregisterInvalidationHandler(
-      syncer::InvalidationHandler* handler) {
-  invalidator_registrar_.UnregisterHandler(handler);
-}
-
-syncer::InvalidatorState FakeServerInvalidationService::GetInvalidatorState()
-    const {
-  return invalidator_registrar_.GetInvalidatorState();
-}
-
-std::string FakeServerInvalidationService::GetInvalidatorClientId() const {
-  return client_id_;
-}
-
-invalidation::InvalidationLogger*
-FakeServerInvalidationService::GetInvalidationLogger() {
-  return NULL;
-}
-
-void FakeServerInvalidationService::RequestDetailedStatus(
-    base::Callback<void(const base::DictionaryValue&)> caller) const {
-  base::DictionaryValue value;
-  caller.Run(value);
-}
-
-IdentityProvider* FakeServerInvalidationService::GetIdentityProvider() {
-  return &identity_provider_;
-}
-
-void FakeServerInvalidationService::OnCommit(
-    syncer::ModelTypeSet committed_model_types) {
-  syncer::ObjectIdSet object_ids = syncer::ModelTypeSetToObjectIdSet(
-      committed_model_types);
-  syncer::ObjectIdInvalidationMap invalidation_map;
-  for (syncer::ObjectIdSet::const_iterator it = object_ids.begin();
-       it != object_ids.end(); ++it) {
-    // TODO(pvalenzuela): Create more refined invalidations instead of
-    // invalidating all items of a given type.
-    invalidation_map.Insert(syncer::Invalidation::InitUnknownVersion(*it));
-  }
-  invalidator_registrar_.DispatchInvalidationsToHandlers(invalidation_map);
-}
-
-}  // namespace fake_server
diff --git a/chrome/browser/sync/test/integration/fake_server_invalidation_service.h b/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
deleted file mode 100644
index 6b57503c..0000000
--- a/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
+++ /dev/null
@@ -1,67 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_FAKE_SERVER_INVALIDATION_SERVICE_H_
-#define CHROME_BROWSER_SYNC_TEST_INTEGRATION_FAKE_SERVER_INVALIDATION_SERVICE_H_
-
-#include <string>
-#include <utility>
-
-#include "base/basictypes.h"
-#include "chrome/browser/invalidation/invalidation_service.h"
-#include "chrome/browser/signin/fake_profile_oauth2_token_service.h"
-#include "google_apis/gaia/fake_identity_provider.h"
-#include "sync/internal_api/public/base/model_type.h"
-#include "sync/notifier/invalidator_registrar.h"
-#include "sync/test/fake_server/fake_server.h"
-
-namespace content {
-class BrowserContext;
-}
-
-namespace invalidation {
-class InvalidationLogger;
-}
-
-namespace fake_server {
-
-// An InvalidationService that is used in conjunction with FakeServer.
-class FakeServerInvalidationService : public invalidation::InvalidationService,
-                                      public FakeServer::Observer {
- public:
-  FakeServerInvalidationService();
-  virtual ~FakeServerInvalidationService();
-
-  static KeyedService* Build(content::BrowserContext* context);
-
-  virtual void RegisterInvalidationHandler(
-      syncer::InvalidationHandler* handler) OVERRIDE;
-  virtual void UpdateRegisteredInvalidationIds(
-      syncer::InvalidationHandler* handler,
-      const syncer::ObjectIdSet& ids) OVERRIDE;
-  virtual void UnregisterInvalidationHandler(
-      syncer::InvalidationHandler* handler) OVERRIDE;
-
-  virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE;
-  virtual std::string GetInvalidatorClientId() const OVERRIDE;
-  virtual invalidation::InvalidationLogger* GetInvalidationLogger() OVERRIDE;
-  virtual void RequestDetailedStatus(
-      base::Callback<void(const base::DictionaryValue&)> caller) const OVERRIDE;
-  virtual IdentityProvider* GetIdentityProvider() OVERRIDE;
-
-  // FakeServer::Observer:
-  virtual void OnCommit(syncer::ModelTypeSet committed_model_types) OVERRIDE;
-
- private:
-  std::string client_id_;
-  syncer::InvalidatorRegistrar invalidator_registrar_;
-  FakeProfileOAuth2TokenService token_service_;
-  FakeIdentityProvider identity_provider_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeServerInvalidationService);
-};
-
-}  // namespace fake_server
-
-#endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_FAKE_SERVER_INVALIDATION_SERVICE_H_
diff --git a/chrome/browser/sync/test/integration/sync_test.cc b/chrome/browser/sync/test/integration/sync_test.cc
index 7da507d..3c8b63a 100644
--- a/chrome/browser/sync/test/integration/sync_test.cc
+++ b/chrome/browser/sync/test/integration/sync_test.cc
@@ -36,7 +36,6 @@
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
-#include "chrome/browser/sync/test/integration/fake_server_invalidation_service.h"
 #include "chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
@@ -235,8 +234,6 @@ void SyncTest::TearDown() {
 
   // Stop the local sync test server. This is a no-op if one wasn't started.
   TearDownLocalTestServer();
-
-  fake_server_.reset();
 }
 
 void SyncTest::SetUpCommandLine(base::CommandLine* cl) {
@@ -329,7 +326,6 @@ bool SyncTest::SetupClients() {
   browsers_.resize(num_clients_);
   clients_.resize(num_clients_);
   invalidation_forwarders_.resize(num_clients_);
-  fake_server_invalidation_services_.resize(num_clients_);
   for (int i = 0; i < num_clients_; ++i) {
     InitializeInstance(i);
   }
@@ -356,6 +352,14 @@ void SyncTest::InitializeInstance(int index) {
   EXPECT_FALSE(GetBrowser(index) == NULL) << "Could not create Browser "
                                           << index << ".";
 
+  invalidation::P2PInvalidationService* p2p_invalidation_service =
+      static_cast<invalidation::P2PInvalidationService*>(
+          InvalidationServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+              GetProfile(index),
+              TestUsesSelfNotifications() ?
+                  BuildSelfNotifyingP2PInvalidationService
+                  : BuildRealisticP2PInvalidationService));
+  p2p_invalidation_service->UpdateCredentials(username_, password_);
 
   // Make sure the ProfileSyncService has been created before creating the
   // ProfileSyncServiceHarness - some tests expect the ProfileSyncService to
@@ -377,7 +381,11 @@ void SyncTest::InitializeInstance(int index) {
           password_);
   EXPECT_FALSE(GetClient(index) == NULL) << "Could not create Client "
                                          << index << ".";
-  InitializeInvalidations(index);
+
+  // Start listening for and emitting notificaitons of commits.
+  invalidation_forwarders_[index] =
+      new P2PInvalidationForwarder(clients_[index]->service(),
+                                   p2p_invalidation_service);
 
   test::WaitForBookmarkModelToLoad(
       BookmarkModelFactory::GetForProfile(GetProfile(index)));
@@ -387,32 +395,6 @@ void SyncTest::InitializeInstance(int index) {
       TemplateURLServiceFactory::GetForProfile(GetProfile(index)));
 }
 
-void SyncTest::InitializeInvalidations(int index) {
-  if (server_type_ == IN_PROCESS_FAKE_SERVER) {
-    CHECK(fake_server_.get());
-    fake_server::FakeServerInvalidationService* invalidation_service =
-        static_cast<fake_server::FakeServerInvalidationService*>(
-            InvalidationServiceFactory::GetInstance()->SetTestingFactoryAndUse(
-                GetProfile(index),
-                fake_server::FakeServerInvalidationService::Build));
-    fake_server_->AddObserver(invalidation_service);
-    fake_server_invalidation_services_[index] = invalidation_service;
-  } else {
-    invalidation::P2PInvalidationService* p2p_invalidation_service =
-        static_cast<invalidation::P2PInvalidationService*>(
-            InvalidationServiceFactory::GetInstance()->SetTestingFactoryAndUse(
-                GetProfile(index),
-                TestUsesSelfNotifications() ?
-                    BuildSelfNotifyingP2PInvalidationService
-                    : BuildRealisticP2PInvalidationService));
-    p2p_invalidation_service->UpdateCredentials(username_, password_);
-    // Start listening for and emitting notifications of commits.
-    invalidation_forwarders_[index] =
-        new P2PInvalidationForwarder(clients_[index]->service(),
-                                     p2p_invalidation_service);
-  }
-}
-
 bool SyncTest::SetupSync() {
   // Create sync profiles and clients if they haven't already been created.
   if (profiles_.empty()) {
@@ -452,19 +434,10 @@ void SyncTest::CleanUpOnMainThread() {
   chrome::CloseAllBrowsers();
   content::RunAllPendingInMessageLoop();
 
-  if (fake_server_.get()) {
-    std::vector<fake_server::FakeServerInvalidationService*>::const_iterator it;
-    for (it = fake_server_invalidation_services_.begin();
-         it != fake_server_invalidation_services_.end(); ++it) {
-      fake_server_->RemoveObserver(*it);
-    }
-  }
-
   // All browsers should be closed at this point, or else we could see memory
   // corruption in QuitBrowser().
   CHECK_EQ(0U, chrome::GetTotalBrowserCount());
   invalidation_forwarders_.clear();
-  fake_server_invalidation_services_.clear();
   clients_.clear();
 }
 
@@ -632,6 +605,8 @@ void SyncTest::SetUpTestServerIfRequired() {
       LOG(FATAL) << "Failed to set up local test server";
   } else if (server_type_ == IN_PROCESS_FAKE_SERVER) {
     fake_server_.reset(new fake_server::FakeServer());
+    // Similar to LOCAL_LIVE_SERVER, we must start this for XMPP.
+    SetUpLocalPythonTestServer();
     SetupMockGaiaResponses();
   } else if (server_type_ == EXTERNAL_LIVE_SERVER) {
     // Nothing to do; we'll just talk to the URL we were given.
@@ -750,10 +725,6 @@ bool SyncTest::IsTestServerRunning() {
 }
 
 void SyncTest::EnableNetwork(Profile* profile) {
-  // TODO(pvalenzuela): Remove this restriction when FakeServer's observers
-  // (namely FakeServerInvaldationService) are aware of a network disconnect.
-  ASSERT_NE(IN_PROCESS_FAKE_SERVER, server_type_)
-      << "FakeServer does not support EnableNetwork.";
   SetProxyConfig(profile->GetRequestContext(),
                  net::ProxyConfig::CreateDirect());
   if (notifications_enabled_) {
@@ -764,10 +735,6 @@ void SyncTest::EnableNetwork(Profile* profile) {
 }
 
 void SyncTest::DisableNetwork(Profile* profile) {
-  // TODO(pvalenzuela): Remove this restriction when FakeServer's observers
-  // (namely FakeServerInvaldationService) are aware of a network disconnect.
-  ASSERT_NE(IN_PROCESS_FAKE_SERVER, server_type_)
-      << "FakeServer does not support DisableNetwork.";
   DisableNotificationsImpl();
   // Set the current proxy configuration to a nonexistent proxy to effectively
   // disable networking.
diff --git a/chrome/browser/sync/test/integration/sync_test.h b/chrome/browser/sync/test/integration/sync_test.h
index 6c32d83..b9e734d 100644
--- a/chrome/browser/sync/test/integration/sync_test.h
+++ b/chrome/browser/sync/test/integration/sync_test.h
@@ -32,7 +32,6 @@ class CommandLine;
 
 namespace fake_server {
 class FakeServer;
-class FakeServerInvalidationService;
 }
 
 namespace net {
@@ -364,10 +363,6 @@ class SyncTest : public InProcessBrowserTest {
   // of test being run and command line args passed in.
   void DecideServerType();
 
-  // Sets up the client-side invalidations infrastructure depending on the
-  // value of |server_type_|.
-  void InitializeInvalidations(int index);
-
   // Python sync test server, started on demand.
   syncer::LocalSyncTestServer sync_server_;
 
@@ -404,10 +399,6 @@ class SyncTest : public InProcessBrowserTest {
   // of this activity to its peer sync clients.
   ScopedVector<P2PInvalidationForwarder> invalidation_forwarders_;
 
-  // Collection of pointers to FakeServerInvalidation objects for each profile.
-  std::vector<fake_server::FakeServerInvalidationService*>
-      fake_server_invalidation_services_;
-
   // Sync profile against which changes to individual profiles are verified. We
   // don't need a corresponding verifier sync client because the contents of the
   // verifier profile are strictly local, and are not meant to be synced.
diff --git a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
index 2b3e38a..ba688ce 100644
--- a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
@@ -1616,8 +1616,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, MC_DuplicateFolders) {
   ASSERT_FALSE(ContainsDuplicateBookmarks(0));
 }
 
-// This test fails when run with FakeServer and FakeServerInvalidationService.
-IN_PROC_BROWSER_TEST_F(LegacyTwoClientBookmarksSyncTest, MC_DeleteBookmark) {
+IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, MC_DeleteBookmark) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   ASSERT_TRUE(GetClient(1)->DisableSyncForDatatype(syncer::BOOKMARKS));
 
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index c1eee64..3afc5f9 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -2192,8 +2192,6 @@
         'browser/sync/test/integration/extension_settings_helper.h',
         'browser/sync/test/integration/extensions_helper.cc',
         'browser/sync/test/integration/extensions_helper.h',
-        'browser/sync/test/integration/fake_server_invalidation_service.cc',
-        'browser/sync/test/integration/fake_server_invalidation_service.h',
         'browser/sync/test/integration/multi_client_status_change_checker.cc',
         'browser/sync/test/integration/multi_client_status_change_checker.h',
         'browser/sync/test/integration/passwords_helper.cc',
diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc
index 798797e..d758e5d 100644
--- a/sync/test/fake_server/fake_server.cc
+++ b/sync/test/fake_server/fake_server.cc
@@ -4,7 +4,6 @@
 
 #include "sync/test/fake_server/fake_server.h"
 
-#include <algorithm>
 #include <limits>
 #include <string>
 #include <vector>
@@ -30,6 +29,7 @@
 using std::string;
 using std::vector;
 
+using base::AutoLock;
 using syncer::GetModelType;
 using syncer::ModelType;
 using syncer::ModelTypeSet;
@@ -238,8 +238,11 @@ void FakeServer::SaveEntity(FakeServerEntity* entity) {
   entities_[entity->GetId()] = entity;
 }
 
-void FakeServer::HandleCommand(const string& request,
-                               const HandleCommandCallback& callback) {
+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);
@@ -256,20 +259,20 @@ void FakeServer::HandleCommand(const string& request,
                                     response_proto.mutable_commit());
       break;
     default:
-      callback.Run(net::ERR_NOT_IMPLEMENTED, 0, string());;
-      return;
+      return net::ERR_NOT_IMPLEMENTED;
   }
 
   if (!success) {
     // TODO(pvalenzuela): Add logging here so that tests have more info about
     // the failure.
-    callback.Run(net::ERR_FAILED, 0, string());
-    return;
+    return net::HTTP_BAD_REQUEST;
   }
 
   response_proto.set_error_code(sync_pb::SyncEnums::SUCCESS);
   response_proto.set_store_birthday(birthday_);
-  callback.Run(0, net::HTTP_OK, response_proto.SerializeAsString());
+  *response_code = net::HTTP_OK;
+  *response = response_proto.SerializeAsString();
+  return 0;
 }
 
 bool FakeServer::HandleGetUpdatesRequest(
@@ -321,13 +324,13 @@ bool FakeServer::HandleGetUpdatesRequest(
   return true;
 }
 
-string FakeServer::CommitEntity(
+bool FakeServer::CommitEntity(
     const sync_pb::SyncEntity& client_entity,
     sync_pb::CommitResponse_EntryResponse* entry_response,
     string client_guid,
-    string parent_id) {
+    std::map<string, string>* client_to_server_ids) {
   if (client_entity.version() == 0 && client_entity.deleted()) {
-    return string();
+    return false;
   }
 
   FakeServerEntity* entity;
@@ -336,7 +339,7 @@ string FakeServer::CommitEntity(
     // TODO(pvalenzuela): Change the behavior of DeleteChilden so that it does
     // not modify server data if it fails.
     if (!DeleteChildren(client_entity.id_string())) {
-      return string();
+      return false;
     }
   } else if (GetModelType(client_entity) == syncer::NIGORI) {
     // NIGORI is the only permanent item type that should be updated by the
@@ -353,6 +356,11 @@ string FakeServer::CommitEntity(
       entity = UniqueClientEntity::CreateNew(client_entity);
     }
   } else {
+    string parent_id = client_entity.parent_id_string();
+    if (client_to_server_ids->find(parent_id) !=
+        client_to_server_ids->end()) {
+      parent_id = (*client_to_server_ids)[parent_id];
+    }
     // TODO(pvalenzuela): Validate entity's parent ID.
     if (entities_.find(client_entity.id_string()) != entities_.end()) {
       entity = BookmarkEntity::CreateUpdatedVersion(
@@ -367,12 +375,17 @@ string FakeServer::CommitEntity(
   if (entity == NULL) {
     // TODO(pvalenzuela): Add logging so that it is easier to determine why
     // creation failed.
-    return string();
+    return false;
+  }
+
+  // Record the ID if it was renamed.
+  if (client_entity.id_string() != entity->GetId()) {
+    (*client_to_server_ids)[client_entity.id_string()] = entity->GetId();
   }
 
   SaveEntity(entity);
   BuildEntryResponseForSuccessfulCommit(entry_response, entity);
-  return entity->GetId();
+  return true;
 }
 
 void FakeServer::BuildEntryResponseForSuccessfulCommit(
@@ -429,7 +442,6 @@ bool FakeServer::HandleCommitRequest(
     sync_pb::CommitResponse* response) {
   std::map<string, string> client_to_server_ids;
   string guid = commit.cache_guid();
-  ModelTypeSet committed_model_types;
 
   // TODO(pvalenzuela): Add validation of CommitMessage.entries.
   ::google::protobuf::RepeatedPtrField<sync_pb::SyncEntity>::const_iterator it;
@@ -437,30 +449,11 @@ bool FakeServer::HandleCommitRequest(
     sync_pb::CommitResponse_EntryResponse* entry_response =
         response->add_entryresponse();
 
-    sync_pb::SyncEntity client_entity = *it;
-    string parent_id = client_entity.parent_id_string();
-    if (client_to_server_ids.find(parent_id) !=
-        client_to_server_ids.end()) {
-      parent_id = client_to_server_ids[parent_id];
-    }
-
-    string entity_id = CommitEntity(client_entity,
-                                    entry_response,
-                                    guid,
-                                    parent_id);
-    if (entity_id.empty()) {
+    if (!CommitEntity(*it, entry_response, guid, &client_to_server_ids)) {
       return false;
     }
-
-    // Record the ID if it was renamed.
-    if (entity_id != client_entity.id_string()) {
-      client_to_server_ids[client_entity.id_string()] = entity_id;
-    }
-    FakeServerEntity* entity = entities_[entity_id];
-    committed_model_types.Put(entity->GetModelType());
   }
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnCommit(committed_model_types));
   return true;
 }
 
@@ -496,12 +489,4 @@ scoped_ptr<base::DictionaryValue> FakeServer::GetEntitiesAsDictionaryValue() {
   return dictionary.Pass();
 }
 
-void FakeServer::AddObserver(Observer* observer) {
-  observers_.AddObserver(observer);
-}
-
-void FakeServer::RemoveObserver(Observer* observer) {
-  observers_.RemoveObserver(observer);
-}
-
 }  // namespace fake_server
diff --git a/sync/test/fake_server/fake_server.h b/sync/test/fake_server/fake_server.h
index 6af77ab..0621bbc 100644
--- a/sync/test/fake_server/fake_server.h
+++ b/sync/test/fake_server/fake_server.h
@@ -7,12 +7,10 @@
 
 #include <map>
 #include <string>
-#include <vector>
 
 #include "base/basictypes.h"
-#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/observer_list.h"
+#include "base/synchronization/lock.h"
 #include "base/values.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/protocol/sync.pb.h"
@@ -23,26 +21,15 @@ namespace fake_server {
 // A fake version of the Sync server used for testing.
 class FakeServer {
  public:
-  typedef base::Callback<void(int, int, const std::string&)>
-      HandleCommandCallback;
-
-  class Observer {
-   public:
-    virtual ~Observer() {}
-
-    // Called after FakeServer has processed a successful commit. The types
-    // updated as part of the commit are passed in |committed_model_types|.
-    virtual void OnCommit(syncer::ModelTypeSet committed_model_types) = 0;
-  };
-
   FakeServer();
   virtual ~FakeServer();
 
-  // Asynchronously handles a /command POST to the server. If the error_code is
-  // passed to |callback| is 0 (success), the POST's response code and content
-  // will also be passed.
-  void HandleCommand(const std::string& request,
-                     const HandleCommandCallback& callback);
+  // Handles a /command POST to the server. If the return value is 0 (success),
+  // |response_code| and |response| will be set. Otherwise, the return value
+  // will be a network error code.
+  int HandleCommand(const std::string& request,
+                    int* response_code,
+                    std::string* response);
 
   // Creates a DicionaryValue representation of all entities present in the
   // server. The dictionary keys are the strings generated by ModelTypeToString
@@ -50,14 +37,6 @@ class FakeServer {
   // names.
   scoped_ptr<base::DictionaryValue> GetEntitiesAsDictionaryValue();
 
-  // Adds |observer| to FakeServer's observer list. This should be called
-  // before the Profile associated with |observer| is connected to the server.
-  void AddObserver(Observer* observer);
-
-  // Removes |observer| from the FakeServer's observer list. This method
-  // must be called if AddObserver was ever called with |observer|.
-  void RemoveObserver(Observer* observer);
-
  private:
   typedef std::map<std::string, FakeServerEntity*> EntityMap;
 
@@ -80,15 +59,14 @@ class FakeServer {
   void SaveEntity(FakeServerEntity* entity);
 
   // Commits a client-side SyncEntity to the server as a FakeServerEntity.
-  // The client that sent the commit is identified via |client_guid|. The
-  // parent ID string present in |client_entity| should be ignored in favor
-  // of |parent_id|. If the commit is successful, the entity's server ID string
-  // is returned and a new FakeServerEntity is saved in |entities_|.
-  std::string CommitEntity(
-      const sync_pb::SyncEntity& client_entity,
-      sync_pb::CommitResponse_EntryResponse* entry_response,
-      std::string client_guid,
-      std::string parent_id);
+  // The client that sent the commit is identified via |client_guid| and all
+  // entity ID renaming is tracked with |client_to_server_ids|. If the commit
+  // is successful, true is returned and the server's version of the SyncEntity
+  // is stored in |server_entity|.
+  bool CommitEntity(const sync_pb::SyncEntity& client_entity,
+                    sync_pb::CommitResponse_EntryResponse* entry_response,
+                    std::string client_guid,
+                    std::map<std::string, std::string>* client_to_server_ids);
 
   // Populates |entry_response| based on |entity|. It is assumed that
   // SaveEntity has already been called on |entity|.
@@ -104,6 +82,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_;
@@ -122,9 +108,6 @@ class FakeServer {
   // are kept track of so that permanent entities are not recreated for new
   // clients.
   syncer::ModelTypeSet created_permanent_entity_types_;
-
-  // FakeServer's observers.
-  ObserverList<Observer, true> observers_;
 };
 
 }  // namespace fake_server
diff --git a/sync/test/fake_server/fake_server_http_post_provider.cc b/sync/test/fake_server/fake_server_http_post_provider.cc
index 5dd45c2..e0b3436 100644
--- a/sync/test/fake_server/fake_server_http_post_provider.cc
+++ b/sync/test/fake_server/fake_server_http_post_provider.cc
@@ -6,11 +6,6 @@
 
 #include <string>
 
-#include "base/bind.h"
-#include "base/location.h"
-#include "base/memory/ref_counted.h"
-#include "base/sequenced_task_runner.h"
-#include "base/synchronization/waitable_event.h"
 #include "sync/test/fake_server/fake_server.h"
 
 using syncer::HttpPostProviderInterface;
@@ -18,9 +13,7 @@ using syncer::HttpPostProviderInterface;
 namespace fake_server {
 
 FakeServerHttpPostProviderFactory::FakeServerHttpPostProviderFactory(
-    FakeServer* fake_server,
-    scoped_refptr<base::SequencedTaskRunner> task_runner)
-        : fake_server_(fake_server), task_runner_(task_runner) { }
+    FakeServer* fake_server) : fake_server_(fake_server) { }
 
 FakeServerHttpPostProviderFactory::~FakeServerHttpPostProviderFactory() { }
 
@@ -28,7 +21,7 @@ void FakeServerHttpPostProviderFactory::Init(const std::string& user_agent) { }
 
 HttpPostProviderInterface* FakeServerHttpPostProviderFactory::Create() {
   FakeServerHttpPostProvider* http =
-      new FakeServerHttpPostProvider(fake_server_, task_runner_);
+      new FakeServerHttpPostProvider(fake_server_);
   http->AddRef();
   return http;
 }
@@ -39,11 +32,7 @@ void FakeServerHttpPostProviderFactory::Destroy(
 }
 
 FakeServerHttpPostProvider::FakeServerHttpPostProvider(
-    FakeServer* fake_server,
-    scoped_refptr<base::SequencedTaskRunner> task_runner)
-        : fake_server_(fake_server),
-          task_runner_(task_runner),
-          post_complete_(false, false) { }
+    FakeServer* fake_server) : fake_server_(fake_server) { }
 
 FakeServerHttpPostProvider::~FakeServerHttpPostProvider() { }
 
@@ -65,35 +54,13 @@ void FakeServerHttpPostProvider::SetPostPayload(const char* content_type,
   request_content_.assign(content, content_length);
 }
 
-void FakeServerHttpPostProvider::OnPostComplete(int error_code,
-                                                int response_code,
-                                                const std::string& response) {
-  post_error_code_ = error_code;
-  post_response_code_ = response_code;
-  response_ = response;
-  post_complete_.Signal();
-}
-
 bool FakeServerHttpPostProvider::MakeSynchronousPost(int* error_code,
                                                      int* response_code) {
-  // It is assumed that a POST is being made to /command.
-  FakeServer::HandleCommandCallback callback = base::Bind(
-      &FakeServerHttpPostProvider::OnPostComplete, base::Unretained(this));
-  task_runner_->PostNonNestableTask(FROM_HERE,
-                                    base::Bind(&FakeServer::HandleCommand,
-                                               base::Unretained(fake_server_),
-                                               base::ConstRef(request_content_),
-                                               base::ConstRef(callback)));
-  const int kTimeoutSecs = 5;
-  bool signaled = post_complete_.TimedWait(
-      base::TimeDelta::FromSeconds(kTimeoutSecs));
-  if (!signaled || *error_code != 0) {
-    return false;
-  }
-
-  *error_code = post_error_code_;
-  *response_code = post_response_code_;
-  return true;
+  // This assumes that a POST is being made to /command.
+  *error_code = fake_server_->HandleCommand(request_content_,
+                                            response_code,
+                                            &response_);
+  return (*error_code == 0);
 }
 
 int FakeServerHttpPostProvider::GetResponseContentLength() const {
diff --git a/sync/test/fake_server/fake_server_http_post_provider.h b/sync/test/fake_server/fake_server_http_post_provider.h
index 9e2aa0e..63f1eda 100644
--- a/sync/test/fake_server/fake_server_http_post_provider.h
+++ b/sync/test/fake_server/fake_server_http_post_provider.h
@@ -8,8 +8,6 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
-#include "base/sequenced_task_runner.h"
-#include "base/synchronization/waitable_event.h"
 #include "sync/internal_api/public/http_post_provider_factory.h"
 #include "sync/internal_api/public/http_post_provider_interface.h"
 
@@ -21,9 +19,7 @@ class FakeServerHttpPostProvider
     : public syncer::HttpPostProviderInterface,
       public base::RefCountedThreadSafe<FakeServerHttpPostProvider> {
  public:
-  explicit FakeServerHttpPostProvider(
-      FakeServer* fake_server,
-      scoped_refptr<base::SequencedTaskRunner> task_runner);
+  explicit FakeServerHttpPostProvider(FakeServer* fake_server);
 
   // HttpPostProviderInterface implementation.
   virtual void SetExtraRequestHeaders(const char* headers) OVERRIDE;
@@ -43,22 +39,13 @@ class FakeServerHttpPostProvider
   virtual ~FakeServerHttpPostProvider();
 
  private:
-  void OnPostComplete(int error_code,
-                      int response_code,
-                      const std::string& response);
-
   FakeServer* const fake_server_;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
-
   std::string response_;
   std::string request_url_;
   int request_port_;
   std::string request_content_;
   std::string request_content_type_;
   std::string extra_request_headers_;
-  int post_error_code_;
-  int post_response_code_;
-  base::WaitableEvent post_complete_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeServerHttpPostProvider);
 };
@@ -66,9 +53,7 @@ class FakeServerHttpPostProvider
 class FakeServerHttpPostProviderFactory
     : public syncer::HttpPostProviderFactory {
  public:
-  explicit FakeServerHttpPostProviderFactory(
-      FakeServer* fake_server,
-      scoped_refptr<base::SequencedTaskRunner> task_runner);
+  explicit FakeServerHttpPostProviderFactory(FakeServer* fake_server);
   virtual ~FakeServerHttpPostProviderFactory();
 
   // HttpPostProviderFactory:
@@ -78,7 +63,6 @@ class FakeServerHttpPostProviderFactory
 
  private:
   FakeServer* const fake_server_;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeServerHttpPostProviderFactory);
 };
diff --git a/sync/test/fake_server/fake_server_network_resources.cc b/sync/test/fake_server/fake_server_network_resources.cc
index c3bedb0..56d5ea2 100644
--- a/sync/test/fake_server/fake_server_network_resources.cc
+++ b/sync/test/fake_server/fake_server_network_resources.cc
@@ -5,7 +5,6 @@
 #include "sync/test/fake_server/fake_server_network_resources.h"
 
 #include "base/memory/scoped_ptr.h"
-#include "base/message_loop/message_loop.h"
 #include "sync/internal_api/public/base/cancelation_signal.h"
 #include "sync/internal_api/public/http_post_provider_factory.h"
 #include "sync/internal_api/public/network_time_update_callback.h"
@@ -29,9 +28,7 @@ scoped_ptr<syncer::HttpPostProviderFactory>
         const NetworkTimeUpdateCallback& network_time_update_callback,
         CancelationSignal* cancelation_signal) {
   return make_scoped_ptr<syncer::HttpPostProviderFactory>(
-      new FakeServerHttpPostProviderFactory(
-          fake_server_,
-          base::MessageLoop::current()->message_loop_proxy()));
+      new FakeServerHttpPostProviderFactory(fake_server_));
 }
 
 }  // namespace fake_server