Don't show FileManagerDialog if another FileManagerDialog is shown in the same tab.

chrome/browser/ui/views/file_manager_dialog_browsertest.cc

Index: chrome/browser/ui/views/file_manager_dialog_browsertest.cc
diff --git a/chrome/browser/ui/views/file_manager_dialog_browsertest.cc b/chrome/browser/ui/views/file_manager_dialog_browsertest.cc
index 38146c11aafaeb86d823eb85a59f92c785c1ac30..2c74f2861793e3bcbb17ab8c5553be343f4d8825 100644
--- a/chrome/browser/ui/views/file_manager_dialog_browsertest.cc
+++ b/chrome/browser/ui/views/file_manager_dialog_browsertest.cc
@@ -90,6 +90,66 @@ class FileManagerDialogBrowserTest : public ExtensionBrowserTest {
     provider->AddMountPoint(path);
   }
 
+  void OpenDialog(SelectFileDialog::Type dialog_type,

[James Cook, 2011/11/10 21:04:54]

Nice that you extracted this shared code!

+                  const FilePath& file_path,
+                  const gfx::NativeWindow& owning_window,
+                  const std::string& additional_message) {
+    // 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 */);
+
+    scoped_ptr<ExtensionTestMessageListener> additional_listener;
+    if (!additional_message.empty()) {
+      additional_listener.reset(
+          new ExtensionTestMessageListener(additional_message, false));
+    }
+
+    dialog_->SelectFile(dialog_type,
+                        string16() /* title */,
+                        file_path,
+                        NULL /* file_types */,
+                         0 /* file_type_index */,
+                        FILE_PATH_LITERAL("") /* default_extension */,
+                        NULL /* source_contents */,
+                        owning_window,
+                        this /* params */);
+
+    LOG(INFO) << "Waiting for JavaScript ready message.";
+    ASSERT_TRUE(init_listener.WaitUntilSatisfied());
+
+    if (additional_listener.get()) {
+      LOG(INFO) << "Waiting for JavaScript " << additional_message
+                << " message.";
+      ASSERT_TRUE(additional_listener->WaitUntilSatisfied());
+    }
+
+    // Dialog should be running now.
+    ASSERT_TRUE(dialog_->IsRunning(owning_window));
+  }
+
+  void CloseDialog(bool press_ok, const gfx::NativeWindow& owning_window) {

[James Cook, 2011/11/10 21:04:54]

Maybe use an enum or a string for the button to press?  This would make the
calling code self-documenting, like: CloseDialog(window, "ok") or
CloseDialog(window, BTN_OK).

[tbarzic, 2011/11/10 22:00:48]

Done.

+    // Inject JavaScript to click the cancel button and wait for notification
+    // that the window has closed.
+    ui_test_utils::WindowedNotificationObserver host_destroyed(
+        content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
+        content::NotificationService::AllSources());
+    RenderViewHost* host = dialog_->GetRenderViewHost();
+    string16 main_frame;
+    std::string button_class = press_ok ? ".ok" : ".cancel";
+    string16 script = ASCIIToUTF16(
+        "console.log(\'Test JavaScript injected.\');"
+        "document.querySelector(\'" + button_class + "\').click();");
+    // The file selection handler closes the dialog and does not return control
+    // to JavaScript, so do not wait for return values.
+    host->ExecuteJavascriptInWebFrame(main_frame, script);
+    LOG(INFO) << "Waiting for window close notification.";
+    host_destroyed.Wait();
+
+    // Dialog no longer believes it is running.
+    ASSERT_FALSE(dialog_->IsRunning(owning_window));
+  }
+
   scoped_ptr<MockSelectFileDialogListener> listener_;
   scoped_refptr<FileManagerDialog> dialog_;
 };
@@ -120,44 +180,13 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogBrowserTest, SelectFileAndCancel) {
   FilePath tmp_dir("/tmp");
   AddMountPoint(tmp_dir);
 
-  // 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 */,
-                      FilePath() /* default_path */,
-                      NULL /* file_types */,
-                       0 /* file_type_index */,
-                      FILE_PATH_LITERAL("") /* default_extension */,
-                      NULL /* source_contents */,
-                      owning_window,
-                      this /* params */);
-  LOG(INFO) << "Waiting for JavaScript ready message.";
-  ASSERT_TRUE(init_listener.WaitUntilSatisfied());
-
-  // Dialog should be running now.
-  ASSERT_TRUE(dialog_->IsRunning(owning_window));
-
-  // Inject JavaScript to click the cancel button and wait for notification
-  // that the window has closed.
-  ui_test_utils::WindowedNotificationObserver host_destroyed(
-      content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
-      content::NotificationService::AllSources());
-  RenderViewHost* host = dialog_->GetRenderViewHost();
-  string16 main_frame;
-  string16 script = ASCIIToUTF16(
-      "console.log(\'Test JavaScript injected.\');"
-      "document.querySelector(\'.cancel\').click();");
-  // The file selection handler closes the dialog and does not return control
-  // to JavaScript, so do not wait for return values.
-  host->ExecuteJavascriptInWebFrame(main_frame, script);
-  LOG(INFO) << "Waiting for window close notification.";
-  host_destroyed.Wait();
-
-  // Dialog no longer believes it is running.
-  ASSERT_FALSE(dialog_->IsRunning(owning_window));
+
+  // FilePath() for default path.
+  OpenDialog(SelectFileDialog::SELECT_OPEN_FILE, FilePath(), owning_window, "");
+
+  // Press cancel button.
+  CloseDialog(false, owning_window);
 
   // Listener should have been informed of the cancellation.
   ASSERT_FALSE(listener_->file_selected());
@@ -183,51 +212,18 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogBrowserTest, SelectFileAndOpen) {
   ASSERT_TRUE(fp != NULL);
   ASSERT_TRUE(file_util::CloseFile(fp));
 
+  gfx::NativeWindow owning_window = browser()->window()->GetNativeHandle();
+
   // 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').
   // 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 */,
-                      test_file,
-                      NULL /* file_types */,
-                      0 /* file_type_index */,
-                      FILE_PATH_LITERAL("") /* default_extension */,
-                      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(selection_listener.WaitUntilSatisfied());
-
-  // Dialog should be running now.
-  ASSERT_TRUE(dialog_->IsRunning(owning_window));
-
-  // Inject JavaScript to click the open button and wait for notification
-  // that the window has closed.
-  ui_test_utils::WindowedNotificationObserver host_destroyed(
-      content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
-      content::NotificationService::AllSources());
-  RenderViewHost* host = dialog_->GetRenderViewHost();
-  string16 main_frame;
-  string16 script = ASCIIToUTF16(
-      "console.log(\'Test JavaScript injected.\');"
-      "document.querySelector('.ok').click();");
-  // The file selection handler closes the dialog and does not return control
-  // to JavaScript, so do not wait for return values.
-  host->ExecuteJavascriptInWebFrame(main_frame, script);
-  LOG(INFO) << "Waiting for window close notification.";
-  host_destroyed.Wait();
-
-  // Dialog no longer believes it is running.
-  ASSERT_FALSE(dialog_->IsRunning(owning_window));
+  OpenDialog(SelectFileDialog::SELECT_OPEN_FILE, test_file, owning_window,
+             "selection-change-complete");
+
+  // Click open button.
+  CloseDialog(true, owning_window);
 
   // Listener should have been informed that the file was opened.
   ASSERT_TRUE(listener_->file_selected());
@@ -249,51 +245,18 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogBrowserTest, SelectFileAndSave) {
   FilePath temp_dir = scoped_temp_dir.path();
   FilePath test_file = temp_dir.AppendASCII("file_manager_test.html");
 
+  gfx::NativeWindow owning_window = browser()->window()->GetNativeHandle();
+
   // Spawn a dialog to save a file, providing a suggested path.
   // Ensure "Save" button is enabled by waiting for notification from
   // chrome.test.sendMessage().
   // 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 */,
-                      test_file,
-                      NULL /* file_types */,
-                      0 /* file_type_index */,
-                      FILE_PATH_LITERAL("") /* default_extension */,
-                      NULL /* source_contents */,
-                      owning_window,
-                      this /* params */);
-  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));
-
-  // Inject JavaScript to click the save button and wait for notification
-  // that the window has closed.
-  ui_test_utils::WindowedNotificationObserver host_destroyed(
-      content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
-      content::NotificationService::AllSources());
-  RenderViewHost* host = dialog_->GetRenderViewHost();
-  string16 main_frame;
-  string16 script = ASCIIToUTF16(
-      "console.log(\'Test JavaScript injected.\');"
-      "document.querySelector('.ok').click();");
-  // The file selection handler closes the dialog and does not return control
-  // to JavaScript, so do not wait for return values.
-  host->ExecuteJavascriptInWebFrame(main_frame, script);
-  LOG(INFO) << "Waiting for window close notification.";
-  host_destroyed.Wait();
-
-  // Dialog no longer believes it is running.
-  ASSERT_FALSE(dialog_->IsRunning(owning_window));
+  OpenDialog(SelectFileDialog::SELECT_SAVEAS_FILE, test_file, owning_window,
+             "directory-change-complete");
+
+  // Click save button.
+  CloseDialog(true, owning_window);
 
   // Listener should have been informed that the file was selected.
   ASSERT_TRUE(listener_->file_selected());
@@ -310,25 +273,9 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogBrowserTest,
   FilePath tmp_dir("/tmp");
   AddMountPoint(tmp_dir);
 
-  // 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 */,
-                      FilePath() /* default_path */,
-                      NULL /* file_types */,
-                       0 /* file_type_index */,
-                      FILE_PATH_LITERAL("") /* default_extension */,
-                      NULL /* source_contents */,
-                      owning_window,
-                      this /* params */);
-  LOG(INFO) << "Waiting for JavaScript ready message.";
-  ASSERT_TRUE(init_listener.WaitUntilSatisfied());
 
-  // Dialog should be running now.
-  ASSERT_TRUE(dialog_->IsRunning(owning_window));
+  OpenDialog(SelectFileDialog::SELECT_OPEN_FILE, FilePath(), owning_window, "");

[James Cook, 2011/11/10 21:04:54]

Yeah, these are much easier to read with the common code extracted.  Good work.

[tbarzic, 2011/11/10 22:00:48]

:)

 
   // Open a singleton tab in background.
   browser::NavigateParams p(browser(), GURL("www.google.com"),
@@ -337,27 +284,51 @@ IN_PROC_BROWSER_TEST_F(FileManagerDialogBrowserTest,
   p.disposition = SINGLETON_TAB;
   browser::Navigate(&p);
 
-  // Inject JavaScript to click the cancel button and wait for notification
-  // that the window has closed.
-  ui_test_utils::WindowedNotificationObserver host_destroyed(
-      content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
-      content::NotificationService::AllSources());
-  RenderViewHost* host = dialog_->GetRenderViewHost();
-  string16 main_frame;
-  string16 script = ASCIIToUTF16(
-      "console.log(\'Test JavaScript injected.\');"
-      "document.querySelector(\'.cancel\').click();");
-  // The file selection handler closes the dialog and does not return control
-  // to JavaScript, so do not wait for return values.
-  host->ExecuteJavascriptInWebFrame(main_frame, script);
-  LOG(INFO) << "Waiting for window close notification.";
-  host_destroyed.Wait();
-
-  // Dialog no longer believes it is running.
-  ASSERT_FALSE(dialog_->IsRunning(owning_window));
+  // Press cancel button.
+  CloseDialog(false, owning_window);
+
+  // Listener should have been informed of the cancellation.
+  ASSERT_FALSE(listener_->file_selected());
+  ASSERT_TRUE(listener_->canceled());
+  ASSERT_EQ(this, listener_->params());
+}
+
+IN_PROC_BROWSER_TEST_F(FileManagerDialogBrowserTest, OpenTwoDialogs) {
+  // 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);
+
+  gfx::NativeWindow owning_window = browser()->window()->GetNativeHandle();
+
+  OpenDialog(SelectFileDialog::SELECT_OPEN_FILE, FilePath(), owning_window, "");
+
+  scoped_ptr<MockSelectFileDialogListener>
+      listener2(new MockSelectFileDialogListener());
+  scoped_refptr<FileManagerDialog>  dialog2 =

[James Cook, 2011/11/10 21:04:54]

Optional: dialog2 could become a member variable of this test. 
FileManagerDialog objects are cheap to construct, so I think it would be OK to
make two of them for every test run.  That would eliminate the extra friend
declaration.  Up to you; I don't feel strongly about adding friends for testing.

[tbarzic, 2011/11/10 22:00:48]

Done.

+      new FileManagerDialog(listener2.get());
+  dialog2->SelectFile(SelectFileDialog::SELECT_OPEN_FILE,
+                      string16() /* title */,
+                      FilePath() /* default_path */,
+                      NULL /* file_types */,
+                      0 /* file_type_index */,
+                      FILE_PATH_LITERAL("") /* default_extension */,
+                      NULL /* source_contents */,
+                      owning_window,
+                      this /* params */);
+
+  // Dialog should not be running now.
+  ASSERT_FALSE(dialog2->IsRunning(owning_window));
+
+  // Click cancel button.
+  CloseDialog(false, owning_window);
 
   // Listener should have been informed of the cancellation.
   ASSERT_FALSE(listener_->file_selected());
   ASSERT_TRUE(listener_->canceled());
   ASSERT_EQ(this, listener_->params());
+
+  dialog2 = NULL;
+  listener2.reset();
 }