file_manager: Fix flakiness in FileManagerBrowserDriveTest

It was not guranteed that the Drive mount point was added before
the tests were run, hence the tests were flaky.

Hopefully, the race can be easily fixed by reusing DriveMountPointWaiter
and AddTestMountPoint() introduced in crrev.com/193586. The implementation
is moved to drive_test_util.cc as is, except that the AddTestMountPoint()
is renamed to WaitUntilDriveMountPointIsAdded() with a Profile parameter.

BUG=230237
TEST=none

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

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

5 files changed, 109 insertions, 58 deletions

diff --git a/chrome/browser/chromeos/extensions/drive_test_util.cc b/chrome/browser/chromeos/extensions/drive_test_util.cc
new file mode 100644
index 0000000..a7a1fc2
--- /dev/null
+++ b/chrome/browser/chromeos/extensions/drive_test_util.cc
@@ -0,0 +1,80 @@
+// Copyright (c) 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 "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/run_loop.h"
+#include "chrome/browser/chromeos/drive/drive_file_system.h"
+#include "chrome/browser/chromeos/drive/drive_file_system_observer.h"
+#include "chrome/browser/chromeos/drive/drive_system_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/browser_context.h"
+#include "webkit/fileapi/external_mount_points.h"
+
+namespace drive_test_util {
+
+namespace {
+
+const char kDriveMountPointName[] = "drive";
+
+// Helper class used to wait for |OnFileSystemMounted| event from a drive file
+// system.
+class DriveMountPointWaiter : public drive::DriveFileSystemObserver {
+ public:
+  explicit DriveMountPointWaiter(drive::DriveFileSystemInterface* file_system)
+      : file_system_(file_system) {
+    file_system_->AddObserver(this);
+  }
+
+  virtual ~DriveMountPointWaiter() {
+    file_system_->RemoveObserver(this);
+  }
+
+  // DriveFileSystemObserver override.
+  virtual void OnFileSystemMounted() OVERRIDE {
+    // Note that it is OK for |run_loop_.Quit| to be called before
+    // |run_loop_.Run|. In this case |Run| will return immediately.
+    run_loop_.Quit();
+  }
+
+  // Runs loop until the file_system_ gets mounted.
+  void Wait() {
+    run_loop_.Run();
+  }
+
+ private:
+  drive::DriveFileSystemInterface* file_system_;
+  base::RunLoop run_loop_;
+};
+
+}  // namespace
+
+void WaitUntilDriveMountPointIsAdded(Profile* profile) {
+  DCHECK(profile);
+
+  // Drive mount point is added by the browser when the drive system service
+  // is first initialized. It is done asynchronously after some other parts of
+  // the service are initialized (e.g. resource metadata and cache), thus racy
+  // with the test start. To handle this raciness, the test verifies that
+  // drive mount point is added before continuing. If this is not the case,
+  // drive file system is observed for FileSystemMounted event (by
+  // |mount_point_waiter|) and test continues once the event is encountered.
+  drive::DriveSystemService* system_service =
+      drive::DriveSystemServiceFactory::FindForProfileRegardlessOfStates(
+          profile);
+  DCHECK(system_service && system_service->file_system());
+
+  DriveMountPointWaiter mount_point_waiter(system_service->file_system());
+
+  base::FilePath ignored;
+  // GetRegisteredPath succeeds iff the mount point exists.
+  if (!content::BrowserContext::GetMountPoints(profile)->
+      GetRegisteredPath(kDriveMountPointName, &ignored)) {
+    LOG(WARNING) << "Waiting for drive mount point to get mounted.";
+    mount_point_waiter.Wait();
+    LOG(WARNING) << "Drive mount point found.";
+  }
+}
+
+}  // namespace drive_test_util
diff --git a/chrome/browser/chromeos/extensions/drive_test_util.h b/chrome/browser/chromeos/extensions/drive_test_util.h
new file mode 100644
index 0000000..049bcc0
--- /dev/null
+++ b/chrome/browser/chromeos/extensions/drive_test_util.h
@@ -0,0 +1,19 @@
+// Copyright (c) 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_EXTENSIONS_DRIVE_TEST_UTIL_H_
+#define CHROME_BROWSER_CHROMEOS_EXTENSIONS_DRIVE_TEST_UTIL_H_
+
+class Profile;
+
+namespace drive_test_util {
+
+// Waits until Drive mount point for |profile| is added. Drive mount point is
+// added by the browser but tests should use this function to ensure that the
+// Drive mount point is added before accessing Drive.
+void WaitUntilDriveMountPointIsAdded(Profile* profile);
+
+}  // namespace drive_test_util
+
+#endif  // CHROME_BROWSER_CHROMEOS_EXTENSIONS_DRIVE_TEST_UTIL_H_
diff --git a/chrome/browser/chromeos/extensions/external_filesystem_apitest.cc b/chrome/browser/chromeos/extensions/external_filesystem_apitest.cc
index 1fc474b..c4fc6f0 100644
--- a/chrome/browser/chromeos/extensions/external_filesystem_apitest.cc
+++ b/chrome/browser/chromeos/extensions/external_filesystem_apitest.cc
@@ -10,12 +10,11 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop_proxy.h"
 #include "base/path_service.h"
-#include "base/run_loop.h"
 #include "base/threading/worker_pool.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/drive/drive_file_system.h"
-#include "chrome/browser/chromeos/drive/drive_file_system_observer.h"
 #include "chrome/browser/chromeos/drive/drive_system_service.h"
+#include "chrome/browser/chromeos/extensions/drive_test_util.h"
 #include "chrome/browser/extensions/event_router.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -68,9 +67,8 @@ namespace {
 
 // Root dirs for file systems expected by the test extensions.
 // NOTE: Root dir for drive file system is set by Chrome's drive implementation,
-// but the test will have to make sure the mount point is mounted before
-// starting a test extension.
-const char kDriveMountPointName[] = "drive";
+// but the test will have to make sure the mount point is added before
+// starting a test extension using WaitUntilDriveMountPointIsAdded().
 const char kLocalMountPointName[] = "local";
 const char kRestrictedMountPointName[] = "restricted";
 
@@ -158,36 +156,6 @@ bool InitializeLocalFileSystem(base::ScopedTempDir* tmp_dir,
   return true;
 }
 
-// Helper class used to wait for |OnFileSystemMounted| event from a drive file
-// system.
-class DriveMountPointWaiter : public drive::DriveFileSystemObserver {
- public:
-  explicit DriveMountPointWaiter(drive::DriveFileSystemInterface* file_system)
-      : file_system_(file_system) {
-    file_system_->AddObserver(this);
-  }
-
-  virtual ~DriveMountPointWaiter() {
-    file_system_->RemoveObserver(this);
-  }
-
-  // DriveFileSystemObserver override.
-  virtual void OnFileSystemMounted() OVERRIDE {
-    // Note that it is OK for |run_loop_.Quit| to be called before
-    // |run_loop_.Run|. In this case |Run| will return immediately.
-    run_loop_.Quit();
-  }
-
-  // Runs loop until the file_system_ gets mounted.
-  void Wait() {
-    run_loop_.Run();
-  }
-
- private:
-  drive::DriveFileSystemInterface* file_system_;
-  base::RunLoop run_loop_;
-};
-
 // Helper class to wait for a background page to load or close again.
 class BackgroundObserver {
  public:
@@ -368,29 +336,7 @@ class DriveFileSystemExtensionApiTest : public FileSystemExtensionApiTestBase {
 
   // FileSystemExtensionApiTestBase OVERRIDE.
   virtual void AddTestMountPoint() OVERRIDE {
-    // Drive mount point is added by the browser when the drive system service
-    // is first initialized. It is done asynchronously after some other parts of
-    // the service are initialized (e.g. resource metadata and cache), thus racy
-    // with the test start. To handle this raciness, the test verifies that
-    // drive mount point is added before continuing. If this is not the case,
-    // drive file system is observed for FileSystemMounted event (by
-    // |mount_point_waiter|) and test continues once the event is encountered.
-    drive::DriveSystemService* system_service =
-        drive::DriveSystemServiceFactory::FindForProfileRegardlessOfStates(
-            browser()->profile());
-    ASSERT_TRUE(system_service && system_service->file_system());
-
-    DriveMountPointWaiter mount_point_waiter(system_service->file_system());
-
-    base::FilePath ignored;
-    // GetRegisteredPath succeeds iff the mount point exists.
-    if (!content::BrowserContext::GetMountPoints(browser()->profile())->
-            GetRegisteredPath(kDriveMountPointName, &ignored)) {
-      LOG(WARNING) << "Waiting for drive mount point to get mounted.";
-      mount_point_waiter.Wait();
-      LOG(WARNING) << "Drive mount point found.";
-    }
-
+    drive_test_util::WaitUntilDriveMountPointIsAdded(browser()->profile());
   }
 
  protected:
diff --git a/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc b/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
index a11d4df..9e805d1 100644
--- a/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
+++ b/chrome/browser/chromeos/extensions/file_manager/file_manager_browsertest.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/chromeos/drive/drive_file_system.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_observer.h"
 #include "chrome/browser/chromeos/drive/drive_system_service.h"
+#include "chrome/browser/chromeos/extensions/drive_test_util.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -704,18 +705,21 @@ IN_PROC_BROWSER_TEST_P(FileManagerBrowserLocalTest, TestKeyboardDelete) {
 }
 
 IN_PROC_BROWSER_TEST_P(FileManagerBrowserDriveTest, TestFileDisplay) {
+  drive_test_util::WaitUntilDriveMountPointIsAdded(browser()->profile());
   StartFileManager("/drive/root");
 
   DoTestFileDisplay();
 }
 
 IN_PROC_BROWSER_TEST_P(FileManagerBrowserDriveTest, TestKeyboardCopy) {
+  drive_test_util::WaitUntilDriveMountPointIsAdded(browser()->profile());
   StartFileManager("/drive/root");
 
   DoTestKeyboardCopy();
 }
 
 IN_PROC_BROWSER_TEST_P(FileManagerBrowserDriveTest, TestKeyboardDelete) {
+  drive_test_util::WaitUntilDriveMountPointIsAdded(browser()->profile());
   StartFileManager("/drive/root");
 
   DoTestKeyboardDelete();
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 9b8b2db..295e588 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1164,6 +1164,8 @@
         'browser/chromeos/drive/drive_system_service_browsertest.cc',
         'browser/chromeos/drive/drive_test_util.cc',
         'browser/chromeos/drive/drive_test_util.h',
+        'browser/chromeos/extensions/drive_test_util.cc',
+        'browser/chromeos/extensions/drive_test_util.h',
         'browser/chromeos/extensions/echo_private_apitest.cc',
         'browser/chromeos/extensions/external_filesystem_apitest.cc',
         'browser/chromeos/extensions/file_manager/file_browser_handler_api_test.cc',