Fix crash/leak issue in native_view_host_gtk.cc.

Fix crash/leak issue in native_view_host_gtk.cc.
 * There are two path to NativeViewDetaching and we need to handle them differently.
  1) Via gtk destroy signal. In this case, we should not try to remove the native view from parent because it's being deleted.
  2) Through NativeViewHost::Detach(). In this case we need to remove the native view from parent because we added it to parent in NativeViewAttached().
 * Fix NativeControlGtk not to destroy the native view because it's now destoryed by  NativeViewHostGtk.
 * Fixed TabContentViewGtk so that it owns the nativew view. The native view was destroyed when Detached.
 * Added more checks so that test can catch regression.

BUG=26154
TEST=The same procedure in bug should now pass. I added several checks that lead tests to fail if this problem exists.

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

[sky, 2009-12-22 23:55:16]

http://codereview.chromium.org/510004/diff/1/2
File chrome/browser/views/tab_contents/tab_contents_view_gtk.cc (right):

http://codereview.chromium.org/510004/diff/1/2#newcode155
chrome/browser/views/tab_contents/tab_contents_view_gtk.cc:155:
gtk_object_ref(GTK_OBJECT(GetNativeView()));
Don't you need an unref to balance this at some point?

http://codereview.chromium.org/510004/diff/1/5
File views/controls/native/native_view_host_gtk.cc (right):

http://codereview.chromium.org/510004/diff/1/5#newcode80
views/controls/native/native_view_host_gtk.cc:80:
DCHECK_NE(static_cast<gfx::NativeView>(NULL),
gtk_widget_get_parent(host_->native_view()));
lines too long here and 82.

http://codereview.chromium.org/510004/diff/1/5#newcode209
views/controls/native/native_view_host_gtk.cc:209: // fixed_ should not have any
child this point.
child-> children at

http://codereview.chromium.org/510004/diff/1/10
File views/controls/native_control_gtk.cc (right):

http://codereview.chromium.org/510004/diff/1/10#newcode53
views/controls/native_control_gtk.cc:53: // Make sure that Detach destroyed
destroy the widget.
remove 'destroy.'

[oshima, 2009-12-23 00:01:43]

http://codereview.chromium.org/510004/diff/1/2
File chrome/browser/views/tab_contents/tab_contents_view_gtk.cc (right):

http://codereview.chromium.org/510004/diff/1/2#newcode155
chrome/browser/views/tab_contents/tab_contents_view_gtk.cc:155:
gtk_object_ref(GTK_OBJECT(GetNativeView()));
On 2009/12/22 23:55:16, sky wrote:
> Don't you need an unref to balance this at some point?

It's destroyed in CloseNew in the destructor.

http://codereview.chromium.org/510004/diff/1/5
File views/controls/native/native_view_host_gtk.cc (right):

http://codereview.chromium.org/510004/diff/1/5#newcode80
views/controls/native/native_view_host_gtk.cc:80:
DCHECK_NE(static_cast<gfx::NativeView>(NULL),
gtk_widget_get_parent(host_->native_view()));
On 2009/12/22 23:55:16, sky wrote:
> lines too long here and 82.

Done.

http://codereview.chromium.org/510004/diff/1/5#newcode209
views/controls/native/native_view_host_gtk.cc:209: // fixed_ should not have any
child this point.
On 2009/12/22 23:55:16, sky wrote:
> child-> children at

Done.

http://codereview.chromium.org/510004/diff/1/10
File views/controls/native_control_gtk.cc (right):

http://codereview.chromium.org/510004/diff/1/10#newcode53
views/controls/native_control_gtk.cc:53: // Make sure that Detach destroyed
destroy the widget.
On 2009/12/22 23:55:16, sky wrote:
> remove 'destroy.'

Done.

[sky, 2009-12-23 00:03:49]

Ok, LGTM.

  -Scott

On 2009/12/23 00:01:43, oshima wrote:
> http://codereview.chromium.org/510004/diff/1/2
> File chrome/browser/views/tab_contents/tab_contents_view_gtk.cc (right):
> 
> http://codereview.chromium.org/510004/diff/1/2#newcode155
> chrome/browser/views/tab_contents/tab_contents_view_gtk.cc:155:
> gtk_object_ref(GTK_OBJECT(GetNativeView()));
> On 2009/12/22 23:55:16, sky wrote:
> > Don't you need an unref to balance this at some point?
> 
> It's destroyed in CloseNew in the destructor.
> 
> http://codereview.chromium.org/510004/diff/1/5
> File views/controls/native/native_view_host_gtk.cc (right):
> 
> http://codereview.chromium.org/510004/diff/1/5#newcode80
> views/controls/native/native_view_host_gtk.cc:80:
> DCHECK_NE(static_cast<gfx::NativeView>(NULL),
> gtk_widget_get_parent(host_->native_view()));
> On 2009/12/22 23:55:16, sky wrote:
> > lines too long here and 82.
> 
> Done.
> 
> http://codereview.chromium.org/510004/diff/1/5#newcode209
> views/controls/native/native_view_host_gtk.cc:209: // fixed_ should not have
any
> child this point.
> On 2009/12/22 23:55:16, sky wrote:
> > child-> children at
> 
> Done.
> 
> http://codereview.chromium.org/510004/diff/1/10
> File views/controls/native_control_gtk.cc (right):
> 
> http://codereview.chromium.org/510004/diff/1/10#newcode53
> views/controls/native_control_gtk.cc:53: // Make sure that Detach destroyed
> destroy the widget.
> On 2009/12/22 23:55:16, sky wrote:
> > remove 'destroy.'
> 
> Done.

[oshima, 2009-12-23 00:15:02]

Re-running linux_view test (it hit AutoUpdator crash)
I'll check in after linux/linux_view and valgrind test become green.

On 2009/12/23 00:03:49, sky wrote:
> Ok, LGTM.
> 
>   -Scott
> 
> On 2009/12/23 00:01:43, oshima wrote:
> > http://codereview.chromium.org/510004/diff/1/2
> > File chrome/browser/views/tab_contents/tab_contents_view_gtk.cc (right):
> > 
> > http://codereview.chromium.org/510004/diff/1/2#newcode155
> > chrome/browser/views/tab_contents/tab_contents_view_gtk.cc:155:
> > gtk_object_ref(GTK_OBJECT(GetNativeView()));
> > On 2009/12/22 23:55:16, sky wrote:
> > > Don't you need an unref to balance this at some point?
> > 
> > It's destroyed in CloseNew in the destructor.
> > 
> > http://codereview.chromium.org/510004/diff/1/5
> > File views/controls/native/native_view_host_gtk.cc (right):
> > 
> > http://codereview.chromium.org/510004/diff/1/5#newcode80
> > views/controls/native/native_view_host_gtk.cc:80:
> > DCHECK_NE(static_cast<gfx::NativeView>(NULL),
> > gtk_widget_get_parent(host_->native_view()));
> > On 2009/12/22 23:55:16, sky wrote:
> > > lines too long here and 82.
> > 
> > Done.
> > 
> > http://codereview.chromium.org/510004/diff/1/5#newcode209
> > views/controls/native/native_view_host_gtk.cc:209: // fixed_ should not have
> any
> > child this point.
> > On 2009/12/22 23:55:16, sky wrote:
> > > child-> children at
> > 
> > Done.
> > 
> > http://codereview.chromium.org/510004/diff/1/10
> > File views/controls/native_control_gtk.cc (right):
> > 
> > http://codereview.chromium.org/510004/diff/1/10#newcode53
> > views/controls/native_control_gtk.cc:53: // Make sure that Detach destroyed
> > destroy the widget.
> > On 2009/12/22 23:55:16, sky wrote:
> > > remove 'destroy.'
> > 
> > Done.