Ensure proper teardown of repost form warning objects.

Make sure a repost form warning is closed properly even when the constrained window is closed from underneath us.
To do this, its controller now calls |Cancel| in its destructor.

We now have a potential cycle of methods calling each other:
* |RepostFormWarningController::CloseDialog| closes the |ConstrainedWindow|.
* This eventually calls |RepostFormWarningWin::DeleteDelegate|, which destroys |RepostFormWarningWin| and |RepostFormWarningController|.
* In its destructor, |RepostFormWarningController| calls |Cancel|, which calls |CloseDialog|.

Therefore, there are some checks in place to make sure we do everything during the teardown exactly once:
* After calling |NavigationController::CancelPendingReload|, we set |tab_contents| to null to make sure we don't call it again.
* During destruction, we set |window_| to null to make sure we don't call |CloseConstrainedWindow| again.

BUG=41367
TEST=RepostFormWarningTest.*

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

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

7 files changed, 23 insertions, 30 deletions

diff --git a/chrome/browser/cocoa/repost_form_warning_mac.h b/chrome/browser/cocoa/repost_form_warning_mac.h
index 7139be6..a1ebeee 100644
--- a/chrome/browser/cocoa/repost_form_warning_mac.h
+++ b/chrome/browser/cocoa/repost_form_warning_mac.h
@@ -7,7 +7,7 @@
 
 #import <Cocoa/Cocoa.h>
 
-#include "base/scoped_nsobject.h"
+#include "base/scoped_ptr.h"
 #include "chrome/browser/cocoa/constrained_window_mac.h"
 
 class RepostFormWarningController;
@@ -31,8 +31,7 @@ class RepostFormWarningMac : public ConstrainedDialogDelegate {
  private:
   virtual ~RepostFormWarningMac();
 
-  // Close the sheet.
-  void Dismiss();
+  scoped_ptr<RepostFormWarningController> controller_;
 
   DISALLOW_COPY_AND_ASSIGN(RepostFormWarningMac);
 };
diff --git a/chrome/browser/cocoa/repost_form_warning_mac.mm b/chrome/browser/cocoa/repost_form_warning_mac.mm
index adfe54e8..e8876c7 100644
--- a/chrome/browser/cocoa/repost_form_warning_mac.mm
+++ b/chrome/browser/cocoa/repost_form_warning_mac.mm
@@ -5,6 +5,7 @@
 #include "chrome/browser/cocoa/repost_form_warning_mac.h"
 
 #include "app/l10n_util_mac.h"
+#include "base/scoped_nsobject.h"
 #include "chrome/browser/repost_form_warning_controller.h"
 #include "grit/generated_resources.h"
 
@@ -51,7 +52,8 @@ RepostFormWarningMac::RepostFormWarningMac(
     : ConstrainedWindowMacDelegateSystemSheet(
         [[[RepostDelegate alloc] initWithWarning:controller]
             autorelease],
-        @selector(alertDidEnd:returnCode:contextInfo:)) {
+        @selector(alertDidEnd:returnCode:contextInfo:)),
+      controller_(controller) {
   scoped_nsobject<NSAlert> alert([[NSAlert alloc] init]);
   [alert setMessageText:
       l10n_util::GetNSStringWithFixup(IDS_HTTP_POST_WARNING_TITLE)];
@@ -68,17 +70,13 @@ RepostFormWarningMac::RepostFormWarningMac(
 }
 
 RepostFormWarningMac::~RepostFormWarningMac() {
-}
-
-void RepostFormWarningMac::DeleteDelegate() {
-  Dismiss();
-  delete this;
-}
-
-void RepostFormWarningMac::Dismiss() {
   NSWindow* window = [(NSAlert*)sheet() window];
   if (window && is_sheet_open()) {
     [NSApp endSheet:window
          returnCode:NSAlertSecondButtonReturn];
   }
 }
+
+void RepostFormWarningMac::DeleteDelegate() {
+  delete this;
+}
diff --git a/chrome/browser/gtk/repost_form_warning_gtk.cc b/chrome/browser/gtk/repost_form_warning_gtk.cc
index 2c3e010..44d70d0 100644
--- a/chrome/browser/gtk/repost_form_warning_gtk.cc
+++ b/chrome/browser/gtk/repost_form_warning_gtk.cc
@@ -65,18 +65,11 @@ GtkWidget* RepostFormWarningGtk::GetWidgetRoot() {
 }
 
 void RepostFormWarningGtk::DeleteDelegate() {
-  Dismiss();
   delete this;
 }
 
 RepostFormWarningGtk::~RepostFormWarningGtk() {
-}
-
-void RepostFormWarningGtk::Dismiss() {
-  if (dialog_) {
-    gtk_widget_destroy(dialog_);
-    dialog_ = NULL;
-  }
+  gtk_widget_destroy(dialog_);
 }
 
 void RepostFormWarningGtk::OnRefresh(GtkWidget* widget) {
diff --git a/chrome/browser/gtk/repost_form_warning_gtk.h b/chrome/browser/gtk/repost_form_warning_gtk.h
index 1c04fda..149216b 100644
--- a/chrome/browser/gtk/repost_form_warning_gtk.h
+++ b/chrome/browser/gtk/repost_form_warning_gtk.h
@@ -8,6 +8,7 @@
 #include <gtk/gtk.h>
 
 #include "app/gtk_signal.h"
+#include "base/scoped_ptr.h"
 #include "chrome/browser/gtk/constrained_window_gtk.h"
 
 class RepostFormWarningController;
@@ -29,8 +30,6 @@ class RepostFormWarningGtk : public ConstrainedDialogDelegate {
  private:
   virtual ~RepostFormWarningGtk();
 
-  void Dismiss();
-
   // Callbacks
   CHROMEGTK_CALLBACK_0(RepostFormWarningGtk, void, OnRefresh);
   CHROMEGTK_CALLBACK_0(RepostFormWarningGtk, void, OnCancel);
@@ -39,7 +38,7 @@ class RepostFormWarningGtk : public ConstrainedDialogDelegate {
                        OnHierarchyChanged,
                        GtkWidget*);
 
-  RepostFormWarningController* controller_;
+  scoped_ptr<RepostFormWarningController> controller_;
 
   GtkWidget* dialog_;
   GtkWidget* ok_;
diff --git a/chrome/browser/repost_form_warning_controller.cc b/chrome/browser/repost_form_warning_controller.cc
index 7b38c4c..2cf6ff5 100644
--- a/chrome/browser/repost_form_warning_controller.cc
+++ b/chrome/browser/repost_form_warning_controller.cc
@@ -21,6 +21,11 @@ RepostFormWarningController::RepostFormWarningController(
 }
 
 RepostFormWarningController::~RepostFormWarningController() {
+  // If we end up here, the constrained window has been closed, so make sure we
+  // don't close it again.
+  window_ = NULL;
+  // Make sure everything is cleaned up.
+  Cancel();
 }
 
 void RepostFormWarningController::Show(
@@ -48,7 +53,8 @@ void RepostFormWarningController::Observe(NotificationType type,
   // Close the dialog if we load a page (because reloading might not apply to
   // the same page anymore) or if the tab is closed, because then we won't have
   // a navigation controller anymore.
-  if ((type == NotificationType::LOAD_START ||
+  if (tab_contents_ &&
+      (type == NotificationType::LOAD_START ||
        type == NotificationType::TAB_CLOSING ||
        type == NotificationType::REPOST_WARNING_SHOWN)) {
     DCHECK_EQ(Source<NavigationController>(source).ptr(),
@@ -58,9 +64,9 @@ void RepostFormWarningController::Observe(NotificationType type,
 }
 
 void RepostFormWarningController::CloseDialog() {
+  // Make sure we won't do anything when |Cancel()| is called again.
   tab_contents_ = NULL;
   if (window_) {
     window_->CloseConstrainedWindow();
   }
-  delete this;
 }
diff --git a/chrome/browser/repost_form_warning_controller.h b/chrome/browser/repost_form_warning_controller.h
index 2b2c0a8..ce22d7b 100644
--- a/chrome/browser/repost_form_warning_controller.h
+++ b/chrome/browser/repost_form_warning_controller.h
@@ -13,11 +13,11 @@ class TabContents;
 
 // This class is used to continue or cancel a pending reload when the
 // repost form warning is shown. It is owned by the platform-dependent
-// |RepostFormWarning{Gtk,Mac,View}| classes and deletes itself after closing
-// the dialog.
+// |RepostFormWarning{Gtk,Mac,View}| classes.
 class RepostFormWarningController : public NotificationObserver {
  public:
   explicit RepostFormWarningController(TabContents* tab_contents);
+  virtual ~RepostFormWarningController();
 
   // Show the warning dialog.
   void Show(ConstrainedWindowDelegate* window_delegate);
@@ -29,8 +29,6 @@ class RepostFormWarningController : public NotificationObserver {
   void Continue();
 
  private:
-  virtual ~RepostFormWarningController();
-
   // NotificationObserver implementation.
   // Watch for a new load or a closed tab and dismiss the dialog if they occur.
   void Observe(NotificationType type,
diff --git a/chrome/browser/views/repost_form_warning_view.h b/chrome/browser/views/repost_form_warning_view.h
index dcac48a..a6932af 100644
--- a/chrome/browser/views/repost_form_warning_view.h
+++ b/chrome/browser/views/repost_form_warning_view.h
@@ -48,7 +48,7 @@ class RepostFormWarningView : public ConstrainedDialogDelegate {
   // The message box view whose commands we handle.
   MessageBoxView* message_box_view_;
 
-  RepostFormWarningController* controller_;
+  scoped_ptr<RepostFormWarningController> controller_;
 
   DISALLOW_COPY_AND_ASSIGN(RepostFormWarningView);
 };