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

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

BUG=424999, 397396
TEST=ExtensionUninstallDialogViewBrowserTest.TrackParentWindowDestruction

Committed: https://crrev.com/ea87b86d5bd1abe26636abfc19af28e755da7585
Cr-Commit-Position: refs/heads/master@{#302691}

[pkotwicz, 2014-11-02 01:13:18]

pkotwicz@chromium.org changed reviewers:
+ asargent@chromium.org

[pkotwicz, 2014-11-02 01:20:34]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2014-11-02 01:26:17]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2014-11-02 01:28:53]

pkotwicz@chromium.org changed reviewers:
+ finnur@chromium.org
- asargent@chromium.org

[pkotwicz, 2014-11-02 01:29:12]

finnur@ PTAL

[Finnur, 2014-11-02 21:38:25]

LGTM

https://codereview.chromium.org/697023002/diff/40001/chrome/browser/ui/views/...
File
chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
(right):

https://codereview.chromium.org/697023002/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc:34:
bool DidCancel() { return canceled_; }
nit: did_cancel() or canceled()

[pkotwicz, 2014-11-03 16:13:50]

pkotwicz@chromium.org changed reviewers:
+ thakis@chromium.org

[pkotwicz, 2014-11-03 16:13:51]

thakis@ for chrome/browser/ui/cocoa OWNERS

[Nico, 2014-11-03 17:51:26]

i don't think this will build

https://codereview.chromium.org/697023002/diff/60001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm
(right):

https://codereview.chromium.org/697023002/diff/60001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm:41: :
extensions::ExtensionUninstallDialog(profile, parent, delegate) {
Don't you need to remove it here? (and I guess the parameter of this constructor
too)

[pkotwicz, 2014-11-04 01:43:48]

Thanks for noticing. thakis@ can you please take another look?

[Nico, 2014-11-04 02:20:38]

cocoa lgtm, but one remark:

https://codereview.chromium.org/697023002/diff/80001/chrome/browser/extension...
File chrome/browser/extensions/extension_uninstall_dialog.h (right):

https://codereview.chromium.org/697023002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/extension_uninstall_dialog.h:46: gfx::NativeWindow
parent,
Shouldn't this parameter go away too now?

[pkotwicz, 2014-11-04 15:08:09]

https://codereview.chromium.org/697023002/diff/80001/chrome/browser/extension...
File chrome/browser/extensions/extension_uninstall_dialog.h (right):

https://codereview.chromium.org/697023002/diff/80001/chrome/browser/extension...
chrome/browser/extensions/extension_uninstall_dialog.h:46: gfx::NativeWindow
parent,
We use the parameter in the views implementation. I moved the member variable
out of the base class and into the views subclass because it was only used there

[pkotwicz, 2014-11-04 15:08:16]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-04 15:09:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/697023002/80001

[commit-bot: I haz the power, 2014-11-04 15:27:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-04 15:27:24]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[pkotwicz, 2014-11-04 18:42:53]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-04 18:43:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/697023002/100001

[commit-bot: I haz the power, 2014-11-04 20:00:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-04 20:00:37]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pkotwicz, 2014-11-04 21:21:52]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-04 21:23:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/697023002/100001

[commit-bot: I haz the power, 2014-11-04 22:42:53]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2014-11-04 22:43:32]

Patchset 4 (id:??) landed as
https://crrev.com/ea87b86d5bd1abe26636abfc19af28e755da7585
Cr-Commit-Position: refs/heads/master@{#302691}

[Andre, 2014-11-12 00:49:55]

andresantoso@chromium.org changed reviewers:
+ andresantoso@chromium.org

[Andre, 2014-11-12 00:49:56]

This change won't compile on MacViews (Mac with toolkit_views=1), becase
MacViews does not use Aura.
Would it be possible to use WidgetObserver or NativeWindowTracker instead?

[pkotwicz, 2014-11-12 02:32:42]

extension_uninstall_dialog_view is part of
chrome_browser_ui_views_non_mac_sources. Unless the file got moved out of that
list recently.
Regardless, the right place to discuss this is as part of a crbug.

Yes, using NativeWindowTracker would be an ok alternative.