Show extension uninstall dialog in browser window.

chrome/browser/extensions/api/management/chrome_management_api_delegate.cc

 // Copyright 2014 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/extensions/api/management/chrome_management_api_delegate.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/bookmark_app_helper.h"
 #include "chrome/browser/extensions/chrome_requirements_checker.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 12 matching lines @@
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/web_application_info.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/safe_json/safe_json_parser.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/utility_process_host.h"
 #include "content/public/browser/utility_process_host_client.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/api/management/management_api.h"
 #include "extensions/browser/api/management/management_api_constants.h"
+#include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/manifest_handlers/background_info.h"
 
 namespace {
 
 class ManagementSetEnabledFunctionInstallPromptDelegate
     : public ExtensionInstallPrompt::Delegate,
       public extensions::InstallPromptDelegate {
  public:
   ManagementSetEnabledFunctionInstallPromptDelegate(
       extensions::ManagementSetEnabledFunction* function,
       const extensions::Extension* extension)
@@ skipping 22 matching lines @@
 
 class ManagementUninstallFunctionUninstallDialogDelegate
     : public extensions::ExtensionUninstallDialog::Delegate,
       public extensions::UninstallDialogDelegate {
  public:
   ManagementUninstallFunctionUninstallDialogDelegate(
       extensions::ManagementUninstallFunctionBase* function,
       const extensions::Extension* target_extension,
       bool show_programmatic_uninstall_ui)
       : function_(function) {
+    Profile* profile = Profile::FromBrowserContext(function->browser_context());
     content::WebContents* web_contents = function->GetSenderWebContents();
+    const extensions::Extension* extension = function->extension();
+    gfx::NativeWindow window =
+        web_contents ? web_contents->GetTopLevelNativeWindow() : nullptr;
+
+    if (extension && extensions::BackgroundInfo::HasBackgroundPage(extension)) {
+      extensions::ExtensionHost* background_host =
+          extensions::ProcessManager::Get(function->browser_context())
+              ->GetBackgroundHostForExtension(extension->id());
+      // Do not use the window of the background page.
+      if (background_host && background_host->host_contents() == web_contents)
+        window = nullptr;
+    }

[not at google - send to devlin, 2015/07/08 23:10:04]

Is this entire block of code necessary? I would have thought that a background
contents would return null for GetTopLevelNativeWindow(), since it doesn't have
one.

[wjywbs, 2015/07/09 00:27:12]

No, GetTopLevelNativeWindow() does not return null for background contents, and
this is why the crash happened. It returns a valid pointer, but calling
window->GetRootWindow() on this pointer will return null.

https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/widget/na...

[not at google - send to devlin, 2015/07/09 16:30:42]

ok, could you check for a null root window then?

[wjywbs, 2015/07/09 16:54:25]

The window->GetRootWindow() function is only defined in aura, but not on Mac.
Should I add a "#if defined(USE_AURA)" instead?

[not at google - send to devlin, 2015/07/09 17:13:39]

ahhh damn it. Alright! This all finally reminded me of a very similar situation
we had with the media galleries API. It did two things:

1. The lack of a place to display the dialogue is detected by the lack of a
"dialogue manager". A bit of a hack but apparently it works:

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

2. In the case where there *isn't* a place to show it (including from a
background page), trace the caller back to the tab that initiated the call to
uninstall() in the first place:

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

i.e. you can access source_tab_id(). Could you try something like that?

I would pull that function I mentioned in (1) into a function on at least
ChromeUIThreadExtensionFunction, "GetVisibleWebContents()". It would be nice to
pull this into UIThreadExtensionFunction but at this point I don't want to add a
dependency from the extensions module into the web_modal component.

[wjywbs, 2015/07/10 02:34:43]

Done.

+
+    if (!window) {
+      Browser* browser =
+          chrome::FindBrowserWithProfile(profile, chrome::GetActiveDesktop());
+      window = browser->window()->GetNativeWindow();
+    }
     extension_uninstall_dialog_.reset(
-        extensions::ExtensionUninstallDialog::Create(
-            Profile::FromBrowserContext(function->browser_context()),
-            web_contents ? web_contents->GetTopLevelNativeWindow() : nullptr,
-            this));
+        extensions::ExtensionUninstallDialog::Create(profile, window, this));
     if (show_programmatic_uninstall_ui) {
       extension_uninstall_dialog_->ConfirmUninstallByExtension(
           target_extension, function->extension(),
           extensions::UNINSTALL_REASON_MANAGEMENT_API);
     } else {
       extension_uninstall_dialog_->ConfirmUninstall(
           target_extension, extensions::UNINSTALL_REASON_MANAGEMENT_API);
     }
   }
   ~ManagementUninstallFunctionUninstallDialogDelegate() override {}
@@ skipping 214 matching lines @@
 
 GURL ChromeManagementAPIDelegate::GetIconURL(
     const extensions::Extension* extension,
     int icon_size,
     ExtensionIconSet::MatchType match,
     bool grayscale,
     bool* exists) const {
   return extensions::ExtensionIconSource::GetIconURL(extension, icon_size,
                                                      match, grayscale, exists);
 }