Commit 40893f4a authored by sashab@chromium.org's avatar sashab@chromium.org

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

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

Cr-Commit-Position: refs/heads/master@{#289227}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289227 0039d316-1c4b-4281-b951-d872f2087c98
parent c49bec08
......@@ -39,6 +39,10 @@ class ExtensionUninstallDialogViews
extensions::ExtensionUninstallDialog::Delegate* delegate);
virtual ~ExtensionUninstallDialogViews();
// Called when the ExtensionUninstallDialogDelegate has been destroyed to make
// sure we invalidate pointers.
void DialogDelegateDestroyed() { view_ = NULL; }
// Forwards the accept and cancels to the delegate.
void ExtensionUninstallAccepted();
void ExtensionUninstallCanceled();
......@@ -121,12 +125,14 @@ void ExtensionUninstallDialogViews::Show() {
void ExtensionUninstallDialogViews::ExtensionUninstallAccepted() {
// The widget gets destroyed when the dialog is accepted.
view_->DialogDestroyed();
view_ = NULL;
delegate_->ExtensionUninstallAccepted();
}
void ExtensionUninstallDialogViews::ExtensionUninstallCanceled() {
// The widget gets destroyed when the dialog is canceled.
view_->DialogDestroyed();
view_ = NULL;
delegate_->ExtensionUninstallCanceled();
}
......@@ -154,6 +160,15 @@ ExtensionUninstallDialogDelegateView::ExtensionUninstallDialogDelegateView(
}
ExtensionUninstallDialogDelegateView::~ExtensionUninstallDialogDelegateView() {
// If we're here, 2 things could have happened. Either the user closed the
// dialog nicely and one of ExtensionUninstallAccepted or
// ExtensionUninstallCanceled has been called (in which case dialog_ will be
// NULL), *or* neither of them have been called and we are being forced closed
// by our parent widget. In this case, we need to make sure to notify dialog_
// not to call us again, since we're about to be freed by the Widget
// framework.
if (dialog_)
dialog_->DialogDelegateDestroyed();
}
base::string16 ExtensionUninstallDialogDelegateView::GetDialogButtonLabel(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment