Fix a crash when we try to close a js dialog that wasn't shown.

Fix a crash when we try to close a js dialog that wasn't shown.

The dialog has been queued, but it hasn't been shown because a
different dialog is already showing.  We try to close the dialog
because a page navigation has occurred.

BUG=47056
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50561

[tony, 2010-06-22 08:26:22]

Suggestions on how to test are welcome.  I'm also not sure why we're getting
NAV_ENTRY_COMMITTED (I would think the load is blocked on waiting for the JS
Alert to show).

A long long time ago, we had a check for dialog_ being NULL here, but it was
removed by me here: http://codereview.chromium.org/63033
Perhaps this has been a bug since then.

[Elliot Glaysher, 2010-06-22 17:19:58]

LGTM

On Tue, Jun 22, 2010 at 1:26 AM,  <tony@chromium.org> wrote:
> Reviewers: Elliot Glaysher, Evan Martin,
>
> Message:
> Suggestions on how to test are welcome.  I'm also not sure why we're getting
> NAV_ENTRY_COMMITTED (I would think the load is blocked on waiting for the JS
> Alert to show).
>
> A long long time ago, we had a check for dialog_ being NULL here, but it was
> removed by me here: http://codereview.chromium.org/63033
> Perhaps this has been a bug since then.
>
> Description:
> Fix a crash when we try to close a js dialog that wasn't shown.
>
> The dialog has been queued, but it hasn't been shown because a
> different dialog is already showing.  We try to close the dialog
> because a page navigation has occurred.
>
> BUG=47056
> TEST=None
>
> Please review this at http://codereview.chromium.org/2803017/show
>
> SVN Base: http://src.chromium.org/git/chromium.git
>
> Affected files:
>  M chrome/browser/app_modal_dialog_gtk.cc
>  M chrome/browser/app_modal_dialog_win.cc
>  M chrome/browser/js_modal_dialog.cc
>
>
> Index: chrome/browser/app_modal_dialog_gtk.cc
> diff --git a/chrome/browser/app_modal_dialog_gtk.cc
> b/chrome/browser/app_modal_dialog_gtk.cc
> index
>
b0268e6b6be0bfb0d5e46ed16bea56ae1ca628e7..2eb91854474dc5740ccb2504a822365b614ee08e
> 100644
> --- a/chrome/browser/app_modal_dialog_gtk.cc
> +++ b/chrome/browser/app_modal_dialog_gtk.cc
> @@ -31,17 +31,21 @@ void AppModalDialog::OnDialogResponse(GtkDialog* dialog,
> gint response_id,
>  }
>
>  void AppModalDialog::ActivateModalDialog() {
> +  DCHECK(dialog_);
>   gtk_window_present(GTK_WINDOW(dialog_));
>  }
>
>  void AppModalDialog::CloseModalDialog() {
> +  DCHECK(dialog_);
>   HandleDialogResponse(GTK_DIALOG(dialog_), GTK_RESPONSE_DELETE_EVENT);
>  }
>
>  void AppModalDialog::AcceptWindow() {
> +  DCHECK(dialog_);
>   HandleDialogResponse(GTK_DIALOG(dialog_), GTK_RESPONSE_OK);
>  }
>
>  void AppModalDialog::CancelWindow() {
> +  DCHECK(dialog_);
>   HandleDialogResponse(GTK_DIALOG(dialog_), GTK_RESPONSE_CANCEL);
>  }
> Index: chrome/browser/app_modal_dialog_win.cc
> diff --git a/chrome/browser/app_modal_dialog_win.cc
> b/chrome/browser/app_modal_dialog_win.cc
> index
>
c892881d027df5740258dba1d55b857c5eb20add..e48d9862bc1cf9a86e6bde6592d14aea6822cef9
> 100644
> --- a/chrome/browser/app_modal_dialog_win.cc
> +++ b/chrome/browser/app_modal_dialog_win.cc
> @@ -19,9 +19,11 @@ void AppModalDialog::CreateAndShowDialog() {
>  }
>
>  void AppModalDialog::ActivateModalDialog() {
> +  DCHECK(dialog_);
>   dialog_->ActivateModalDialog();
>  }
>
>  void AppModalDialog::CloseModalDialog() {
> +  DCHECK(dialog_);
>   dialog_->CloseModalDialog();
>  }
> Index: chrome/browser/js_modal_dialog.cc
> diff --git a/chrome/browser/js_modal_dialog.cc
> b/chrome/browser/js_modal_dialog.cc
> index
>
37253b93b1e09ea8d698269759f90647546b64dc..111aa01af5c640a79ff157058d6f11a1f14e0516
> 100644
> --- a/chrome/browser/js_modal_dialog.cc
> +++ b/chrome/browser/js_modal_dialog.cc
> @@ -29,6 +29,9 @@ JavaScriptAppModalDialog::JavaScriptAppModalDialog(
>     bool is_before_unload_dialog,
>     IPC::Message* reply_msg)
>     : AppModalDialog(client->AsTabContents(), title),
> +#if defined(OS_MACOSX)
> +      dialog_(NULL),
> +#endif
>       client_(client),
>       extension_host_(client->AsExtensionHost()),
>       dialog_flags_(dialog_flags),
> @@ -64,7 +67,8 @@ void JavaScriptAppModalDialog::Observe(NotificationType
> type,
>   // Also clear the client, since it's now invalid.
>   skip_this_dialog_ = true;
>   client_ = NULL;
> -  CloseModalDialog();
> +  if (dialog_)
> +    CloseModalDialog();
>  }
>
>  void JavaScriptAppModalDialog::InitNotifications() {
> @@ -136,4 +140,3 @@ void JavaScriptAppModalDialog::Cleanup() {
>   }
>   AppModalDialog::Cleanup();
>  }
> -
>
>
>