Merge branch 'template-child-dispose-order' into 'main'

gtk: Set widget template children to NULL before destroy unref

See merge request GNOME/gtk!4611

2 files changed, 150 insertions, 20 deletions

diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c
index ab75402d8c..ac732533ef 100644
--- a/gtk/gtkwidget.c
+++ b/gtk/gtkwidget.c
@@ -7442,6 +7442,7 @@ gtk_widget_real_destroy (GtkWidget *object)
     {
       GtkWidgetClass *class;
       GSList *l;
+      GHashTable *auto_children;
 
 #ifdef G_ENABLE_CONSISTENCY_CHECKS
       GSList *assertions = NULL;
@@ -7490,34 +7491,29 @@ gtk_widget_real_destroy (GtkWidget *object)
         }
 #endif /* G_ENABLE_CONSISTENCY_CHECKS */
 
-      /* Release references to all automated children */
-      g_object_set_qdata (G_OBJECT (widget), quark_auto_children, NULL);
+      /* Prepare to release references to all automated children */
+      auto_children = g_object_steal_qdata (G_OBJECT (widget), quark_auto_children);
 
-#ifdef G_ENABLE_CONSISTENCY_CHECKS
-      for (l = assertions; l; l = l->next)
-        {
-          FinalizeAssertion *assertion = l->data;
-
-          if (!assertion->did_finalize)
-            g_critical ("Automated component '%s' of class '%s' did not finalize in dispose()"
-            "Current reference count is %d",
-            assertion->child_class->name,
-            g_type_name (assertion->widget_type),
-            assertion->object->ref_count);
-
-          g_slice_free (FinalizeAssertion, assertion);
-        }
-      g_slist_free (assertions);
-#endif /* G_ENABLE_CONSISTENCY_CHECKS */
-
-      /* Set any automatic private data pointers to NULL */
+      /* Set any automatic private data pointers to NULL and release child references */
       for (class = GTK_WIDGET_GET_CLASS (widget);
            GTK_IS_WIDGET_CLASS (class);
            class = g_type_class_peek_parent (class))
         {
+          GHashTable *auto_child_hash = NULL;
+
           if (!class->priv->template)
             continue;
 
+          if (auto_children)
+            {
+              GType type = G_TYPE_FROM_CLASS (class);
+
+              g_hash_table_steal_extended (auto_children,
+                                           GSIZE_TO_POINTER (type),
+                                           NULL,
+                                           (gpointer *) &auto_child_hash);
+            }
+
           for (l = class->priv->template->children; l; l = l->next)
             {
               AutomaticChildClass *child_class = l->data;
@@ -7530,8 +7526,34 @@ gtk_widget_real_destroy (GtkWidget *object)
                   field_p = G_STRUCT_MEMBER_P (widget, child_class->offset);
                   (* (gpointer *) field_p) = NULL;
                 }
+
+              /* Release the references in order after setting the pointer to NULL */
+              if (auto_child_hash)
+                g_hash_table_remove (auto_child_hash, child_class->name);
             }
+
+          g_clear_pointer (&auto_child_hash, g_hash_table_unref);
         }
+
+      /* Free the child reference hash table */
+      g_clear_pointer (&auto_children, g_hash_table_unref);
+
+#ifdef G_ENABLE_CONSISTENCY_CHECKS
+      for (l = assertions; l; l = l->next)
+        {
+          FinalizeAssertion *assertion = l->data;
+
+          if (!assertion->did_finalize)
+            g_critical ("Automated component '%s' of class '%s' did not finalize in dispose()"
+            "Current reference count is %d",
+            assertion->child_class->name,
+            g_type_name (assertion->widget_type),
+            assertion->object->ref_count);
+
+          g_slice_free (FinalizeAssertion, assertion);
+        }
+      g_slist_free (assertions);
+#endif /* G_ENABLE_CONSISTENCY_CHECKS */
     }
 
   /* Callers of add_mnemonic_label() should disconnect on ::destroy */
diff --git a/testsuite/gtk/builder.c b/testsuite/gtk/builder.c
index 1e15588b50..db49159cb7 100644
--- a/testsuite/gtk/builder.c
+++ b/testsuite/gtk/builder.c
@@ -2689,6 +2689,113 @@ test_expressions (void)
     }
 }
 
+#define MY_GTK_BOX_TEMPLATE "\
+<interface>\n\
+ <template class=\"MyGtkBox\" parent=\"GtkWidget\">\n\
+  <child>\n\
+   <object class=\"GtkLabel\" id=\"label\">\n\
+    <property name=\"label\">First</property>\n\
+   </object>\n\
+  </child>\n\
+  <child>\n\
+   <object class=\"GtkLabel\" id=\"label2\">\n\
+    <property name=\"label\">Second</property>\n\
+   </object>\n\
+  </child>\n\
+ </template>\n\
+</interface>\n"
+
+#define MY_TYPE_GTK_BOX (my_gtk_box_get_type ())
+G_DECLARE_FINAL_TYPE    (MyGtkBox, my_gtk_box, MY, GTK_BOX, GtkWidget)
+
+struct _MyGtkBox
+{
+  GtkWidget  parent_instance;
+  GtkLabel  *label;
+  GtkLabel  *label2;
+};
+
+G_DEFINE_TYPE (MyGtkBox, my_gtk_box, GTK_TYPE_WIDGET);
+
+static void
+my_gtk_box_init (MyGtkBox *grid)
+{
+  gtk_widget_init_template (GTK_WIDGET (grid));
+}
+
+static void
+my_gtk_box_dispose (GObject *obj)
+{
+  MyGtkBox  *my_gtk_box = MY_GTK_BOX (obj);
+  GtkWidget *child;
+
+  while ((child = gtk_widget_get_first_child (GTK_WIDGET (my_gtk_box))))
+    gtk_widget_unparent (child);
+
+  G_OBJECT_CLASS (my_gtk_box_parent_class)->dispose (obj);
+}
+
+static void
+my_gtk_box_class_init (MyGtkBoxClass *klass)
+{
+  GBytes *template = g_bytes_new_static (MY_GTK_BOX_TEMPLATE, strlen (MY_GTK_BOX_TEMPLATE));
+  GtkWidgetClass *widget_class = GTK_WIDGET_CLASS (klass);
+
+  gtk_widget_class_set_template (widget_class, template);
+  gtk_widget_class_bind_template_child (widget_class, MyGtkBox, label);
+  gtk_widget_class_bind_template_child (widget_class, MyGtkBox, label2);
+
+  G_OBJECT_CLASS (klass)->dispose = my_gtk_box_dispose;
+}
+
+typedef struct
+{
+  MyGtkBox *my_gtk_box;
+  guint     destroy_count;
+} BoxDestroyData;
+
+static void
+my_label_destroy (GtkLabel *label, BoxDestroyData *data)
+{
+  g_assert_true (MY_IS_GTK_BOX (data->my_gtk_box));
+  /* Make sure the other label is null if it was disposed first */
+  g_assert_true (!data->my_gtk_box->label2 || GTK_IS_LABEL (data->my_gtk_box->label2));
+  data->destroy_count++;
+}
+
+static void
+my_label2_destroy (GtkLabel *label2, BoxDestroyData *data)
+{
+  g_assert_true (MY_IS_GTK_BOX (data->my_gtk_box));
+  /* Make sure the other label is null if it was disposed first */
+  g_assert_true (!data->my_gtk_box->label || GTK_IS_LABEL (data->my_gtk_box->label));
+  data->destroy_count++;
+}
+
+static void
+test_child_dispose_order (void)
+{
+  BoxDestroyData data = { 0, };
+
+  /* make sure the type we are trying to register does not exist */
+  g_assert_false (g_type_from_name ("MyGtkBox"));
+
+  /* create the template object */
+  data.my_gtk_box = g_object_ref_sink (g_object_new (MY_TYPE_GTK_BOX, NULL));
+
+  /* Check everything is fine */
+  g_assert_true (g_type_from_name ("MyGtkBox"));
+  g_assert_true (MY_IS_GTK_BOX (data.my_gtk_box));
+  g_assert_true (GTK_IS_LABEL (data.my_gtk_box->label));
+  g_assert_true (GTK_IS_LABEL (data.my_gtk_box->label2));
+
+  /* Check if both labels are destroyed */
+  g_signal_connect (data.my_gtk_box->label, "destroy", G_CALLBACK (my_label_destroy), &data);
+  g_signal_connect (data.my_gtk_box->label2, "destroy", G_CALLBACK (my_label2_destroy), &data);
+  g_object_unref (data.my_gtk_box);
+  g_assert_cmpuint (data.destroy_count, ==, 2);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -2736,6 +2843,7 @@ main (int argc, char **argv)
   g_test_add_func ("/Builder/Shortcuts", test_shortcuts);
   g_test_add_func ("/Builder/Transforms", test_transforms);
   g_test_add_func ("/Builder/Expressions", test_expressions);
+  g_test_add_func ("/Builder/Child Dispose Order", test_child_dispose_order);
 
   return g_test_run();
 }