Cancel uninstall if the uninstall dialog's parent window is destroyed prior to

the uninstall dialog showing.

BUG=424999, 397396
TEST=ExtensionUninstallDialogViewBrowserTest.TrackParentWindowDestruction

Review URL: https://codereview.chromium.org/697023002

Cr-Commit-Position: refs/heads/master@{#302691}

chrome/browser/extensions/extension_uninstall_dialog.cc

@@ -39,10 +39,8 @@ SkBitmap GetDefaultIconBitmapForMaxScaleFactor(bool is_app) {
 
 ExtensionUninstallDialog::ExtensionUninstallDialog(
     Profile* profile,
-    gfx::NativeWindow parent,
     ExtensionUninstallDialog::Delegate* delegate)
     : profile_(profile),
-      parent_(parent),
       delegate_(delegate),
       extension_(NULL),
       triggering_extension_(NULL),

chrome/browser/extensions/extension_uninstall_dialog.h

@@ -63,17 +63,11 @@ class ExtensionUninstallDialog
 
  protected:
   // Constructor used by the derived classes.
-  ExtensionUninstallDialog(Profile* profile,
-                           gfx::NativeWindow parent,
-                           Delegate* delegate);
+  ExtensionUninstallDialog(Profile* profile, Delegate* delegate);
 
   // TODO(sashab): Remove protected members: crbug.com/397395
   Profile* const profile_;
 
-  // TODO(sashab): Investigate lifetime issue of this window variable:
-  // crbug.com/397396
-  gfx::NativeWindow parent_;
-
   // The delegate we will call Accepted/Canceled on after confirmation dialog.
   Delegate* delegate_;
 

chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm

@@ -25,9 +25,7 @@ namespace {
 class ExtensionUninstallDialogCocoa
     : public extensions::ExtensionUninstallDialog {
  public:
-  ExtensionUninstallDialogCocoa(Profile* profile,
-                                gfx::NativeWindow parent,
-                                Delegate* delegate);
+  ExtensionUninstallDialogCocoa(Profile* profile, Delegate* delegate);
   ~ExtensionUninstallDialogCocoa() override;
 
  private:
@@ -36,9 +34,8 @@ class ExtensionUninstallDialogCocoa
 
 ExtensionUninstallDialogCocoa::ExtensionUninstallDialogCocoa(
     Profile* profile,
-    gfx::NativeWindow parent,
     extensions::ExtensionUninstallDialog::Delegate* delegate)
-    : extensions::ExtensionUninstallDialog(profile, parent, delegate) {
+    : extensions::ExtensionUninstallDialog(profile, delegate) {
 }
 
 ExtensionUninstallDialogCocoa::~ExtensionUninstallDialogCocoa() {}
@@ -73,5 +70,5 @@ extensions::ExtensionUninstallDialog*
 extensions::ExtensionUninstallDialog::Create(Profile* profile,
                                              gfx::NativeWindow parent,
                                              Delegate* delegate) {
-  return new ExtensionUninstallDialogCocoa(profile, parent, delegate);
+  return new ExtensionUninstallDialogCocoa(profile, delegate);
 }

chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc

@@ -12,6 +12,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "components/constrained_window/constrained_window_views.h"
 #include "extensions/common/extension.h"
+#include "ui/aura/window_tracker.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/layer.h"
@@ -35,7 +36,7 @@ class ExtensionUninstallDialogViews
  public:
   ExtensionUninstallDialogViews(
       Profile* profile,
-      gfx::NativeWindow parent,
+      aura::Window* parent,
       extensions::ExtensionUninstallDialog::Delegate* delegate);
   ~ExtensionUninstallDialogViews() override;
 
@@ -52,6 +53,12 @@ class ExtensionUninstallDialogViews
 
   ExtensionUninstallDialogDelegateView* view_;
 
+  // The dialog's parent window.
+  aura::Window* parent_;
+
+  // Tracks whether |parent_| got destroyed.
+  aura::WindowTracker parent_window_tracker_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionUninstallDialogViews);
 };
 
@@ -100,10 +107,13 @@ class ExtensionUninstallDialogDelegateView : public views::DialogDelegateView {
 
 ExtensionUninstallDialogViews::ExtensionUninstallDialogViews(
     Profile* profile,
-    gfx::NativeWindow parent,
+    aura::Window* parent,
     extensions::ExtensionUninstallDialog::Delegate* delegate)
-    : extensions::ExtensionUninstallDialog(profile, parent, delegate),
-      view_(NULL) {
+    : extensions::ExtensionUninstallDialog(profile, delegate),
+      view_(NULL),
+      parent_(parent) {
+  if (parent_)
+    parent_window_tracker_.Add(parent_);
 }
 
 ExtensionUninstallDialogViews::~ExtensionUninstallDialogViews() {
@@ -115,6 +125,11 @@ ExtensionUninstallDialogViews::~ExtensionUninstallDialogViews() {
 }
 
 void ExtensionUninstallDialogViews::Show() {
+  if (parent_ && !parent_window_tracker_.Contains(parent_)) {
+    delegate_->ExtensionUninstallCanceled();
+    return;
+  }
+
   view_ = new ExtensionUninstallDialogDelegateView(
       this, extension_, triggering_extension_, &icon_);
   CreateBrowserModalDialogViews(view_, parent_)->Show();

chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.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 "base/run_loop.h"
+#include "chrome/browser/extensions/extension_uninstall_dialog.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_builder.h"
+#include "extensions/common/value_builder.h"
+
+namespace {
+
+scoped_refptr<extensions::Extension> BuildTestExtension() {
+  return extensions::ExtensionBuilder()
+      .SetManifest(extensions::DictionaryBuilder()
+                       .Set("name", "foo")
+                       .Set("version", "1.0"))
+      .Build();
+}
+
+class TestExtensionUninstallDialogDelegate
+    : public extensions::ExtensionUninstallDialog::Delegate {
+ public:
+  explicit TestExtensionUninstallDialogDelegate(
+      const base::Closure& quit_closure)
+      : quit_closure_(quit_closure), canceled_(false) {}
+
+  ~TestExtensionUninstallDialogDelegate() override {}
+
+  bool canceled() { return canceled_; }
+
+ private:
+  void ExtensionUninstallAccepted() override { quit_closure_.Run(); }
+
+  void ExtensionUninstallCanceled() override {
+    canceled_ = true;
+    quit_closure_.Run();
+  }
+
+  base::Closure quit_closure_;
+  bool canceled_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestExtensionUninstallDialogDelegate);
+};
+
+}  // namespace
+
+typedef InProcessBrowserTest ExtensionUninstallDialogViewBrowserTest;
+
+// Test that ExtensionUninstallDialog cancels the uninstall if the aura::Window
+// which is passed to ExtensionUninstallDialog::Create() is destroyed.
+IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
+                       TrackParentWindowDestruction) {
+  // Create a second browser to prevent the app from exiting when the browser is
+  // closed.
+  CreateBrowser(browser()->profile());
+
+  scoped_refptr<extensions::Extension> extension(BuildTestExtension());
+
+  base::RunLoop run_loop;
+  TestExtensionUninstallDialogDelegate delegate(run_loop.QuitClosure());
+  scoped_ptr<extensions::ExtensionUninstallDialog> dialog(
+      extensions::ExtensionUninstallDialog::Create(
+          browser()->profile(), browser()->window()->GetNativeWindow(),
+          &delegate));
+  browser()->window()->Close();
+  content::RunAllPendingInMessageLoop();
+
+  dialog->ConfirmUninstall(extension.get());
+  run_loop.Run();
+  EXPECT_TRUE(delegate.canceled());
+}

chrome/chrome_tests.gypi

@@ -638,6 +638,7 @@
       'browser/ui/views/autofill/password_generation_popup_view_tester_views.cc',
       'browser/ui/views/autofill/password_generation_popup_view_tester_views.h',
       'browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc',
+      'browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc',
       'browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc',
       'browser/ui/views/frame/browser_view_browsertest.cc',
       'browser/ui/views/frame/browser_window_property_manager_browsertest_win.cc',