Fix some browser shutdown issues on Linux.

Fix some browser shutdown issues on Linux.

The previous code ended up destroying the BrowserWindow (removing it from BrowserList) while BrowserList was in the middle of iterating.  Push the deletion onto the message loop (using DeleteSoon) to emulate what I'm guessing happens on Windows (the message is queue instead of processed right away).

This fixes a second bug involving re-entrancy issues with Close() and window_, we now make sure to set window_ to NULL before any events might fire.

BUG=8712

[Dean McNamee, 2009-03-12 15:17:07]

This is kind of gnarly, and I hate to use DeleteSoon.  Ben, can you confirm how
it works on Windows?  Evan, can you explain why the destroy signal is processed
right away on GTK?

The code seems to work at least, we no longer crash when you close the browser
window, or exit with wrench -> exit.

[Dean McNamee, 2009-03-12 15:27:54]

After digging, WidgetWin::Close() also posts a task to schedule closing the
window.  We now schedule the destruction of the object, which is not exactly the
same, but more straightforward in our case.

We could get rid a lot of this by making BrowserList aware of changes to itself
(doing if (prev_size != browsers_.size()) start again) sort of thing.  Any
thoughts Ben?

On 2009/03/12 15:17:07, Dean McNamee wrote:
> This is kind of gnarly, and I hate to use DeleteSoon.  Ben, can you confirm
how
> it works on Windows?  Evan, can you explain why the destroy signal is
processed
> right away on GTK?
> 
> The code seems to work at least, we no longer crash when you close the browser
> window, or exit with wrench -> exit.

[Evan Martin, 2009-03-12 17:35:42]

This feels gnarly to me and I don't understand it.

You asked: "Evan, can you explain why the destroy signal is processed right away
on GTK?" I don't really understand the question.  So here's some random info
that may help.
 - "destroy" is sent on all GtkObjects and matches the destructor in C++.  So
it's part of the synchronous call stack of gtk_widget_destroy() because that is
where the widget is actually destroyed.
 - "destroy-event" is this other thing that maybe corresponds to WM_DESTROY on
windows in that it's telling you about an X window going away; the GTK docs say
you rarely recieve this because of the way destruction order happens (first you
destroy your window, then yourself).
 - finally, top-level windows have a "delete-event" which comes from the close
button being clicked and by default it destroys the window.

Depending on how sensitive our destruction order is to this stuff, one option
might be using something like
http://library.gnome.org/devel/gtk/stable/GtkWidget.html#gtk-widget-hide-on-d...
to make it so closing the window doesn't actually close it, but instead hides it
and invokes the same code path as the normal "exit" command.

[Dean McNamee, 2009-03-12 17:47:06]

Hey Evan,

Thanks, this was super helpful.  I am starting to think of it differently, and I
think I know how it should be done.  I'll need to get a fresh Windows build
tomorrow and spend some time understanding how it happens there.

On 2009/03/12 17:35:42, Evan Martin wrote:
> This feels gnarly to me and I don't understand it.
> 
> You asked: "Evan, can you explain why the destroy signal is processed right
away
> on GTK?" I don't really understand the question.  So here's some random info
> that may help.
>  - "destroy" is sent on all GtkObjects and matches the destructor in C++.  So
> it's part of the synchronous call stack of gtk_widget_destroy() because that
is
> where the widget is actually destroyed.
>  - "destroy-event" is this other thing that maybe corresponds to WM_DESTROY on
> windows in that it's telling you about an X window going away; the GTK docs
say
> you rarely recieve this because of the way destruction order happens (first
you
> destroy your window, then yourself).
>  - finally, top-level windows have a "delete-event" which comes from the close
> button being clicked and by default it destroys the window.
> 
> Depending on how sensitive our destruction order is to this stuff, one option
> might be using something like
>
http://library.gnome.org/devel/gtk/stable/GtkWidget.html#gtk-widget-hide-on-d...
> to make it so closing the window doesn't actually close it, but instead hides
it
> and invokes the same code path as the normal "exit" command.

[darin (slow to review), 2009-03-12 22:26:25]

http://codereview.chromium.org/43135/diff/6/7
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/43135/diff/6/7#newcode434
Line 434: MessageLoop::current()->DeleteSoon(FROM_HERE, browser_win);
i love problems like this :-P

this LGTM.  DeleteSoon uses PostNonNestableTask, so you know for sure that the
stack will have unwound completely before this object gets deleted.

[Dean McNamee, 2009-03-13 13:31:05]

Hey Evan,

I updated the bug with some more details about what's happening on Windows. 
Basically from what I can tell, it does basically what this code is doing.  I am
going to commit this, although I agree it is a bit gnarly, I don't see an
obvious way around it right now.