Show extension uninstall dialog in browser window.

chrome/browser/extensions/chrome_extension_function_details.cc

Index: chrome/browser/extensions/chrome_extension_function_details.cc
diff --git a/chrome/browser/extensions/chrome_extension_function_details.cc b/chrome/browser/extensions/chrome_extension_function_details.cc
index 922c96be53d7f720dcdcc1c6b0283b241d441893..d9ff40f253b8e2e3c91dde6d7ea91ca6a960a707 100644
--- a/chrome/browser/extensions/chrome_extension_function_details.cc
+++ b/chrome/browser/extensions/chrome_extension_function_details.cc
@@ -4,14 +4,18 @@
 
 #include "chrome/browser/extensions/chrome_extension_function_details.h"
 
+#include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/window_controller.h"
 #include "chrome/browser/extensions/window_controller_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/extension_function.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 
@@ -113,3 +117,37 @@ ChromeExtensionFunctionDetails::GetAssociatedWebContents() {
     return NULL;
   return browser->tab_strip_model()->GetActiveWebContents();
 }
+
+content::WebContents* ChromeExtensionFunctionDetails::GetVisibleWebContents() {
+  WebContents* contents =
+      WebContents::FromRenderFrameHost(function_->render_frame_host());

[not at google - send to devlin, 2015/07/10 17:02:25]

You should be able to use GetSenderWebContents() here. It knows how to pull out
the WebContents from render_frame_host.

You should also null-check it and return nullptr if there is no sender.

+  web_modal::WebContentsModalDialogManager* web_contents_modal_dialog_manager =

[not at google - send to devlin, 2015/07/10 17:02:25]

Add comment:

// Hack: use the existence of a WebContentsModalDialogManager to decide whether
// the sender web contents is visible.

I'd also prefer to early-return rather than putting the rest of the function
body behind this check.

+      web_modal::WebContentsModalDialogManager::FromWebContents(contents);
+  if (!web_contents_modal_dialog_manager) {
+    // If there is no WebContentsModalDialogManager, then this contents is
+    // probably the background page for an app. Try to find a app window to
+    // host the dialog.
+    contents = nullptr;
+
+    Profile* profile = GetProfile();
+    const extensions::Extension* extension = function_->extension();
+    if (extension) {
+      extensions::AppWindow* window =

[not at google - send to devlin, 2015/07/10 17:57:03]

Actually, perhaps you should be using WindowControllerList here?

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

like:

// Get the WindowControllerList from somewhere. Where?
WindowControllerList::CurrentWindowForFunction(function_)->window()->GetNativeWindow();

but I'd (or you'd) need to double check that CurrentWindowForFunction does what
you'd expect. I *think* it does. This would let you pick up devtools windows as
well, I believe.

[wjywbs, 2015/07/10 19:54:22]

Turns out that CurrentWindowForFunction() can properly find extension's window
but not app's window, but I used CurrentWindowForFunction() in
GetNativeWindowForUI().

+          extensions::AppWindowRegistry::Get(profile)
+              ->GetCurrentAppWindowForApp(extension->id());
+      if (window)
+        contents = window->web_contents();

[not at google - send to devlin, 2015/07/10 17:02:25]

Likewise, early-return (even once moved to the end):

if (window)
  return window->web_contents();

+    }
+
+    int source_tab_id = function_->source_tab_id();

[not at google - send to devlin, 2015/07/10 17:02:25]

Tiny request: put the checks for the tab above the check for the app window.
It's more likely to pass, so it saves looking up an app window probably
unnecessarily.

+    if (!contents && source_tab_id != TabStripModel::kNoTab) {
+      // When the request originated from a background page, but there is no
+      // app window open, check to see if it originated from a tab and display
+      // the dialog in that tab.
+      extensions::ExtensionTabUtil::GetTabById(source_tab_id, profile,
+                                               profile->IsOffTheRecord(), NULL,

[not at google - send to devlin, 2015/07/10 17:02:25]

just use "true" here, not "profile->IsOffTheRecord()". But do comment it:

...
true,  // include_incognito
...

+                                               NULL, &contents, NULL);

[not at google - send to devlin, 2015/07/10 17:02:25]

always use nullptr not NULL now.

+    }

[not at google - send to devlin, 2015/07/10 17:02:25]

and early-return.

+  }
+  return contents;
+}