b=522635 destroy child nsWindows when destroying the parent r=roc
authorKarl Tomlinson <karlt+@karlt.net>
Wed, 11 Nov 2009 10:57:25 +1300
changeset 32880 53af277ed12fc64f3e4fe71cea2fcddd1cd61efe
parent 32879 f212daadb16b1a57bdea2b72e701f18f6214cd59
child 32881 96a63cd3a544517b853e7a7acb5a10a4ed0fea32
push id658
push userktomlinson@mozilla.com
push dateThu, 12 Nov 2009 02:35:05 +0000
reviewersroc
bugs522635
milestone1.9.2b3pre
b=522635 destroy child nsWindows when destroying the parent r=roc
widget/src/gtk2/nsWindow.cpp
widget/src/gtk2/nsWindow.h
--- a/widget/src/gtk2/nsWindow.cpp
+++ b/widget/src/gtk2/nsWindow.cpp
@@ -654,19 +654,17 @@ CheckDestroyInvisibleContainer()
         // Make sure to destroy the GtkWindow also.
         gtk_widget_destroy(gInvisibleContainer->parent);
         gInvisibleContainer = NULL;
     }
 }
 
 // Change the containing GtkWidget on a sub-hierarchy of GdkWindows belonging
 // to aOldWidget and rooted at aWindow, and reparent any child GtkWidgets of
-// the GdkWindow hierarchy.  If aNewWidget is NULL, the reference to
-// aOldWidget is removed from its GdkWindows, and child GtkWidgets are
-// destroyed.
+// the GdkWindow hierarchy to aNewWidget.
 static void
 SetWidgetForHierarchy(GdkWindow *aWindow,
                       GtkWidget *aOldWidget,
                       GtkWidget *aNewWidget)
 {
     gpointer data;
     gdk_window_get_user_data(aWindow, &data);
 
@@ -675,36 +673,58 @@ SetWidgetForHierarchy(GdkWindow *aWindow
             return;
 
         GtkWidget* widget = static_cast<GtkWidget*>(data);
         if (widget->parent != aOldWidget)
             return;
 
         // This window belongs to a child widget, which will no longer be a
         // child of aOldWidget.
-        if (aNewWidget) {
-            gtk_widget_reparent(widget, aNewWidget);
-        } else {
-            // aNewWidget == NULL indicates that the window is about to be
-            // destroyed.
-            gtk_widget_destroy(widget);
-        }
+        gtk_widget_reparent(widget, aNewWidget);
 
         return;
     }
 
     GList *children = gdk_window_get_children(aWindow);
     for(GList *list = children; list; list = list->next) {
         SetWidgetForHierarchy(GDK_WINDOW(list->data), aOldWidget, aNewWidget);
     }
     g_list_free(children);
 
     gdk_window_set_user_data(aWindow, aNewWidget);
 }
 
+// Walk the list of child windows and call destroy on them.
+void
+nsWindow::DestroyChildWindows()
+{
+    if (!mGdkWindow)
+        return;
+
+    GList *children = gdk_window_get_children(mGdkWindow);
+
+    for(GList *list = children; list; list = list->next) {
+        GdkWindow *child = GDK_WINDOW(children->data);
+        nsWindow *kid = get_window_for_gdk_window(child);
+        if (kid) {
+            kid->Destroy();
+        } else {
+            // This child is not an nsWindow.
+            // Destroy the child GtkWidget.
+            gpointer data;
+            gdk_window_get_user_data(child, &data);
+            if (GTK_IS_WIDGET(data)) {
+                gtk_widget_destroy(static_cast<GtkWidget*>(data));
+            }
+        }
+    }
+
+    g_list_free(children);
+}
+
 NS_IMETHODIMP
 nsWindow::Destroy(void)
 {
     if (mIsDestroyed || !mCreated)
         return NS_OK;
 
     LOG(("nsWindow::Destroy [%p]\n", (void *)this));
     mIsDestroyed = PR_TRUE;
@@ -732,25 +752,16 @@ nsWindow::Destroy(void)
         if (gRollupListener)
             gRollupListener->Rollup(nsnull, nsnull);
         gRollupWindow = nsnull;
         gRollupListener = nsnull;
     }
 
     NativeShow(PR_FALSE);
 
-    // walk the list of children and call destroy on them.  Have to be
-    // careful, though -- calling destroy on a kid may actually remove
-    // it from our child list, losing its sibling links.
-    for (nsIWidget* kid = mFirstChild; kid; ) {
-        nsIWidget* next = kid->GetNextSibling();
-        kid->Destroy();
-        kid = next;
-    }
-
 #ifdef USE_XIM
     IMEDestroyContext();
 #endif
 
     // make sure that we remove ourself as the focus window
     if (gFocusWindow == this) {
         LOGFOCUS(("automatically losing focus...\n"));
         gFocusWindow = nsnull;
@@ -792,26 +803,23 @@ nsWindow::Destroy(void)
     }
     else if (mContainer) {
         gtk_widget_destroy(GTK_WIDGET(mContainer));
         mContainer = nsnull;
         NS_ABORT_IF_FALSE(!mGdkWindow,
                           "mGdkWindow should be NULL when mContainer is destroyed");
     }
     else if (mGdkWindow) {
-        // Remove references from GdkWindows back to their container
-        // widget while the GdkWindow hierarchy is still available.
-        // (OnContainerUnrealize does this when the MozContainer widget is
-        // destroyed.)
-        if (owningWidget) {
-            SetWidgetForHierarchy(mGdkWindow, owningWidget, NULL);
-        }
-        NS_ASSERTION(!get_gtk_widget_for_gdk_window(mGdkWindow),
-                     "widget reference not removed");
-
+        // Destroy child windows to ensure that their mThebesSurfaces are
+        // released and to remove references from GdkWindows back to their
+        // container widget.  (OnContainerUnrealize() does this when the
+        // MozContainer widget is destroyed.)
+        DestroyChildWindows();
+
+        gdk_window_set_user_data(mGdkWindow, NULL);
         g_object_set_data(G_OBJECT(mGdkWindow), "nsWindow", NULL);
         gdk_window_destroy(mGdkWindow);
         mGdkWindow = nsnull;
     }
 
     if (gInvisibleContainer && owningWidget == gInvisibleContainer) {
         CheckDestroyInvisibleContainer();
     }
@@ -2428,17 +2436,17 @@ nsWindow::OnContainerUnrealize(GtkWidget
     // The GdkWindows are about to be destroyed (but not deleted), so remove
     // their references back to their container widget while the GdkWindow
     // hierarchy is still available.
 
     NS_ASSERTION(mContainer == MOZ_CONTAINER(aWidget),
                  "unexpected \"unrealize\" signal");
 
     if (mGdkWindow) {
-        SetWidgetForHierarchy(mGdkWindow, aWidget, NULL);
+        DestroyChildWindows();
 
         g_object_set_data(G_OBJECT(mGdkWindow), "nsWindow", NULL);
         mGdkWindow = NULL;
     }
 }
 
 void
 nsWindow::OnSizeAllocate(GtkWidget *aWidget, GtkAllocation *aAllocation)
@@ -7258,16 +7266,19 @@ nsWindow::GetSurfaceForGdkDrawable(GdkDr
     return result;
 }
 #endif
 
 // return the gfxASurface for rendering to this widget
 gfxASurface*
 nsWindow::GetThebesSurface()
 {
+    if (!mGdkWindow)
+        return nsnull;
+
     GdkDrawable* d;
     gint x_offset, y_offset;
     gdk_window_get_internal_paint_info(mGdkWindow, &d, &x_offset, &y_offset);
 
 #ifdef MOZ_X11
     gint width, height;
     gdk_drawable_get_size(d, &width, &height);
     // Owen Taylor says this is the right thing to do!
--- a/widget/src/gtk2/nsWindow.h
+++ b/widget/src/gtk2/nsWindow.h
@@ -429,16 +429,17 @@ protected:
     PRPackedBool        mEnabled;
     // has the native window for this been created yet?
     PRPackedBool        mCreated;
     // Has anyone set an x/y location for this widget yet? Toplevels
     // shouldn't be automatically set to 0,0 for first show.
     PRPackedBool        mPlaced;
 
 private:
+    void               DestroyChildWindows();
     void               GetToplevelWidget(GtkWidget **aWidget);
     GtkWidget         *GetMozContainerWidget();
     nsWindow          *GetContainerWindow();
     void               SetUrgencyHint(GtkWidget *top_window, PRBool state);
     void              *SetupPluginPort(void);
     nsresult           SetWindowIconList(const nsTArray<nsCString> &aIconList);
     void               SetDefaultIcon(void);
     void               InitButtonEvent(nsMouseEvent &aEvent, GdkEventButton *aGdkEvent);