Adopt OnceCallback for OnSyncStarting()

The callback must be called at most once, so let's migrate away from
deprecated APIs and adopt base::OnceCallback.

Bug: 681921
Change-Id: I2cc0fb6b75771b4a203584154178a1a897a305b8
Reviewed-on: https://chromium-review.googlesource.com/1012063Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550976}

components/sync/base/bind_to_task_runner.h

@@ -42,25 +42,39 @@ struct BindToTaskRunnerTrampoline;
 template <typename... Args>
 struct BindToTaskRunnerTrampoline<void(Args...)> {
   static void Run(const scoped_refptr<base::TaskRunner>& task_runner,
-                  const base::Callback<void(Args...)>& cb,
+                  base::OnceCallback<void(Args...)> cb,
                   Args... args) {
-    task_runner->PostTask(FROM_HERE,
-                          base::BindOnce(cb, std::forward<Args>(args)...));
+    task_runner->PostTask(
+        FROM_HERE, base::BindOnce(std::move(cb), std::forward<Args>(args)...));
   }
 };
 
 }  // namespace bind_helpers
 
 template <typename T>
-base::Callback<T> BindToTaskRunner(
+base::OnceCallback<T> BindToTaskRunner(
     const scoped_refptr<base::TaskRunner>& task_runner,
-    const base::Callback<T>& cb) {
-  return base::Bind(&bind_helpers::BindToTaskRunnerTrampoline<T>::Run,
-                    task_runner, cb);
+    base::OnceCallback<T> cb) {
+  return base::BindOnce(&bind_helpers::BindToTaskRunnerTrampoline<T>::Run,
+                        task_runner, std::move(cb));
 }
 
 template <typename T>
-base::Callback<T> BindToCurrentThread(const base::Callback<T>& cb) {
+base::RepeatingCallback<T> BindToTaskRunner(
+    const scoped_refptr<base::TaskRunner>& task_runner,
+    const base::RepeatingCallback<T>& cb) {
+  return base::BindRepeating(&bind_helpers::BindToTaskRunnerTrampoline<T>::Run,
+                             task_runner, cb);
+}
+
+template <typename T>
+base::OnceCallback<T> BindToCurrentThread(base::OnceCallback<T> cb) {
+  return BindToTaskRunner(base::ThreadTaskRunnerHandle::Get(), std::move(cb));
+}
+
+template <typename T>
+base::RepeatingCallback<T> BindToCurrentThread(
+    const base::RepeatingCallback<T>& cb) {
   return BindToTaskRunner(base::ThreadTaskRunnerHandle::Get(), cb);
 }
 

components/sync/driver/model_type_controller.cc

@@ -27,10 +27,9 @@ using BridgeTask = ModelTypeController::BridgeTask;
 
 namespace {
 
-void OnSyncStartingHelper(
-    const ModelErrorHandler& error_handler,
-    const ModelTypeChangeProcessor::StartCallback& callback,
-    ModelTypeSyncBridge* bridge) {
+void OnSyncStartingHelper(const ModelErrorHandler& error_handler,
+                          ModelTypeChangeProcessor::StartCallback callback,
+                          ModelTypeSyncBridge* bridge) {
   bridge->OnSyncStarting(std::move(error_handler), std::move(callback));
 }
 
@@ -96,8 +95,8 @@ void ModelTypeController::LoadModels(
 
   // Callback that posts back to the UI thread.
   ModelTypeChangeProcessor::StartCallback callback =
-      BindToCurrentThread(base::Bind(&ModelTypeController::OnProcessorStarted,
-                                     base::AsWeakPtr(this)));
+      BindToCurrentThread(base::BindOnce(
+          &ModelTypeController::OnProcessorStarted, base::AsWeakPtr(this)));
 
   ModelErrorHandler error_handler = base::BindRepeating(
       &ReportError, type(), base::ThreadTaskRunnerHandle::Get(),
@@ -106,8 +105,8 @@ void ModelTypeController::LoadModels(
 
   // Start the type processor on the model thread.
   PostBridgeTask(FROM_HERE,
-                 base::Bind(&OnSyncStartingHelper, std::move(error_handler),
-                            std::move(callback)));
+                 base::BindOnce(&OnSyncStartingHelper, std::move(error_handler),
+                                std::move(callback)));
 }
 
 void ModelTypeController::BeforeLoadModels(ModelTypeConfigurer* configurer) {}
@@ -208,7 +207,8 @@ void ModelTypeController::Stop() {
       sync_prefs_.GetPreferredDataTypes(ModelTypeSet(type()));
   if ((state() == MODEL_LOADED || state() == RUNNING) &&
       (!sync_prefs_.IsFirstSetupComplete() || !preferred_types.Has(type()))) {
-    PostBridgeTask(FROM_HERE, base::Bind(&ModelTypeSyncBridge::DisableSync));
+    PostBridgeTask(FROM_HERE,
+                   base::BindOnce(&ModelTypeSyncBridge::DisableSync));
   }
 
   state_ = NOT_RUNNING;
@@ -219,19 +219,21 @@ DataTypeController::State ModelTypeController::state() const {
 }
 
 void ModelTypeController::GetAllNodes(const AllNodesCallback& callback) {
-  PostBridgeTask(FROM_HERE, base::Bind(&ModelTypeDebugInfo::GetAllNodes,
-                                       BindToCurrentThread(callback)));
+  PostBridgeTask(FROM_HERE, base::BindOnce(&ModelTypeDebugInfo::GetAllNodes,
+                                           BindToCurrentThread(callback)));
 }
 
 void ModelTypeController::GetStatusCounters(
     const StatusCountersCallback& callback) {
-  PostBridgeTask(FROM_HERE,
-                 base::Bind(&ModelTypeDebugInfo::GetStatusCounters, callback));
+  PostBridgeTask(
+      FROM_HERE,
+      base::BindOnce(&ModelTypeDebugInfo::GetStatusCounters, callback));
 }
 
 void ModelTypeController::RecordMemoryUsageHistogram() {
-  PostBridgeTask(FROM_HERE,
-                 base::Bind(&ModelTypeDebugInfo::RecordMemoryUsageHistogram));
+  PostBridgeTask(
+      FROM_HERE,
+      base::BindOnce(&ModelTypeDebugInfo::RecordMemoryUsageHistogram));
 }
 
 void ModelTypeController::ReportModelError(const ModelError& error) {

components/sync/driver/model_type_controller_unittest.cc

@@ -49,7 +49,7 @@ class TestModelTypeProcessor : public FakeModelTypeChangeProcessor,
 
   // ModelTypeChangeProcessor implementation.
   void OnSyncStarting(const ModelErrorHandler& error_handler,
-                      const StartCallback& callback) override {
+                      StartCallback callback) override {
     std::unique_ptr<ActivationContext> activation_context =
         std::make_unique<ActivationContext>();
     activation_context->model_type_state.set_initial_sync_done(
@@ -57,7 +57,7 @@ class TestModelTypeProcessor : public FakeModelTypeChangeProcessor,
     activation_context->type_processor =
         std::make_unique<ModelTypeProcessorProxy>(
             weak_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get());
-    callback.Run(std::move(activation_context));
+    std::move(callback).Run(std::move(activation_context));
   }
   void DisableSync() override { (*disable_sync_call_count_)++; }
 

components/sync/model/fake_model_type_change_processor.cc

@@ -4,6 +4,8 @@
 
 #include "components/sync/model/fake_model_type_change_processor.h"
 
+#include <utility>
+
 #include "base/callback.h"
 #include "base/memory/ptr_util.h"
 #include "components/sync/model/metadata_batch.h"
@@ -48,9 +50,9 @@ void FakeModelTypeChangeProcessor::ModelReadyToSync(
 
 void FakeModelTypeChangeProcessor::OnSyncStarting(
     const ModelErrorHandler& error_handler,
-    const StartCallback& callback) {
+    StartCallback callback) {
   if (!callback.is_null()) {
-    callback.Run(nullptr);
+    std::move(callback).Run(nullptr);
   }
 }
 

components/sync/model/fake_model_type_change_processor.h

@@ -38,7 +38,7 @@ class FakeModelTypeChangeProcessor : public ModelTypeChangeProcessor {
   void ModelReadyToSync(ModelTypeSyncBridge* bridge,
                         std::unique_ptr<MetadataBatch> batch) override;
   void OnSyncStarting(const ModelErrorHandler& error_handler,
-                      const StartCallback& callback) override;
+                      StartCallback callback) override;
   void DisableSync() override;
   bool IsTrackingMetadata() override;
   void ReportError(const ModelError& error) override;

components/sync/model/mock_model_type_change_processor.cc

@@ -47,8 +47,8 @@ class ForwardingModelTypeChangeProcessor : public ModelTypeChangeProcessor {
   }
 
   void OnSyncStarting(const ModelErrorHandler& error_handler,
-                      const StartCallback& callback) override {
-    other_->OnSyncStarting(error_handler, callback);
+                      StartCallback callback) override {
+    other_->OnSyncStarting(error_handler, std::move(callback));
   }
 
   void DisableSync() override { other_->DisableSync(); }
@@ -82,6 +82,12 @@ void MockModelTypeChangeProcessor::ModelReadyToSync(
   DoModelReadyToSync(bridge, batch.get());
 }
 
+void MockModelTypeChangeProcessor::OnSyncStarting(
+    const ModelErrorHandler& error_handler,
+    StartCallback callback) {
+  DoOnSyncStarting(error_handler, &callback);
+}
+
 std::unique_ptr<ModelTypeChangeProcessor>
 MockModelTypeChangeProcessor::CreateForwardingProcessor() {
   return base::WrapUnique<ModelTypeChangeProcessor>(

components/sync/model/mock_model_type_change_processor.h

@@ -42,9 +42,13 @@ class MockModelTypeChangeProcessor : public ModelTypeChangeProcessor {
                void(ModelTypeSyncBridge* bridge, MetadataBatch* batch));
   void ModelReadyToSync(ModelTypeSyncBridge* bridge,
                         std::unique_ptr<MetadataBatch> batch) override;
-  MOCK_METHOD2(OnSyncStarting,
+  // TODO(crbug.com/729950): Use unique_ptr here directly once move-only
+  // arguments are supported in gMock.
+  MOCK_METHOD2(DoOnSyncStarting,
                void(const ModelErrorHandler& error_handler,
-                    const StartCallback& callback));
+                    StartCallback* callback));
+  void OnSyncStarting(const ModelErrorHandler& error_handler,
+                      StartCallback callback) override;
   MOCK_METHOD0(DisableSync, void());
   MOCK_METHOD0(IsTrackingMetadata, bool());
   MOCK_METHOD1(ReportError, void(const ModelError& error));
@@ -52,6 +56,9 @@ class MockModelTypeChangeProcessor : public ModelTypeChangeProcessor {
   // Returns a processor that forwards all calls to
   // |this|. |*this| must outlive the returned processor.
   std::unique_ptr<ModelTypeChangeProcessor> CreateForwardingProcessor();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockModelTypeChangeProcessor);
 };
 
 }  //  namespace syncer

components/sync/model/model_type_change_processor.h

@@ -25,7 +25,7 @@ class ModelTypeSyncBridge;
 class ModelTypeChangeProcessor {
  public:
   using StartCallback =
-      base::Callback<void(std::unique_ptr<ActivationContext>)>;
+      base::OnceCallback<void(std::unique_ptr<ActivationContext>)>;
 
   ModelTypeChangeProcessor();
   virtual ~ModelTypeChangeProcessor();
@@ -77,7 +77,7 @@ class ModelTypeChangeProcessor {
   // processor. If an error is encountered, |error_handler| should be called
   // and |callback| should not.
   virtual void OnSyncStarting(const ModelErrorHandler& error_handler,
-                              const StartCallback& callback) = 0;
+                              StartCallback callback) = 0;
 
   // Indicates that sync is being disabled permanently for this data type. All
   // metadata should be erased from storage.

components/sync/model/model_type_sync_bridge.cc

@@ -35,8 +35,9 @@ ConflictResolution ModelTypeSyncBridge::ResolveConflict(
 
 void ModelTypeSyncBridge::OnSyncStarting(
     const ModelErrorHandler& error_handler,
-    const ModelTypeChangeProcessor::StartCallback& start_callback) {
-  change_processor_->OnSyncStarting(std::move(error_handler), start_callback);
+    ModelTypeChangeProcessor::StartCallback start_callback) {
+  change_processor_->OnSyncStarting(std::move(error_handler),
+                                    std::move(start_callback));
 }
 
 void ModelTypeSyncBridge::DisableSync() {

components/sync/model/model_type_sync_bridge.h

@@ -128,9 +128,8 @@ class ModelTypeSyncBridge : public base::SupportsWeakPtr<ModelTypeSyncBridge> {
   // before the processor can be connected to a sync worker. Once the
   // metadata has been loaded, the info is collected and given to |callback|.
   // When overriding, the bridge must notify the processor.
-  virtual void OnSyncStarting(
-      const ModelErrorHandler& error_handler,
-      const ModelTypeChangeProcessor::StartCallback& callback);
+  virtual void OnSyncStarting(const ModelErrorHandler& error_handler,
+                              ModelTypeChangeProcessor::StartCallback callback);
 
   // Indicates that we no longer want to do any sync-related things for this
   // data type. Severs all ties to the sync thread, deletes all local sync

components/sync/model/model_type_sync_bridge_unittest.cc

@@ -41,7 +41,7 @@ class ModelTypeSyncBridgeTest : public ::testing::Test {
 
 // OnSyncStarting should create a processor and call OnSyncStarting on it.
 TEST_F(ModelTypeSyncBridgeTest, OnSyncStarting) {
-  EXPECT_CALL(*processor(), OnSyncStarting(_, _));
+  EXPECT_CALL(*processor(), DoOnSyncStarting(_, _));
   OnSyncStarting();
 }
 

components/sync/model_impl/client_tag_based_model_type_processor.cc

@@ -69,7 +69,7 @@ ClientTagBasedModelTypeProcessor::~ClientTagBasedModelTypeProcessor() {
 
 void ClientTagBasedModelTypeProcessor::OnSyncStarting(
     const ModelErrorHandler& error_handler,
-    const StartCallback& start_callback) {
+    StartCallback start_callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!IsConnected());
   DCHECK(error_handler);
@@ -77,7 +77,7 @@ void ClientTagBasedModelTypeProcessor::OnSyncStarting(
   DVLOG(1) << "Sync is starting for " << ModelTypeToString(type_);
 
   error_handler_ = error_handler;
-  start_callback_ = start_callback;
+  start_callback_ = std::move(start_callback);
   ConnectIfReady();
 }
 
@@ -135,7 +135,7 @@ void ClientTagBasedModelTypeProcessor::ConnectIfReady() {
         std::make_unique<ModelTypeProcessorProxy>(
             weak_ptr_factory_.GetWeakPtr(),
             base::ThreadTaskRunnerHandle::Get());
-    start_callback_.Run(std::move(activation_context));
+    std::move(start_callback_).Run(std::move(activation_context));
   }
 
   start_callback_.Reset();

components/sync/model_impl/client_tag_based_model_type_processor.h

@@ -68,7 +68,7 @@ class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
   void ModelReadyToSync(ModelTypeSyncBridge* bridge,
                         std::unique_ptr<MetadataBatch> batch) override;
   void OnSyncStarting(const ModelErrorHandler& error_handler,
-                      const StartCallback& callback) override;
+                      StartCallback callback) override;
   void DisableSync() override;
   bool IsTrackingMetadata() override;
   void ReportError(const ModelError& error) override;

components/sync/user_events/user_event_sync_bridge.cc

@@ -155,9 +155,10 @@ std::string UserEventSyncBridge::GetStorageKey(const EntityData& entity_data) {
 
 void UserEventSyncBridge::OnSyncStarting(
     const ModelErrorHandler& error_handler,
-    const ModelTypeChangeProcessor::StartCallback& start_callback) {
+    ModelTypeChangeProcessor::StartCallback start_callback) {
   DCHECK(!GetAuthenticatedAccountId().empty());
-  change_processor()->OnSyncStarting(std::move(error_handler), start_callback);
+  change_processor()->OnSyncStarting(std::move(error_handler),
+                                     std::move(start_callback));
 
   bool was_sync_starting_or_started = is_sync_starting_or_started_;
   is_sync_starting_or_started_ = true;

components/sync/user_events/user_event_sync_bridge.h

@@ -45,7 +45,7 @@ class UserEventSyncBridge : public ModelTypeSyncBridge {
   std::string GetStorageKey(const EntityData& entity_data) override;
   void OnSyncStarting(
       const ModelErrorHandler& error_handler,
-      const ModelTypeChangeProcessor::StartCallback& callback) override;
+      ModelTypeChangeProcessor::StartCallback callback) override;
   void ApplyDisableSyncChanges(
       std::unique_ptr<MetadataChangeList> delete_metadata_change_list) override;