[nautilus-actions] Fix reference count errors in NACT user interface

[Pierre Wieser <pwieser src gnome org>]

commit 0fe2b10d017b02490522bd450c7b6bee0777ed4f
Author: Pierre Wieser <pwieser trychlos org>
Date:   Fri Feb 26 00:06:17 2010 +0100

    Fix reference count errors in NACT user interface

 ChangeLog                         |   16 +++++++++++
 src/core/na-object-id.c           |   15 ++++++++++
 src/core/na-object-item-factory.c |    4 +-
 src/core/na-object-item.c         |   20 +++++++-------
 src/core/na-object.c              |    1 -
 src/nact/nact-tree-model.c        |   54 +++++++++++++++++--------------------
 6 files changed, 68 insertions(+), 42 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 0e46404..f91a984 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2009-02-25 Pierre Wieser <pwieser trychlos org>
 
+	Fix reference count errors in NACT user interface.
+
+	* src/core/na-object-id.c (instance_dispose):
+	Remove the disposing child from its parent childs list.
+
+	* src/core/na-object-item-factory.c (free_items_list):
+	* src/core/na-object-item.c (instance_dispose):
+	* src/core/na-object.c (na_object_object_unref):
+	Fix minor typo.
+
+	* src/core/na-object-item.c (na_object_item_unref_items):
+	Do not recursively unref objects here as this is done in dispose.
+
+	* src/nact/nact-tree-model.c (nact_tree_model_dispose):
+	Add a debug message.
+
 	Make the menus exportable.
 
 	* src/nact/nact-iactions-list.c:
diff --git a/src/core/na-object-id.c b/src/core/na-object-id.c
index daad211..9f5ca10 100644
--- a/src/core/na-object-id.c
+++ b/src/core/na-object-id.c
@@ -137,11 +137,18 @@ instance_init( GTypeInstance *instance, gpointer klass )
 	self->private = g_new0( NAObjectIdPrivate, 1 );
 }
 
+/*
+ * note that when the tree store is cleared, Gtk begins with the deepest
+ * levels, so that children are disposed before their parent
+ * as we try to dispose all children when disposing a parent, we have to
+ * remove a disposing child from its parent
+ */
 static void
 instance_dispose( GObject *object )
 {
 	static const gchar *thisfn = "na_object_id_instance_dispose";
 	NAObjectId *self;
+	NAObjectItem *parent;
 
 	g_debug( "%s: object=%p (%s)", thisfn, ( void * ) object, G_OBJECT_TYPE_NAME( object ));
 
@@ -153,6 +160,14 @@ instance_dispose( GObject *object )
 
 		self->private->dispose_has_run = TRUE;
 
+		parent = na_object_get_parent( object );
+		if( parent ){
+			na_object_remove_item( parent, object );
+			na_object_set_parent( object, NULL );
+		}
+
+		self->private->dispose_has_run = TRUE;
+
 		/* chain up to the parent class */
 		if( G_OBJECT_CLASS( st_parent_class )->dispose ){
 			G_OBJECT_CLASS( st_parent_class )->dispose( object );
diff --git a/src/core/na-object-item-factory.c b/src/core/na-object-item-factory.c
index 0d72f32..99dabd2 100644
--- a/src/core/na-object-item-factory.c
+++ b/src/core/na-object-item-factory.c
@@ -195,9 +195,9 @@ NADataDef data_def_item [] = {
 };
 
 static void
-free_items_list( void * list )
+free_items_list( void *list )
 {
-	static const gchar *thisfn = "na_object_item_enum_free_items_list";
+	static const gchar *thisfn = "na_object_item_factory_free_items_list";
 
 	g_debug( "%s: list=%p (count=%d)", thisfn, list, g_list_length(( GList * ) list ));
 
diff --git a/src/core/na-object-item.c b/src/core/na-object-item.c
index eab06fc..e1552c5 100644
--- a/src/core/na-object-item.c
+++ b/src/core/na-object-item.c
@@ -154,7 +154,7 @@ instance_dispose( GObject *object )
 {
 	static const gchar *thisfn = "na_object_item_instance_dispose";
 	NAObjectItem *self;
-	GList *items, *it;
+	GList *children, *it;
 
 	g_debug( "%s: object=%p (%s)", thisfn, ( void * ) object, G_OBJECT_TYPE_NAME( object ));
 
@@ -166,11 +166,11 @@ instance_dispose( GObject *object )
 
 		self->private->dispose_has_run = TRUE;
 
-		items = na_object_get_items( self );
-		for( it = items ; it ; it = it->next ){
+		children = na_object_get_items( self );
+		for( it = children ; it ; it = it->next ){
 			g_object_unref( it->data );
 		}
-		g_list_free( items );
+		g_list_free( children );
 		na_object_set_items( self, NULL );
 
 		/* chain up to the parent class */
@@ -554,16 +554,16 @@ na_object_item_insert_item( NAObjectItem *item, const NAObject *object, const NA
 void
 na_object_item_remove_item( NAObjectItem *item, const NAObjectId *object )
 {
-	GList *childs;
+	GList *children;
 
 	g_return_if_fail( NA_IS_OBJECT_ITEM( item ));
 	g_return_if_fail( NA_IS_OBJECT_ID( object ));
 
 	if( !item->private->dispose_has_run ){
 
-		childs = na_object_get_items( item );
-		childs = g_list_remove( childs, ( gconstpointer ) object );
-		na_object_set_items( item, childs );
+		children = na_object_get_items( item );
+		children = g_list_remove( children, ( gconstpointer ) object );
+		na_object_set_items( item, children );
 	}
 }
 
@@ -683,8 +683,8 @@ na_object_item_unref_items( GList *items )
 	GList *it;
 
 	for( it = items ; it ; it = it->next ){
-
-		na_object_unref( it->data );
+		/*na_object_unref( it->data );*/
+		g_object_unref( it->data );
 	}
 
 	g_list_free( items );
diff --git a/src/core/na-object.c b/src/core/na-object.c
index 0b1d082..9faae4e 100644
--- a/src/core/na-object.c
+++ b/src/core/na-object.c
@@ -660,7 +660,6 @@ na_object_object_unref( NAObject *object )
 	if( !object->private->dispose_has_run ){
 
 		if( NA_IS_OBJECT_ITEM( object )){
-
 			children = na_object_get_items( object );
 
 			for( ic = children ; ic ; ic = ic->next ){
diff --git a/src/nact/nact-tree-model.c b/src/nact/nact-tree-model.c
index 8530adf..74aebda 100644
--- a/src/nact/nact-tree-model.c
+++ b/src/nact/nact-tree-model.c
@@ -430,6 +430,8 @@ nact_tree_model_dispose( NactTreeModel *model )
 		ts_model = GTK_TREE_STORE( gtk_tree_model_filter_get_model( GTK_TREE_MODEL_FILTER( model )));
 
 		gtk_tree_store_clear( ts_model );
+
+		g_debug( "%s: end of tree store clear", thisfn );
 	}
 }
 
@@ -605,6 +607,8 @@ nact_tree_model_fill( NactTreeModel *model, GList *items, gboolean are_profiles_
 		for( it = items ; it ; it = it->next ){
 			duplicate = ( NAObject * ) na_object_duplicate( it->data );
 			fill_tree_store( ts_model, model->private->treeview, duplicate, are_profiles_displayed, NULL );
+			/*g_debug( "%s: before_na_object_unref: duplicate=%p (%s, ref_count=%d)", thisfn,
+					( void * ) duplicate, G_OBJECT_TYPE_NAME( duplicate ), G_OBJECT( duplicate )->ref_count );*/
 			na_object_unref( duplicate );
 		}
 	}
@@ -917,29 +921,25 @@ fill_tree_store( GtkTreeStore *model, GtkTreeView *treeview,
 	GList *subitems, *it;
 	GtkTreeIter iter;
 
-	g_debug( "%s: object=%p (%s, ref_count=%d)", thisfn,
+	g_debug( "%s entering: object=%p (%s, ref_count=%d)", thisfn,
 			( void * ) object, G_OBJECT_TYPE_NAME( object ), G_OBJECT( object )->ref_count );
 
-	if( NA_IS_OBJECT_MENU( object )){
+	/* an action or a menu
+	 */
+	if( NA_IS_OBJECT_ITEM( object )){
 		append_item( model, treeview, parent, &iter, object );
 		subitems = na_object_get_items( object );
 		for( it = subitems ; it ; it = it->next ){
 			fill_tree_store( model, treeview, it->data, are_profiles_displayed, &iter );
 		}
-	}
 
-	if( NA_IS_OBJECT_ACTION( object )){
-		g_return_if_fail( na_object_get_items_count( object ) >= 1 );
+	} else {
+		g_return_if_fail( NA_IS_OBJECT_PROFILE( object ));
 		append_item( model, treeview, parent, &iter, object );
-		subitems = na_object_get_items( object );
-		for( it = subitems ; it ; it = it->next ){
-			fill_tree_store( model, treeview, it->data, are_profiles_displayed, &iter );
-		}
 	}
 
-	if( NA_IS_OBJECT_PROFILE( object )){
-		append_item( model, treeview, parent, &iter, object );
-	}
+	/*g_debug( "%s quitting: object=%p (%s, ref_count=%d)", thisfn,
+			( void * ) object, G_OBJECT_TYPE_NAME( object ), G_OBJECT( object )->ref_count );*/
 }
 
 /*
@@ -959,33 +959,29 @@ filter_visible( GtkTreeModel *store, GtkTreeIter *iter, NactTreeModel *model )
 	/* is a GtkTreeStore */
 
 	gtk_tree_model_get( store, iter, IACTIONS_LIST_NAOBJECT_COLUMN, &object, -1 );
-	g_object_unref( object );
 
 	if( object ){
+		g_object_unref( object );
 		/*na_object_dump( object );*/
 
-		if( NA_IS_OBJECT_ACTION( object )){
-			return( TRUE );
-		}
-
-		if( NA_IS_OBJECT_MENU( object )){
+		/* an action or a menu
+		 */
+		if( NA_IS_OBJECT_ITEM( object )){
 			return( TRUE );
 		}
 
+		g_return_val_if_fail( NA_IS_OBJECT_PROFILE( object ), FALSE );
 		are_profiles_displayed = NACT_TREE_MODEL( model )->private->are_profiles_displayed;
 
-		if( NA_IS_OBJECT_PROFILE( object )){
-
-			if( !are_profiles_displayed ){
-				return( FALSE );
-			}
-
-			action = NA_OBJECT_ACTION( na_object_get_parent( object ));
-			count = na_object_get_items_count( action );
-			/*g_debug( "action=%p: count=%d", ( void * ) action, count );*/
-			/*return( TRUE );*/
-			return( count > 1 );
+		if( !are_profiles_displayed ){
+			return( FALSE );
 		}
+
+		action = NA_OBJECT_ACTION( na_object_get_parent( object ));
+		count = na_object_get_items_count( action );
+		/*g_debug( "action=%p: count=%d", ( void * ) action, count );*/
+		/*return( TRUE );*/
+		return( count > 1 );
 	}
 
 	return( FALSE );