[SyncFS] Make routines using metadata database async

chrome/browser/sync_file_system/drive_backend/sync_engine.cc

 // Copyright 2013 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_engine.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/metrics/histogram.h"
@@ skipping 94 matching lines @@
       sync_engine_->UpdateServiceState(state, description);
       return;
     }
 
     ui_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&SyncEngine::UpdateServiceState,
                    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_;
 
   DISALLOW_COPY_AND_ASSIGN(WorkerObserver);
 };
 
 namespace {
 
 void EmptyStatusCallback(SyncStatusCode status) {}
@@ skipping 259 matching lines @@
 
 void SyncEngine::SetSyncEnabled(bool enabled) {
   worker_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&SyncWorker::SetSyncEnabled,
                  base::Unretained(sync_worker_.get()),
                  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(
     const FileChange& local_change,
     const base::FilePath& local_path,
     const SyncFileMetadata& local_metadata,
     const fileapi::FileSystemURL& url,
     const SyncStatusCallback& callback) {
   worker_task_runner_->PostTask(
       FROM_HERE,
@@ skipping 48 matching lines @@
 }
 
 drive::DriveServiceInterface* SyncEngine::GetDriveService() {
   return drive_service_.get();
 }
 
 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();
 }
 
 SyncEngine::SyncEngine(
     scoped_ptr<drive::DriveServiceInterface> drive_service,
     scoped_ptr<drive::DriveUploaderInterface> drive_uploader,
     base::SequencedTaskRunner* worker_task_runner,
     drive::DriveNotificationManager* notification_manager,
     ExtensionServiceInterface* extension_service,
     SigninManagerBase* signin_manager)
@@ skipping 27 matching lines @@
 
 void SyncEngine::UpdateServiceState(RemoteServiceState state,
                                     const std::string& description) {
   service_state_ = state;
 
   FOR_EACH_OBSERVER(
       Observer, service_observers_,
       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);
 
   // Update the status of every origin using status from ExtensionService.
   for (std::vector<std::string>::const_iterator itr = app_ids.begin();
        itr != app_ids.end(); ++itr) {
     const std::string& app_id = *itr;
     GURL origin =
         extensions::Extension::GetBaseURLFromExtensionId(app_id);
@@ skipping 16 matching lines @@
         tracker.tracker_kind() == TRACKER_KIND_APP_ROOT;
     if (is_app_enabled && !is_app_root_tracker_enabled)
       EnableOrigin(origin, base::Bind(&EmptyStatusCallback));
     else if (!is_app_enabled && is_app_root_tracker_enabled)
       DisableOrigin(origin, base::Bind(&EmptyStatusCallback));
   }
 }
 
 }  // namespace drive_backend
 }  // namespace sync_file_system