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.

Review URL: http://codereview.chromium.org/510004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35220 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 40 insertions, 15 deletions

diff --git a/views/controls/native/native_view_host.cc b/views/controls/native/native_view_host.cc
index ce2e4d02..1274e5c 100644
--- a/views/controls/native/native_view_host.cc
+++ b/views/controls/native/native_view_host.cc
@@ -40,6 +40,7 @@ NativeViewHost::~NativeViewHost() {
 }
 
 void NativeViewHost::Attach(gfx::NativeView native_view) {
+  DCHECK(native_view);
   DCHECK(!native_view_);
   native_view_ = native_view;
   // If set_focus_view() has not been invoked, this view is the one that should
@@ -50,9 +51,7 @@ void NativeViewHost::Attach(gfx::NativeView native_view) {
 }
 
 void NativeViewHost::Detach() {
-  DCHECK(native_view_);
-  native_wrapper_->NativeViewDetaching();
-  native_view_ = NULL;
+  Detach(false);
 }
 
 void NativeViewHost::SetPreferredSize(const gfx::Size& size) {
@@ -63,7 +62,7 @@ void NativeViewHost::SetPreferredSize(const gfx::Size& size) {
 void NativeViewHost::NativeViewDestroyed() {
   // Detach so we can clear our state and notify the native_wrapper_ to release
   // ref on the native view.
-  Detach();
+  Detach(true);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -146,4 +145,13 @@ void NativeViewHost::Focus() {
   native_wrapper_->SetFocus();
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// NativeViewHost, private:
+
+void NativeViewHost::Detach(bool destroyed) {
+  DCHECK(native_view_);
+  native_wrapper_->NativeViewDetaching(destroyed);
+  native_view_ = NULL;
+}
+
 }  // namespace views
diff --git a/views/controls/native/native_view_host.h b/views/controls/native/native_view_host.h
index 9970901..d6115b2 100644
--- a/views/controls/native/native_view_host.h
+++ b/views/controls/native/native_view_host.h
@@ -81,6 +81,10 @@ class NativeViewHost : public View {
   virtual std::string GetClassName() const;
 
  private:
+  // Detach the native view. |destroyed| is true if the native view is
+  // detached because it's being destroyed, or false otherwise.
+  void Detach(bool destroyed);
+
   // The attached native view.
   gfx::NativeView native_view_;
 
diff --git a/views/controls/native/native_view_host_gtk.cc b/views/controls/native/native_view_host_gtk.cc
index 581f08d..ed4f5ba 100644
--- a/views/controls/native/native_view_host_gtk.cc
+++ b/views/controls/native/native_view_host_gtk.cc
@@ -63,7 +63,7 @@ void NativeViewHostGtk::NativeViewAttached() {
   // TODO(port): figure out focus.
 }
 
-void NativeViewHostGtk::NativeViewDetaching() {
+void NativeViewHostGtk::NativeViewDetaching(bool destroyed) {
   DCHECK(host_->native_view());
 
   g_signal_handler_disconnect(G_OBJECT(host_->native_view()),
@@ -76,6 +76,14 @@ void NativeViewHostGtk::NativeViewDetaching() {
 
   installed_clip_ = false;
 
+  if (fixed_ && !destroyed) {
+    DCHECK_NE(static_cast<gfx::NativeView>(NULL),
+              gtk_widget_get_parent(host_->native_view()));
+    gtk_container_remove(GTK_CONTAINER(fixed_), host_->native_view());
+    DCHECK_EQ(
+        0U, g_list_length(gtk_container_get_children(GTK_CONTAINER(fixed_))));
+  }
+
   g_object_unref(G_OBJECT(host_->native_view()));
 }
 
@@ -200,7 +208,9 @@ void NativeViewHostGtk::DestroyFixed() {
     // widget from the moment it is attached.
     gtk_container_remove(GTK_CONTAINER(fixed_), host_->native_view());
   }
-
+  // fixed_ should not have any children this point.
+  DCHECK_EQ(0U,
+            g_list_length(gtk_container_get_children(GTK_CONTAINER(fixed_))));
   gtk_widget_destroy(fixed_);
   fixed_ = NULL;
 }
diff --git a/views/controls/native/native_view_host_gtk.h b/views/controls/native/native_view_host_gtk.h
index 81a2a7a1..b1bcc50 100644
--- a/views/controls/native/native_view_host_gtk.h
+++ b/views/controls/native/native_view_host_gtk.h
@@ -28,7 +28,7 @@ class NativeViewHostGtk : public NativeViewHostWrapper {
 
   // Overridden from NativeViewHostWrapper:
   virtual void NativeViewAttached();
-  virtual void NativeViewDetaching();
+  virtual void NativeViewDetaching(bool destroyed);
   virtual void AddedToWidget();
   virtual void RemovedFromWidget();
   virtual void InstallClip(int x, int y, int w, int h);
@@ -86,4 +86,3 @@ class NativeViewHostGtk : public NativeViewHostWrapper {
 }  // namespace views
 
 #endif  // VIEWS_CONTROLS_NATIVE_HOST_VIEW_GTK_H_
-
diff --git a/views/controls/native/native_view_host_win.cc b/views/controls/native/native_view_host_win.cc
index c316d6e..4a8f357 100644
--- a/views/controls/native/native_view_host_win.cc
+++ b/views/controls/native/native_view_host_win.cc
@@ -39,7 +39,7 @@ void NativeViewHostWin::NativeViewAttached() {
   host_->Layout();
 }
 
-void NativeViewHostWin::NativeViewDetaching() {
+void NativeViewHostWin::NativeViewDetaching(bool destroyed) {
   installed_clip_ = false;
 }
 
diff --git a/views/controls/native/native_view_host_win.h b/views/controls/native/native_view_host_win.h
index 51bc616..489c9d8 100644
--- a/views/controls/native/native_view_host_win.h
+++ b/views/controls/native/native_view_host_win.h
@@ -20,7 +20,7 @@ class NativeViewHostWin : public NativeViewHostWrapper {
 
   // Overridden from NativeViewHostWrapper:
   virtual void NativeViewAttached();
-  virtual void NativeViewDetaching();
+  virtual void NativeViewDetaching(bool destroyed);
   virtual void AddedToWidget();
   virtual void RemovedFromWidget();
   virtual void InstallClip(int x, int y, int w, int h);
diff --git a/views/controls/native/native_view_host_wrapper.h b/views/controls/native/native_view_host_wrapper.h
index 948f0ce..056584e 100644
--- a/views/controls/native/native_view_host_wrapper.h
+++ b/views/controls/native/native_view_host_wrapper.h
@@ -23,8 +23,9 @@ class NativeViewHostWrapper {
 
   // Called before the attached gfx::NativeView is detached from the
   // NativeViewHost, allowing the wrapper to perform platform-specific
-  // cleanup.
-  virtual void NativeViewDetaching() = 0;
+  // cleanup. |destroyed| is true if the native view is detached
+  // because it's being destroyed, or false otherwise.
+  virtual void NativeViewDetaching(bool destroyed) = 0;
 
   // Called when our associated NativeViewHost is added to a View hierarchy
   // rooted at a valid Widget.
diff --git a/views/controls/native_control_gtk.cc b/views/controls/native_control_gtk.cc
index 49453ef..66e6994 100644
--- a/views/controls/native_control_gtk.cc
+++ b/views/controls/native_control_gtk.cc
@@ -49,9 +49,9 @@ void NativeControlGtk::VisibilityChanged(View* starting_from, bool is_visible) {
     if (native_view()) {
       // We destroy the child widget when we become invisible because of the
       // performance cost of maintaining widgets that aren't currently needed.
-      GtkWidget* widget = native_view();
       Detach();
-      gtk_widget_destroy(widget);
+      // Make sure that Detach destroyed the widget.
+      DCHECK(!native_view());
     }
   } else if (!native_view()) {
     if (GetWidget())
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index 9729606..a2d699f 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -112,6 +112,7 @@ WidgetGtk::WidgetGtk(Type type)
 }
 
 WidgetGtk::~WidgetGtk() {
+  DCHECK(delete_on_destroy_ || widget_ == NULL);
   if (type_ != TYPE_CHILD)
     ActiveWindowWatcherX::RemoveObserver(this);
   MessageLoopForUI::current()->RemoveObserver(this);
@@ -441,8 +442,10 @@ void WidgetGtk::Close() {
 }
 
 void WidgetGtk::CloseNow() {
-  if (widget_)
+  if (widget_) {
     gtk_widget_destroy(widget_);
+    widget_ = NULL;
+  }
 }
 
 void WidgetGtk::Show() {