Allow ChromeOS file selection dialog to be shown from shell windows.

chrome/browser/ui/views/select_file_dialog_extension.cc

Index: chrome/browser/ui/views/select_file_dialog_extension.cc
diff --git a/chrome/browser/ui/views/select_file_dialog_extension.cc b/chrome/browser/ui/views/select_file_dialog_extension.cc
index b8e3d26aa968a48a2d43028f7f94d3a5cc4e28f2..bc934ad2e3127616413f82420fec3a77a350eff5 100644
--- a/chrome/browser/ui/views/select_file_dialog_extension.cc
+++ b/chrome/browser/ui/views/select_file_dialog_extension.cc
@@ -10,15 +10,19 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/extensions/file_browser_private_api.h"
 #include "chrome/browser/chromeos/extensions/file_manager_util.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/shell_window_registry.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sessions/restore_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/extensions/shell_window.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/views/extensions/extension_dialog.h"
 #include "content/public/browser/browser_thread.h"
@@ -96,7 +100,7 @@ SelectFileDialogExtension::SelectFileDialogExtension(Listener* listener)
     : SelectFileDialog(listener),
       has_multiple_file_type_choices_(false),
       tab_id_(0),
-      owner_browser_(NULL),
+      profile_(NULL),
       owner_window_(NULL),
       selection_type_(CANCEL),
       selection_index_(0),
@@ -121,7 +125,7 @@ void SelectFileDialogExtension::ListenerDestroyed() {
 
 void SelectFileDialogExtension::ExtensionDialogClosing(
     ExtensionDialog* dialog) {
-  owner_browser_ = NULL;
+  profile_ = NULL;
   owner_window_ = NULL;
   // Release our reference to the dialog to allow it to close.
   extension_dialog_ = NULL;
@@ -144,10 +148,10 @@ void SelectFileDialogExtension::ExtensionTerminated(
   // time, the extension subsystem would automatically reload it for us. At
   // this time though this is broken because of some faulty wiring in
   // ExtensionProcessManager::CreateViewHost. Once that is fixed, remove this.
-  if (owner_browser_) {
+  if (profile_) {
     MessageLoop::current()->PostTask(FROM_HERE,
         base::Bind(&ExtensionService::ReloadExtension,
-            base::Unretained(owner_browser_->profile()->GetExtensionService()),
+            base::Unretained(profile_->GetExtensionService()),
         extension_id));
   }
 
@@ -246,19 +250,58 @@ void SelectFileDialogExtension::SelectFileImpl(
     LOG(ERROR) << "File dialog already in use!";
     return;
   }
-  // Extension background pages may not supply an owner_window.
-  owner_browser_ = (owner_window ?
+
+  // The tab contents to associate the dialog with. If this is not set
+  // by the time the dialog is created, it means the dialog should be shown
+  // full screen and not associated with any tab contents.
+  TabContents* tab = NULL;
+
+  // Extension background pages may not supply an owner_window, in which

[Mihai Parparita -not on Chrome, 2012/06/20 05:21:06]

In cases where the the file picker is triggered from a platform app's background
page and there are no browser windows open, will you end up hitting the
NOTREACHED on line 297?

[benwells, 2012/06/20 05:41:32]

No, as that will fail in the file system API code which already checks that
there is a ShellWindow associated with the calling RenderViewHost. (See
FileSystemPickerFunction::ShowPicker in file_system_api.cc, conventiently also
modified in this change).

+  // case we use the last active browser window.
+  Browser* owner_browser = (owner_window ?
       browser::FindBrowserWithWindow(owner_window) :
       BrowserList::GetLastActive());
-  if (!owner_browser_) {
-    NOTREACHED() << "Can't find owning browser";
-    return;
+  if (owner_browser) {

[Mihai Parparita -not on Chrome, 2012/06/20 05:21:06]

In the scenario where you have a shell window and a browser open, it seems like
the dialog will end up being attached to the browser window, even if the shell
window is the one that triggered this. It seems like you should always do the
GetShellWindowForNativeWindow(owner_window) check.

[benwells, 2012/06/20 05:41:32]

As owner_window will be set it will call FindBrowserWithWindow, which will
return NULL, so I think it will work.

The logic was pretty convoluted though. Hopefully it's clearer now.

+    tab = owner_browser->GetActiveTabContents();
+    DCHECK(tab);
+    profile_ = tab->profile();
+  } else if (owner_window) {
+    // If there is no browser associated with owner_window, it could be for
+    // a shell window, so check this here.
+    // TODO(benwells): Find a better way to get a shell window from a native
+    // window.
+    // At the moment shell windows are misunderstood, with most of the system
+    // only understanding Browser objects as top level windows. Until shell
+    // windows are better understood we will show the dialog full screen,
+    // and only allow one dialog to be shown like this at a time.
+    // TODO(benwells): Allow ExtensionDialogs to also be shown on shell windows
+    // as they are for Browsers (centered in the window and associated with
+    // the window's tab contents).
+    std::vector<Profile*> profiles =
+        g_browser_process->profile_manager()->GetLoadedProfiles();
+    for (std::vector<Profile*>::const_iterator iter = profiles.begin();
+        iter < profiles.end(); iter++) {
+      ShellWindowRegistry* registry = ShellWindowRegistry::Get(*iter);
+      DCHECK(registry);
+      ShellWindow* shell_window = registry->GetShellWindowForNativeWindow(
+          owner_window);
+      if (shell_window) {
+        profile_ = *iter;
+        break;
+      }
+    }
   }
 
-  TabContents* tab = owner_browser_->GetActiveTabContents();
+  // If we still have no profile something has gone wrong.
+  if (!profile_) {
+    NOTREACHED() << "Can't find profile of owning window.";
+    return;
+  }
 
   // Check if we have another dialog opened in the tab. It's unlikely, but
-  // possible.
+  // possible. If there is no tab contents use a tab_id of 0. A dialog without
+  // an associated tab contents will be shown fully screen; only one at a time
+  // is allowed in this state.
   int32 tab_id = tab ? tab->restore_tab_helper()->session_id().id() : 0;
   if (PendingExists(tab_id)) {
     DLOG(WARNING) << "Pending dialog exists with id " << tab_id;
@@ -267,7 +310,7 @@ void SelectFileDialogExtension::SelectFileImpl(
 
   FilePath virtual_path;
   if (!file_manager_util::ConvertFileToRelativeFileSystemPath(
-          owner_browser_->profile(), default_path, &virtual_path)) {
+          profile_, default_path, &virtual_path)) {
     virtual_path = default_path.BaseName();
   }
 
@@ -278,16 +321,24 @@ void SelectFileDialogExtension::SelectFileImpl(
       type, title, virtual_path, file_types, file_type_index,
       default_extension);
 
-  ExtensionDialog* dialog = ExtensionDialog::Show(file_browser_url,
-      owner_browser_, tab->web_contents(),
-      kFileManagerWidth, kFileManagerHeight,
 #if defined(USE_AURA)
-      file_manager_util::GetTitleFromType(type),
+  string16 dialog_title = file_manager_util::GetTitleFromType(type);
 #else
-      // HTML-based header used.
-      string16(),
+  // HTML-based header used.
+  string16 dialog_title;
 #endif
-      this /* ExtensionDialog::Observer */);
+
+  // At this point either both owner_browser and tab should be set,
+  // or neither.
+  ExtensionDialog* dialog;
+  if (owner_browser) {
+    dialog = ExtensionDialog::Show(file_browser_url,
+        owner_browser, tab->web_contents(), kFileManagerWidth,
+        kFileManagerHeight, dialog_title, this /* ExtensionDialog::Observer */);
+  } else {
+    dialog = ExtensionDialog::ShowFullscreen(file_browser_url,
+        profile_, dialog_title, this /* ExtensionDialog::Observer */);
+  }
   if (!dialog) {
     LOG(ERROR) << "Unable to create extension dialog";
     return;