linux: cleanup browser-side GtkSockets when plugins are destroyed

linux: cleanup browser-side GtkSockets when plugins are destroyed

WillDestroyWindow is already plumbed over to the browser process in the
multiproc case; we want to plumb it to our RenderView delegate so we
can do similar cleanup in test_shell.

BUG=15423

[Evan Martin, 2009-06-26 23:38:28]

http://codereview.chromium.org/147251/diff/1/2
File webkit/glue/plugins/webplugin_delegate_impl_gtk.cc (right):

http://codereview.chromium.org/147251/diff/1/2#newcode287
Line 287: plugin_->WillDestroyWindow(windowed_handle_);
This code matches the original webplugindelegateimpl, by the way.

[Antoine Labour, 2009-06-27 01:29:32]

lgtm other than this minor thing.

http://codereview.chromium.org/147251/diff/1/5
File webkit/glue/webview_delegate.h (right):

http://codereview.chromium.org/147251/diff/1/5#newcode138
Line 138: virtual void WillDestroyPluginWindow(unsigned long id) { }
Can we use PluginWindowHandle here (and other places we use unsigned long for
it) ? It's a bit more descriptive.

[Evan Martin, 2009-06-27 01:32:27]

On 2009/06/27 01:29:32, Antoine Labour wrote:
> lgtm other than this minor thing.
> 
> http://codereview.chromium.org/147251/diff/1/5
> File webkit/glue/webview_delegate.h (right):
> 
> http://codereview.chromium.org/147251/diff/1/5#newcode138
> Line 138: virtual void WillDestroyPluginWindow(unsigned long id) { }
> Can we use PluginWindowHandle here (and other places we use unsigned long for
> it) ? It's a bit more descriptive.

Doing that means that the code in glue will depend on base.  I had thought
that's not allowed?  This is what the "I am unhappy" is referring to in the
chagne description.  But now that I've looked harder I think you are right.

[Antoine Labour, 2009-06-27 01:37:02]

On 2009/06/27 01:32:27, Evan Martin wrote:
> On 2009/06/27 01:29:32, Antoine Labour wrote:
> > lgtm other than this minor thing.
> > 
> > http://codereview.chromium.org/147251/diff/1/5
> > File webkit/glue/webview_delegate.h (right):
> > 
> > http://codereview.chromium.org/147251/diff/1/5#newcode138
> > Line 138: virtual void WillDestroyPluginWindow(unsigned long id) { }
> > Can we use PluginWindowHandle here (and other places we use unsigned long
for
> > it) ? It's a bit more descriptive.
> 
> Doing that means that the code in glue will depend on base.  I had thought
> that's not allowed?  This is what the "I am unhappy" is referring to in the
> chagne description.  But now that I've looked harder I think you are right.

I believe it is fine, since we do in other places, but I'll defer to John.

[jam, 2009-06-27 02:40:57]

lgtm (I thought I already sent this earlier, but apparently not)

On 2009/06/27 01:37:02, Antoine Labour wrote:
> On 2009/06/27 01:32:27, Evan Martin wrote:
> > On 2009/06/27 01:29:32, Antoine Labour wrote:
> > > lgtm other than this minor thing.
> > > 
> > > http://codereview.chromium.org/147251/diff/1/5
> > > File webkit/glue/webview_delegate.h (right):
> > > 
> > > http://codereview.chromium.org/147251/diff/1/5#newcode138
> > > Line 138: virtual void WillDestroyPluginWindow(unsigned long id) { }
> > > Can we use PluginWindowHandle here (and other places we use unsigned long
> for
> > > it) ? It's a bit more descriptive.
> > 
> > Doing that means that the code in glue will depend on base.  I had thought
> > that's not allowed?  This is what the "I am unhappy" is referring to in the
> > chagne description.  But now that I've looked harder I think you are right.
> 
> I believe it is fine, since we do in other places, but I'll defer to John.

Darin can correct me if I'm wrong, but it's fine to use base in the
implementation of WebKit APIs (where glue is headed) since that will reside in
Chrome's repository.  But the headers will be in WebKit, in which case we'll
need to probably move the gfx::PluginWindowHandle stuff into the WebKit
namespace.

[darin (slow to review), 2009-06-29 17:50:31]

> Darin can correct me if I'm wrong, but it's fine to use base in the
> implementation of WebKit APIs (where glue is headed) since that will reside in
> Chrome's repository.  But the headers will be in WebKit, in which case we'll
> need to probably move the gfx::PluginWindowHandle stuff into the WebKit
> namespace.

Yeah... I wonder about this.  Can we get away with a WebPluginDelegate method
here instead?