Make the DownloadRequestDialogDelegateGtk always report some response to the download request manager.

We weren't reporting Cancel on page navigation, which meant the download request manager had a stale pointer that it later tried to dereference.

BUG=18021

Review URL: http://codereview.chromium.org/160361

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22011 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 30 insertions, 27 deletions

diff --git a/chrome/browser/download/download_request_manager.cc b/chrome/browser/download/download_request_manager.cc
index e5745d9..83811e2 100644
--- a/chrome/browser/download/download_request_manager.cc
+++ b/chrome/browser/download/download_request_manager.cc
@@ -156,12 +156,6 @@ void DownloadRequestManager::TabDownloadState::NotifyCallbacks(bool allow) {
     host_->ScheduleNotification(callbacks[i], allow);
 }
 
-namespace {
-
-// DialogDelegateImpl ----------------------------------------------------------
-
-}  // namespace
-
 // DownloadRequestManager ------------------------------------------------------
 
 DownloadRequestManager::DownloadRequestManager(MessageLoop* io_loop,
diff --git a/chrome/browser/gtk/download_request_dialog_delegate_gtk.cc b/chrome/browser/gtk/download_request_dialog_delegate_gtk.cc
index 8740dcc..8903ee2 100644
--- a/chrome/browser/gtk/download_request_dialog_delegate_gtk.cc
+++ b/chrome/browser/gtk/download_request_dialog_delegate_gtk.cc
@@ -26,8 +26,8 @@ DownloadRequestDialogDelegateGtk::~DownloadRequestDialogDelegateGtk() {
 DownloadRequestDialogDelegateGtk::DownloadRequestDialogDelegateGtk(
     TabContents* tab,
     DownloadRequestManager::TabDownloadState* host)
-    : DownloadRequestDialogDelegate(host) {
-
+    : DownloadRequestDialogDelegate(host),
+      responded_(false) {
   // Create dialog.
   root_.Own(gtk_vbox_new(NULL, gtk_util::kContentAreaBorder));
   GtkWidget* label = gtk_label_new(
@@ -46,7 +46,7 @@ DownloadRequestDialogDelegateGtk::DownloadRequestDialogDelegateGtk(
   gtk_button_set_image(
       GTK_BUTTON(deny),
       gtk_image_new_from_stock(GTK_STOCK_CANCEL, GTK_ICON_SIZE_BUTTON));
-  g_signal_connect(deny, "clicked", G_CALLBACK(OnDenyClicked), this);
+  g_signal_connect(deny, "clicked", G_CALLBACK(OnDenyClickedThunk), this);
   gtk_box_pack_end(GTK_BOX(hbox), deny, FALSE, FALSE, 0);
 
   GtkWidget* allow = gtk_button_new_with_label(
@@ -54,7 +54,7 @@ DownloadRequestDialogDelegateGtk::DownloadRequestDialogDelegateGtk(
   gtk_button_set_image(
       GTK_BUTTON(allow),
       gtk_image_new_from_stock(GTK_STOCK_OK, GTK_ICON_SIZE_BUTTON));
-  g_signal_connect(allow, "clicked", G_CALLBACK(OnAllowClicked), this);
+  g_signal_connect(allow, "clicked", G_CALLBACK(OnAllowClickedThunk), this);
   gtk_box_pack_end(GTK_BOX(hbox), allow, FALSE, FALSE, 0);
 
   // Attach to window.
@@ -75,22 +75,20 @@ GtkWidget* DownloadRequestDialogDelegateGtk::GetWidgetRoot() {
 }
 
 void DownloadRequestDialogDelegateGtk::DeleteDelegate() {
-  // On windows, host_ is set to NULL when the window is closed by not pressing
-  // one of the two buttons (i.e. by navigating to another site). We don't do
-  // that, hence we can't DCHECK(!host_) here.
+  if (!responded_)
+    DoCancel();
+  DCHECK(!host_);
   delete this;
 }
 
-void DownloadRequestDialogDelegateGtk::OnAllowClicked(
-    GtkButton *button,
-    DownloadRequestDialogDelegateGtk* handler) {
-  handler->DoAccept();
-  handler->CloseWindow();
+void DownloadRequestDialogDelegateGtk::OnAllowClicked() {
+  DoAccept();
+  responded_ = true;
+  CloseWindow();
 }
 
-void DownloadRequestDialogDelegateGtk::OnDenyClicked(
-    GtkButton *button,
-    DownloadRequestDialogDelegateGtk* handler) {
-  handler->DoCancel();
-  handler->CloseWindow();
+void DownloadRequestDialogDelegateGtk::OnDenyClicked() {
+  DoCancel();
+  responded_ = true;
+  CloseWindow();
 }
diff --git a/chrome/browser/gtk/download_request_dialog_delegate_gtk.h b/chrome/browser/gtk/download_request_dialog_delegate_gtk.h
index f68523e..5f65f99 100644
--- a/chrome/browser/gtk/download_request_dialog_delegate_gtk.h
+++ b/chrome/browser/gtk/download_request_dialog_delegate_gtk.h
@@ -27,10 +27,16 @@ class DownloadRequestDialogDelegateGtk : public DownloadRequestDialogDelegate,
   virtual void DeleteDelegate();
 
   // Other methods.
-  static void OnAllowClicked(GtkButton *button,
-                             DownloadRequestDialogDelegateGtk* handler);
-  static void OnDenyClicked(GtkButton *button,
-                             DownloadRequestDialogDelegateGtk* handler);
+  static void OnAllowClickedThunk(GtkButton* button,
+                                  DownloadRequestDialogDelegateGtk* delegate) {
+    delegate->OnAllowClicked();
+  }
+  void OnAllowClicked();
+  static void OnDenyClickedThunk(GtkButton* button,
+                                 DownloadRequestDialogDelegateGtk* delegate) {
+    delegate->OnDenyClicked();
+  }
+  void OnDenyClicked();
 
   // The ConstrainedWindow that is hosting our DownloadRequestDialog.
   ConstrainedWindow* window_;
@@ -38,6 +44,11 @@ class DownloadRequestDialogDelegateGtk : public DownloadRequestDialogDelegate,
   // Our root widget.
   OwnedWidgetGtk root_;
 
+  // Tracks whether we have responded so we can send a cancel response
+  // when we are getting deleted. DRDDWin gets this for free from its
+  // views::DialogDelegate superclass.
+  bool responded_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadRequestDialogDelegateGtk);
 };