Sync: store FakeServer as WeakPtr

This CL changes the FakeServerNetworkResources stack to store FakeServer
as a WeakPtr. This is done because FakeServerHttpPostProvider posts a
task to another thread and FakeServer may be deleted before the task
executes.

This CL adds a ThreadChecker to FakeServer to ensure that all actions
(especially AsWeakPtr() and destruction) happen on the same thread. As
a result of this thread restriction, FakeServerHelper has been changed
to call all FakeServer methods on the UI thread.

FakeServerHttpPostProvider's task-posting has also been reconfigured to
be cleaner. This class now calls PostTask (instead of unnecessarily
calling PostNonNestableTask previously) and has removed the possibility
of FakeServer::HandleCommand (UI thread) calling a method on
FakeServerHttpPostProvider (Sync thread).

BUG=481192

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

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

9 files changed, 207 insertions, 81 deletions

diff --git a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java
index 9094a18..94b3950 100644
--- a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java
+++ b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java
@@ -90,7 +90,12 @@ public class FakeServerHelper {
      * @return the FakeServer pointer
      */
     public long createFakeServer() {
-        return nativeCreateFakeServer(mNativeFakeServerHelperAndroid);
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Long>() {
+            @Override
+            public Long call() {
+                return nativeCreateFakeServer(mNativeFakeServerHelperAndroid);
+            }
+        });
     }
 
     /**
@@ -100,8 +105,14 @@ public class FakeServerHelper {
      * @param nativeFakeServer pointer to a native FakeServer object.
      * @return the NetworkResources pointer
      */
-    public long createNetworkResources(long nativeFakeServer) {
-        return nativeCreateNetworkResources(mNativeFakeServerHelperAndroid, nativeFakeServer);
+    public long createNetworkResources(final long nativeFakeServer) {
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Long>() {
+            @Override
+            public Long call() {
+                return nativeCreateNetworkResources(
+                        mNativeFakeServerHelperAndroid, nativeFakeServer);
+            }
+        });
     }
 
     /**
@@ -109,8 +120,14 @@ public class FakeServerHelper {
      *
      * @param nativeFakeServer the pointer to be deleted
      */
-    public void deleteFakeServer(long nativeFakeServer) {
-        nativeDeleteFakeServer(mNativeFakeServerHelperAndroid, nativeFakeServer);
+    public void deleteFakeServer(final long nativeFakeServer) {
+        ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Void>() {
+            @Override
+            public Void call() {
+                nativeDeleteFakeServer(mNativeFakeServerHelperAndroid, nativeFakeServer);
+                return null;
+            }
+        });
     }
 
     /**
@@ -123,17 +140,29 @@ public class FakeServerHelper {
      *
      * @return whether the number of specified entities exist
      */
-    public boolean verifyEntityCountByTypeAndName(int count, ModelType modelType, String name) {
+    public boolean verifyEntityCountByTypeAndName(final int count, final ModelType modelType,
+            final String name) {
         checkFakeServerInitialized(
                 "useFakeServer must be called before data verification.");
-        return nativeVerifyEntityCountByTypeAndName(mNativeFakeServerHelperAndroid,
-            sNativeFakeServer, count, modelType.toString(), name);
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() {
+            @Override
+            public Boolean call() {
+                return nativeVerifyEntityCountByTypeAndName(mNativeFakeServerHelperAndroid,
+                        sNativeFakeServer, count, modelType.toString(), name);
+            }
+        });
     }
 
-    public boolean verifySessions(String[] urls) {
+    public boolean verifySessions(final String[] urls) {
         checkFakeServerInitialized(
                 "useFakeServer must be called before data verification.");
-        return nativeVerifySessions(mNativeFakeServerHelperAndroid, sNativeFakeServer, urls);
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() {
+            @Override
+            public Boolean call() {
+                return nativeVerifySessions(mNativeFakeServerHelperAndroid, sNativeFakeServer,
+                        urls);
+            }
+        });
     }
 
     /**
@@ -144,12 +173,18 @@ public class FakeServerHelper {
      *             SyncEntity.name value
      * @param entitySpecifics the EntitySpecifics proto that represents the entity to inject
      */
-    public void injectUniqueClientEntity(String name, EntitySpecifics entitySpecifics) {
+    public void injectUniqueClientEntity(final String name, final EntitySpecifics entitySpecifics) {
         checkFakeServerInitialized("useFakeServer must be called before data injection.");
-        // The protocol buffer is serialized as a byte array because it can be easily deserialized
-        // from this format in native code.
-        nativeInjectUniqueClientEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, name,
-                MessageNano.toByteArray(entitySpecifics));
+        ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Void>() {
+            @Override
+            public Void call() {
+                // The protocol buffer is serialized as a byte array because it can be easily
+                // deserialized from this format in native code.
+                nativeInjectUniqueClientEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer,
+                        name, MessageNano.toByteArray(entitySpecifics));
+                return null;
+            }
+        });
     }
 
     /**
@@ -160,10 +195,16 @@ public class FakeServerHelper {
      *            class, so it must be a valid URL under its definition.
      * @param parentId the ID of the desired parent bookmark folder
      */
-    public void injectBookmarkEntity(String title, String url, String parentId) {
+    public void injectBookmarkEntity(final String title, final String url, final String parentId) {
         checkFakeServerInitialized("useFakeServer must be called before data injection.");
-        nativeInjectBookmarkEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, title, url,
-                parentId);
+        ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Void>() {
+            @Override
+            public Void call() {
+                nativeInjectBookmarkEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, title,
+                        url, parentId);
+                return null;
+            }
+        });
     }
 
     /**
@@ -173,9 +214,15 @@ public class FakeServerHelper {
      *
      * @param id the server ID of the entity to delete
      */
-    public void deleteEntity(String id) {
+    public void deleteEntity(final String id) {
         checkFakeServerInitialized("useFakeServer must be called before deleting an entity.");
-        nativeDeleteEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, id);
+        ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Void>() {
+            @Override
+            public Void call() {
+                nativeDeleteEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, id);
+                return null;
+            }
+        });
     }
 
     /**
@@ -186,7 +233,13 @@ public class FakeServerHelper {
      */
     public String getBookmarkBarFolderId() {
         checkFakeServerInitialized("useFakeServer must be called before access");
-        return nativeGetBookmarkBarFolderId(mNativeFakeServerHelperAndroid, sNativeFakeServer);
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<String>() {
+            @Override
+            public String call() {
+                return nativeGetBookmarkBarFolderId(mNativeFakeServerHelperAndroid,
+                        sNativeFakeServer);
+            }
+        });
     }
 
     private static void checkFakeServerInitialized(String failureMessage) {
diff --git a/chrome/browser/sync/test/integration/sync_test.cc b/chrome/browser/sync/test/integration/sync_test.cc
index ec05a7e..39d9c0f 100644
--- a/chrome/browser/sync/test/integration/sync_test.cc
+++ b/chrome/browser/sync/test/integration/sync_test.cc
@@ -462,7 +462,8 @@ void SyncTest::InitializeInstance(int index) {
     // TODO(pvalenzuela): Run the fake server via EmbeddedTestServer.
     profile_sync_service->OverrideNetworkResourcesForTest(
         make_scoped_ptr<syncer::NetworkResources>(
-            new fake_server::FakeServerNetworkResources(fake_server_.get())));
+            new fake_server::FakeServerNetworkResources(
+                fake_server_->AsWeakPtr())));
   }
 
   ProfileSyncServiceHarness::SigninType singin_type = UsingExternalServers()
diff --git a/sync/test/fake_server/android/fake_server_helper_android.cc b/sync/test/fake_server/android/fake_server_helper_android.cc
index cfb42e9..2cc64f5 100644
--- a/sync/test/fake_server/android/fake_server_helper_android.cc
+++ b/sync/test/fake_server/android/fake_server_helper_android.cc
@@ -46,7 +46,7 @@ jlong FakeServerHelperAndroid::CreateNetworkResources(JNIEnv* env,
   fake_server::FakeServer* fake_server_ptr =
       reinterpret_cast<fake_server::FakeServer*>(fake_server);
   syncer::NetworkResources* resources =
-      new fake_server::FakeServerNetworkResources(fake_server_ptr);
+      new fake_server::FakeServerNetworkResources(fake_server_ptr->AsWeakPtr());
   return reinterpret_cast<intptr_t>(resources);
 }
 
diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc
index dbe3105..e8f8dc0 100644
--- a/sync/test/fake_server/fake_server.cc
+++ b/sync/test/fake_server/fake_server.cc
@@ -170,17 +170,22 @@ FakeServer::FakeServer() : version_(0),
                            error_type_(sync_pb::SyncEnums::SUCCESS),
                            alternate_triggered_errors_(false),
                            request_counter_(0),
-                           network_enabled_(true) {
+                           network_enabled_(true),
+                           weak_ptr_factory_(this) {
   keystore_keys_.push_back(kDefaultKeystoreKey);
-  CHECK(CreateDefaultPermanentItems());
+
+  const bool create_result = CreateDefaultPermanentItems();
+  DCHECK(create_result) << "Permanent items were not created successfully.";
 }
 
 FakeServer::~FakeServer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   STLDeleteContainerPairSecondPointers(entities_.begin(), entities_.end());
 }
 
 bool FakeServer::CreatePermanentBookmarkFolder(const std::string& server_tag,
                                                const std::string& name) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   FakeServerEntity* entity =
       PermanentEntity::Create(syncer::BOOKMARKS, server_tag, name,
                               ModelTypeToRootTag(syncer::BOOKMARKS));
@@ -222,15 +227,25 @@ void FakeServer::SaveEntity(FakeServerEntity* entity) {
 }
 
 void FakeServer::HandleCommand(const string& request,
-                               const HandleCommandCallback& callback) {
+                               const base::Closure& completion_closure,
+                               int* error_code,
+                               int* response_code,
+                               std::string* response) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!network_enabled_) {
-    callback.Run(net::ERR_FAILED, net::ERR_FAILED, string());
+    *error_code = net::ERR_FAILED;
+    *response_code = net::ERR_FAILED;
+    *response = string();
+    completion_closure.Run();
     return;
   }
   request_counter_++;
 
   if (!authenticated_) {
-    callback.Run(0, net::HTTP_UNAUTHORIZED, string());
+    *error_code = 0;
+    *response_code = net::HTTP_UNAUTHORIZED;
+    *response = string();
+    completion_closure.Run();
     return;
   }
 
@@ -263,14 +278,20 @@ void FakeServer::HandleCommand(const string& request,
                                       response_proto.mutable_commit());
         break;
       default:
-        callback.Run(net::ERR_NOT_IMPLEMENTED, 0, string());;
+        *error_code = net::ERR_NOT_IMPLEMENTED;
+        *response_code = 0;
+        *response = string();
+        completion_closure.Run();
         return;
     }
 
     if (!success) {
       // TODO(pvalenzuela): Add logging here so that tests have more info about
       // the failure.
-      callback.Run(net::ERR_FAILED, 0, string());
+      *error_code = net::ERR_FAILED;
+      *response_code = 0;
+      *response = string();
+      completion_closure.Run();
       return;
     }
 
@@ -278,7 +299,11 @@ void FakeServer::HandleCommand(const string& request,
   }
 
   response_proto.set_store_birthday(store_birthday_);
-  callback.Run(0, net::HTTP_OK, response_proto.SerializeAsString());
+
+  *error_code = 0;
+  *response_code = net::HTTP_OK;
+  *response = response_proto.SerializeAsString();
+  completion_closure.Run();
 }
 
 bool FakeServer::HandleGetUpdatesRequest(
@@ -469,6 +494,7 @@ bool FakeServer::HandleCommitRequest(
 }
 
 scoped_ptr<base::DictionaryValue> FakeServer::GetEntitiesAsDictionaryValue() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   scoped_ptr<base::DictionaryValue> dictionary(new base::DictionaryValue());
 
   // Initialize an empty ListValue for all ModelTypes.
@@ -503,6 +529,7 @@ scoped_ptr<base::DictionaryValue> FakeServer::GetEntitiesAsDictionaryValue() {
 std::vector<sync_pb::SyncEntity> FakeServer::GetSyncEntitiesByModelType(
     ModelType model_type) {
   std::vector<sync_pb::SyncEntity> sync_entities;
+  DCHECK(thread_checker_.CalledOnValidThread());
   for (EntityMap::const_iterator it = entities_.begin(); it != entities_.end();
        ++it) {
     FakeServerEntity* entity = it->second;
@@ -516,10 +543,12 @@ std::vector<sync_pb::SyncEntity> FakeServer::GetSyncEntitiesByModelType(
 }
 
 void FakeServer::InjectEntity(scoped_ptr<FakeServerEntity> entity) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   SaveEntity(entity.release());
 }
 
 bool FakeServer::SetNewStoreBirthday(const string& store_birthday) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (store_birthday_ == store_birthday)
     return false;
 
@@ -528,14 +557,17 @@ bool FakeServer::SetNewStoreBirthday(const string& store_birthday) {
 }
 
 void FakeServer::SetAuthenticated() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   authenticated_ = true;
 }
 
 void FakeServer::SetUnauthenticated() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   authenticated_ = false;
 }
 
 bool FakeServer::TriggerError(const sync_pb::SyncEnums::ErrorType& error_type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (triggered_actionable_error_.get()) {
     DVLOG(1) << "Only one type of error can be triggered at any given time.";
     return false;
@@ -550,6 +582,7 @@ bool FakeServer::TriggerActionableError(
     const string& description,
     const string& url,
     const sync_pb::SyncEnums::Action& action) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (error_type_ != sync_pb::SyncEnums::SUCCESS) {
     DVLOG(1) << "Only one type of error can be triggered at any given time.";
     return false;
@@ -566,6 +599,7 @@ bool FakeServer::TriggerActionableError(
 }
 
 bool FakeServer::EnableAlternatingTriggeredErrors() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (error_type_ == sync_pb::SyncEnums::SUCCESS &&
       !triggered_actionable_error_.get()) {
     DVLOG(1) << "No triggered error set. Alternating can't be enabled.";
@@ -588,22 +622,27 @@ bool FakeServer::ShouldSendTriggeredError() const {
 }
 
 void FakeServer::AddObserver(Observer* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   observers_.AddObserver(observer);
 }
 
 void FakeServer::RemoveObserver(Observer* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   observers_.RemoveObserver(observer);
 }
 
 void FakeServer::EnableNetwork() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   network_enabled_ = true;
 }
 
 void FakeServer::DisableNetwork() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   network_enabled_ = false;
 }
 
 std::string FakeServer::GetBookmarkBarFolderId() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   for (EntityMap::const_iterator it = entities_.begin(); it != entities_.end();
        ++it) {
     FakeServerEntity* entity = it->second;
@@ -617,4 +656,9 @@ std::string FakeServer::GetBookmarkBarFolderId() const {
   return "";
 }
 
+base::WeakPtr<FakeServer> FakeServer::AsWeakPtr() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 }  // namespace fake_server
diff --git a/sync/test/fake_server/fake_server.h b/sync/test/fake_server/fake_server.h
index 8478076..f4374e7 100644
--- a/sync/test/fake_server/fake_server.h
+++ b/sync/test/fake_server/fake_server.h
@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
+#include "base/threading/thread_checker.h"
 #include "base/values.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/protocol/sync.pb.h"
@@ -20,12 +21,10 @@
 
 namespace fake_server {
 
-// A fake version of the Sync server used for testing.
+// A fake version of the Sync server used for testing. This class is not thread
+// safe.
 class FakeServer {
  public:
-  typedef base::Callback<void(int, int, const std::string&)>
-      HandleCommandCallback;
-
   class Observer {
    public:
     virtual ~Observer() {}
@@ -40,11 +39,16 @@ class FakeServer {
   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.
+  // Handles a /command POST (with the given |request|) to the server. Three
+  // output arguments, |error_code|, |response_code|, and |response|, are used
+  // to pass data back to the caller. The command has failed if the value
+  // pointed to by |error_code| is nonzero. |completion_closure| will be called
+  // immediately before return.
   void HandleCommand(const std::string& request,
-                     const HandleCommandCallback& callback);
+                     const base::Closure& completion_closure,
+                     int* error_code,
+                     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
@@ -119,6 +123,9 @@ class FakeServer {
   // Returns the entity ID of the Bookmark Bar folder.
   std::string GetBookmarkBarFolderId() const;
 
+  // Returns the current FakeServer as a WeakPtr.
+  base::WeakPtr<FakeServer> AsWeakPtr();
+
  private:
   typedef std::map<std::string, FakeServerEntity*> EntityMap;
 
@@ -211,6 +218,13 @@ class FakeServer {
   // When true, the server operates normally. When false, a failure is returned
   // on every request. This is used to simulate a network failure on the client.
   bool network_enabled_;
+
+  // Used to verify that FakeServer is only used from one thread.
+  base::ThreadChecker thread_checker_;
+
+  // Creates WeakPtr versions of the current FakeServer. This must be the last
+  // data member!
+  base::WeakPtrFactory<FakeServer> weak_ptr_factory_;
 };
 
 }  // 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 fbfe16f..75c5d46e 100644
--- a/sync/test/fake_server/fake_server_http_post_provider.cc
+++ b/sync/test/fake_server/fake_server_http_post_provider.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/synchronization/waitable_event.h"
 #include "sync/test/fake_server/fake_server.h"
@@ -18,9 +19,10 @@ 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) { }
+    const base::WeakPtr<FakeServer>& fake_server,
+    scoped_refptr<base::SequencedTaskRunner> fake_server_task_runner)
+        : fake_server_(fake_server),
+          fake_server_task_runner_(fake_server_task_runner) { }
 
 FakeServerHttpPostProviderFactory::~FakeServerHttpPostProviderFactory() { }
 
@@ -28,7 +30,7 @@ void FakeServerHttpPostProviderFactory::Init(const std::string& user_agent) { }
 
 HttpPostProviderInterface* FakeServerHttpPostProviderFactory::Create() {
   FakeServerHttpPostProvider* http =
-      new FakeServerHttpPostProvider(fake_server_, task_runner_);
+      new FakeServerHttpPostProvider(fake_server_, fake_server_task_runner_);
   http->AddRef();
   return http;
 }
@@ -39,11 +41,10 @@ void FakeServerHttpPostProviderFactory::Destroy(
 }
 
 FakeServerHttpPostProvider::FakeServerHttpPostProvider(
-    FakeServer* fake_server,
-    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    const base::WeakPtr<FakeServer>& fake_server,
+    scoped_refptr<base::SequencedTaskRunner> fake_server_task_runner)
         : fake_server_(fake_server),
-          task_runner_(task_runner),
-          post_complete_(false, false) { }
+          fake_server_task_runner_(fake_server_task_runner) { }
 
 FakeServerHttpPostProvider::~FakeServerHttpPostProvider() { }
 
@@ -65,26 +66,35 @@ 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)));
-  post_complete_.Wait();
+  int post_error_code = -1;
+  int post_response_code = -1;
+  std::string post_response;
+
+  base::WaitableEvent post_complete(false, false);
+  base::Closure signal_closure = base::Bind(&base::WaitableEvent::Signal,
+                                            base::Unretained(&post_complete));
+
+  bool result = fake_server_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&FakeServer::HandleCommand,
+                 fake_server_,
+                 base::ConstRef(request_content_),
+                 base::ConstRef(signal_closure),
+                 &post_error_code,
+                 &post_response_code,
+                 &post_response));
+
+  if (!result)
+    return false;
+
+  post_complete.Wait();
+  post_error_code_ = post_error_code;
+  post_response_code_ = post_response_code;
+  response_ = post_response;
+
   *error_code = post_error_code_;
   *response_code = post_response_code_;
   return *error_code == 0;
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 50d2317..593be1d 100644
--- a/sync/test/fake_server/fake_server_http_post_provider.h
+++ b/sync/test/fake_server/fake_server_http_post_provider.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/synchronization/waitable_event.h"
 #include "sync/internal_api/public/http_post_provider_factory.h"
@@ -22,8 +23,8 @@ class FakeServerHttpPostProvider
       public base::RefCountedThreadSafe<FakeServerHttpPostProvider> {
  public:
   FakeServerHttpPostProvider(
-      FakeServer* fake_server,
-      scoped_refptr<base::SequencedTaskRunner> task_runner);
+      const base::WeakPtr<FakeServer>& fake_server,
+      scoped_refptr<base::SequencedTaskRunner> fake_server_task_runner);
 
   // HttpPostProviderInterface implementation.
   void SetExtraRequestHeaders(const char* headers) override;
@@ -43,12 +44,10 @@ class FakeServerHttpPostProvider
   ~FakeServerHttpPostProvider() override;
 
  private:
-  void OnPostComplete(int error_code,
-                      int response_code,
-                      const std::string& response);
-
-  FakeServer* const fake_server_;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  // |fake_server_| should only be dereferenced on the same thread as
+  // |fake_server_task_runner_| runs on.
+  base::WeakPtr<FakeServer> fake_server_;
+  scoped_refptr<base::SequencedTaskRunner> fake_server_task_runner_;
 
   std::string response_;
   std::string request_url_;
@@ -58,7 +57,6 @@ class FakeServerHttpPostProvider
   std::string extra_request_headers_;
   int post_error_code_;
   int post_response_code_;
-  base::WaitableEvent post_complete_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeServerHttpPostProvider);
 };
@@ -67,8 +65,8 @@ class FakeServerHttpPostProviderFactory
     : public syncer::HttpPostProviderFactory {
  public:
   FakeServerHttpPostProviderFactory(
-      FakeServer* fake_server,
-      scoped_refptr<base::SequencedTaskRunner> task_runner);
+      const base::WeakPtr<FakeServer>& fake_server,
+      scoped_refptr<base::SequencedTaskRunner> fake_server_task_runner);
   ~FakeServerHttpPostProviderFactory() override;
 
   // HttpPostProviderFactory:
@@ -77,8 +75,10 @@ class FakeServerHttpPostProviderFactory
   void Destroy(syncer::HttpPostProviderInterface* http) override;
 
  private:
-  FakeServer* const fake_server_;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  // |fake_server_| should only be dereferenced on the same thread as
+  // |fake_server_task_runner_| runs on.
+  base::WeakPtr<FakeServer> fake_server_;
+  scoped_refptr<base::SequencedTaskRunner> fake_server_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 8d7c647..b6b573d 100644
--- a/sync/test/fake_server/fake_server_network_resources.cc
+++ b/sync/test/fake_server/fake_server_network_resources.cc
@@ -5,6 +5,7 @@
 #include "sync/test/fake_server/fake_server_network_resources.h"
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_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"
@@ -18,8 +19,9 @@ using syncer::NetworkTimeUpdateCallback;
 
 namespace fake_server {
 
-FakeServerNetworkResources::FakeServerNetworkResources(FakeServer* fake_server)
-    : fake_server_(fake_server) { }
+FakeServerNetworkResources::FakeServerNetworkResources(
+    const base::WeakPtr<FakeServer>& fake_server)
+        : fake_server_(fake_server) { }
 
 FakeServerNetworkResources::~FakeServerNetworkResources() {}
 
diff --git a/sync/test/fake_server/fake_server_network_resources.h b/sync/test/fake_server/fake_server_network_resources.h
index f6e1fe3..ba8579e 100644
--- a/sync/test/fake_server/fake_server_network_resources.h
+++ b/sync/test/fake_server/fake_server_network_resources.h
@@ -6,6 +6,7 @@
 #define SYNC_TEST_FAKE_SERVER_FAKE_SERVER_NETWORK_RESOURCES_H_
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "sync/internal_api/public/network_resources.h"
 #include "sync/internal_api/public/network_time_update_callback.h"
 
@@ -20,7 +21,8 @@ class HttpPostProviderFactory;
 
 class FakeServerNetworkResources : public syncer::NetworkResources {
  public:
-  explicit FakeServerNetworkResources(FakeServer* fake_server);
+  explicit FakeServerNetworkResources(
+      const base::WeakPtr<FakeServer>& fake_server);
   ~FakeServerNetworkResources() override;
 
   // NetworkResources
@@ -31,7 +33,7 @@ class FakeServerNetworkResources : public syncer::NetworkResources {
       syncer::CancelationSignal* cancelation_signal) override;
 
  private:
-  FakeServer* const fake_server_;
+  base::WeakPtr<FakeServer> fake_server_;
 };
 
 }  // namespace fake_server