From af4b26f5812b990a7f4af725d9b3a09e7c4ac63a Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 26 Feb 2014 02:29:36 +0100 Subject: [PATCH] a11y: Fix refcounting for treeview cells Old code assumed that AT-SPI would keep track of references and therefore tried to only hold weak references. On the other hand it also tried to keep objects alive so it referenced objects very randomly. All of that lead to cycles and leaking. As AT-SPI does not keep track of objects at all, the treeview now does. The refcounting looks as follows: GtkTreeViewAccessible => creates per row/column GtkTreeViewAccessibleCellInfo => which references 1 GtkCellAccessible If there is only one cell, this accessible is a GtkRendererCellAccessible, otherwise a GtkContainerCellAccessible is created and that accessible holds references to the GtkRendererCellAccessibles that are created for every cell renderer. https://bugzilla.gnome.org/show_bug.cgi?id=554618 --- gtk/a11y/gtkcellaccessible.c | 39 ++++++++++++++++----------- gtk/a11y/gtkcontainercellaccessible.c | 4 +++ gtk/a11y/gtktreeviewaccessible.c | 2 +- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/gtk/a11y/gtkcellaccessible.c b/gtk/a11y/gtkcellaccessible.c index b5846e8c21..8251771128 100644 --- a/gtk/a11y/gtkcellaccessible.c +++ b/gtk/a11y/gtkcellaccessible.c @@ -23,6 +23,11 @@ #include "gtkcellaccessibleprivate.h" #include "gtkcellaccessibleparent.h" +struct _GtkCellAccessiblePrivate +{ + AtkObject *parent; +}; + static const struct { AtkState atk_state; GtkCellRendererState renderer_state; @@ -43,6 +48,7 @@ static void atk_action_interface_init (AtkActionIface *iface); static void atk_component_interface_init (AtkComponentIface *iface); G_DEFINE_TYPE_WITH_CODE (GtkCellAccessible, gtk_cell_accessible, GTK_TYPE_ACCESSIBLE, + G_ADD_PRIVATE (GtkCellAccessible) G_IMPLEMENT_INTERFACE (ATK_TYPE_ACTION, atk_action_interface_init) G_IMPLEMENT_INTERFACE (ATK_TYPE_COMPONENT, atk_component_interface_init)) @@ -83,15 +89,14 @@ gtk_cell_accessible_get_index_in_parent (AtkObject *obj) cell = GTK_CELL_ACCESSIBLE (obj); - parent = atk_object_get_parent (obj); - if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (parent)) - return g_list_index (gtk_container_cell_accessible_get_children (GTK_CONTAINER_CELL_ACCESSIBLE (parent)), obj); + if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (cell->priv->parent)) + return g_list_index (gtk_container_cell_accessible_get_children (GTK_CONTAINER_CELL_ACCESSIBLE (cell->priv->parent)), obj); parent = gtk_widget_get_accessible (gtk_accessible_get_widget (GTK_ACCESSIBLE (cell))); if (parent == NULL) return -1; - return gtk_cell_accessible_parent_get_child_index (GTK_CELL_ACCESSIBLE_PARENT (parent), cell); + return gtk_cell_accessible_parent_get_child_index (GTK_CELL_ACCESSIBLE_PARENT (cell->priv->parent), cell); } static AtkStateSet * @@ -139,6 +144,13 @@ gtk_cell_accessible_ref_state_set (AtkObject *accessible) return state_set; } +static AtkObject * +gtk_cell_accessible_get_parent (AtkObject *object) +{ + GtkCellAccessible *cell = GTK_CELL_ACCESSIBLE (object); + + return cell->priv->parent; +} static void gtk_cell_accessible_class_init (GtkCellAccessibleClass *klass) @@ -150,11 +162,13 @@ gtk_cell_accessible_class_init (GtkCellAccessibleClass *klass) class->get_index_in_parent = gtk_cell_accessible_get_index_in_parent; class->ref_state_set = gtk_cell_accessible_ref_state_set; + class->get_parent = gtk_cell_accessible_get_parent; } static void gtk_cell_accessible_init (GtkCellAccessible *cell) { + cell->priv = gtk_cell_accessible_get_instance_private (cell); } void @@ -163,7 +177,7 @@ _gtk_cell_accessible_initialize (GtkCellAccessible *cell, AtkObject *parent) { gtk_accessible_set_widget (GTK_ACCESSIBLE (cell), widget); - atk_object_set_parent (ATK_OBJECT (cell), parent); + cell->priv->parent = parent; } gboolean @@ -171,8 +185,6 @@ _gtk_cell_accessible_add_state (GtkCellAccessible *cell, AtkStateType state_type, gboolean emit_signal) { - AtkObject *parent; - /* The signal should only be generated if the value changed, * not when the cell is set up. So states that are set * initially should pass FALSE as the emit_signal argument. @@ -188,9 +200,8 @@ _gtk_cell_accessible_add_state (GtkCellAccessible *cell, /* If the parent is a flyweight container cell, propagate the state * change to it also */ - parent = atk_object_get_parent (ATK_OBJECT (cell)); - if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (parent)) - _gtk_cell_accessible_add_state (GTK_CELL_ACCESSIBLE (parent), state_type, emit_signal); + if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (cell->priv->parent)) + _gtk_cell_accessible_add_state (GTK_CELL_ACCESSIBLE (cell->priv->parent), state_type, emit_signal); return TRUE; } @@ -200,10 +211,6 @@ _gtk_cell_accessible_remove_state (GtkCellAccessible *cell, AtkStateType state_type, gboolean emit_signal) { - AtkObject *parent; - - parent = atk_object_get_parent (ATK_OBJECT (cell)); - /* The signal should only be generated if the value changed, * not when the cell is set up. So states that are set * initially should pass FALSE as the emit_signal argument. @@ -219,8 +226,8 @@ _gtk_cell_accessible_remove_state (GtkCellAccessible *cell, /* If the parent is a flyweight container cell, propagate the state * change to it also */ - if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (parent)) - _gtk_cell_accessible_remove_state (GTK_CELL_ACCESSIBLE (parent), state_type, emit_signal); + if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (cell->priv->parent)) + _gtk_cell_accessible_remove_state (GTK_CELL_ACCESSIBLE (cell->priv->parent), state_type, emit_signal); return TRUE; } diff --git a/gtk/a11y/gtkcontainercellaccessible.c b/gtk/a11y/gtkcontainercellaccessible.c index 519b16b0e8..ea8ada0767 100644 --- a/gtk/a11y/gtkcontainercellaccessible.c +++ b/gtk/a11y/gtkcontainercellaccessible.c @@ -141,6 +141,8 @@ gtk_container_cell_accessible_add_child (GtkContainerCellAccessible *container, g_return_if_fail (GTK_IS_CONTAINER_CELL_ACCESSIBLE (container)); g_return_if_fail (GTK_IS_CELL_ACCESSIBLE (child)); + g_object_ref (child); + container->priv->n_children++; container->priv->children = g_list_append (container->priv->children, child); atk_object_set_parent (ATK_OBJECT (child), ATK_OBJECT (container)); @@ -156,6 +158,8 @@ gtk_container_cell_accessible_remove_child (GtkContainerCellAccessible *containe container->priv->children = g_list_remove (container->priv->children, child); container->priv->n_children--; + + g_object_unref (child); } GList * diff --git a/gtk/a11y/gtktreeviewaccessible.c b/gtk/a11y/gtktreeviewaccessible.c index 6c579b29fe..5a5b11a16f 100644 --- a/gtk/a11y/gtktreeviewaccessible.c +++ b/gtk/a11y/gtktreeviewaccessible.c @@ -1448,7 +1448,7 @@ cell_info_new (GtkTreeViewAccessible *accessible, cell_info->tree = tree; cell_info->node = node; cell_info->cell_col_ref = tv_col; - cell_info->cell = g_object_ref (cell); + cell_info->cell = cell; cell_info->view = accessible; g_object_set_qdata (G_OBJECT (cell),