USS SyncContextProxy / data type activation refactoring

This refactoring prepares the code to introduction of
NonBlockingDataTypeController for USS datatypes. The goal
was to split the very large NonBlockingDataTypeController
change into a couple of smaller to make it easier to review
and verify.

The following changes are included here:
1) Introduced ActivationContext which is a structure that
   contains all arguments needed to activate a USS datatype.
   For now ActivationContext is passed via SyncContext /
   SyncContextProxy, but the goal is to pass it directly
   via BackendDataTypeConfigurer as an argument for
   ActivateNonBlockingDataType.
   ActivationContext is needed as a separate class because
   NonBlockingDataTypeController will have to receive it
   from the type processor's callback and temporarily hold
   on to it.
2) BackendDataTypeConfigurer - two activation methods are
   renamed to be directory specific and two more activation
   methods for non-blocking data types are added.
3) DataTypeController cleanup - OnModelLoaded() virtual
   method shouldn't be on the base class because it applies
   only to some of the subclasses and is never invoked via the
   base class.

BUG=515962

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

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

chrome/browser/sync/glue/non_frontend_data_type_controller.cc

@@ -203,10 +203,6 @@ void NonFrontendDataTypeController::LoadModels(
   model_load_callback.Run(type(), syncer::SyncError());
 }
 
-void NonFrontendDataTypeController::OnModelLoaded() {
-  NOTREACHED();
-}
-
 void NonFrontendDataTypeController::StartAssociating(
     const StartCallback& start_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);

chrome/browser/sync/glue/non_frontend_data_type_controller.h

@@ -92,9 +92,6 @@ class NonFrontendDataTypeController
 
   ~NonFrontendDataTypeController() override;
 
-  // DataTypeController interface.
-  void OnModelLoaded() override;
-
   // Start any dependent services that need to be running before we can
   // associate models. The default implementation is a no-op.
   // Return value:

chrome/browser/sync/glue/non_frontend_data_type_controller_mock.h

@@ -19,7 +19,6 @@ class NonFrontendDataTypeControllerMock : public NonFrontendDataTypeController {
   MOCK_METHOD1(StartAssociating,
                  void(const StartCallback& start_callback));
   MOCK_METHOD1(LoadModels, void(const ModelLoadCallback& model_load_callback));
-  MOCK_METHOD0(OnModelLoaded, void());
 
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD0(enabled, bool());

chrome/browser/sync/glue/sync_backend_host_impl.cc

@@ -25,6 +25,7 @@
 #include "components/sync_driver/sync_prefs.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
+#include "sync/internal_api/public/activation_context.h"
 #include "sync/internal_api/public/base_transaction.h"
 #include "sync/internal_api/public/events/protocol_event.h"
 #include "sync/internal_api/public/http_bridge.h"
@@ -452,16 +453,28 @@ void SyncBackendHostImpl::EnableEncryptEverything() {
       base::Bind(&SyncBackendHostCore::DoEnableEncryptEverything, core_.get()));
 }
 
-void SyncBackendHostImpl::ActivateDataType(
-    syncer::ModelType type, syncer::ModelSafeGroup group,
+void SyncBackendHostImpl::ActivateDirectoryDataType(
+    syncer::ModelType type,
+    syncer::ModelSafeGroup group,
     sync_driver::ChangeProcessor* change_processor) {
   registrar_->ActivateDataType(type, group, change_processor, GetUserShare());
 }
 
-void SyncBackendHostImpl::DeactivateDataType(syncer::ModelType type) {
+void SyncBackendHostImpl::DeactivateDirectoryDataType(syncer::ModelType type) {
   registrar_->DeactivateDataType(type);
 }
 
+void SyncBackendHostImpl::ActivateNonBlockingDataType(
+    syncer::ModelType type,
+    scoped_ptr<syncer_v2::ActivationContext> activation_context) {
+  sync_context_proxy_->ConnectTypeToSync(type, activation_context.Pass());
+}
+
+void SyncBackendHostImpl::DeactivateNonBlockingDataType(
+    syncer::ModelType type) {
+  sync_context_proxy_->Disconnect(type);
+}
+
 syncer::UserShare* SyncBackendHostImpl::GetUserShare() const {
   return core_->sync_manager()->GetUserShare();
 }

chrome/browser/sync/glue/sync_backend_host_impl.h

@@ -112,11 +112,15 @@ class SyncBackendHostImpl
       const base::Callback<void(syncer::ModelTypeSet, syncer::ModelTypeSet)>&
           ready_task,
       const base::Callback<void()>& retry_callback) override;
-  void ActivateDataType(
+  void ActivateDirectoryDataType(
       syncer::ModelType type,
       syncer::ModelSafeGroup group,
       sync_driver::ChangeProcessor* change_processor) override;
-  void DeactivateDataType(syncer::ModelType type) override;
+  void DeactivateDirectoryDataType(syncer::ModelType type) override;
+  void ActivateNonBlockingDataType(
+      syncer::ModelType type,
+      scoped_ptr<syncer_v2::ActivationContext>) override;
+  void DeactivateNonBlockingDataType(syncer::ModelType type) override;
   void EnableEncryptEverything() override;
   syncer::UserShare* GetUserShare() const override;
   scoped_ptr<syncer_v2::SyncContextProxy> GetSyncContextProxy() override;

chrome/browser/sync/glue/sync_backend_host_mock.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/sync/glue/sync_backend_host_mock.h"
 
 #include "components/sync_driver/sync_frontend.h"
+#include "sync/internal_api/public/activation_context.h"
 
 namespace browser_sync {
 
@@ -70,10 +71,18 @@ syncer::ModelTypeSet SyncBackendHostMock::ConfigureDataTypes(
 
 void SyncBackendHostMock::EnableEncryptEverything() {}
 
-void SyncBackendHostMock::ActivateDataType(
-    syncer::ModelType type, syncer::ModelSafeGroup group,
+void SyncBackendHostMock::ActivateDirectoryDataType(
+    syncer::ModelType type,
+    syncer::ModelSafeGroup group,
     sync_driver::ChangeProcessor* change_processor) {}
-void SyncBackendHostMock::DeactivateDataType(syncer::ModelType type) {}
+void SyncBackendHostMock::DeactivateDirectoryDataType(syncer::ModelType type) {}
+
+void SyncBackendHostMock::ActivateNonBlockingDataType(
+    syncer::ModelType type,
+    scoped_ptr<syncer_v2::ActivationContext> activation_context) {}
+
+void SyncBackendHostMock::DeactivateNonBlockingDataType(
+    syncer::ModelType type) {}
 
 syncer::UserShare* SyncBackendHostMock::GetUserShare() const {
   return NULL;

chrome/browser/sync/glue/sync_backend_host_mock.h

@@ -67,11 +67,16 @@ class SyncBackendHostMock : public SyncBackendHost {
 
   void EnableEncryptEverything() override;
 
-  void ActivateDataType(
+  void ActivateDirectoryDataType(
       syncer::ModelType type,
       syncer::ModelSafeGroup group,
       sync_driver::ChangeProcessor* change_processor) override;
-  void DeactivateDataType(syncer::ModelType type) override;
+  void DeactivateDirectoryDataType(syncer::ModelType type) override;
+
+  void ActivateNonBlockingDataType(
+      syncer::ModelType type,
+      scoped_ptr<syncer_v2::ActivationContext>) override;
+  void DeactivateNonBlockingDataType(syncer::ModelType type) override;
 
   syncer::UserShare* GetUserShare() const override;
 

chrome/browser/sync/profile_sync_service.cc

@@ -93,7 +93,6 @@
 #include "sync/internal_api/public/network_resources.h"
 #include "sync/internal_api/public/sessions/type_debug_info_observer.h"
 #include "sync/internal_api/public/shutdown_reason.h"
-#include "sync/internal_api/public/sync_context_proxy.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/util/sync_db_util.h"

components/sync_driver/backend_data_type_configurer.h

@@ -12,6 +12,10 @@
 #include "sync/internal_api/public/configure_reason.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 
+namespace syncer_v2 {
+struct ActivationContext;
+}
+
 namespace sync_driver {
 
 class ChangeProcessor;
@@ -61,16 +65,25 @@ class BackendDataTypeConfigurer {
   static syncer::ModelTypeSet GetDataTypesInState(
       DataTypeConfigState state, const DataTypeConfigStateMap& state_map);
 
-  // Activates change processing for the given data type.  This must
+  // Activates change processing for the given directory data type.  This must
   // be called synchronously with the data type's model association so
   // no changes are dropped between model association and change
   // processor activation.
-  virtual void ActivateDataType(
-      syncer::ModelType type, syncer::ModelSafeGroup group,
-      ChangeProcessor* change_processor) = 0;
+  virtual void ActivateDirectoryDataType(syncer::ModelType type,
+                                         syncer::ModelSafeGroup group,
+                                         ChangeProcessor* change_processor) = 0;
 
   // Deactivates change processing for the given data type.
-  virtual void DeactivateDataType(syncer::ModelType type) = 0;
+  virtual void DeactivateDirectoryDataType(syncer::ModelType type) = 0;
+
+  // Activates change processing for the given non-blocking data type.
+  // This must be called synchronously with the data type's model association.
+  virtual void ActivateNonBlockingDataType(
+      syncer::ModelType type,
+      scoped_ptr<syncer_v2::ActivationContext> activation_context) = 0;
+
+  // Deactivates change processing for the given non-blocking data type.
+  virtual void DeactivateNonBlockingDataType(syncer::ModelType type) = 0;
 
   // Set state of |types| in |state_map| to |state|.
   static void SetDataTypesState(DataTypeConfigState state,

components/sync_driver/data_type_controller.h

@@ -149,11 +149,6 @@ class DataTypeController
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread,
       const base::Closure& error_callback);
 
-  // If the DTC is waiting for models to load, once the models are
-  // loaded the datatype service will call this function on DTC to let
-  // us know that it is safe to start associating.
-  virtual void OnModelLoaded() = 0;
-
   ~DataTypeController() override;
 
   // The callback that will be invoked when an unrecoverable error occurs.

components/sync_driver/data_type_manager_impl_unittest.cc

@@ -11,6 +11,7 @@
 #include "components/sync_driver/data_type_manager_observer.h"
 #include "components/sync_driver/data_type_status_table.h"
 #include "components/sync_driver/fake_data_type_controller.h"
+#include "sync/internal_api/public/activation_context.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/configure_reason.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -102,15 +103,25 @@ class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer {
     return ready_types_;
   }
 
-  void ActivateDataType(syncer::ModelType type,
-                        syncer::ModelSafeGroup group,
-                        ChangeProcessor* change_processor) override {
+  void ActivateDirectoryDataType(syncer::ModelType type,
+                                 syncer::ModelSafeGroup group,
+                                 ChangeProcessor* change_processor) override {
     activated_types_.Put(type);
   }
-  void DeactivateDataType(syncer::ModelType type) override {
+  void DeactivateDirectoryDataType(syncer::ModelType type) override {
     activated_types_.Remove(type);
   }
 
+  void ActivateNonBlockingDataType(
+      syncer::ModelType type,
+      scoped_ptr<syncer_v2::ActivationContext> activation_context) override {
+    // TODO (stanisc): crbug.com/515962: Add test coverage.
+  }
+
+  void DeactivateNonBlockingDataType(syncer::ModelType type) override {
+    // TODO (stanisc): crbug.com/515962: Add test coverage.
+  }
+
   base::Callback<void(ModelTypeSet, ModelTypeSet)> last_ready_task() const {
     return last_ready_task_;
   }

components/sync_driver/directory_data_type_controller.cc

@@ -19,13 +19,13 @@ void DirectoryDataTypeController::ActivateDataType(
     BackendDataTypeConfigurer* configurer) {
   // Tell the backend about the change processor for this type so it can
   // begin routing changes to it.
-  configurer->ActivateDataType(type(), model_safe_group(),
-                               GetChangeProcessor());
+  configurer->ActivateDirectoryDataType(type(), model_safe_group(),
+                                        GetChangeProcessor());
 }
 
 void DirectoryDataTypeController::DeactivateDataType(
     BackendDataTypeConfigurer* configurer) {
-  configurer->DeactivateDataType(type());
+  configurer->DeactivateDirectoryDataType(type());
 }
 
 }  // namespace sync_driver

components/sync_driver/fake_data_type_controller.cc

@@ -44,10 +44,6 @@ void FakeDataTypeController::LoadModels(
   }
 }
 
-void FakeDataTypeController::OnModelLoaded() {
-  NOTREACHED();
-}
-
 // MODEL_LOADED -> MODEL_STARTING.
 void FakeDataTypeController::StartAssociating(
    const StartCallback& start_callback) {

components/sync_driver/fake_data_type_controller.h

@@ -24,7 +24,6 @@ class FakeDataTypeController : public DirectoryDataTypeController {
   explicit FakeDataTypeController(syncer::ModelType type);
 
   void LoadModels(const ModelLoadCallback& model_load_callback) override;
-  void OnModelLoaded() override;
   void StartAssociating(const StartCallback& start_callback) override;
   void Stop() override;
   syncer::ModelType type() const override;

components/sync_driver/frontend_data_type_controller.h

@@ -80,9 +80,6 @@ class FrontendDataTypeController
   // Datatype specific creation of sync components.
   virtual void CreateSyncComponents() = 0;
 
-  // DataTypeController interface.
-  void OnModelLoaded() override;
-
   // Perform any DataType controller specific state cleanup before stopping
   // the datatype controller. The default implementation is a no-op.
   virtual void CleanUpState();
@@ -111,6 +108,11 @@ class FrontendDataTypeController
       const tracked_objects::Location& from_here,
       const std::string& message);
 
+  // If the DTC is waiting for models to load, once the models are
+  // loaded the datatype service will call this function on DTC to let
+  // us know that it is safe to start associating.
+  void OnModelLoaded();
+
   sync_driver::SyncClient* const sync_client_;
 
   State state_;

components/sync_driver/frontend_data_type_controller_mock.h

@@ -19,7 +19,6 @@ class FrontendDataTypeControllerMock : public FrontendDataTypeController {
   MOCK_METHOD1(StartAssociating,
                void(const StartCallback& start_callback));
   MOCK_METHOD1(LoadModels, void(const ModelLoadCallback& model_load_callback));
-  MOCK_METHOD0(OnModelLoaded, void());
 
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD0(enabled, bool());

components/sync_driver/non_blocking_data_type_controller_unittest.cc

@@ -10,9 +10,11 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/test/test_simple_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "components/sync_driver/non_blocking_data_type_controller.h"
 #include "sync/engine/commit_queue.h"
 #include "sync/engine/model_type_processor_impl.h"
+#include "sync/internal_api/public/activation_context.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/sync_context_proxy.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -81,20 +83,16 @@ class MockSyncContextProxy : public syncer_v2::SyncContextProxy {
 
   void ConnectTypeToSync(
       syncer::ModelType type,
-      const syncer_v2::DataTypeState& data_type_state,
-      const syncer_v2::UpdateResponseDataList& saved_pending_updates,
-      const base::WeakPtr<syncer_v2::ModelTypeProcessor>& type_processor)
-      override {
+      scoped_ptr<syncer_v2::ActivationContext> activation_context) override {
     // Normally we'd use ThreadTaskRunnerHandle::Get() as the TaskRunner
     // argument
     // to Connect().  That won't work here in this test, so we use the
     // model_task_runner_ that was injected for this purpose instead.
-    sync_task_runner_->PostTask(FROM_HERE,
-                                base::Bind(&MockSyncContext::Connect,
-                                           base::Unretained(mock_sync_context_),
-                                           type,
-                                           model_task_runner_,
-                                           type_processor));
+    sync_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&MockSyncContext::Connect,
+                   base::Unretained(mock_sync_context_), type,
+                   model_task_runner_, activation_context->type_processor));
   }
 
   void Disconnect(syncer::ModelType type) override {
@@ -123,6 +121,7 @@ class NonBlockingDataTypeControllerTest : public testing::Test {
       : type_processor_(syncer::DICTIONARY,
                         base::WeakPtr<syncer_v2::ModelTypeStore>()),
         model_thread_(new base::TestSimpleTaskRunner()),
+        model_thread_handle_(model_thread_),
         sync_thread_(new base::TestSimpleTaskRunner()),
         controller_(syncer::DICTIONARY, true),
         mock_context_proxy_(&mock_sync_context_, model_thread_, sync_thread_),
@@ -185,6 +184,7 @@ class NonBlockingDataTypeControllerTest : public testing::Test {
  protected:
   syncer_v2::ModelTypeProcessorImpl type_processor_;
   scoped_refptr<base::TestSimpleTaskRunner> model_thread_;
+  base::ThreadTaskRunnerHandle model_thread_handle_;
   scoped_refptr<base::TestSimpleTaskRunner> sync_thread_;
 
   NonBlockingDataTypeController controller_;

components/sync_driver/non_ui_data_type_controller.h

@@ -5,8 +5,6 @@
 #ifndef COMPONENTS_SYNC_DRIVER_NON_UI_DATA_TYPE_CONTROLLER_H_
 #define COMPONENTS_SYNC_DRIVER_NON_UI_DATA_TYPE_CONTROLLER_H_
 
-#include <string>
-
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
@@ -48,9 +46,6 @@ class NonUIDataTypeController : public DirectoryDataTypeController {
   // DataTypeController is RefCounted.
   ~NonUIDataTypeController() override;
 
-  // DataTypeController interface.
-  void OnModelLoaded() override;
-
   // Start any dependent services that need to be running before we can
   // associate models. The default implementation is a no-op.
   // Return value:
@@ -99,6 +94,11 @@ class NonUIDataTypeController : public DirectoryDataTypeController {
   // and shutdown, use a factory method to create the SharedChangeProcessor.
   virtual SharedChangeProcessor* CreateSharedChangeProcessor();
 
+  // If the DTC is waiting for models to load, once the models are
+  // loaded the datatype service will call this function on DTC to let
+  // us know that it is safe to start associating.
+  void OnModelLoaded();
+
  private:
 
   // Posted on the backend thread by StartAssociationAsync().

components/sync_driver/non_ui_data_type_controller_mock.h

@@ -20,7 +20,6 @@ class NonUIDataTypeControllerMock
   MOCK_METHOD1(StartAssociating,
                  void(const StartCallback& start_callback));
   MOCK_METHOD1(LoadModels, void(const ModelLoadCallback& model_load_callback));
-  MOCK_METHOD0(OnModelLoaded, void());
   MOCK_METHOD0(Stop, void());
   MOCK_CONST_METHOD0(type, syncer::ModelType());
   MOCK_CONST_METHOD0(name, std::string());