[SyncFS] Post tasks between SyncEngine and SyncWorker

chrome/browser/sync_file_system/drive_backend/sync_worker.cc

Index: chrome/browser/sync_file_system/drive_backend/sync_worker.cc
diff --git a/chrome/browser/sync_file_system/drive_backend/sync_worker.cc b/chrome/browser/sync_file_system/drive_backend/sync_worker.cc
index f58c66c3f83b268e4f5dfd2fb23dea0744b4bd74..d1b9ea8664f041f9194d1f79acecf7579680c4db 100644
--- a/chrome/browser/sync_file_system/drive_backend/sync_worker.cc
+++ b/chrome/browser/sync_file_system/drive_backend/sync_worker.cc
@@ -56,6 +56,46 @@ namespace {
 
 void EmptyStatusCallback(SyncStatusCode status) {}
 
+void RelayToTaskRunner(base::TaskRunner* task_runner,

[peria, 2014/04/16 05:56:12]

I have to merge some RelayToTaskRunner().

+                       const tracked_objects::Location& from_here,
+                       const base::Callback<void(void)>& callback) {

[tzik, 2014/04/16 17:21:19]

s/base::Callback<void(void)>/base::Closure/

[peria, 2014/04/17 01:40:19]

Done.

+  task_runner->PostTask(from_here, callback);
+}
+
+template <typename T>
+void RelayToTaskRunner(base::TaskRunner* task_runner,
+                       const tracked_objects::Location& from_here,
+                       const base::Callback<void(T)>& callback,
+                       const T& p1) {

[tzik, 2014/04/16 17:21:19]

# I'll revisit later.

This probably needs base::internal::CallbackParamTraits to avoid 2^n overload.

[tzik, 2014/04/16 18:26:21]

Whether we should do it or not, we can do it by:
s/const T& p1/typename base::internal::CallbackParamTraits<T>::ForwardType/
s/base::Bind(callback, p1)/base::Bind(callback,
base::internal::CallbackForward(p1))/

[peria, 2014/04/17 01:40:19]

Done.

+  task_runner->PostTask(from_here, base::Bind(callback, p1));
+}
+
+template <typename T>
+void RelayToTaskRunner(base::TaskRunner* task_runner,
+                       const tracked_objects::Location& from_here,
+                       const base::Callback<void(const T& p1)>& callback,
+                       const T& p1) {
+  task_runner->PostTask(from_here, base::Bind(callback, p1));
+}
+
+template <typename T1, typename T2>
+void RelayToTaskRunner(base::TaskRunner* task_runner,
+                       const tracked_objects::Location& from_here,
+                       const base::Callback<void(T1, const T2&)>& callback,
+                       const T1& p1,
+                       const T2& p2) {
+  task_runner->PostTask(from_here, base::Bind(callback, p1, p2));
+}
+
+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,
+                       const T2& p2) {
+  task_runner->PostTask(from_here, base::Bind(callback, p1, p2));
+}
+
 }  // namespace
 
 scoped_ptr<SyncWorker> SyncWorker::CreateOnWorker(
@@ -211,8 +251,11 @@ void SyncWorker::SetSyncEnabled(bool enabled) {
   if (old_state == GetCurrentState())
     return;
 
-  // TODO(peria): PostTask()
-  sync_engine_->UpdateSyncEnabled(enabled);
+  RelayToTaskRunner(context_->GetUiTaskRunner(),
+                    FROM_HERE,
+                    base::Bind(&SyncEngine::UpdateSyncEnabled,
+                               base::Unretained(sync_engine_.get())),

[tzik, 2014/04/16 17:21:19]

Pass WeakPtr directly instead of raw pointer, since SyncWorker is going to
outlive SyncEngine.

[tzik, 2014/04/16 17:42:20]

BTW, just ui_task_runner->PostTask() seems to work for this case.

[peria, 2014/04/17 01:40:19]

Done.

+                    enabled);
 }
 
 SyncStatusCode SyncWorker::SetDefaultConflictResolutionPolicy(
@@ -274,8 +317,10 @@ void SyncWorker::NotifyLastOperationStatus(
   UpdateServiceStateFromSyncStatusCode(status, used_network);
 
   if (GetMetadataDatabase()) {
-    // TODO(peria): Post task
-    sync_engine_->NotifyLastOperationStatus();
+    RelayToTaskRunner(context_->GetUiTaskRunner(),
+                      FROM_HERE,
+                      base::Bind(&SyncEngine::NotifyLastOperationStatus,
+                                 base::Unretained(sync_engine_.get())));
   }
 }
 
@@ -410,8 +455,10 @@ void SyncWorker::DidInitialize(SyncEngineInitializer* initializer,
   if (metadata_database)
     context_->SetMetadataDatabase(metadata_database.Pass());
 
-  // TODO(peria): Post task
-  sync_engine_->UpdateRegisteredApps();
+  RelayToTaskRunner(context_->GetUiTaskRunner(),
+                    FROM_HERE,
+                    base::Bind(&SyncEngine::UpdateRegisteredApps,
+                               base::Unretained(sync_engine_.get())));
 }
 
 void SyncWorker::DidProcessRemoteChange(RemoteToLocalSyncer* syncer,
@@ -420,13 +467,20 @@ void SyncWorker::DidProcessRemoteChange(RemoteToLocalSyncer* syncer,
   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);
+    RelayToTaskRunner(context_->GetUiTaskRunner(),
+                      FROM_HERE,
+                      base::Bind(&SyncEngine::DidProcessRemoteChange,
+                                 base::Unretained(sync_engine_.get())),
+                      syncer);
 
     if (syncer->sync_action() == SYNC_ACTION_DELETED &&
         syncer->url().is_valid() &&
@@ -435,13 +489,18 @@ void SyncWorker::DidProcessRemoteChange(RemoteToLocalSyncer* syncer,
     }
     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);

[peria, 2014/04/16 05:56:12]

This method needs more than replacing with RelayToTaskRunner.

 
   if (status == SYNC_STATUS_UNKNOWN_ORIGIN && syncer->url().is_valid()) {
@@ -583,8 +642,12 @@ void SyncWorker::UpdateServiceState(RemoteServiceState state,
   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);
+
+  RelayToTaskRunner(context_->GetUiTaskRunner(),
+                    FROM_HERE,
+                    base::Bind(&SyncEngine::UpdateServiceState,
+                               base::Unretained(sync_engine_.get())),
+                    description);
 }
 
 }  // namespace drive_backend