Fixed bug where Uninstall dialog forces its own widget to close twice

Fixed bug where Uninstall dialog forces its own widget to close twice

Previously, ExtensionUninstallDialogViews called CloseNow() on it's own
delegate's Widget if it was destroyed before an Accept/Cancel handler
was called. However, the Widget had already been freed by the Widget
hierarchy, resulting in a crash.

Made ExtensionUninstallDialogViews notify
ExtensionUninstallDialogDelegateView when it closes, so now either order
is possible: either class can be destroyed first (either by the user, or
by the views hierarchy) and the other will be safely destroyed.

TEST=Open the App List in ChromeOS, then right-click on an app and
select 'App Info', then click 'Remove'. Now click away from the app list
to dismiss it. Previously, this would cause a crash.
TEST=Open the App List in Linux, then right-click on an app and
select 'App Info', then click 'Remove'. Now click the 'close' button in
the app info dialog to close the dismiss it. Previously, this would
cause a crash.

BUG=390414, 400909
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289227

[sashab, 2014-07-22 23:55:35]

Scott: requesting feedback from your general views knowledge :)

[sky, 2014-07-23 15:29:37]

https://codereview.chromium.org/412483006/diff/1/chrome/browser/ui/views/exte...
File chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
(right):

https://codereview.chromium.org/412483006/diff/1/chrome/browser/ui/views/exte...
chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc:129:
view_->GetWidget()->Close();
If you do this, it means the WidgetDelegate/DialogDelegate of the widget
(ExtensionUninstallDialogViews) has been destroyed but the widget is still
alive. Most likely widget (and related classes) are going to try calling into
the delegate until the widget is actually destroyed. This is why the
widgetdelegate is usually owned by the widget, but at a minimum it must be
around as long as the widget.

[sashab, 2014-08-06 03:09:00]

https://codereview.chromium.org/412483006/diff/1/chrome/browser/ui/views/exte...
File chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
(right):

https://codereview.chromium.org/412483006/diff/1/chrome/browser/ui/views/exte...
chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc:129:
view_->GetWidget()->Close();
On 2014/07/23 15:29:37, sky wrote:
> If you do this, it means the WidgetDelegate/DialogDelegate of the widget
> (ExtensionUninstallDialogViews) has been destroyed but the widget is still
> alive. Most likely widget (and related classes) are going to try calling into
> the delegate until the widget is actually destroyed. This is why the
> widgetdelegate is usually owned by the widget, but at a minimum it must be
> around as long as the widget.

If that's true, then I don't think this destructor code is needed at all. By the
time the ExtensionUninstallDialogViews is destroyed,  you can assume the widget
will be destroyed on its own (since its probably its owner).

This code is currently not working properly anyway - there is a code path where
view_ is not-NULL but already freed, and this destructor is reached.

[sashab, 2014-08-13 00:11:58]

Alright. Take a look. :)

[sky, 2014-08-13 03:26:41]

LGTM

[sashab, 2014-08-13 06:12:55]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2014-08-13 06:13:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sashab@chromium.org/412483006/40001

[commit-bot: I haz the power, 2014-08-13 09:17:11]

Change committed as 289227