[SyncFS] Make routines using metadata database async

chrome/browser/sync_file_system/drive_backend/sync_engine.cc

Index: chrome/browser/sync_file_system/drive_backend/sync_engine.cc
diff --git a/chrome/browser/sync_file_system/drive_backend/sync_engine.cc b/chrome/browser/sync_file_system/drive_backend/sync_engine.cc
index 9dc0c8181bcdcc349ff1e47a07d705178251241a..780eed74b761ae8bc3e3df450d04410dfe99f156 100644
--- a/chrome/browser/sync_file_system/drive_backend/sync_engine.cc
+++ b/chrome/browser/sync_file_system/drive_backend/sync_engine.cc
@@ -112,6 +112,13 @@ class SyncEngine::WorkerObserver : public SyncWorker::Observer {
                    sync_engine_, state, description));
   }
 
+  virtual void OnPromoteDemotedChanges(size_t num_trackers) OVERRIDE {
+    ui_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&SyncEngine::DidPromoteDemotedChanges,
+                   sync_engine_, num_trackers));
+  }
+
  private:
   scoped_refptr<base::SequencedTaskRunner> ui_task_runner_;
   base::WeakPtr<SyncEngine> sync_engine_;
@@ -391,14 +398,10 @@ void SyncEngine::SetSyncEnabled(bool enabled) {
 }
 
 void SyncEngine::PromoteDemotedChanges() {
-  MetadataDatabase* metadata_db = GetMetadataDatabase();
-  if (metadata_db && metadata_db->HasLowPriorityDirtyTracker()) {
-    metadata_db->PromoteLowerPriorityTrackersToNormal();
-    FOR_EACH_OBSERVER(
-        Observer,
-        service_observers_,
-        OnRemoteChangeQueueUpdated(metadata_db->CountDirtyTracker()));
-  }
+  worker_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncWorker::PromoteDemotedChanges,
+                 base::Unretained(sync_worker_.get())));
 }
 
 void SyncEngine::ApplyLocalChange(
@@ -467,8 +470,7 @@ drive::DriveUploaderInterface* SyncEngine::GetDriveUploader() {
   return drive_uploader_.get();
 }
 
-MetadataDatabase* SyncEngine::GetMetadataDatabase() {
-  // TODO(peria): Post task
+MetadataDatabase* SyncEngine::GetMetadataDatabaseForTesting() {

[nhiroki, 2014/05/29 01:56:13]

In tests, callers of this function need to ensure that the worker is suspended
while accessing the database on the main thread. Is it right?

I guess this might cause flakiness on multiple-threads tests because future test
implementers forget the assumption. I'm not sure what the best solution is, but
at least can we have a comment about it?

[peria, 2014/05/29 02:15:29]

I think it wrong.
In such a case, sync worker and DB will run independently, in racy and flaky
condition.
I agree.
I think it good to handle DB through methods of SyncEngine, even in tests.

[nhiroki, 2014/05/29 04:12:58]

Sorry, "... ensure that the worker is suspended ..." is confusing. I mean to say
"... ensure that the worker isn't working on any tasks which interact with the
database ...". Does this make sense?

   return sync_worker_->GetMetadataDatabase();
 }
 
@@ -516,11 +518,18 @@ void SyncEngine::UpdateServiceState(RemoteServiceState state,
       OnRemoteServiceStateUpdated(state, description));
 }
 
-void SyncEngine::UpdateRegisteredApps() {
+void SyncEngine::DidPromoteDemotedChanges(size_t num_trackers) {

[tzik, 2014/05/29 02:24:25]

Please merge this to OnPendingFileListUpdated

[peria, 2014/05/29 03:11:46]

Done.

+  FOR_EACH_OBSERVER(
+      Observer,
+      service_observers_,
+      OnRemoteChangeQueueUpdated(num_trackers));
+}
+
+void SyncEngine::UpdateRegisteredAppsForTesting() {
   if (!extension_service_)
     return;
 
-  MetadataDatabase* metadata_db = GetMetadataDatabase();
+  MetadataDatabase* metadata_db = GetMetadataDatabaseForTesting();
   DCHECK(metadata_db);
   std::vector<std::string> app_ids;
   metadata_db->GetRegisteredAppIDs(&app_ids);