[SyncFS] Post tasks between SyncEngine and SyncWorker

chrome/browser/sync_file_system/drive_backend/sync_worker.cc

 // 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 "chrome/browser/sync_file_system/drive_backend/sync_worker.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/memory/weak_ptr.h"
@@ skipping 38 matching lines @@
 namespace sync_file_system {
 
 class RemoteChangeProcessor;
 
 namespace drive_backend {
 
 namespace {
 
 void EmptyStatusCallback(SyncStatusCode status) {}
 
+template <typename T>
+void RelayToTaskRunner(
+    base::TaskRunner* task_runner,
+    const tracked_objects::Location& from_here,
+    const base::Callback<void(T)>& callback,
+    typename base::internal::CallbackParamTraits<T>::ForwardType& p1) {
+  task_runner->PostTask(from_here,
+                        base::Bind(callback,
+                                   base::internal::CallbackForward(p1)));
+}
+
+template <typename T1, typename T2>
+void RelayToTaskRunner(
+    base::TaskRunner* task_runner,
+    const tracked_objects::Location& from_here,
+    const base::Callback<void(T1, T2)>& callback,
+    const T1& p1,
+    typename base::internal::CallbackParamTraits<T2>::ForwardType& p2) {
+  task_runner->PostTask(from_here,
+                        base::Bind(callback,
+                                   p1,
+                                   base::internal::CallbackForward(p2)));
+}
+
 }  // namespace
 
 scoped_ptr<SyncWorker> SyncWorker::CreateOnWorker(
     const base::WeakPtr<drive_backend::SyncEngine>& sync_engine,
     const base::FilePath& base_dir,
     scoped_ptr<SyncEngineContext> sync_engine_context,
     leveldb::Env* env_override) {
   scoped_ptr<SyncWorker> sync_worker(
       new SyncWorker(sync_engine,
                      base_dir,
@@ skipping 23 matching lines @@
 
 void SyncWorker::RegisterOrigin(
     const GURL& origin,
     const SyncStatusCallback& callback) {
   if (!GetMetadataDatabase() && GetDriveService()->HasRefreshToken())
     PostInitializeTask();
 
   scoped_ptr<RegisterAppTask> task(
       new RegisterAppTask(context_.get(), origin.host()));
   if (task->CanFinishImmediately()) {
-    callback.Run(SYNC_STATUS_OK);
+    RelayToTaskRunner(context_->GetBlockingTaskRunner(),
+                      FROM_HERE, callback, SYNC_STATUS_OK);

[tzik, 2014/04/17 17:48:32]

All existing RelayToTaskRunner usage in this file can be replaced with PostTask.

GetBlockingTaskRunner() looks not the right place to post the callback from
here.
Could you directly post it to GetUiTaskRunner()?

[peria, 2014/04/18 05:19:36]

Done.
I may have misunderstood the usage of RelayToTaskRunner.
I remove all RelayToTaskRunner in this CL, and add it again in another CL.

     return;
   }
 
   task_manager_->ScheduleSyncTask(
       FROM_HERE,
       task.PassAs<SyncTask>(),
       SyncTaskManager::PRIORITY_HIGH,
       callback);

[tzik, 2014/04/17 17:48:32]

This callback is called on the worker thread.
It's not necessary to be done in this CL, but we need to wrap this callback by
RelayToTaskRunner.

[peria, 2014/04/18 05:19:36]

It may be done in the next CL.
I put TODO comments instead.

 }
 
 void SyncWorker::EnableOrigin(
     const GURL& origin,
     const SyncStatusCallback& callback) {
   task_manager_->ScheduleTask(
       FROM_HERE,
       base::Bind(&SyncWorker::DoEnableApp,
                  weak_ptr_factory_.GetWeakPtr(),
                  origin.host()),
@@ skipping 83 matching lines @@
 
 void SyncWorker::SetSyncEnabled(bool enabled) {
   if (sync_enabled_ == enabled)
     return;
 
   RemoteServiceState old_state = GetCurrentState();
   sync_enabled_ = enabled;
   if (old_state == GetCurrentState())
     return;
 
-  // TODO(peria): PostTask()
-  sync_engine_->UpdateSyncEnabled(enabled);
+  context_->GetUiTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncEngine::UpdateSyncEnabled, sync_engine_, enabled));
 }
 
 SyncStatusCode SyncWorker::SetDefaultConflictResolutionPolicy(
     ConflictResolutionPolicy policy) {
   default_conflict_resolution_policy_ = policy;
   return SYNC_STATUS_OK;
 }
 
 SyncStatusCode SyncWorker::SetConflictResolutionPolicy(
     const GURL& origin,
@@ skipping 41 matching lines @@
 
   MaybeStartFetchChanges();
 }
 
 void SyncWorker::NotifyLastOperationStatus(
     SyncStatusCode status,
     bool used_network) {
   UpdateServiceStateFromSyncStatusCode(status, used_network);
 
   if (GetMetadataDatabase()) {
-    // TODO(peria): Post task
-    sync_engine_->NotifyLastOperationStatus();
+    context_->GetUiTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&SyncEngine::NotifyLastOperationStatus, sync_engine_));
   }
 }
 
 void SyncWorker::OnNotificationReceived() {
   if (service_state_ == REMOTE_SERVICE_TEMPORARY_UNAVAILABLE)
     UpdateServiceState(REMOTE_SERVICE_OK, "Got push notification for Drive.");
 
   should_check_remote_change_ = true;
   MaybeScheduleNextTask();
 }
@@ skipping 64 matching lines @@
       sync_enabled_(false),
       default_conflict_resolution_policy_(
           CONFLICT_RESOLUTION_POLICY_LAST_WRITE_WIN),
       network_available_(false),
       context_(sync_engine_context.Pass()),
       sync_engine_(sync_engine),
       weak_ptr_factory_(this) {}
 
 void SyncWorker::DoDisableApp(const std::string& app_id,
                               const SyncStatusCallback& callback) {
-  if (GetMetadataDatabase())
+  if (GetMetadataDatabase()) {
     GetMetadataDatabase()->DisableApp(app_id, callback);
-  else
-    callback.Run(SYNC_STATUS_OK);
+  } else {
+    RelayToTaskRunner(context_->GetBlockingTaskRunner(),
+                      FROM_HERE, callback, SYNC_STATUS_OK);
+  }
 }
 
 void SyncWorker::DoEnableApp(const std::string& app_id,
                              const SyncStatusCallback& callback) {
-  if (GetMetadataDatabase())
+  if (GetMetadataDatabase()) {
     GetMetadataDatabase()->EnableApp(app_id, callback);
-  else
-    callback.Run(SYNC_STATUS_OK);
+  } else {
+    RelayToTaskRunner(context_->GetBlockingTaskRunner(),
+                      FROM_HERE, callback, SYNC_STATUS_OK);
+  }
 }
 
 void SyncWorker::PostInitializeTask() {
   DCHECK(!GetMetadataDatabase());
 
   // This initializer task may not run if MetadataDatabase in context_ is
   // already initialized when it runs.
   SyncEngineInitializer* initializer =
       new SyncEngineInitializer(context_.get(),
                                 context_->GetBlockingTaskRunner(),
@@ skipping 18 matching lines @@
                          "Authentication required.");
     }
     return;
   }
 
   scoped_ptr<MetadataDatabase> metadata_database =
       initializer->PassMetadataDatabase();
   if (metadata_database)
     context_->SetMetadataDatabase(metadata_database.Pass());
 
-  // TODO(peria): Post task
-  sync_engine_->UpdateRegisteredApps();
+  context_->GetUiTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncEngine::UpdateRegisteredApps, sync_engine_));
 }
 
 void SyncWorker::DidProcessRemoteChange(RemoteToLocalSyncer* syncer,
                                         const SyncFileCallback& callback,
                                         SyncStatusCode status) {
   if (syncer->is_sync_root_deletion()) {
     MetadataDatabase::ClearDatabase(context_->PassMetadataDatabase());
     PostInitializeTask();
-    callback.Run(status, syncer->url());
+    RelayToTaskRunner(context_->GetUiTaskRunner(),
+                      FROM_HERE,
+                      callback,
+                      status,
+                      syncer->url());
     return;
   }
 
   if (status == SYNC_STATUS_OK) {
-    // TODO(peria): Post task
-    sync_engine_->DidProcessRemoteChange(syncer);
+    context_->GetUiTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&SyncEngine::DidProcessRemoteChange, sync_engine_, syncer));
 
     if (syncer->sync_action() == SYNC_ACTION_DELETED &&
         syncer->url().is_valid() &&
         fileapi::VirtualPath::IsRootPath(syncer->url().path())) {
       RegisterOrigin(syncer->url().origin(), base::Bind(&EmptyStatusCallback));
     }
     should_check_conflict_ = true;
   }
-  callback.Run(status, syncer->url());
+
+  RelayToTaskRunner(context_->GetUiTaskRunner(),
+                    FROM_HERE,
+                    callback,
+                    status,
+                    syncer->url());
 }
 
 void SyncWorker::DidApplyLocalChange(LocalToRemoteSyncer* syncer,
                                      const SyncStatusCallback& callback,
                                      SyncStatusCode status) {
-  // TODO(peria): Post task
+  // TODO(peria): PostTask (Simple replace fails DriveBackendSync* tests)
   sync_engine_->DidApplyLocalChange(syncer, status);
 
   if (status == SYNC_STATUS_UNKNOWN_ORIGIN && syncer->url().is_valid()) {
     RegisterOrigin(syncer->url().origin(),
                    base::Bind(&EmptyStatusCallback));
   }
 
   if (syncer->needs_remote_change_listing() &&
       !listing_remote_changes_) {
     task_manager_->ScheduleSyncTask(
         FROM_HERE,
         scoped_ptr<SyncTask>(new ListChangesTask(context_.get())),
         SyncTaskManager::PRIORITY_HIGH,
         base::Bind(&SyncWorker::DidFetchChanges,
                    weak_ptr_factory_.GetWeakPtr()));
     should_check_remote_change_ = false;
     listing_remote_changes_ = true;
     time_to_check_changes_ =
         base::TimeTicks::Now() +
         base::TimeDelta::FromSeconds(kListChangesRetryDelaySeconds);
   }
 
-  if (status != SYNC_STATUS_OK &&
-      status != SYNC_STATUS_NO_CHANGE_TO_SYNC) {
-    callback.Run(status);
-    return;
-  }
-
   if (status == SYNC_STATUS_OK)
     should_check_conflict_ = true;
 
-  callback.Run(status);
+  RelayToTaskRunner(context_->GetBlockingTaskRunner(),
+                    FROM_HERE, callback, status);
 }
 
 void SyncWorker::MaybeStartFetchChanges() {
   if (GetCurrentState() == REMOTE_SERVICE_DISABLED)
     return;
 
   if (!GetMetadataDatabase())
     return;
 
   if (listing_remote_changes_)
@@ skipping 89 matching lines @@
                                     const std::string& description) {
   RemoteServiceState old_state = GetCurrentState();
   service_state_ = state;
 
   if (old_state == GetCurrentState())
     return;
 
   util::Log(logging::LOG_VERBOSE, FROM_HERE,
             "Service state changed: %d->%d: %s",
             old_state, GetCurrentState(), description.c_str());
-  // TODO(peria): Post task
-  sync_engine_->UpdateServiceState(description);
+
+  context_->GetUiTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncEngine::UpdateServiceState, sync_engine_, description));
 }
 
 }  // namespace drive_backend
 }  // namespace sync_file_system