Chromium Code Reviews

Unified Diff: views/controls/native/native_view_host_gtk.cc

Issue 159751: Fix leak of GtkWidgets in NativeViewHost, and in doing so fix a crash on shut... (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: '' Created 11 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Index: views/controls/native/native_view_host_gtk.cc
===================================================================
--- views/controls/native/native_view_host_gtk.cc (revision 22173)
+++ views/controls/native/native_view_host_gtk.cc (working copy)
@@ -47,6 +47,10 @@
// Always layout though.
host_->Layout();
+ // We own the native view as long as it's attached, so that we can safely
+ // reparent it in multiple passes.
+ gtk_widget_ref(host_->native_view());
+
// TODO(port): figure out focus.
}
@@ -60,6 +64,9 @@
// TODO(port): focus.
// FocusManager::UninstallFocusSubclass(native_view());
installed_clip_ = false;
+
+ // Release ownership back to the caller.
+ gtk_widget_unref(host_->native_view());
}
void NativeViewHostGtk::AddedToWidget() {
@@ -87,8 +94,6 @@
if (!host_->native_view())
return;
- // TODO(beng): We leak host_->native_view() here. Fix: make all widgets not be
- // refcounted.
DestroyFixed();
}
@@ -156,7 +161,7 @@
// NativeViewHostGtk, private:
void NativeViewHostGtk::CreateFixed(bool needs_window) {
- bool native_view_addrefed = DestroyFixed();
+ DestroyFixed();
fixed_ = gtk_fixed_new();
gtk_fixed_set_has_window(GTK_FIXED(fixed_), needs_window);
@@ -168,28 +173,23 @@
widget_gtk->AddChild(fixed_);
if (host_->native_view())
gtk_container_add(GTK_CONTAINER(fixed_), host_->native_view());
- if (native_view_addrefed)
- gtk_widget_unref(host_->native_view());
}
-bool NativeViewHostGtk::DestroyFixed() {
- bool native_view_addrefed = false;
+void NativeViewHostGtk::DestroyFixed() {
if (!fixed_)
- return native_view_addrefed;
+ return;
gtk_widget_hide(fixed_);
GetHostWidget()->RemoveChild(fixed_);
if (host_->native_view()) {
- // We can't allow the hosted NativeView's refcount to drop to zero.
- gtk_widget_ref(host_->native_view());
- native_view_addrefed = true;
+ // We can safely remove the widget from its container since we own the
+ // widget from the moment it is attached.
gtk_container_remove(GTK_CONTAINER(fixed_), host_->native_view());
}
gtk_widget_destroy(fixed_);
fixed_ = NULL;
- return native_view_addrefed;
}
WidgetGtk* NativeViewHostGtk::GetHostWidget() const {
« views/controls/native/native_view_host_gtk.h ('K') | « views/controls/native/native_view_host_gtk.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine