Consolidate into a single shutdown path.

Consolidate into a single shutdown path.

This consolidates the shutdown path.  We used to have two paths depending
of whether the user hit the X on the window manager or used wrench -> exit.
Now both do the same thing: call BrowserWindowGtk::Close().

[Dean McNamee, 2009-03-19 10:15:29]

It's nice to have a single path, but right now I don't see how it made anything
really simpler (and I'm worried it might not be exactly correct).

Also, it seems like it might still be a good idea to have a destroy event, just
in case there is some closing situation where we don't get it through
delete-event?  I guess I'd have to know more about GTK and X to know if that's
ever possible.

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

http://codereview.chromium.org/42372/diff/6/7#newcode259
Line 259: MessageLoop::current()->DeleteSoon(FROM_HERE, this);
I will have to spend some more time thinking about this, I am just a bit worried
since it's not parallel to how the Windows code works.

[tony, 2009-03-19 17:30:55]

I think this is more like how windows works.  delete-event is more like
WM_CLOSE, which just calls WindowWin::Close.  Here's what I'm seeing on windows:

red x or alt+4 end up calling:
-> WindowWin::Close()
  -> Browser::CloseAllTabs()
    -> Browser::TabStripEmpty()
      PostTask(&Browser::CloseFrame())
-> Browser::CloseFrame()
  -> WindowWin::Close()
    -> WidgetWin::Close()
      PostTask(&WidgetWin::CloseNow())
-> WidgetWin::CloseNow()
  -> delete the window (WM_DESTROY)

Our BrowserWindowGtk::Close should be more like WindowWin::Close.  I'm planning
on hooking up the CloseAllTabs stuff next.  For now, it just deletes itself
which is like the PostTask to WidgetWin::CloseNow.

I'm pretty sure we always get a delete-event if the user it trying to close the
window.

On 2009/03/19 10:15:29, Dean McNamee wrote:
> It's nice to have a single path, but right now I don't see how it made
anything
> really simpler (and I'm worried it might not be exactly correct).
> 
> Also, it seems like it might still be a good idea to have a destroy event,
just
> in case there is some closing situation where we don't get it through
> delete-event?  I guess I'd have to know more about GTK and X to know if that's
> ever possible.
> 
> http://codereview.chromium.org/42372/diff/6/7
> File chrome/browser/gtk/browser_window_gtk.cc (right):
> 
> http://codereview.chromium.org/42372/diff/6/7#newcode259
> Line 259: MessageLoop::current()->DeleteSoon(FROM_HERE, this);
> I will have to spend some more time thinking about this, I am just a bit
worried
> since it's not parallel to how the Windows code works.

[Dean McNamee, 2009-03-19 17:34:15]

On 2009/03/19 17:30:55, tony wrote:
> I think this is more like how windows works.  delete-event is more like
> WM_CLOSE, which just calls WindowWin::Close.  Here's what I'm seeing on
windows:
> 
> red x or alt+4 end up calling:
> -> WindowWin::Close()
>   -> Browser::CloseAllTabs()
>     -> Browser::TabStripEmpty()
>       PostTask(&Browser::CloseFrame())
> -> Browser::CloseFrame()
>   -> WindowWin::Close()
>     -> WidgetWin::Close()
>       PostTask(&WidgetWin::CloseNow())
> -> WidgetWin::CloseNow()
>   -> delete the window (WM_DESTROY)

On windows, delete the underlying object happens in NC_DESTROY, here we are
doing it in Close().  What I was saying is it would be closer if the destructor
was called during destroy(), although even that is not exactly the same.

> 
> Our BrowserWindowGtk::Close should be more like WindowWin::Close.  I'm
planning
> on hooking up the CloseAllTabs stuff next.  For now, it just deletes itself
> which is like the PostTask to WidgetWin::CloseNow.
> 
> I'm pretty sure we always get a delete-event if the user it trying to close
the
> window.
> 
> On 2009/03/19 10:15:29, Dean McNamee wrote:
> > It's nice to have a single path, but right now I don't see how it made
> anything
> > really simpler (and I'm worried it might not be exactly correct).
> > 
> > Also, it seems like it might still be a good idea to have a destroy event,
> just
> > in case there is some closing situation where we don't get it through
> > delete-event?  I guess I'd have to know more about GTK and X to know if
that's
> > ever possible.
> > 
> > http://codereview.chromium.org/42372/diff/6/7
> > File chrome/browser/gtk/browser_window_gtk.cc (right):
> > 
> > http://codereview.chromium.org/42372/diff/6/7#newcode259
> > Line 259: MessageLoop::current()->DeleteSoon(FROM_HERE, this);
> > I will have to spend some more time thinking about this, I am just a bit
> worried
> > since it's not parallel to how the Windows code works.

[tony, 2009-03-19 18:22:08]

I think I understand what you're saying now.  I re-added the destroy handler
which deletes the object.  This change makes it so we don't need to null out the
window because the object is always destroyed from BrowserWindowGtk::Close (so
it's already null).

On 2009/03/19 17:34:15, Dean McNamee wrote:
> On 2009/03/19 17:30:55, tony wrote:
> > I think this is more like how windows works.  delete-event is more like
> > WM_CLOSE, which just calls WindowWin::Close.  Here's what I'm seeing on
> windows:
> > 
> > red x or alt+4 end up calling:
> > -> WindowWin::Close()
> >   -> Browser::CloseAllTabs()
> >     -> Browser::TabStripEmpty()
> >       PostTask(&Browser::CloseFrame())
> > -> Browser::CloseFrame()
> >   -> WindowWin::Close()
> >     -> WidgetWin::Close()
> >       PostTask(&WidgetWin::CloseNow())
> > -> WidgetWin::CloseNow()
> >   -> delete the window (WM_DESTROY)
> 
> On windows, delete the underlying object happens in NC_DESTROY, here we are
> doing it in Close().  What I was saying is it would be closer if the
destructor
> was called during destroy(), although even that is not exactly the same.
> 
> > 
> > Our BrowserWindowGtk::Close should be more like WindowWin::Close.  I'm
> planning
> > on hooking up the CloseAllTabs stuff next.  For now, it just deletes itself
> > which is like the PostTask to WidgetWin::CloseNow.
> > 
> > I'm pretty sure we always get a delete-event if the user it trying to close
> the
> > window.
> > 
> > On 2009/03/19 10:15:29, Dean McNamee wrote:
> > > It's nice to have a single path, but right now I don't see how it made
> > anything
> > > really simpler (and I'm worried it might not be exactly correct).
> > > 
> > > Also, it seems like it might still be a good idea to have a destroy event,
> > just
> > > in case there is some closing situation where we don't get it through
> > > delete-event?  I guess I'd have to know more about GTK and X to know if
> that's
> > > ever possible.
> > > 
> > > http://codereview.chromium.org/42372/diff/6/7
> > > File chrome/browser/gtk/browser_window_gtk.cc (right):
> > > 
> > > http://codereview.chromium.org/42372/diff/6/7#newcode259
> > > Line 259: MessageLoop::current()->DeleteSoon(FROM_HERE, this);
> > > I will have to spend some more time thinking about this, I am just a bit
> > worried
> > > since it's not parallel to how the Windows code works.

[Dean McNamee, 2009-03-20 17:33:21]

I don't see the harm in not setting window_ to NULL like we did before, but it's
ok.  LG.