[SyncFS] Make DriveBackendSyncTest run in multi-threaded env.

chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.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 <algorithm>
 #include <stack>
 
 #include "base/file_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
@@ skipping 21 matching lines @@
 #include "third_party/leveldatabase/src/include/leveldb/env.h"
 #include "webkit/browser/fileapi/file_system_context.h"
 
 #define FPL(a) FILE_PATH_LITERAL(a)
 
 namespace sync_file_system {
 namespace drive_backend {
 
 typedef fileapi::FileSystemOperation::FileEntryList FileEntryList;
 
+namespace {
+
+void SetSyncStatus(const base::Closure& closure,
+                   SyncStatusCode* status_out,
+                   SyncStatusCode status_in) {

[nhiroki, 2014/05/26 09:56:00]

In syncfs, we haven't used "_in" suffix for input arguments. I don't have a
strong opinion about this, but generally it seems to be a good practice to go
along with existing code. Wdyt?

[peria, 2014/05/27 02:40:12]

Done.
Going along with current code is better, I think.

+  *status_out = status_in;
+  closure.Run();
+}
+
+void SetSyncStatusAndUrl(const base::Closure& closure,
+                         SyncStatusCode* status_out,
+                         fileapi::FileSystemURL* url_out,
+                         SyncStatusCode status_in,
+                         const fileapi::FileSystemURL& url_in) {
+  *status_out = status_in;
+  *url_out = url_in;
+  closure.Run();
+}
+
+}  // namespace
+
 class DriveBackendSyncTest : public testing::Test,
                              public LocalFileSyncService::Observer,
                              public RemoteFileSyncService::Observer {
  public:
   DriveBackendSyncTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
         pending_remote_changes_(0),
         pending_local_changes_(0) {}
   virtual ~DriveBackendSyncTest() {}
 
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(base_dir_.CreateUniqueTempDir());
     in_memory_env_.reset(leveldb::NewMemEnv(leveldb::Env::Default()));
 
+    scoped_refptr<base::SequencedWorkerPool> worker_pool(
+        content::BrowserThread::GetBlockingPool());
+

[nhiroki, 2014/05/26 09:56:00]

Can you move this to line 83 or just call
content::BrowserThread::GetBlockingPool()->GetSequenced...?

[peria, 2014/05/27 02:40:12]

Done.

     io_task_runner_ = content::BrowserThread::GetMessageLoopProxyForThread(
         content::BrowserThread::IO);
     file_task_runner_ = content::BrowserThread::GetMessageLoopProxyForThread(
         content::BrowserThread::FILE);
+    worker_task_runner_ =
+        worker_pool->GetSequencedTaskRunnerWithShutdownBehavior(
+            worker_pool->GetSequenceToken(),
+            base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
 
     RegisterSyncableFileSystem();
     local_sync_service_ = LocalFileSyncService::CreateForTesting(
         &profile_, in_memory_env_.get());
     local_sync_service_->AddChangeObserver(this);
 
     scoped_ptr<drive::FakeDriveService>
         drive_service(new drive::FakeDriveService);
     drive_service->Initialize("test@example.com");
     ASSERT_TRUE(drive::test_util::SetUpTestEntries(drive_service.get()));
 
     scoped_ptr<drive::DriveUploaderInterface> uploader(
         new drive::DriveUploader(drive_service.get(),
                                  file_task_runner_.get()));
 
     fake_drive_service_helper_.reset(new FakeDriveServiceHelper(
         drive_service.get(), uploader.get(),
         kSyncRootFolderTitle));
 
     remote_sync_service_.reset(new SyncEngine(
         drive_service.PassAs<drive::DriveServiceInterface>(),
         uploader.Pass(),
         file_task_runner_.get(),
         NULL, NULL, NULL));
     remote_sync_service_->AddServiceObserver(this);
     remote_sync_service_->Initialize(base_dir_.path(),
-                                     base::MessageLoopProxy::current(),
+                                     worker_task_runner_.get(),
                                      in_memory_env_.get());
     remote_sync_service_->SetSyncEnabled(true);
 
     local_sync_service_->SetLocalChangeProcessor(remote_sync_service_.get());
     remote_sync_service_->SetRemoteChangeProcessor(local_sync_service_.get());
   }
 
   virtual void TearDown() OVERRIDE {
     typedef std::map<std::string, CannedSyncableFileSystem*>::iterator iterator;
     for (iterator itr = file_systems_.begin();
@@ skipping 57 matching lines @@
 
   SyncStatusCode RegisterApp(const std::string& app_id) {
     GURL origin = extensions::Extension::GetBaseURLFromExtensionId(app_id);
     if (!ContainsKey(file_systems_, app_id)) {
       CannedSyncableFileSystem* file_system = new CannedSyncableFileSystem(
           origin, in_memory_env_.get(),
           io_task_runner_.get(), file_task_runner_.get());
       file_system->SetUp(CannedSyncableFileSystem::QUOTA_DISABLED);
 
       SyncStatusCode status = SYNC_STATUS_UNKNOWN;
-      local_sync_service_->MaybeInitializeFileSystemContext(
-          origin, file_system->file_system_context(),
-          CreateResultReceiver(&status));
-      base::RunLoop().RunUntilIdle();
+      {
+        base::RunLoop run_loop;
+        local_sync_service_->MaybeInitializeFileSystemContext(
+            origin, file_system->file_system_context(),
+            base::Bind(&SetSyncStatus, run_loop.QuitClosure(), &status));
+        run_loop.Run();
+      }

[nhiroki, 2014/05/26 09:56:00]

You are already surrounded with if-block.

[peria, 2014/05/27 02:40:12]

Done.

       EXPECT_EQ(SYNC_STATUS_OK, status);
 
       file_system->backend()->sync_context()->
           set_mock_notify_changes_duration_in_sec(0);
 
       EXPECT_EQ(base::File::FILE_OK, file_system->OpenFileSystem());
       file_systems_[app_id] = file_system;
     }
 
     SyncStatusCode status = SYNC_STATUS_UNKNOWN;
-    remote_sync_service_->RegisterOrigin(origin, CreateResultReceiver(&status));
-    base::RunLoop().RunUntilIdle();
+    {
+      base::RunLoop run_loop;
+      remote_sync_service_->RegisterOrigin(
+          origin,
+          base::Bind(&SetSyncStatus, run_loop.QuitClosure(), &status));
+      run_loop.Run();
+    }
     return status;
   }
 
   void AddLocalFolder(const std::string& app_id,
                       const base::FilePath::StringType& path) {
     ASSERT_TRUE(ContainsKey(file_systems_, app_id));
     EXPECT_EQ(base::File::FILE_OK,
               file_systems_[app_id]->CreateDirectory(
                   CreateURL(app_id, path)));
   }
@@ skipping 25 matching lines @@
     EXPECT_EQ(base::File::FILE_OK,
               file_systems_[app_id]->Remove(
                   CreateURL(app_id, path),
                   true /* recursive */));
     base::RunLoop().RunUntilIdle();
   }
 
   SyncStatusCode ProcessLocalChange() {
     SyncStatusCode status = SYNC_STATUS_UNKNOWN;
     fileapi::FileSystemURL url;
-    local_sync_service_->ProcessLocalChange(
-        CreateResultReceiver(&status, &url));
-    base::RunLoop().RunUntilIdle();
+    base::RunLoop run_loop;
+    local_sync_service_->ProcessLocalChange(base::Bind(
+        &SetSyncStatusAndUrl, run_loop.QuitClosure(), &status, &url));
+    run_loop.Run();
     return status;
   }
 
   SyncStatusCode ProcessRemoteChange() {
     SyncStatusCode status = SYNC_STATUS_UNKNOWN;
     fileapi::FileSystemURL url;
-    remote_sync_service_->ProcessRemoteChange(
-        CreateResultReceiver(&status, &url));
-    base::RunLoop().RunUntilIdle();
+    base::RunLoop run_loop;
+    remote_sync_service_->ProcessRemoteChange(base::Bind(
+        &SetSyncStatusAndUrl, run_loop.QuitClosure(), &status, &url));
+    run_loop.Run();
     return status;
   }
 
   int64 GetLargestChangeID() {
     scoped_ptr<google_apis::AboutResource> about_resource;
     EXPECT_EQ(google_apis::HTTP_SUCCESS,
               fake_drive_service_helper()->GetAboutResource(&about_resource));
     if (!about_resource)
       return 0;
     return about_resource->largest_change_id();
@@ skipping 237 matching lines @@
   scoped_ptr<LocalFileSyncService> local_sync_service_;
 
   int64 pending_remote_changes_;
   int64 pending_local_changes_;
 
   scoped_ptr<FakeDriveServiceHelper> fake_drive_service_helper_;
   std::map<std::string, CannedSyncableFileSystem*> file_systems_;
 
 
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+  scoped_refptr<base::SequencedTaskRunner> worker_task_runner_;

[nhiroki, 2014/05/26 09:56:00]

Can you move this after |file_task_runner_| to sort them in setup-order.

[peria, 2014/05/27 02:40:12]

Done.
I changed the order of setting them up.
I and Tsuiki-san have an agreement to sort those task runners in
UI->worker->file order.

   scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(DriveBackendSyncTest);
 };
 
 TEST_F(DriveBackendSyncTest, LocalToRemoteBasicTest) {
   std::string app_id = "example";
 
   RegisterApp(app_id);
   AddOrUpdateLocalFile(app_id, FPL("file"), "abcde");
@@ skipping 1071 matching lines @@
 
   EXPECT_EQ(1u, CountApp());
   EXPECT_EQ(1u, CountLocalFile(app_id));
 
   EXPECT_EQ(2u, CountMetadata());
   EXPECT_EQ(2u, CountTracker());
 }
 
 }  // namespace drive_backend
 }  // namespace sync_file_system