Ensure proper teardown of repost form warning objects.

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.*

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

[sky, 2010-04-14 15:09:38]

http://codereview.chromium.org/1530032/diff/5001/6003
File chrome/browser/gtk/repost_form_warning_gtk.cc (right):

http://codereview.chromium.org/1530032/diff/5001/6003#newcode68
chrome/browser/gtk/repost_form_warning_gtk.cc:68: delete this;
Do you need to set dialog_ to NULL here to make sure you don't try to destroy
the dialog again?

[Bernhard Bauer, 2010-04-14 15:20:58]

http://codereview.chromium.org/1530032/diff/5001/6003
File chrome/browser/gtk/repost_form_warning_gtk.cc (right):

http://codereview.chromium.org/1530032/diff/5001/6003#newcode68
chrome/browser/gtk/repost_form_warning_gtk.cc:68: delete this;
On 2010/04/14 15:09:39, sky wrote:
> Do you need to set dialog_ to NULL here to make sure you don't try to destroy
> the dialog again?

The destructor should really be only called once (and |DeleteDelegate()| is only
called once, provided the |ConstrainedWindow| is only closed once...). I think
the code in the destructor could actually be simplified to
|gtk_widget_destroy(dialog_)|; everything else is vestigial.

[sky, 2010-04-14 15:34:54]

http://codereview.chromium.org/1530032/diff/5001/6003
File chrome/browser/gtk/repost_form_warning_gtk.cc (right):

http://codereview.chromium.org/1530032/diff/5001/6003#newcode68
chrome/browser/gtk/repost_form_warning_gtk.cc:68: delete this;
On 2010/04/14 15:20:58, Bernhard Bauer wrote:
> On 2010/04/14 15:09:39, sky wrote:
> > Do you need to set dialog_ to NULL here to make sure you don't try to
destroy
> > the dialog again?
> 
> The destructor should really be only called once (and |DeleteDelegate()| is
only
> called once, provided the |ConstrainedWindow| is only closed once...). I think
> the code in the destructor could actually be simplified to
> |gtk_widget_destroy(dialog_)|; everything else is vestigial.

I'm thinking of the case where the user closes the constrained dialog. Does that
invoke gtk_widget_destroy so that by the time the destructor is run you execute
gtk_widget_destroy again?

[Bernhard Bauer, 2010-04-14 16:05:54]

On 2010/04/14 15:34:54, sky wrote:
> http://codereview.chromium.org/1530032/diff/5001/6003
> File chrome/browser/gtk/repost_form_warning_gtk.cc (right):
> 
> http://codereview.chromium.org/1530032/diff/5001/6003#newcode68
> chrome/browser/gtk/repost_form_warning_gtk.cc:68: delete this;
> On 2010/04/14 15:20:58, Bernhard Bauer wrote:
> > On 2010/04/14 15:09:39, sky wrote:
> > > Do you need to set dialog_ to NULL here to make sure you don't try to
> destroy
> > > the dialog again?
> > 
> > The destructor should really be only called once (and |DeleteDelegate()| is
> only
> > called once, provided the |ConstrainedWindow| is only closed once...). I
think
> > the code in the destructor could actually be simplified to
> > |gtk_widget_destroy(dialog_)|; everything else is vestigial.
> 
> I'm thinking of the case where the user closes the constrained dialog. Does
that
> invoke gtk_widget_destroy so that by the time the destructor is run you
execute
> gtk_widget_destroy again?

In Gtk, the constrained dialog doesn't have any window controls, so the only way
to close it is by hitting cancel, or triggering a notification to the
controller.

[sky, 2010-04-14 16:11:13]

Ok, LGTM.