[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) {}
 
+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(
     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 135 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);
+  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(
     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();
+    RelayToTaskRunner(context_->GetUiTaskRunner(),
+                      FROM_HERE,
+                      base::Bind(&SyncEngine::NotifyLastOperationStatus,
+                                 base::Unretained(sync_engine_.get())));
   }
 }
 
 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 114 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();
+  RelayToTaskRunner(context_->GetUiTaskRunner(),
+                    FROM_HERE,
+                    base::Bind(&SyncEngine::UpdateRegisteredApps,
+                               base::Unretained(sync_engine_.get())));
 }
 
 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);
+    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() &&
         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);

[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()) {
     RegisterOrigin(syncer->url().origin(),
                    base::Bind(&EmptyStatusCallback));
   }
 
   if (syncer->needs_remote_change_listing() &&
       !listing_remote_changes_) {
     task_manager_->ScheduleSyncTask(
         FROM_HERE,
@@ skipping 120 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);
+
+  RelayToTaskRunner(context_->GetUiTaskRunner(),
+                    FROM_HERE,
+                    base::Bind(&SyncEngine::UpdateServiceState,
+                               base::Unretained(sync_engine_.get())),
+                    description);
 }
 
 }  // namespace drive_backend
 }  // namespace sync_file_system