sync: Add interface for SyncCoreProxy

Defines an interface for SyncCoreProxy.  This will be used for testing.

The SyncCoreProxy is a natural boundary for tests.  To its clients, it
represents the sync thread and all of its functionality.  By allowing it
to be overloaded in tests, we can stub out all of the sync thread
functionality so we can better test the parts that communicate with it.

This required some fairly large changes.  In order to allow it to be
used as an interface, the SyncCoreProxy could no longer be stored on the
stack and copied around everywhere.  The objects that call its virtual
functions must manage it as a pointer or reference, rather than
a concrete object.

For now, ownership is settled by having one or two elements on each
thread hold a scoped_ptr to their own copy of it.  The SyncCoreProxy is
passed around as a pointer, and ownership is not transferred in these
calls.  Objects that want to keep their own private copy can make use of
the Clone() method and store the resulting copy in a scoped_ptr.

BUG=351005

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

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

24 files changed, 224 insertions, 105 deletions

diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 3f7ea4d..e9fb105 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -161,7 +161,9 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
 
   // Called on |frontend_loop_| to obtain a handle to the SyncCore needed by
   // the non-blocking sync types to communicate with the server.
-  virtual syncer::SyncCoreProxy GetSyncCoreProxy() = 0;
+  //
+  // Should be called only when the backend is initialized.
+  virtual scoped_ptr<syncer::SyncCoreProxy> GetSyncCoreProxy() = 0;
 
   // Called from any thread to obtain current status information in detailed or
   // summarized form.
diff --git a/chrome/browser/sync/glue/sync_backend_host_core.cc b/chrome/browser/sync/glue/sync_backend_host_core.cc
index 54c1c51..b5e2eb3 100644
--- a/chrome/browser/sync/glue/sync_backend_host_core.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_core.cc
@@ -16,6 +16,7 @@
 #include "sync/internal_api/public/http_post_provider_factory.h"
 #include "sync/internal_api/public/internal_components_factory.h"
 #include "sync/internal_api/public/sessions/sync_session_snapshot.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/internal_api/public/sync_manager.h"
 #include "sync/internal_api/public/sync_manager_factory.h"
 
@@ -481,19 +482,12 @@ void SyncBackendHostCore::DoFinishInitialProcessControlTypes() {
                                synced_device_tracker_.get(),
                                sync_manager_->GetUserShare());
 
-  base::WeakPtr<syncer::SyncCore> sync_core = sync_manager_->GetSyncCore();
-
-  // De-reference the WeakPtr while we're here to signal to the debugging
-  // mechanisms that it belongs to the sync thread.  This helps us DCHECK
-  // earlier if the pointer is misused.
-  sync_core.get();
-
   host_.Call(
       FROM_HERE,
       &SyncBackendHostImpl::HandleInitializationSuccessOnFrontendLoop,
       js_backend_,
       debug_info_listener_,
-      syncer::SyncCoreProxy(base::MessageLoopProxy::current(), sync_core));
+      sync_manager_->GetSyncCoreProxy());
 
   js_backend_.Reset();
   debug_info_listener_.Reset();
diff --git a/chrome/browser/sync/glue/sync_backend_host_core.h b/chrome/browser/sync/glue/sync_backend_host_core.h
index d458d40..eb267a6 100644
--- a/chrome/browser/sync/glue/sync_backend_host_core.h
+++ b/chrome/browser/sync/glue/sync_backend_host_core.h
@@ -212,10 +212,6 @@ class SyncBackendHostCore
   void SendBufferedProtocolEventsAndEnableForwarding();
   void DisableProtocolEventForwarding();
 
-  // Returns handle to sync functionality used by non-blocking sync types.
-  // Should only be called when the backend is initialized.
-  syncer::SyncCoreProxy GetSyncCoreProxy();
-
   // Delete the sync data folder to cleanup backend data.  Happens the first
   // time sync is enabled for a user (to prevent accidentally reusing old
   // sync databases), as well as shutdown when you're no longer syncing.
diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.cc b/chrome/browser/sync/glue/sync_backend_host_impl.cc
index 5049879..1d7e482 100644
--- a/chrome/browser/sync/glue/sync_backend_host_impl.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_impl.cc
@@ -432,8 +432,8 @@ syncer::UserShare* SyncBackendHostImpl::GetUserShare() const {
   return core_->sync_manager()->GetUserShare();
 }
 
-syncer::SyncCoreProxy SyncBackendHostImpl::GetSyncCoreProxy() {
-  return *sync_core_proxy_.get();
+scoped_ptr<syncer::SyncCoreProxy> SyncBackendHostImpl::GetSyncCoreProxy() {
+  return scoped_ptr<syncer::SyncCoreProxy>(sync_core_proxy_->Clone());
 }
 
 SyncBackendHostImpl::Status SyncBackendHostImpl::GetDetailedStatus() {
@@ -597,15 +597,16 @@ void SyncBackendHostImpl::HandleInitializationSuccessOnFrontendLoop(
     const syncer::WeakHandle<syncer::JsBackend> js_backend,
     const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>
         debug_info_listener,
-    syncer::SyncCoreProxy sync_core_proxy) {
+    syncer::SyncCoreProxy* sync_core_proxy) {
   DCHECK_EQ(base::MessageLoop::current(), frontend_loop_);
+
+  sync_core_proxy_ = sync_core_proxy->Clone();
+
   if (!frontend_)
     return;
 
   initialized_ = true;
 
-  sync_core_proxy_.reset(new syncer::SyncCoreProxy(sync_core_proxy));
-
   invalidator_->RegisterInvalidationHandler(this);
   invalidation_handler_registered_ = true;
 
diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.h b/chrome/browser/sync/glue/sync_backend_host_impl.h
index 84f9491..9ba56b3 100644
--- a/chrome/browser/sync/glue/sync_backend_host_impl.h
+++ b/chrome/browser/sync/glue/sync_backend_host_impl.h
@@ -112,7 +112,7 @@ class SyncBackendHostImpl
       ChangeProcessor* change_processor) OVERRIDE;
   virtual void DeactivateDataType(syncer::ModelType type) OVERRIDE;
   virtual syncer::UserShare* GetUserShare() const OVERRIDE;
-  virtual syncer::SyncCoreProxy GetSyncCoreProxy() OVERRIDE;
+  virtual scoped_ptr<syncer::SyncCoreProxy> GetSyncCoreProxy() OVERRIDE;
   virtual Status GetDetailedStatus() OVERRIDE;
   virtual syncer::sessions::SyncSessionSnapshot
       GetLastSessionSnapshot() const OVERRIDE;
@@ -164,11 +164,15 @@ class SyncBackendHostImpl
 
   // Reports backend initialization success.  Includes some objects from sync
   // manager initialization to be passed back to the UI thread.
+  //
+  // |sync_core_proxy| points to an object owned by the SyncManager.  Ownership
+  // is not transferred, but we can obtain our own copy of the object using its
+  // Clone() method.
   virtual void HandleInitializationSuccessOnFrontendLoop(
     const syncer::WeakHandle<syncer::JsBackend> js_backend,
     const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>
         debug_info_listener,
-    syncer::SyncCoreProxy sync_core_proxy);
+    syncer::SyncCoreProxy* sync_core_proxy);
 
   // Downloading of control types failed and will be retried. Invokes the
   // frontend's sync configure retry method.
diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.cc b/chrome/browser/sync/glue/sync_backend_host_mock.cc
index 21eeedb8..aee76c6 100644
--- a/chrome/browser/sync/glue/sync_backend_host_mock.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_mock.cc
@@ -69,8 +69,8 @@ syncer::UserShare* SyncBackendHostMock::GetUserShare() const {
   return NULL;
 }
 
-syncer::SyncCoreProxy SyncBackendHostMock::GetSyncCoreProxy() {
-  return syncer::SyncCoreProxy::GetInvalidSyncCoreProxyForTest();
+scoped_ptr<syncer::SyncCoreProxy> SyncBackendHostMock::GetSyncCoreProxy() {
+  return scoped_ptr<syncer::SyncCoreProxy>();
 }
 
 SyncBackendHost::Status SyncBackendHostMock::GetDetailedStatus() {
diff --git a/chrome/browser/sync/glue/sync_backend_host_mock.h b/chrome/browser/sync/glue/sync_backend_host_mock.h
index 60af243..0b0782d 100644
--- a/chrome/browser/sync/glue/sync_backend_host_mock.h
+++ b/chrome/browser/sync/glue/sync_backend_host_mock.h
@@ -72,7 +72,7 @@ class SyncBackendHostMock : public SyncBackendHost {
 
   virtual syncer::UserShare* GetUserShare() const OVERRIDE;
 
-  virtual syncer::SyncCoreProxy GetSyncCoreProxy() OVERRIDE;
+  virtual scoped_ptr<syncer::SyncCoreProxy> GetSyncCoreProxy() OVERRIDE;
 
   virtual Status GetDetailedStatus() OVERRIDE;
 
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index fce9958..6c368da 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -947,8 +947,6 @@ void ProfileSyncService::OnBackendInitialized(
     backend_->RequestBufferedProtocolEventsAndEnableForwarding();
   }
 
-  syncer::SyncCoreProxy sync_core_proxy_ = backend_->GetSyncCoreProxy();
-
   // If we have a cached passphrase use it to decrypt/encrypt data now that the
   // backend is initialized. We want to call this before notifying observers in
   // case this operation affects the "passphrase required" status.
diff --git a/sync/internal_api/non_blocking_type_processor.cc b/sync/internal_api/non_blocking_type_processor.cc
index 242c42f..8fd0d27 100644
--- a/sync/internal_api/non_blocking_type_processor.cc
+++ b/sync/internal_api/non_blocking_type_processor.cc
@@ -6,6 +6,7 @@
 
 #include "base/message_loop/message_loop_proxy.h"
 #include "sync/engine/non_blocking_type_processor_core.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
 
 namespace syncer {
 
@@ -25,9 +26,9 @@ ModelType NonBlockingTypeProcessor::GetModelType() const {
   return type_;
 }
 
-void NonBlockingTypeProcessor::Enable(SyncCoreProxy core_proxy_) {
+void NonBlockingTypeProcessor::Enable(SyncCoreProxy* core_proxy) {
   DCHECK(CalledOnValidThread());
-  core_proxy_.ConnectTypeToCore(
+  core_proxy->ConnectTypeToCore(
       GetModelType(),
       AsWeakPtr());
 }
diff --git a/sync/internal_api/public/non_blocking_type_processor.h b/sync/internal_api/public/non_blocking_type_processor.h
index a9079d5a..0198c27 100644
--- a/sync/internal_api/public/non_blocking_type_processor.h
+++ b/sync/internal_api/public/non_blocking_type_processor.h
@@ -5,17 +5,16 @@
 #ifndef SYNC_ENGINE_NON_BLOCKING_TYPE_PROCESSOR_H_
 #define SYNC_ENGINE_NON_BLOCKING_TYPE_PROCESSOR_H_
 
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/threading/non_thread_safe.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/protocol/sync.pb.h"
 
 namespace syncer {
 
+class SyncCoreProxy;
 class NonBlockingTypeProcessorCore;
 
 // A sync component embedded on the synced type's thread that helps to handle
@@ -32,7 +31,7 @@ class SYNC_EXPORT_PRIVATE NonBlockingTypeProcessor : base::NonThreadSafe {
   ModelType GetModelType() const;
 
   // Starts the handshake with the sync thread.
-  void Enable(SyncCoreProxy core_proxy_);
+  void Enable(SyncCoreProxy* core_proxy);
 
   // Severs all ties to the sync thread.
   // Another call to Enable() can be used to re-establish this connection.
diff --git a/sync/internal_api/public/sync_core_proxy.h b/sync/internal_api/public/sync_core_proxy.h
index dd5a4ac..8750b42 100644
--- a/sync/internal_api/public/sync_core_proxy.h
+++ b/sync/internal_api/public/sync_core_proxy.h
@@ -5,49 +5,30 @@
 #ifndef SYNC_INTERNAL_API_PUBLIC_SYNC_CORE_PROXY_H_
 #define SYNC_INTERNAL_API_PUBLIC_SYNC_CORE_PROXY_H_
 
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/sequenced_task_runner.h"
-#include "sync/base/sync_export.h"
 #include "sync/internal_api/public/base/model_type.h"
 
 namespace syncer {
 
-class SyncCore;
 class NonBlockingTypeProcessor;
 
-// Encapsulates a reference to the sync core and the thread it's running on.
-// Used by sync's data types to connect with the sync core.
+// Interface for the datatype integration logic from non-sync threads.
 //
-// It is epxected that this object will be copied to and used on many different
-// threads.  It is small and safe to pass by value.
+// See SyncCoreProxyImpl for an actual implementation.
 class SYNC_EXPORT_PRIVATE SyncCoreProxy {
  public:
-  SyncCoreProxy(
-      scoped_refptr<base::SequencedTaskRunner> sync_task_runner,
-      base::WeakPtr<SyncCore> sync_core);
-  ~SyncCoreProxy();
+  SyncCoreProxy();
+  virtual ~SyncCoreProxy();
 
   // Attempts to connect a non-blocking type to the sync core.
   //
-  // This may fail under some unusual circumstances, like shutdown.  Due to the
-  // nature of WeakPtrs and cross-thread communication, the caller will be
-  // unable to distinguish a slow success from failure.
-  //
   // Must be called from the thread where the data type lives.
-  void ConnectTypeToCore(
+  virtual void ConnectTypeToCore(
       syncer::ModelType type,
-      base::WeakPtr<NonBlockingTypeProcessor> type_processor);
-
-  // Constructs and returns a useless instance of this object.
-  static SyncCoreProxy GetInvalidSyncCoreProxyForTest();
-
- private:
-  // A SequencedTaskRunner representing the thread where the SyncCore lives.
-  scoped_refptr<base::SequencedTaskRunner> sync_task_runner_;
+      base::WeakPtr<NonBlockingTypeProcessor> type_processor) = 0;
 
-  // The SyncCore this object is wrapping.
-  base::WeakPtr<SyncCore> sync_core_;
+  // Creates a clone of this SyncCoreProxy.
+  virtual scoped_ptr<SyncCoreProxy> Clone() = 0;
 };
 
 }  // namespace syncer
diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h
index d10b998..7488586 100644
--- a/sync/internal_api/public/sync_manager.h
+++ b/sync/internal_api/public/sync_manager.h
@@ -22,6 +22,7 @@
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/engine/sync_status.h"
 #include "sync/internal_api/public/events/protocol_event.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/internal_api/public/util/report_unrecoverable_error_function.h"
 #include "sync/internal_api/public/util/unrecoverable_error_handler.h"
@@ -44,7 +45,7 @@ class HttpPostProviderFactory;
 class InternalComponentsFactory;
 class JsBackend;
 class JsEventHandler;
-class SyncCore;
+class SyncCoreProxy;
 class SyncEncryptionHandler;
 class ProtocolEvent;
 class SyncScheduler;
@@ -334,7 +335,7 @@ class SYNC_EXPORT SyncManager : public syncer::InvalidationHandler {
   virtual UserShare* GetUserShare() = 0;
 
   // Returns an instance of the main interface for non-blocking sync types.
-  virtual base::WeakPtr<syncer::SyncCore> GetSyncCore() = 0;
+  virtual syncer::SyncCoreProxy* GetSyncCoreProxy() = 0;
 
   // Returns the cache_guid of the currently open database.
   // Requires that the SyncManager be initialized.
diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h
index fae4c8e..55e4f3b 100644
--- a/sync/internal_api/public/test/fake_sync_manager.h
+++ b/sync/internal_api/public/test/fake_sync_manager.h
@@ -10,6 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "sync/internal_api/public/sync_manager.h"
+#include "sync/internal_api/public/test/null_sync_core_proxy.h"
 #include "sync/internal_api/public/test/test_user_share.h"
 #include "sync/notifier/invalidator_registrar.h"
 
@@ -119,7 +120,7 @@ class FakeSyncManager : public SyncManager {
   virtual void SaveChanges() OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;
   virtual UserShare* GetUserShare() OVERRIDE;
-  virtual base::WeakPtr<syncer::SyncCore> GetSyncCore() OVERRIDE;
+  virtual syncer::SyncCoreProxy* GetSyncCoreProxy() OVERRIDE;
   virtual const std::string cache_guid() OVERRIDE;
   virtual bool ReceivedExperiment(Experiments* experiments) OVERRIDE;
   virtual bool HasUnsyncedItems() OVERRIDE;
@@ -163,6 +164,8 @@ class FakeSyncManager : public SyncManager {
 
   TestUserShare test_user_share_;
 
+  NullSyncCoreProxy null_sync_core_proxy_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeSyncManager);
 };
 
diff --git a/sync/internal_api/public/test/null_sync_core_proxy.h b/sync/internal_api/public/test/null_sync_core_proxy.h
new file mode 100644
index 0000000..410a992
--- /dev/null
+++ b/sync/internal_api/public/test/null_sync_core_proxy.h
@@ -0,0 +1,31 @@
+// 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 SYNC_INTERNAL_API_PUBLIC_TEST_NULL_SYNC_CORE_PROXY_H_
+#define SYNC_INTERNAL_API_PUBLIC_TEST_NULL_SYNC_CORE_PROXY_H_
+
+#include "sync/internal_api/public/sync_core_proxy.h"
+#include "base/memory/weak_ptr.h"
+
+namespace syncer {
+
+class NonBlockingTypeProcessor;
+
+// A non-functional implementation of SyncCoreProxy.
+//
+// It supports Clone(), but not much else.  Useful for testing.
+class NullSyncCoreProxy : public SyncCoreProxy {
+ public:
+  NullSyncCoreProxy();
+  virtual ~NullSyncCoreProxy();
+
+  virtual void ConnectTypeToCore(
+      syncer::ModelType type,
+      base::WeakPtr<NonBlockingTypeProcessor> processor) OVERRIDE;
+  virtual scoped_ptr<SyncCoreProxy> Clone() OVERRIDE;
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_INTERNAL_API_PUBLIC_TEST_NULL_SYNC_CORE_PROXY_H_
diff --git a/sync/internal_api/sync_core_proxy.cc b/sync/internal_api/sync_core_proxy.cc
index 5ba3135..4e6f7f6 100644
--- a/sync/internal_api/sync_core_proxy.cc
+++ b/sync/internal_api/sync_core_proxy.cc
@@ -4,37 +4,10 @@
 
 #include "sync/internal_api/public/sync_core_proxy.h"
 
-#include "base/bind.h"
-#include "base/location.h"
-#include "base/message_loop/message_loop_proxy.h"
-#include "sync/internal_api/sync_core.h"
-
 namespace syncer {
 
-SyncCoreProxy::SyncCoreProxy(
-    scoped_refptr<base::SequencedTaskRunner> sync_task_runner,
-    base::WeakPtr<SyncCore> sync_core)
-    : sync_task_runner_(sync_task_runner),
-      sync_core_(sync_core) {}
+SyncCoreProxy::SyncCoreProxy() {}
 
 SyncCoreProxy::~SyncCoreProxy() {}
 
-void SyncCoreProxy::ConnectTypeToCore(
-    ModelType type,
-    base::WeakPtr<NonBlockingTypeProcessor> type_processor) {
-  VLOG(1) << "ConnectSyncTypeToCore: " << ModelTypeToString(type);
-  sync_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&SyncCore::ConnectSyncTypeToCore,
-                 sync_core_,
-                 type,
-                 base::MessageLoopProxy::current(),
-                 type_processor));
-}
-
-SyncCoreProxy SyncCoreProxy::GetInvalidSyncCoreProxyForTest() {
-  return SyncCoreProxy(scoped_refptr<base::SequencedTaskRunner>(),
-                       base::WeakPtr<SyncCore>());
-}
-
 }  // namespace syncer
diff --git a/sync/internal_api/sync_core_proxy_impl.cc b/sync/internal_api/sync_core_proxy_impl.cc
new file mode 100644
index 0000000..bfdd146
--- /dev/null
+++ b/sync/internal_api/sync_core_proxy_impl.cc
@@ -0,0 +1,40 @@
+// 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 "sync/internal_api/sync_core_proxy_impl.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/message_loop/message_loop_proxy.h"
+#include "sync/internal_api/sync_core.h"
+
+namespace syncer {
+
+SyncCoreProxyImpl::SyncCoreProxyImpl(
+    scoped_refptr<base::SequencedTaskRunner> sync_task_runner,
+    base::WeakPtr<SyncCore> sync_core)
+    : sync_task_runner_(sync_task_runner),
+      sync_core_(sync_core) {}
+
+SyncCoreProxyImpl::~SyncCoreProxyImpl() {}
+
+void SyncCoreProxyImpl::ConnectTypeToCore(
+    ModelType type,
+    base::WeakPtr<NonBlockingTypeProcessor> type_processor) {
+  VLOG(1) << "ConnectTypeToCore: " << ModelTypeToString(type);
+  sync_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncCore::ConnectSyncTypeToCore,
+                 sync_core_,
+                 type,
+                 base::MessageLoopProxy::current(),
+                 type_processor));
+}
+
+scoped_ptr<SyncCoreProxy> SyncCoreProxyImpl::Clone() {
+  return scoped_ptr<SyncCoreProxy>(
+      new SyncCoreProxyImpl(sync_task_runner_, sync_core_));
+}
+
+}  // namespace syncer
diff --git a/sync/internal_api/sync_core_proxy_impl.h b/sync/internal_api/sync_core_proxy_impl.h
new file mode 100644
index 0000000..7ccb26a
--- /dev/null
+++ b/sync/internal_api/sync_core_proxy_impl.h
@@ -0,0 +1,55 @@
+// 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 SYNC_INTERNAL_API_SYNC_CORE_PROXY_IMPL_H_
+#define SYNC_INTERNAL_API_SYNC_CORE_PROXY_IMPL_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner.h"
+#include "sync/base/sync_export.h"
+#include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
+
+namespace syncer {
+
+class SyncCore;
+class NonBlockingTypeProcessor;
+
+// Encapsulates a reference to the sync core and the thread it's running on.
+// Used by sync's data types to connect with the sync core.
+//
+// It is epxected that this object will be copied to and used on many different
+// threads.  It is small and safe to pass by value.
+class SYNC_EXPORT_PRIVATE SyncCoreProxyImpl : public SyncCoreProxy {
+ public:
+  SyncCoreProxyImpl(
+      scoped_refptr<base::SequencedTaskRunner> sync_task_runner,
+      base::WeakPtr<SyncCore> sync_core);
+  virtual ~SyncCoreProxyImpl();
+
+  // Attempts to connect a non-blocking type to the sync core.
+  //
+  // This may fail under some unusual circumstances, like shutdown.  Due to the
+  // nature of WeakPtrs and cross-thread communication, the caller will be
+  // unable to distinguish a slow success from failure.
+  //
+  // Must be called from the thread where the data type lives.
+  virtual void ConnectTypeToCore(
+      syncer::ModelType type,
+      base::WeakPtr<NonBlockingTypeProcessor> type_processor) OVERRIDE;
+
+  virtual scoped_ptr<SyncCoreProxy> Clone() OVERRIDE;
+
+ private:
+  // A SequencedTaskRunner representing the thread where the SyncCore lives.
+  scoped_refptr<base::SequencedTaskRunner> sync_task_runner_;
+
+  // The SyncCore this object is wrapping.
+  base::WeakPtr<SyncCore> sync_core_;
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_INTERNAL_API_SYNC_CORE_PROXY_IMPL_H_
diff --git a/sync/internal_api/sync_core_proxy_unittest.cc b/sync/internal_api/sync_core_proxy_impl_unittest.cc
index d6885bf..a3e228c 100644
--- a/sync/internal_api/sync_core_proxy_unittest.cc
+++ b/sync/internal_api/sync_core_proxy_impl_unittest.cc
@@ -8,16 +8,16 @@
 #include "base/sequenced_task_runner.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/non_blocking_type_processor.h"
-#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/internal_api/sync_core.h"
+#include "sync/internal_api/sync_core_proxy_impl.h"
 #include "sync/sessions/model_type_registry.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
 
-class SyncCoreProxyTest : public ::testing::Test {
+class SyncCoreProxyImplTest : public ::testing::Test {
  public:
-  SyncCoreProxyTest()
+  SyncCoreProxyImplTest()
       : sync_task_runner_(base::MessageLoopProxy::current()),
         type_task_runner_(base::MessageLoopProxy::current()),
         core_(new SyncCore(&registry_)),
@@ -31,8 +31,8 @@ class SyncCoreProxyTest : public ::testing::Test {
     core_.reset();
   }
 
-  SyncCoreProxy GetProxy() {
-    return core_proxy_;
+  SyncCoreProxy* GetProxy() {
+    return &core_proxy_;
   }
 
  private:
@@ -41,11 +41,11 @@ class SyncCoreProxyTest : public ::testing::Test {
   scoped_refptr<base::SequencedTaskRunner> type_task_runner_;
   ModelTypeRegistry registry_;
   scoped_ptr<SyncCore> core_;
-  SyncCoreProxy core_proxy_;
+  SyncCoreProxyImpl core_proxy_;
 };
 
 // Try to connect a type to a SyncCore that has already shut down.
-TEST_F(SyncCoreProxyTest, FailToConnect1) {
+TEST_F(SyncCoreProxyImplTest, FailToConnect1) {
   NonBlockingTypeProcessor themes_processor(syncer::THEMES);
   DisableSync();
   themes_processor.Enable(GetProxy());
@@ -56,7 +56,7 @@ TEST_F(SyncCoreProxyTest, FailToConnect1) {
 }
 
 // Try to connect a type to a SyncCore as it shuts down.
-TEST_F(SyncCoreProxyTest, FailToConnect2) {
+TEST_F(SyncCoreProxyImplTest, FailToConnect2) {
   NonBlockingTypeProcessor themes_processor(syncer::THEMES);
   themes_processor.Enable(GetProxy());
   DisableSync();
@@ -67,7 +67,7 @@ TEST_F(SyncCoreProxyTest, FailToConnect2) {
 }
 
 // Tests the case where the type's processor shuts down first.
-TEST_F(SyncCoreProxyTest, TypeDisconnectsFirst) {
+TEST_F(SyncCoreProxyImplTest, TypeDisconnectsFirst) {
   scoped_ptr<NonBlockingTypeProcessor> themes_processor
       (new NonBlockingTypeProcessor(syncer::THEMES));
   themes_processor->Enable(GetProxy());
@@ -80,7 +80,7 @@ TEST_F(SyncCoreProxyTest, TypeDisconnectsFirst) {
 }
 
 // Tests the case where the sync thread shuts down first.
-TEST_F(SyncCoreProxyTest, SyncDisconnectsFirst) {
+TEST_F(SyncCoreProxyImplTest, SyncDisconnectsFirst) {
   scoped_ptr<NonBlockingTypeProcessor> themes_processor
       (new NonBlockingTypeProcessor(syncer::THEMES));
   themes_processor->Enable(GetProxy());
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 7cb649b..7658f58 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -29,11 +29,13 @@
 #include "sync/internal_api/public/internal_components_factory.h"
 #include "sync/internal_api/public/read_node.h"
 #include "sync/internal_api/public/read_transaction.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/internal_api/public/user_share.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
 #include "sync/internal_api/sync_core.h"
+#include "sync/internal_api/sync_core_proxy_impl.h"
 #include "sync/internal_api/syncapi_internal.h"
 #include "sync/internal_api/syncapi_server_connection_manager.h"
 #include "sync/notifier/invalidation_util.h"
@@ -397,6 +399,14 @@ void SyncManagerImpl::Init(
 
   sync_core_.reset(new SyncCore(model_type_registry_.get()));
 
+  // Bind the SyncCore WeakPtr to this thread.  This helps us crash earlier if
+  // the pointer is misused in debug mode.
+  base::WeakPtr<SyncCore> weak_core = sync_core_->AsWeakPtr();
+  weak_core.get();
+
+  sync_core_proxy_.reset(
+      new SyncCoreProxyImpl(base::MessageLoopProxy::current(), weak_core));
+
   // Build a SyncSessionContext and store the worker in it.
   DVLOG(1) << "Sync is bringing up SyncSessionContext.";
   std::vector<SyncEngineEventListener*> listeners;
@@ -1017,9 +1027,9 @@ UserShare* SyncManagerImpl::GetUserShare() {
   return &share_;
 }
 
-base::WeakPtr<syncer::SyncCore> SyncManagerImpl::GetSyncCore() {
+syncer::SyncCoreProxy* SyncManagerImpl::GetSyncCoreProxy() {
   DCHECK(initialized_);
-  return sync_core_->AsWeakPtr();
+  return sync_core_proxy_.get();
 }
 
 const std::string SyncManagerImpl::cache_guid() {
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index 003471c..653e48a 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -19,6 +19,7 @@
 #include "sync/internal_api/js_sync_encryption_handler_observer.h"
 #include "sync/internal_api/js_sync_manager_observer.h"
 #include "sync/internal_api/protocol_event_buffer.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/internal_api/public/sync_manager.h"
 #include "sync/internal_api/public/user_share.h"
 #include "sync/internal_api/sync_encryption_handler_impl.h"
@@ -33,6 +34,7 @@ namespace syncer {
 
 class ModelTypeRegistry;
 class SyncAPIServerConnectionManager;
+class SyncCore;
 class WriteNode;
 class WriteTransaction;
 
@@ -110,7 +112,7 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl :
   virtual void SaveChanges() OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;
   virtual UserShare* GetUserShare() OVERRIDE;
-  virtual base::WeakPtr<syncer::SyncCore> GetSyncCore() OVERRIDE;
+  virtual syncer::SyncCoreProxy* GetSyncCoreProxy() OVERRIDE;
   virtual const std::string cache_guid() OVERRIDE;
   virtual bool ReceivedExperiment(Experiments* experiments) OVERRIDE;
   virtual bool HasUnsyncedItems() OVERRIDE;
@@ -298,8 +300,9 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl :
   // This state changes when entering or exiting a configuration cycle.
   scoped_ptr<ModelTypeRegistry> model_type_registry_;
 
-  // The main interface for non-blocking sync types.
+  // The main interface for non-blocking sync types and a thread-safe wrapper.
   scoped_ptr<SyncCore> sync_core_;
+  scoped_ptr<SyncCoreProxy> sync_core_proxy_;
 
   // A container of various bits of information used by the SyncScheduler to
   // create SyncSessions.  Must outlive the SyncScheduler.
diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc
index 9f1abb8..c7873a8 100644
--- a/sync/internal_api/test/fake_sync_manager.cc
+++ b/sync/internal_api/test/fake_sync_manager.cc
@@ -218,8 +218,8 @@ UserShare* FakeSyncManager::GetUserShare() {
   return test_user_share_.user_share();
 }
 
-base::WeakPtr<syncer::SyncCore> FakeSyncManager::GetSyncCore() {
-  return base::WeakPtr<syncer::SyncCore>();
+syncer::SyncCoreProxy* FakeSyncManager::GetSyncCoreProxy() {
+  return &null_sync_core_proxy_;
 }
 
 const std::string FakeSyncManager::cache_guid() {
diff --git a/sync/internal_api/test/null_sync_core_proxy.cc b/sync/internal_api/test/null_sync_core_proxy.cc
new file mode 100644
index 0000000..866e68b
--- /dev/null
+++ b/sync/internal_api/test/null_sync_core_proxy.cc
@@ -0,0 +1,23 @@
+// 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 "sync/internal_api/public/test/null_sync_core_proxy.h"
+
+namespace syncer {
+
+NullSyncCoreProxy::NullSyncCoreProxy() {}
+
+NullSyncCoreProxy::~NullSyncCoreProxy() {}
+
+void NullSyncCoreProxy::ConnectTypeToCore(
+    syncer::ModelType type,
+      base::WeakPtr<NonBlockingTypeProcessor> processor) {
+  NOTREACHED() << "NullSyncCoreProxy is not meant to be used";
+}
+
+scoped_ptr<SyncCoreProxy> NullSyncCoreProxy::Clone() {
+  return scoped_ptr<SyncCoreProxy>(new NullSyncCoreProxy());
+}
+
+}  // namespace syncer
diff --git a/sync/sync_internal_api.gypi b/sync/sync_internal_api.gypi
index b0a476b..3a572df 100644
--- a/sync/sync_internal_api.gypi
+++ b/sync/sync_internal_api.gypi
@@ -125,6 +125,8 @@
     'internal_api/sync_core.cc',
     'internal_api/sync_core.h',
     'internal_api/sync_core_proxy.cc',
+    'internal_api/sync_core_proxy_impl.cc',
+    'internal_api/sync_core_proxy_impl.h',
     'internal_api/sync_encryption_handler_impl.cc',
     'internal_api/sync_encryption_handler_impl.h',
     'internal_api/sync_manager_factory.cc',
diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi
index 9998438..6841bec 100644
--- a/sync/sync_tests.gypi
+++ b/sync/sync_tests.gypi
@@ -191,11 +191,13 @@
         'internal_api/public/base/invalidation_test_util.cc',
         'internal_api/public/base/invalidation_test_util.h',
         'internal_api/public/test/fake_sync_manager.h',
+        'internal_api/public/test/null_sync_core_proxy.h',
         'internal_api/public/test/sync_manager_factory_for_profile_sync_test.h',
         'internal_api/public/test/test_entry_factory.h',
         'internal_api/public/test/test_internal_components_factory.h',
         'internal_api/public/test/test_user_share.h',
         'internal_api/test/fake_sync_manager.cc',
+        'internal_api/test/null_sync_core_proxy.cc',
         'internal_api/test/sync_manager_factory_for_profile_sync_test.cc',
         'internal_api/test/sync_manager_for_profile_sync_test.cc',
         'internal_api/test/sync_manager_for_profile_sync_test.h',
@@ -418,7 +420,7 @@
           'internal_api/protocol_event_buffer_unittest.cc',
           'internal_api/public/change_record_unittest.cc',
           'internal_api/public/sessions/sync_session_snapshot_unittest.cc',
-          'internal_api/sync_core_proxy_unittest.cc',
+          'internal_api/sync_core_proxy_impl_unittest.cc',
           'internal_api/sync_encryption_handler_impl_unittest.cc',
           'internal_api/sync_manager_impl_unittest.cc',
           'internal_api/syncapi_server_connection_manager_unittest.cc',