Merge 88759 - CrOS - Fix crash when pressing Enter in file browser dialog

If focus is on the cancel button, pressing enter will both cancel the dialog and attempt to select a file.  Fix by ensuring that SelectFileDialog::Listener objects are invoked exactly once (some listeners delete themselves after being invoked).

BUG=chromium-os:16349
TEST=manual, see bug.  Also browsertest FileManagerDialogTest.*

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

TBR=jamescook@chromium.org
Review URL: http://codereview.chromium.org/7003150

git-svn-id: svn://svn.chromium.org/chrome/branches/782/src@88857 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 34 insertions, 5 deletions

diff --git a/chrome/browser/extensions/extension_file_browser_private_api.cc b/chrome/browser/extensions/extension_file_browser_private_api.cc
index 4c85345..d369152 100644
--- a/chrome/browser/extensions/extension_file_browser_private_api.cc
+++ b/chrome/browser/extensions/extension_file_browser_private_api.cc
@@ -816,6 +816,12 @@ const FileDialogFunction::Callback& FileDialogFunction::GetCallback() const {
   return Callback::Find(GetTabId());
 }
 
+void FileDialogFunction::RemoveCallback() {
+  // Listeners expect to be invoked exactly once, so we need to remove our
+  // callback objects afterwards.
+  Callback::Remove(GetTabId());
+}
+
 // GetFileSystemRootPathOnFileThread can only be called from the file thread,
 // so here we are. This function takes a vector of virtual paths, converts
 // them to local paths and calls GetLocalPathsResponseOnUIThread with the
@@ -902,6 +908,7 @@ void SelectFileFunction::GetLocalPathsResponseOnUIThread(
     callback.listener()->FileSelected(files[0],
                                       index,
                                       callback.params());
+    RemoveCallback();  // Listeners expect to be invoked exactly once.
   }
   SendResponse(true);
 }
@@ -996,6 +1003,7 @@ void SelectFilesFunction::GetLocalPathsResponseOnUIThread(
   DCHECK(!callback.IsNull());
   if (!callback.IsNull()) {
     callback.listener()->MultiFilesSelected(files, callback.params());
+    RemoveCallback();  // Listeners expect to be invoked exactly once.
   }
   SendResponse(true);
 }
@@ -1005,6 +1013,7 @@ bool CancelFileDialogFunction::RunImpl() {
   DCHECK(!callback.IsNull());
   if (!callback.IsNull()) {
     callback.listener()->FileSelectionCanceled(callback.params());
+    RemoveCallback();  // Listeners expect to be invoked exactly once.
   }
   SendResponse(true);
   return true;
diff --git a/chrome/browser/extensions/extension_file_browser_private_api.h b/chrome/browser/extensions/extension_file_browser_private_api.h
index 207d281..2673874 100644
--- a/chrome/browser/extensions/extension_file_browser_private_api.h
+++ b/chrome/browser/extensions/extension_file_browser_private_api.h
@@ -126,6 +126,9 @@ class FileDialogFunction
   // Get the callback for the hosting tab.
   const Callback& GetCallback() const;
 
+  // Remove the callback for the hosting tab.
+  void RemoveCallback();
+
  private:
   // Figure out the tab_id of the hosting tab.
   int32 GetTabId() const;
diff --git a/chrome/browser/resources/file_manager/js/file_manager.js b/chrome/browser/resources/file_manager/js/file_manager.js
index 831d8ae..0175a4b 100644
--- a/chrome/browser/resources/file_manager/js/file_manager.js
+++ b/chrome/browser/resources/file_manager/js/file_manager.js
@@ -1999,8 +1999,10 @@ FileManager.prototype = {
         if (this.selection.totalCount == 1 &&
             this.selection.leadEntry.isDirectory &&
             this.dialogType_ != FileManager.SELECT_FOLDER) {
+          event.preventDefault();
           this.changeDirectory(this.selection.leadEntry.fullPath);
         } else if (!this.okButton_.disabled) {
+          event.preventDefault();
           this.onOk_();
         }
         break;
diff --git a/chrome/browser/ui/views/file_manager_dialogs.cc b/chrome/browser/ui/views/file_manager_dialogs.cc
index 0c3dd5e..3bc9b99 100644
--- a/chrome/browser/ui/views/file_manager_dialogs.cc
+++ b/chrome/browser/ui/views/file_manager_dialogs.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/views/extensions/extension_dialog.h"
 #include "chrome/browser/ui/views/window.h"
 #include "content/browser/browser_thread.h"
@@ -22,6 +23,19 @@ namespace {
 const int kFileManagerWidth = 720;  // pixels
 const int kFileManagerHeight = 580;  // pixels
 
+// Returns the browser represented by |window| or NULL if not found.
+// TODO(jamescook):  Move this to browser_list.h.
+Browser* FindBrowserWithWindow(gfx::NativeWindow window) {
+  for (BrowserList::const_iterator it = BrowserList::begin();
+       it != BrowserList::end();
+       ++it) {
+    Browser* browser = *it;
+    if (browser->window() && browser->window()->GetNativeHandle() == window)
+      return browser;
+  }
+  return NULL;
+}
+
 }
 
 // Shows a dialog box for selecting a file or a folder.
@@ -113,20 +127,21 @@ void FileManagerDialog::SelectFileImpl(
     LOG(ERROR) << "File dialog already in use!";
     return;
   }
-  Browser* active_browser = BrowserList::GetLastActive();
-  if (!active_browser)
+  Browser* owner_browser = FindBrowserWithWindow(owner_window);
+  if (!owner_browser) {
+    NOTREACHED() << "Can't find owning browser";
     return;
+  }
 
   GURL file_browser_url = FileManagerUtil::GetFileBrowserUrlWithParams(
       type, title, default_path, file_types, file_type_index,
       default_extension);
   extension_dialog_ = ExtensionDialog::Show(file_browser_url,
-      active_browser, kFileManagerWidth, kFileManagerHeight,
+      owner_browser, kFileManagerWidth, kFileManagerHeight,
       this /* ExtensionDialog::Observer */);
 
   // Connect our listener to FileDialogFunction's per-tab callbacks.
-  Browser* extension_browser = extension_dialog_->host()->view()->browser();
-  TabContents* contents = extension_browser->GetSelectedTabContents();
+  TabContents* contents = owner_browser->GetSelectedTabContents();
   int32 tab_id = (contents ? contents->controller().session_id().id() : 0);
   FileDialogFunction::Callback::Add(tab_id, listener_, params);