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

5 files changed, 48 insertions, 33 deletions

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_