[Extensions] Remove ChromeExtensionFunctionDetails::GetOriginWebContents

chrome/browser/extensions/api/media_galleries/media_galleries_api.cc

Index: chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
diff --git a/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc b/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
index 6daf3bdca3e0966d60ec24f60f9b690dc0ac3460..c1b0f19cd5d7eb792ea57b21c91a18cf30b9df9c 100644
--- a/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
+++ b/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
@@ -41,6 +41,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/storage_monitor/storage_info.h"
+#include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/blob_handle.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
@@ -49,6 +50,8 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/blob_holder.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
@@ -262,6 +265,30 @@ class SelectDirectoryDialog : public ui::SelectFileDialog::Listener,
   DISALLOW_COPY_AND_ASSIGN(SelectDirectoryDialog);
 };
 
+// Returns a web contents to use as the source for a prompt showing to the user.
+// The web contents has to support modal dialogs, so it can't be the app's
+// background page.
+content::WebContents* GetWebContentsForPrompt(

[tommycli, 2016/11/22 21:59:41]

AFAICT, this function basically does: If the sender web contents supports
dialogs, use that. Otherwise use the "current" window for the app, which is
guaranteed to support modal dialogs.

That sounds good - but I am a bit surprised that this function is in the
anonymous section of media_galleries_api rather than UIThreadExtensionFunction.

It doesn't seem media galleries specific at all... just happens that Media
Galleries is the first (or only?) API that needs to display a modal dialog?

[Devlin, 2016/11/22 22:42:29]

More or less.  I'm actually moving it from ChromeExtensionFunctionDetails. 
Right now, media galleries is the only thing that needs this implementation, and
the implementation changes depending on e.g. whether or not the calling item is
an extension or an app, whether or not using a foreground tab or the browser
window is acceptable, etc.  Additionally, a name like GetOriginWebContents()
really doesn't describe what it's doing, and doesn't cover these questions of
acceptable criteria.

I could see in the future using a variant of GetNativeWindowForUI(), but I don't
think that's quite right here either, for the same reasons of ambiguity around
what's acceptable.  If/when we clean that up, it's probably what we'd want.

+    content::WebContents* sender_web_contents,
+    content::BrowserContext* browser_context,
+    const std::string& app_id) {
+  // Check if the sender web contents supports modal dialogs.
+  web_modal::WebContentsModalDialogManager* web_contents_modal_dialog_manager =
+      web_modal::WebContentsModalDialogManager::FromWebContents(
+          sender_web_contents);
+  if (web_contents_modal_dialog_manager)
+    return sender_web_contents;
+  // Otherwise, check for the current app window for the app (app windows
+  // support modal dialogs).
+  if (!app_id.empty()) {
+    AppWindow* window = AppWindowRegistry::Get(browser_context)
+                            ->GetCurrentAppWindowForApp(app_id);
+    if (window)
+      return window->web_contents();
+  }
+  return nullptr;
+}
+
 }  // namespace
 
 MediaGalleriesEventRouter::MediaGalleriesEventRouter(
@@ -445,7 +472,9 @@ void MediaGalleriesGetMediaFileSystemsFunction::ReturnGalleries(
 void MediaGalleriesGetMediaFileSystemsFunction::ShowDialog() {
   media_galleries::UsageCount(media_galleries::SHOW_DIALOG);
   WebContents* contents =
-      ChromeExtensionFunctionDetails(this).GetOriginWebContents();
+      GetWebContentsForPrompt(GetSenderWebContents(),

[lazyboy, 2016/11/22 20:47:41]

Previously the code would check if GetSenderWebContents() is null:

chrome/browser/extensions/chrome_extension_function_details.cc:109, should we
add this back as WebContentsModalDialogManager::FromWebContents in
GetWebContentsForPrompt will try to deref the WebContents*?

[Devlin, 2016/11/22 22:42:29]

Good call, now checking in GetWebContentsForPrompt.

+                              browser_context(),
+                              extension()->id());
   if (!contents) {
     SendResponse(false);
     return;
@@ -482,10 +511,11 @@ bool MediaGalleriesAddUserSelectedFolderFunction::RunAsync() {
 }
 
 void MediaGalleriesAddUserSelectedFolderFunction::OnPreferencesInit() {
-  Profile* profile = GetProfile();
   const std::string& app_id = extension()->id();
   WebContents* contents =
-      ChromeExtensionFunctionDetails(this).GetOriginWebContents();
+      GetWebContentsForPrompt(GetSenderWebContents(),
+                              browser_context(),
+                              app_id);
   if (!contents) {
     SendResponse(false);
     return;
@@ -498,7 +528,7 @@ void MediaGalleriesAddUserSelectedFolderFunction::OnPreferencesInit() {
 
   base::FilePath last_used_path =
       extensions::file_system_api::GetLastChooseEntryDirectory(
-          extensions::ExtensionPrefs::Get(profile), app_id);
+          extensions::ExtensionPrefs::Get(browser_context()), app_id);
   SelectDirectoryDialog::Callback callback = base::Bind(
       &MediaGalleriesAddUserSelectedFolderFunction::OnDirectorySelected, this);
   scoped_refptr<SelectDirectoryDialog> select_directory_dialog =