CrOS - Fix flaky FileManagerDialogTest

We were not waiting for the file manager extension to initialize its
Web Worker process before starting the test, which lead to occasional
crashes if the test completed before the init was done.

BUG=chromium:89733
TEST=browser_tests --gtest_filter=FileManagerDialogTest.* --gtest_repeat=10 --gtest_shuffle

Review URL: http://codereview.chromium.org/7528003

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

chrome/browser/resources/file_manager/js/file_manager.js

@@ -148,6 +148,8 @@ function FileManager(dialogDom, filesystem, rootEntries, params) {
   this.metadataReader_ = new Worker('js/metadata_dispatcher.js');
   this.metadataReader_.onmessage = this.onMetadataMessage_.bind(this);
   this.metadataReader_.postMessage({verb: 'init'});
+  // Initialization is not complete until the Worker sends back the
+  // 'initialized' message.  See below.
 }
 
 FileManager.prototype = {
@@ -1499,8 +1501,17 @@ FileManager.prototype = {
     this.onMetadataResult_(fileURL, {});
   };
 
-  FileManager.prototype.onMetadataFilter_ = function(regexp) {
+  /**
+   * Handles the 'initialized' message from the metadata reader Worker.
+   */
+  FileManager.prototype.onMetadataInitialized_ = function(regexp) {
     this.metadataUrlFilter_ = regexp;
+    // We're ready to run.  Tests can monitor for this state with
+    // ExtensionTestMessageListener listener("worker-initialized");
+    // ASSERT_TRUE(listener.WaitUntilSatisfied());
+    // Automated tests need to wait for this, otherwise we crash in browser_test
+    // cleanup because the worker process still has URL requests in-flight.
+    chrome.test.sendMessage('worker-initialized');
   };
 
   FileManager.prototype.onMetadataLog_ = function(arglist) {

chrome/browser/resources/file_manager/js/main.js

@@ -26,10 +26,6 @@ function init() {
   function onEntriesFound(filesystem, entries) {
     FileManager.initStrings(function () {
       fileManager = new FileManager(document.body, filesystem, entries, params);
-      // We're ready to run.  Tests can monitor for this state with
-      // ExtensionTestMessageListener listener("ready");
-      // ASSERT_TRUE(listener.WaitUntilSatisfied());
-      chrome.test.sendMessage('ready');
     });
   }
 

chrome/browser/resources/file_manager/js/metadata_dispatcher.js

@@ -51,7 +51,9 @@ MetadataDispatcher.prototype.verbose = false;
 
 MetadataDispatcher.prototype.messageHandlers = {
   init: function() {
-    this.postMessage('filter', [this.parserRegexp_]);
+    // Inform our owner that we're done initializing.
+    // If we need to pass more data back, we can add it to the param array.
+    this.postMessage('initialized', [this.parserRegexp_]);
     this.log('initialized with URL filter ' + this.parserRegexp_);
   },
 

chrome/browser/ui/views/file_manager_dialog_browsertest.cc

@@ -108,17 +108,17 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, FileManagerDestroyListener) {
   listener_.reset();
 }
 
-// Flaky: http://crbug.com/89733
-IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, FLAKY_SelectFileAndCancel) {
+IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, SelectFileAndCancel) {
   // Add tmp mount point even though this test won't use it directly.
   // We need this to make sure that at least one top-level directory exists
   // in the file browser.
   FilePath tmp_dir("/tmp");
   AddMountPoint(tmp_dir);
 
-  // Spawn a dialog to open a file.  The dialog will signal that it is done
-  // loading via chrome.test.sendMessage('ready') in the extension JavaScript.
-  ExtensionTestMessageListener msg_listener("ready", false /* will_reply */);
+  // Spawn a dialog to open a file.  The dialog will signal that it is ready
+  // via chrome.test.sendMessage() in the extension JavaScript.
+  ExtensionTestMessageListener init_listener("worker-initialized",
+                                             false /* will_reply */);
   gfx::NativeWindow owning_window = browser()->window()->GetNativeHandle();
   dialog_->SelectFile(SelectFileDialog::SELECT_OPEN_FILE,
                       string16() /* title */,
@@ -130,7 +130,7 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, FLAKY_SelectFileAndCancel) {
                       owning_window,
                       this /* params */);
   LOG(INFO) << "Waiting for JavaScript ready message.";
-  ASSERT_TRUE(msg_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(init_listener.WaitUntilSatisfied());
 
   // Dialog should be running now.
   ASSERT_TRUE(dialog_->IsRunning(owning_window));
@@ -181,8 +181,12 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, SelectFileAndOpen) {
   // Spawn a dialog to open a file.  Provide the path to the file so the dialog
   // will automatically select it.  Ensure that the OK button is enabled by
   // waiting for chrome.test.sendMessage('selection-change-complete').
-  ExtensionTestMessageListener msg_listener("selection-change-complete",
-                                            false /* will_reply */);
+  // The extension starts a Web Worker to read file metadata, so it may send
+  // 'selection-change-complete' before 'worker-initialized'.  This is OK.
+  ExtensionTestMessageListener init_listener("worker-initialized",
+                                             false /* will_reply */);
+  ExtensionTestMessageListener selection_listener("selection-change-complete",
+                                                  false /* will_reply */);
   gfx::NativeWindow owning_window = browser()->window()->GetNativeHandle();
   dialog_->SelectFile(SelectFileDialog::SELECT_OPEN_FILE,
                       string16() /* title */,
@@ -193,8 +197,10 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, SelectFileAndOpen) {
                       NULL /* source_contents */,
                       owning_window,
                       this /* params */);
+  LOG(INFO) << "Waiting for JavaScript initialized message.";
+  ASSERT_TRUE(init_listener.WaitUntilSatisfied());
   LOG(INFO) << "Waiting for JavaScript selection-change-complete message.";
-  ASSERT_TRUE(msg_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(selection_listener.WaitUntilSatisfied());
 
   // Dialog should be running now.
   ASSERT_TRUE(dialog_->IsRunning(owning_window));
@@ -225,8 +231,7 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, SelectFileAndOpen) {
   ASSERT_EQ(this, listener_->params());
 }
 
-// Flaky: http://crbug.com/89733
-IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, DISABLED_SelectFileAndSave) {
+IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, SelectFileAndSave) {
   // Allow the tmp directory to be mounted.  We explicitly use /tmp because
   // it it whitelisted for file system access on Chrome OS.
   FilePath tmp_dir("/tmp");
@@ -242,8 +247,12 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, DISABLED_SelectFileAndSave) {
   // Spawn a dialog to save a file, providing a suggested path.
   // Ensure "Save" button is enabled by waiting for notification from
   // chrome.test.sendMessage().
-  ExtensionTestMessageListener msg_listener("directory-change-complete",
-                                            false /* will_reply */);
+  // The extension starts a Web Worker to read file metadata, so it may send
+  // 'directory-change-complete' before 'worker-initialized'.  This is OK.
+  ExtensionTestMessageListener init_listener("worker-initialized",
+                                             false /* will_reply */);
+  ExtensionTestMessageListener dir_change_listener("directory-change-complete",
+                                                   false /* will_reply */);
   gfx::NativeWindow owning_window = browser()->window()->GetNativeHandle();
   dialog_->SelectFile(SelectFileDialog::SELECT_SAVEAS_FILE,
                       string16() /* title */,
@@ -254,8 +263,10 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, DISABLED_SelectFileAndSave) {
                       NULL /* source_contents */,
                       owning_window,
                       this /* params */);
-  LOG(INFO) << "Waiting for JavaScript message.";
-  ASSERT_TRUE(msg_listener.WaitUntilSatisfied());
+  LOG(INFO) << "Waiting for JavaScript initialized message.";
+  ASSERT_TRUE(init_listener.WaitUntilSatisfied());
+  LOG(INFO) << "Waiting for JavaScript directory-change-complete message.";
+  ASSERT_TRUE(dir_change_listener.WaitUntilSatisfied());
 
   // Dialog should be running now.
   ASSERT_TRUE(dialog_->IsRunning(owning_window));