[SyncFS] Run LocalToRemoteSyncer as SyncTask

 * Replace base class of LocalToRemoteSyncer with SyncTask
 * Add testing SyncTaskToken.
 * Clean up LocalToRemoteSyncer implementation.

BUG=344769

Review URL: https://codereview.chromium.org/297803011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272494 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync_file_system/drive_backend/conflict_resolver_unittest.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/sync_file_system/drive_backend/sync_engine_context.h"
 #include "chrome/browser/sync_file_system/drive_backend/sync_engine_initializer.h"
 #include "chrome/browser/sync_file_system/drive_backend/sync_task_manager.h"
+#include "chrome/browser/sync_file_system/drive_backend/sync_task_token.h"
 #include "chrome/browser/sync_file_system/fake_remote_change_processor.h"
 #include "chrome/browser/sync_file_system/sync_file_system_test_util.h"
 #include "chrome/browser/sync_file_system/syncable_file_system_util.h"
@@ -202,7 +203,8 @@ class ConflictResolverTest : public testing::Test {
         context_.get(),
         SyncFileMetadata(file_change.file_type(), 0, base::Time()),
         file_change, local_path, url));
-    syncer->RunExclusive(CreateResultReceiver(&status));
+    syncer->RunPreflight(SyncTaskToken::CreateForTesting(
+        CreateResultReceiver(&status)));
     base::RunLoop().RunUntilIdle();
     if (status == SYNC_STATUS_OK)
       remote_change_processor_->ClearLocalChanges(url);

chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer.cc

@@ -21,6 +21,8 @@
 #include "chrome/browser/sync_file_system/drive_backend/metadata_database.h"
 #include "chrome/browser/sync_file_system/drive_backend/metadata_database.pb.h"
 #include "chrome/browser/sync_file_system/drive_backend/sync_engine_context.h"
+#include "chrome/browser/sync_file_system/drive_backend/sync_task_manager.h"
+#include "chrome/browser/sync_file_system/drive_backend/sync_task_token.h"
 #include "chrome/browser/sync_file_system/logger.h"
 #include "google_apis/drive/drive_api_parser.h"
 #include "webkit/common/fileapi/file_system_util.h"
@@ -74,19 +76,24 @@ LocalToRemoteSyncer::LocalToRemoteSyncer(SyncEngineContext* sync_context,
 LocalToRemoteSyncer::~LocalToRemoteSyncer() {
 }
 
-void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
+void LocalToRemoteSyncer::RunPreflight(scoped_ptr<SyncTaskToken> token) {
+  scoped_ptr<BlockingFactor> blocking_factor(new BlockingFactor);
+  blocking_factor->exclusive = true;
+  SyncTaskManager::UpdateBlockingFactor(
+      token.Pass(), blocking_factor.Pass(),
+      base::Bind(&LocalToRemoteSyncer::RunExclusive,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void LocalToRemoteSyncer::RunExclusive(scoped_ptr<SyncTaskToken> token) {
   if (!IsContextReady()) {
     util::Log(logging::LOG_VERBOSE, FROM_HERE,
               "[Local -> Remote] Context not ready.");
     NOTREACHED();
-    callback.Run(SYNC_STATUS_FAILED);
+    SyncTaskManager::NotifyTaskDone(token.Pass(), SYNC_STATUS_FAILED);
     return;
   }
 
-  SyncStatusCallback wrapped_callback = base::Bind(
-      &LocalToRemoteSyncer::SyncCompleted, weak_ptr_factory_.GetWeakPtr(),
-      callback);
-
   util::Log(logging::LOG_VERBOSE, FROM_HERE,
             "[Local -> Remote] Start: %s on %s@%s %s",
             local_change_.DebugString().c_str(),
@@ -98,7 +105,7 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
     // Stray file, we can just return.
     util::Log(logging::LOG_VERBOSE, FROM_HERE,
               "[Local -> Remote]: Missing file for non-delete change");
-    callback.Run(SYNC_STATUS_OK);
+    SyncTaskManager::NotifyTaskDone(token.Pass(), SYNC_STATUS_OK);
     return;
   }
 
@@ -113,7 +120,7 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
     // The app is disabled or not registered.
     util::Log(logging::LOG_VERBOSE, FROM_HERE,
               "[Local -> Remote]: App is disabled or not registered");
-    callback.Run(SYNC_STATUS_UNKNOWN_ORIGIN);
+    SyncTaskManager::NotifyTaskDone(token.Pass(), SYNC_STATUS_UNKNOWN_ORIGIN);
     return;
   }
   DCHECK(active_ancestor_tracker->active());
@@ -131,12 +138,10 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
   if (active_ancestor_path.empty()) {
     missing_entries = path;
   } else if (active_ancestor_path != path) {
-    bool should_success = active_ancestor_path.AppendRelativePath(
-        path, &missing_entries);
-    if (!should_success) {
+    if (!active_ancestor_path.AppendRelativePath(path, &missing_entries)) {
       NOTREACHED() << "[Local -> Remote]: Detected invalid ancestor: "
                    << active_ancestor_path.value();
-      callback.Run(SYNC_STATUS_FAILED);
+      SyncTaskManager::NotifyTaskDone(token.Pass(), SYNC_STATUS_FAILED);
       return;
     }
   }
@@ -154,11 +159,15 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
 
       // Local file is deleted and remote file is missing, already deleted or
       // not yet synced.  There is nothing to do for the file.
-      callback.Run(SYNC_STATUS_OK);
+      SyncTaskManager::NotifyTaskDone(token.Pass(), SYNC_STATUS_OK);
       return;
     }
   }
 
+  SyncStatusCallback callback = base::Bind(
+      &LocalToRemoteSyncer::SyncCompleted, weak_ptr_factory_.GetWeakPtr(),
+      base::Passed(&token));
+
   if (missing_components.size() > 1) {
     // The original target doesn't have remote file and parent.
     // Try creating the parent first.
@@ -167,7 +176,7 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
       target_path_ = active_ancestor_path.Append(missing_components[0]);
       util::Log(logging::LOG_VERBOSE, FROM_HERE,
                 "[Local -> Remote]: Detected missing parent folder.");
-      CreateRemoteFolder(wrapped_callback);
+      CreateRemoteFolder(callback);
       return;
     }
 
@@ -181,8 +190,7 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
               "[Local -> Remote]: Detected non-folder file in its path.");
     DeleteRemoteFile(base::Bind(&LocalToRemoteSyncer::DidDeleteForCreateFolder,
                                 weak_ptr_factory_.GetWeakPtr(),
-                                wrapped_callback));
-
+                                callback));
     return;
   }
 
@@ -200,12 +208,12 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
                 "[Local -> Remote]: Detected conflicting dirty tracker:%"
                 PRId64, remote_file_tracker_->tracker_id());
       // Both local and remote file has pending modification.
-      HandleConflict(wrapped_callback);
+      HandleConflict(callback);
       return;
     }
 
     // Non-conflicting file/folder update case.
-    HandleExistingRemoteFile(wrapped_callback);
+    HandleExistingRemoteFile(callback);
     return;
   }
 
@@ -220,15 +228,15 @@ void LocalToRemoteSyncer::RunExclusive(const SyncStatusCallback& callback) {
   if (local_change_.file_type() == SYNC_FILE_TYPE_FILE) {
     util::Log(logging::LOG_VERBOSE, FROM_HERE,
               "[Local -> Remote]: Detected a new file.");
-    UploadNewFile(wrapped_callback);
+    UploadNewFile(callback);
     return;
   }
   util::Log(logging::LOG_VERBOSE, FROM_HERE,
             "[Local -> Remote]: Detected a new folder.");
-  CreateRemoteFolder(wrapped_callback);
+  CreateRemoteFolder(callback);
 }
 
-void LocalToRemoteSyncer::SyncCompleted(const SyncStatusCallback& callback,
+void LocalToRemoteSyncer::SyncCompleted(scoped_ptr<SyncTaskToken> token,
                                         SyncStatusCode status) {
   if (status == SYNC_STATUS_OK && target_path_ != url_.path())
     status = SYNC_STATUS_RETRY;
@@ -243,7 +251,7 @@ void LocalToRemoteSyncer::SyncCompleted(const SyncStatusCallback& callback,
             target_path_.AsUTF8Unsafe().c_str(),
             url_.origin().host().c_str());
 
-  callback.Run(status);
+  SyncTaskManager::NotifyTaskDone(token.Pass(), status);
 }
 
 void LocalToRemoteSyncer::HandleConflict(const SyncStatusCallback& callback) {
@@ -265,9 +273,8 @@ void LocalToRemoteSyncer::HandleConflict(const SyncStatusCallback& callback) {
   DCHECK(local_change_.IsDirectory());
   // Check if we can reuse the remote folder.
   FileMetadata remote_file_metadata;
-  bool should_success = metadata_database()->FindFileByFileID(
-      remote_file_tracker_->file_id(), &remote_file_metadata);
-  if (!should_success) {
+  if (!metadata_database()->FindFileByFileID(
+          remote_file_tracker_->file_id(), &remote_file_metadata)) {
     NOTREACHED();
     CreateRemoteFolder(callback);
     return;
@@ -463,9 +470,8 @@ void LocalToRemoteSyncer::DidUpdateDatabaseForUploadExistingFile(
   }
 
   FileMetadata file;
-  bool should_success = metadata_database()->FindFileByFileID(
-      remote_file_tracker_->file_id(), &file);
-  if (!should_success) {
+  if (!metadata_database()->FindFileByFileID(
+          remote_file_tracker_->file_id(), &file)) {
     NOTREACHED();
     callback.Run(SYNC_STATUS_FAILED);
     return;

chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer.h

@@ -38,7 +38,7 @@ class FolderCreator;
 class MetadataDatabase;
 class SyncEngineContext;
 
-class LocalToRemoteSyncer : public ExclusiveTask {
+class LocalToRemoteSyncer : public SyncTask {
  public:
   LocalToRemoteSyncer(SyncEngineContext* sync_context,
                       const SyncFileMetadata& local_metadata,
@@ -46,7 +46,8 @@ class LocalToRemoteSyncer : public ExclusiveTask {
                       const base::FilePath& local_path,
                       const fileapi::FileSystemURL& url);
   virtual ~LocalToRemoteSyncer();
-  virtual void RunExclusive(const SyncStatusCallback& callback) OVERRIDE;
+  virtual void RunPreflight(scoped_ptr<SyncTaskToken> token) OVERRIDE;
+  void RunExclusive(scoped_ptr<SyncTaskToken> token);
 
   const fileapi::FileSystemURL& url() const { return url_; }
   const base::FilePath& target_path() const { return target_path_; }
@@ -56,7 +57,7 @@ class LocalToRemoteSyncer : public ExclusiveTask {
   }
 
  private:
-  void SyncCompleted(const SyncStatusCallback& callback,
+  void SyncCompleted(scoped_ptr<SyncTaskToken> token,
                      SyncStatusCode status);
 
   void HandleConflict(const SyncStatusCallback& callback);

chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer_unittest.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/sync_file_system/drive_backend/sync_engine_context.h"
 #include "chrome/browser/sync_file_system/drive_backend/sync_engine_initializer.h"
 #include "chrome/browser/sync_file_system/drive_backend/sync_task_manager.h"
+#include "chrome/browser/sync_file_system/drive_backend/sync_task_token.h"
 #include "chrome/browser/sync_file_system/fake_remote_change_processor.h"
 #include "chrome/browser/sync_file_system/sync_file_system_test_util.h"
 #include "chrome/browser/sync_file_system/syncable_file_system_util.h"
@@ -169,7 +170,8 @@ class LocalToRemoteSyncerTest : public testing::Test {
         context_.get(),
         SyncFileMetadata(file_change.file_type(), 0, base::Time()),
         file_change, local_path, url));
-    syncer->RunExclusive(CreateResultReceiver(&status));
+    syncer->RunPreflight(SyncTaskToken::CreateForTesting(
+        CreateResultReceiver(&status)));
     base::RunLoop().RunUntilIdle();
     return status;
   }

chrome/browser/sync_file_system/drive_backend/sync_task_manager.cc

@@ -124,6 +124,14 @@ void SyncTaskManager::NotifyTaskDone(scoped_ptr<SyncTaskToken> token,
   DCHECK(token);
 
   SyncTaskManager* manager = token->manager();
+  if (token->token_id() == SyncTaskToken::kTestingTaskTokenID) {
+    DCHECK(!manager);
+    SyncStatusCallback callback = token->callback();
+    token->clear_callback();
+    callback.Run(status);
+    return;
+  }
+
   if (manager)
     manager->NotifyTaskDoneBody(token.Pass(), status);
 }
@@ -136,6 +144,12 @@ void SyncTaskManager::UpdateBlockingFactor(
   DCHECK(current_task_token);
 
   SyncTaskManager* manager = current_task_token->manager();
+  if (current_task_token->token_id() == SyncTaskToken::kTestingTaskTokenID) {
+    DCHECK(!manager);
+    continuation.Run(current_task_token.Pass());
+    return;
+  }
+
   if (!manager)
     return;
 

chrome/browser/sync_file_system/drive_backend/sync_task_token.cc

@@ -11,16 +11,28 @@
 namespace sync_file_system {
 namespace drive_backend {
 
+const int64 SyncTaskToken::kTestingTaskTokenID = -1;
 const int64 SyncTaskToken::kForegroundTaskTokenID = 0;
 const int64 SyncTaskToken::kMinimumBackgroundTaskTokenID = 1;
 
+// static
+scoped_ptr<SyncTaskToken> SyncTaskToken::CreateForTesting(
+    const SyncStatusCallback& callback) {
+  return make_scoped_ptr(new SyncTaskToken(
+      base::WeakPtr<SyncTaskManager>(),
+      kTestingTaskTokenID,
+      scoped_ptr<BlockingFactor>(),
+      callback));
+}
+
 // static
 scoped_ptr<SyncTaskToken> SyncTaskToken::CreateForForegroundTask(
     const base::WeakPtr<SyncTaskManager>& manager) {
   return make_scoped_ptr(new SyncTaskToken(
       manager,
       kForegroundTaskTokenID,
-      scoped_ptr<BlockingFactor>()));
+      scoped_ptr<BlockingFactor>(),
+      SyncStatusCallback()));
 }
 
 // static
@@ -31,7 +43,8 @@ scoped_ptr<SyncTaskToken> SyncTaskToken::CreateForBackgroundTask(
   return make_scoped_ptr(new SyncTaskToken(
       manager,
       token_id,
-      blocking_factor.Pass()));
+      blocking_factor.Pass(),
+      SyncStatusCallback()));
 }
 
 void SyncTaskToken::UpdateTask(const tracked_objects::Location& location,
@@ -55,7 +68,8 @@ SyncTaskToken::~SyncTaskToken() {
     // Reinitializes the token.
     SyncTaskManager::NotifyTaskDone(
         make_scoped_ptr(new SyncTaskToken(
-            manager_, token_id_, blocking_factor_.Pass())),
+            manager_, token_id_, blocking_factor_.Pass(),
+            SyncStatusCallback())),
         SYNC_STATUS_OK);
   }
 }
@@ -81,9 +95,11 @@ void SyncTaskToken::clear_blocking_factor() {
 
 SyncTaskToken::SyncTaskToken(const base::WeakPtr<SyncTaskManager>& manager,
                              int64 token_id,
-                             scoped_ptr<BlockingFactor> blocking_factor)
+                             scoped_ptr<BlockingFactor> blocking_factor,
+                             const SyncStatusCallback& callback)
     : manager_(manager),
       token_id_(token_id),
+      callback_(callback),
       blocking_factor_(blocking_factor.Pass()) {
 }
 

chrome/browser/sync_file_system/drive_backend/sync_task_token.h

@@ -21,9 +21,12 @@ struct BlockingFactor;
 // should run exclusively, and held by SyncTaskManager when no task is running.
 class SyncTaskToken {
  public:
+  static const int64 kTestingTaskTokenID;
   static const int64 kForegroundTaskTokenID;
   static const int64 kMinimumBackgroundTaskTokenID;
 
+  static scoped_ptr<SyncTaskToken> CreateForTesting(
+      const SyncStatusCallback& callback);
   static scoped_ptr<SyncTaskToken> CreateForForegroundTask(
       const base::WeakPtr<SyncTaskManager>& manager);
   static scoped_ptr<SyncTaskToken> CreateForBackgroundTask(
@@ -53,7 +56,8 @@ class SyncTaskToken {
  private:
   SyncTaskToken(const base::WeakPtr<SyncTaskManager>& manager,
                 int64 token_id,
-                scoped_ptr<BlockingFactor> blocking_factor);
+                scoped_ptr<BlockingFactor> blocking_factor,
+                const SyncStatusCallback& callback);
 
   base::WeakPtr<SyncTaskManager> manager_;
   tracked_objects::Location location_;