CrOS - Add browsertest for file open/save dialog.

* Extracts a public header for FileManagerDialog.
* Enables component extensions for CrOS tests, because file manager is implemented as one.
* Tests some memory management for FileManagerDialog.
* Injects JavaScript into FileManagerDialog to simulate clicking the cancel button and tests window closing.

BUG=chromium-os:15584
TEST=FileManagerDialogTest.*

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

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

12 files changed, 215 insertions, 70 deletions

diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 20daa161..3733ecb 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -499,16 +499,9 @@ void ProfileImpl::RegisterComponentExtensions() {
       IDR_BOOKMARKS_MANIFEST));
 
 #if defined(FILE_MANAGER_EXTENSION)
-#if defined(OS_CHROMEOS)
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kSkipChromeOSComponents)) {
-#endif
     component_extensions.push_back(std::make_pair(
         FILE_PATH_LITERAL("file_manager"),
         IDR_FILEMANAGER_MANIFEST));
-#if defined(OS_CHROMEOS)
-   }
-#endif
 #endif
 
 #if defined(TOUCH_UI)
@@ -518,8 +511,6 @@ void ProfileImpl::RegisterComponentExtensions() {
 #endif
 
 #if defined(OS_CHROMEOS)
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kSkipChromeOSComponents)) {
     component_extensions.push_back(std::make_pair(
         FILE_PATH_LITERAL("/usr/share/chromeos-assets/mobile"),
         IDR_MOBILE_MANIFEST));
@@ -531,7 +522,6 @@ void ProfileImpl::RegisterComponentExtensions() {
           IDR_HELP_MANIFEST));
     }
 #endif
-  }
 #endif
 
   // Web Store.
diff --git a/chrome/browser/resources/file_manager/js/main.js b/chrome/browser/resources/file_manager/js/main.js
index 8eb5dc3..7181285 100644
--- a/chrome/browser/resources/file_manager/js/main.js
+++ b/chrome/browser/resources/file_manager/js/main.js
@@ -26,6 +26,10 @@ function init() {
   function onEntriesFound(entries) {
     FileManager.initStrings(function () {
       fileManager = new FileManager(document.body, 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');
     });
   }
 
diff --git a/chrome/browser/ui/views/extensions/extension_dialog.cc b/chrome/browser/ui/views/extensions/extension_dialog.cc
index fc5e29d..50ac515 100644
--- a/chrome/browser/ui/views/extensions/extension_dialog.cc
+++ b/chrome/browser/ui/views/extensions/extension_dialog.cc
@@ -69,6 +69,7 @@ ExtensionDialog* ExtensionDialog::Show(
   if (!manager)
     return NULL;
   ExtensionHost* host = manager->CreateDialogHost(url, browser);
+  DCHECK(host);
   views::Widget* frame = BrowserView::GetBrowserViewForNativeWindow(
       browser->window()->GetNativeHandle())->GetWidget();
   gfx::Rect relative_to = browser->window()->GetBounds();
diff --git a/chrome/browser/ui/views/file_manager_dialogs.cc b/chrome/browser/ui/views/file_manager_dialog.cc
index 0c3dd5e..fbc1895 100644
--- a/chrome/browser/ui/views/file_manager_dialogs.cc
+++ b/chrome/browser/ui/views/file_manager_dialog.cc
@@ -2,20 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/ui/views/file_manager_dialog.h"
+
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/extensions/extension_file_browser_private_api.h"
 #include "chrome/browser/extensions/file_manager_util.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/views/extensions/extension_dialog.h"
 #include "chrome/browser/ui/views/window.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "views/window/window.h"
-#include "ui/gfx/rect.h"
-#include "ui/gfx/size.h"
 
 namespace {
 
@@ -24,46 +23,6 @@ const int kFileManagerHeight = 580;  // pixels
 
 }
 
-// Shows a dialog box for selecting a file or a folder.
-class FileManagerDialog
-    : public SelectFileDialog,
-      public ExtensionDialog::Observer {
-
- public:
-  explicit FileManagerDialog(Listener* listener);
-
-  // BaseShellDialog implementation.
-  virtual bool IsRunning(gfx::NativeWindow owner_window) const OVERRIDE;
-  virtual void ListenerDestroyed() OVERRIDE;
-
-  // ExtensionDialog::Observer implementation.
-  virtual void ExtensionDialogIsClosing(ExtensionDialog* dialog) OVERRIDE;
-
- protected:
-  // SelectFileDialog implementation.
-  virtual void SelectFileImpl(Type type,
-                              const string16& title,
-                              const FilePath& default_path,
-                              const FileTypeInfo* file_types,
-                              int file_type_index,
-                              const FilePath::StringType& default_extension,
-                              gfx::NativeWindow owning_window,
-                              void* params) OVERRIDE;
-
- private:
-  virtual ~FileManagerDialog();
-
-  // Host for the extension that implements this dialog.
-  scoped_refptr<ExtensionDialog> extension_dialog_;
-
-  // ID of the tab that spawned this dialog, used to route callbacks.
-  int32 tab_id_;
-
-  gfx::NativeWindow owner_window_;
-
-  DISALLOW_COPY_AND_ASSIGN(FileManagerDialog);
-};
-
 // Linking this implementation of SelectFileDialog::Create into the target
 // selects FileManagerDialog as the dialog of choice.
 // static
@@ -72,6 +31,8 @@ SelectFileDialog* SelectFileDialog::Create(Listener* listener) {
   return new FileManagerDialog(listener);
 }
 
+/////////////////////////////////////////////////////////////////////////////
+
 FileManagerDialog::FileManagerDialog(Listener* listener)
     : SelectFileDialog(listener),
       tab_id_(0),
@@ -100,6 +61,12 @@ void FileManagerDialog::ExtensionDialogIsClosing(ExtensionDialog* dialog) {
   FileDialogFunction::Callback::Remove(tab_id_);
 }
 
+RenderViewHost* FileManagerDialog::GetRenderViewHost() {
+  if (extension_dialog_)
+    return extension_dialog_->host()->render_view_host();
+  return NULL;
+}
+
 void FileManagerDialog::SelectFileImpl(
     Type type,
     const string16& title,
diff --git a/chrome/browser/ui/views/file_manager_dialog.h b/chrome/browser/ui/views/file_manager_dialog.h
new file mode 100644
index 0000000..22e0677
--- /dev/null
+++ b/chrome/browser/ui/views/file_manager_dialog.h
@@ -0,0 +1,60 @@
+// Copyright (c) 2011 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_UI_VIEWS_FILE_MANAGER_DIALOG_H_
+#define CHROME_BROWSER_UI_VIEWS_FILE_MANAGER_DIALOG_H_
+#pragma once
+
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/ui/shell_dialogs.h"  // SelectFileDialog
+#include "chrome/browser/ui/views/extensions/extension_dialog.h"
+
+class RenderViewHost;
+
+// Shows a dialog box for selecting a file or a folder, using the
+// file manager extension implementation.
+class FileManagerDialog
+    : public SelectFileDialog,
+      public ExtensionDialog::Observer {
+
+ public:
+  explicit FileManagerDialog(SelectFileDialog::Listener* listener);
+
+  // BaseShellDialog implementation.
+  virtual bool IsRunning(gfx::NativeWindow owner_window) const OVERRIDE;
+  virtual void ListenerDestroyed() OVERRIDE;
+
+  // ExtensionDialog::Observer implementation.
+  virtual void ExtensionDialogIsClosing(ExtensionDialog* dialog) OVERRIDE;
+
+  // For testing, so we can inject JavaScript into the contained view.
+  RenderViewHost* GetRenderViewHost();
+
+ protected:
+  // SelectFileDialog implementation.
+  virtual void SelectFileImpl(Type type,
+                              const string16& title,
+                              const FilePath& default_path,
+                              const FileTypeInfo* file_types,
+                              int file_type_index,
+                              const FilePath::StringType& default_extension,
+                              gfx::NativeWindow owning_window,
+                              void* params) OVERRIDE;
+
+ private:
+  // Object is ref-counted, see SelectFileDialog.
+  virtual ~FileManagerDialog();
+
+  // Host for the extension that implements this dialog.
+  scoped_refptr<ExtensionDialog> extension_dialog_;
+
+  // ID of the tab that spawned this dialog, used to route callbacks.
+  int32 tab_id_;
+
+  gfx::NativeWindow owner_window_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileManagerDialog);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_FILE_MANAGER_DIALOG_H_
diff --git a/chrome/browser/ui/views/file_manager_dialog_browsertest.cc b/chrome/browser/ui/views/file_manager_dialog_browsertest.cc
new file mode 100644
index 0000000..cc189ae
--- /dev/null
+++ b/chrome/browser/ui/views/file_manager_dialog_browsertest.cc
@@ -0,0 +1,127 @@
+// Copyright (c) 2011 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 "chrome/browser/ui/views/file_manager_dialog.h"
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
+#include "base/threading/platform_thread.h"
+#include "build/build_config.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/shell_dialogs.h"  // SelectFileDialog
+#include "chrome/test/ui_test_utils.h"
+
+class FileManagerDialogTest : public ExtensionBrowserTest {
+};
+
+class MockSelectFileDialogListener : public SelectFileDialog::Listener {
+ public:
+  MockSelectFileDialogListener()
+    : canceled_(false) {
+  }
+
+  bool canceled() const { return canceled_; }
+
+  // SelectFileDialog::Listener implementation.
+  virtual void FileSelected(const FilePath& path, int index, void* params) {}
+  virtual void MultiFilesSelected(
+      const std::vector<FilePath>& files, void* params) {}
+  virtual void FileSelectionCanceled(void* params) {
+    canceled_ = true;
+  }
+
+ private:
+  bool canceled_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockSelectFileDialogListener);
+};
+
+IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, FileManagerCreateAndDestroy) {
+  // Browser window must be up for us to test dialog window parent.
+  gfx::NativeWindow native_window = browser()->window()->GetNativeHandle();
+  ASSERT_TRUE(native_window != NULL);
+
+  // Create the dialog wrapper object, but don't show it yet.
+  scoped_ptr<MockSelectFileDialogListener> listener(
+      new MockSelectFileDialogListener());
+  scoped_refptr<FileManagerDialog> dialog =
+      new FileManagerDialog(listener.get());
+
+  // Before we call SelectFile, dialog is not running/visible.
+  ASSERT_FALSE(dialog->IsRunning(native_window));
+
+  // Release the dialog first, as it holds a pointer to the listener.
+  dialog.release();
+  listener.reset();
+}
+
+IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, FileManagerDestroyListener) {
+  // Create the dialog wrapper object, but don't show it yet.
+  scoped_ptr<MockSelectFileDialogListener> listener(
+      new MockSelectFileDialogListener());
+  scoped_refptr<FileManagerDialog> dialog =
+      new FileManagerDialog(listener.get());
+
+  // Some users of SelectFileDialog destroy their listener before cleaning
+  // up the dialog.  Make sure we don't crash.
+  dialog->ListenerDestroyed();
+  listener.reset();
+
+  dialog.release();
+}
+
+IN_PROC_BROWSER_TEST_F(FileManagerDialogTest, SelectFileAndCancel) {
+  // Create the dialog wrapper object, but don't show it yet.
+  scoped_ptr<MockSelectFileDialogListener> listener(
+      new MockSelectFileDialogListener());
+  scoped_refptr<FileManagerDialog> dialog =
+      new FileManagerDialog(listener.get());
+
+  // 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 */);
+  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,
+                     NULL /* params */);
+  LOG(INFO) << "Waiting for JavaScript ready message.";
+  ASSERT_TRUE(msg_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(
+      NotificationType::RENDER_WIDGET_HOST_DESTROYED,
+      NotificationService::AllSources());
+  RenderViewHost* host = dialog->GetRenderViewHost();
+  std::wstring script =
+      L"console.log('Test JavaScript injected.');"
+      L"document.querySelector('.cancel').click();";
+  ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(
+      host, L"" /* frame_xpath */, script));
+  LOG(INFO) << "Waiting for window close notification.";
+  host_destroyed.Wait();
+
+  // Dialog no longer believes it is running.
+  ASSERT_FALSE(dialog->IsRunning(owning_window));
+
+  // Listener should have been informed of the cancellation.
+  ASSERT_TRUE(listener->canceled());
+
+  // Enforce deleting the dialog first.
+  dialog.release();
+  listener.reset();
+}
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 16ee15d..1870402 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -3032,7 +3032,8 @@
         'browser/ui/views/extensions/extension_view.h',
         'browser/ui/views/external_protocol_dialog.cc',
         'browser/ui/views/external_protocol_dialog.h',
-        'browser/ui/views/file_manager_dialogs.cc',
+        'browser/ui/views/file_manager_dialog.cc',
+        'browser/ui/views/file_manager_dialog.h',
         'browser/ui/views/find_bar_host.cc',
         'browser/ui/views/find_bar_host.h',
         'browser/ui/views/find_bar_host_gtk.cc',
@@ -4437,14 +4438,17 @@
              'browser/ui/crypto_module_password_dialog_openssl.cc',
           ]},
         ],
+        # File manager extension replaces the native OS file open/save dialog.
         ['file_manager_extension==1', {
           'sources/': [
             ['exclude', '^browser/ui/gtk/dialogs_gtk.cc'],
             ['exclude', '^browser/ui/views/select_file_dialog.cc'],
-            ['include', '^browser/ui/views/file_manager_dialogs.cc'],
+            ['include', '^browser/ui/views/file_manager_dialog.cc'],
+            ['include', '^browser/ui/views/file_manager_dialog.h'],
           ]}, {
           'sources/': [
-            ['exclude', '^browser/ui/views/file_manager_dialogs.cc'],
+            ['exclude', '^browser/ui/views/file_manager_dialog.cc'],
+            ['exclude', '^browser/ui/views/file_manager_dialog.h'],
           ]}
         ],
       ],
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 4c2f366..a8a3b77 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -2470,6 +2470,7 @@
         'browser/ui/panels/panel_browser_view_browsertest.cc',
         'browser/ui/views/browser_actions_container_browsertest.cc',
         'browser/ui/views/dom_view_browsertest.cc',
+        'browser/ui/views/file_manager_dialog_browsertest.cc',
         'browser/ui/views/html_dialog_view_browsertest.cc',
         'browser/ui/webui/chrome_url_data_manager_browsertest.cc',
         'browser/ui/webui/ntp/most_visited_browsertest.cc',
@@ -2534,6 +2535,11 @@
             'browser/service/service_process_control_browsertest.cc',
           ],
         }],
+        ['file_manager_extension==0', {
+          'sources!': [
+            'browser/ui/views/file_manager_dialog_browsertest.cc',
+          ],
+        }],
         ['toolkit_views==0', {
           'sources!': [
             'browser/extensions/extension_input_apitest.cc',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 7d9ba43..084acd3 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1064,12 +1064,6 @@ const char kScreenSaverUrl[]                = "screen-saver-url";
 // Flag to trigger ChromeOS system log compression during feedback submit.
 const char kCompressSystemFeedback[]        = "compress-sys-feedback";
 
-// Flag to skip loading ChromeOS specific component extensions. This one is
-// needed to prevent these component interfering with the some of the tests.
-// TODO(zelidrag): http://crosbug.com/14463 - we should remove this switch once
-// get rid of ChromeOS component extensions with background pages.
-const char kSkipChromeOSComponents[]        = "skip-chromeos-components";
-
 // Forces usage of libcros stub implementation. For testing purposes, this
 // switch separates chrome code from the rest of ChromeOS.
 const char kForceStubLibcros[]              = "force-stub-libcros";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 50a9699..38896e9 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -297,7 +297,6 @@ extern const char kGuestSession[];
 extern const char kStubCros[];
 extern const char kScreenSaverUrl[];
 extern const char kCompressSystemFeedback[];
-extern const char kSkipChromeOSComponents[];
 #endif
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index ccb73c2..69c8890 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -145,10 +145,6 @@ void InProcessBrowserTest::SetUp() {
   // Disable audio mixer as it can cause hang.
   // see http://crosbug.com/17058.
   chromeos::AudioHandler::Disable();
-
-  // Prevent loading ChromeOS component extension to prevent their interference
-  // with other browser tests.
-  command_line->AppendSwitch(switches::kSkipChromeOSComponents);
 #endif  // defined(OS_CHROMEOS)
 
   SandboxInitWrapper sandbox_wrapper;
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index cc5917a..2830848 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -198,9 +198,6 @@ void UITestBase::SetLaunchSwitches() {
     launch_arguments_.AppendSwitchASCII(switches::kHomePage, homepage_);
   if (!test_name_.empty())
     launch_arguments_.AppendSwitchASCII(switches::kTestName, test_name_);
-#if defined(OS_CHROMEOS)
-  launch_arguments_.AppendSwitch(switches::kSkipChromeOSComponents);
-#endif
 }
 
 void UITestBase::LaunchBrowser() {