The main breakage is that reference counting is based on current_path which is a GtkTreePath. This is not a good idea because 'stored' TreePaths will become inaccurate or invalid when nodes around that one change. For example, a node with path "3" will become "2" if a node above it goes away, but the current code doesn't account for that. I can easily reproduce this crash due to this bug: Gtk-CRITICAL **: gtk_tree_model_get_iter: assertion `path != NULL' failed #0 0x00002b08800f3839 in raise () from /lib/tls/libc.so.6 #1 0x00002b08800f4cde in abort () from /lib/tls/libc.so.6 #2 0x00002b087f998825 in g_logv () from /usr/lib/libglib-2.0.so.0 #3 0x00002b087f9988b3 in g_log () from /usr/lib/libglib-2.0.so.0 #4 0x00002b087def1caf in gtk_tree_model_get_iter () from /usr/lib/libgtk-x11-2.0.so.0 #5 0x000000000042215f in navigation_model_path_deref (model=0x631c80, path=) at navigation-tree.c:1461 #6 0x0000000000424f60 in navigation_selection_changed ( treeselection=0x631d00, user_data=) at navigation-tree.c:1071 #7 0x00002b087f7446e9 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #8 0x00002b087f753836 in g_signal_has_handler_pending () from /usr/lib/libgobject-2.0.so.0 #9 0x00002b087f754a30 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #10 0x00002b087f754cf3 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #11 0x00002b087defc1da in gtk_tree_selection_select_path () from /usr/lib/libgtk-x11-2.0.so.0 #12 0x00002b087defc2bd in gtk_tree_selection_select_iter () from /usr/lib/libgtk-x11-2.0.so.0 #13 0x00000000004235dc in navigation_tree_select_session (navtree=0x65a9e0, sess=) at navigation-tree.c:490 #14 0x0000000000424010 in navigation_tree_create_new_network_entry ( navtree=0x65a9e0, sess=0x8cf300) at navigation-tree.c:239 Additionally, the current code seems to forget that calling gtk_tree_selection_select_* will cause the 'changed' event to be fired (you can see that in the above trace). The functions calling a select function do *not* need to do book-keeping on current_path, since the selection callback handles that. Index: src/fe-gnome/navigation-tree.h =================================================================== --- src/fe-gnome/navigation-tree.h (revision 10171) +++ src/fe-gnome/navigation-tree.h (working copy) @@ -46,10 +46,10 @@ NavModel *model; - /* current_path stores a GtkTreePath to the most recently selected + /* current_rowref stores a GtkTreeRowReference to the most recently selected * item in the NavTree. */ - GtkTreePath *current_path; + GtkTreeRowReference *current_rowref; /* We need the handler id for the selection_changed call back so that we * can block it sometimes. @@ -136,8 +136,8 @@ /* Ref counting for the selected items in the model. */ /* Find the row by path. */ -void navigation_model_path_ref (NavModel *model, GtkTreePath *path); -void navigation_model_path_deref (NavModel *model, GtkTreePath *path); +void navigation_model_rowref_ref (GtkTreeRowReference *rowref); +void navigation_model_rowref_deref (GtkTreeRowReference *rowref); /* Find the row by iter. XXX: The iter becomes invalid after a call to this * function, make sure if you're going to use it again that you re-get the Index: src/fe-gnome/navigation-tree.c =================================================================== --- src/fe-gnome/navigation-tree.c (revision 10171) +++ src/fe-gnome/navigation-tree.c (working copy) @@ -143,7 +143,7 @@ GtkTreeSelection *select; g_object_set ((gpointer) navtree, "headers-visible", FALSE, NULL); - navtree->current_path = NULL; + navtree->current_rowref = NULL; navtree->model = NULL; navtree->selection_changed_id = 0; @@ -206,8 +206,8 @@ navigation_tree_finalize (GObject *object) { NavTree *navtree = (NavTree *) object; - gtk_tree_path_free (navtree->current_path); - navtree->current_path = NULL; + gtk_tree_row_reference_free (navtree->current_rowref); + navtree->current_rowref = NULL; } /* New NavTree. */ @@ -230,7 +230,6 @@ void navigation_tree_create_new_network_entry (NavTree *navtree, struct session *sess) { - GtkTreeIter *iter; GtkWidget *menuitem, *button; navigation_model_add_new_network (navtree->model, sess); @@ -238,18 +237,8 @@ /* Select the new network. */ navigation_tree_select_session (navtree, sess); - /* Because we added a network it is likely that the path to the current session has changed - * so we update it. - */ - iter = navigation_model_get_sorted_iter (navtree->model, gui.current_session); - if (iter) { - if (navtree->current_path) - gtk_tree_path_free (navtree->current_path); + /* We rely on the above select_session call triggering the selection 'changed' event to do book-keeping */ - navtree->current_path = gtk_tree_model_get_path (GTK_TREE_MODEL (navtree->model->sorted), iter); - gtk_tree_iter_free (iter); - } - /* Our row references to the last channel and server should also be * updated. */ @@ -269,7 +258,6 @@ void navigation_tree_create_new_channel_entry (NavTree *navtree, struct session *sess) { - GtkTreeIter *iter; GtkWidget *menuitem, *button; GtkTreeView *treeview; ircnet *net; @@ -278,21 +266,8 @@ navigation_tree_select_session (navtree, sess); - /* Because we're adding a new channel it's possible that the path to the current - * session has changed, so we'll update it. - * XXX: We could probably add some kind of test to only update current_path - * if it's truly necessary (e.g. the new channel is on the same network as - * the old one), but do we need to bother? - */ - iter = navigation_model_get_sorted_iter (navtree->model, gui.current_session); - if (iter) { - if (navtree->current_path) - gtk_tree_path_free (navtree->current_path); + /* We rely on the above select_session call triggering the selection 'changed' event to do book-keeping */ - navtree->current_path = gtk_tree_model_get_path (GTK_TREE_MODEL (navtree->model->sorted), iter); - gtk_tree_iter_free (iter); - } - /* Update the row refs here. */ navigation_tree_update_refs (navtree); @@ -320,7 +295,7 @@ void navigation_tree_remove_channel (NavTree *navtree, struct session *sess) { - GtkTreePath *path = gtk_tree_path_copy (navtree->current_path); + GtkTreePath *path = gtk_tree_row_reference_get_path (navtree->current_rowref); GtkTreeSelection *select = gtk_tree_view_get_selection (GTK_TREE_VIEW (navtree)); GtkTreeIter iter; GtkTreeModel *sorted; @@ -343,13 +318,7 @@ gtk_tree_selection_select_path (select, path); navigation_model_remove (navtree->model, sess); - /* Make sure that the navtree's current_path is valid. If it selected the next channel - * in the list because there was no previous channel, current_path will be one item - * too high. In all other cases this is extraneous. - */ - gtk_tree_path_free (navtree->current_path); - gtk_tree_selection_get_selected (select, &sorted, &iter); - navtree->current_path = gtk_tree_model_get_path (sorted, &iter); + /* We rely on the above select_path call triggering the selection 'changed' event to do book-keeping */ navigation_tree_update_refs (navtree); @@ -359,7 +328,7 @@ void navigation_tree_remove_server (NavTree *navtree, struct session *sess) { - GtkTreePath *path = gtk_tree_path_copy (navtree->current_path); + GtkTreePath *path = gtk_tree_row_reference_get_path (navtree->current_rowref); GtkTreeSelection *select = gtk_tree_view_get_selection (GTK_TREE_VIEW (navtree)); if (!gtk_tree_path_prev (path)) { @@ -374,11 +343,7 @@ gtk_tree_selection_select_iter (select, &iter); navigation_model_remove (navtree->model, sess); - /* After removing the first server the navtree's current_path - * is invalid so we set it to the root of the tree. - */ - gtk_tree_path_free (navtree->current_path); - navtree->current_path = gtk_tree_path_new_first (); + /* We rely on the above select_iter call triggering the selection 'changed' event to do book-keeping */ } else { /* The first server is the only server. */ GtkTreeModel *store = gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT (sorted)); @@ -709,12 +674,12 @@ /* Try to get the current selection. */ if (!gtk_tree_selection_get_selected (selection, &model, &iter)) { model = gtk_tree_view_get_model (GTK_TREE_VIEW (navtree)); - if (navtree->current_path) { - /* If nothing is selected and we have a current_path set the iter to that path. */ - path = gtk_tree_path_copy (navtree->current_path); + if (navtree->current_rowref) { + /* If nothing is selected and we have a current_rowref set the iter to that path. */ + path = gtk_tree_row_reference_get_path (navtree->current_rowref); gtk_tree_model_get_iter (model, &iter, path); } else { - /* If we have no current_path and nothing selected select the first server and return. */ + /* If we have no current_rowref and nothing selected select the first server and return. */ gtk_tree_model_get_iter_first (model, &iter); gtk_tree_selection_select_iter (selection, &iter); return; @@ -730,6 +695,9 @@ gtk_tree_model_iter_parent (model, &parent, &iter); iter = parent; } + + gtk_tree_path_free(path); + if (!gtk_tree_model_iter_next (model, &iter)) { /* If we can't move to the next we need to move back to the root. Move iter to the root. */ GtkTreeIter root; @@ -756,9 +724,9 @@ /* If there is nothing selected in the GtkTreeSelection... */ /* Get the model. */ model = gtk_tree_view_get_model (GTK_TREE_VIEW (navtree)); - if (navtree->current_path) { - /* If there's a current_path set path to that. */ - path = gtk_tree_path_copy (navtree->current_path); + if (navtree->current_rowref) { + /* If there's a current_rowref set path to that. */ + path = gtk_tree_row_reference_get_path (navtree->current_rowref); } else { /* Otherwise set path to the root. */ path = gtk_tree_path_new_from_string ("0"); @@ -787,6 +755,7 @@ /* Select the path in our GtkTreeSelection. */ gtk_tree_selection_select_path (selection, path); + gtk_tree_path_free(path); } static session* @@ -1031,8 +1000,10 @@ gtk_tree_path_free (path); } - model = GTK_TREE_MODEL (NAVTREE (treeview)->model->sorted); - gtk_tree_model_get_iter (model, &iter, NAVTREE (treeview)->current_path); + model = gtk_tree_row_reference_get_model (NAVTREE (treeview)->current_rowref); + path = gtk_tree_row_reference_get_path (NAVTREE (treeview)->current_rowref); + gtk_tree_model_get_iter (model, &iter, path); + gtk_tree_path_free(path); gtk_tree_model_get (model, &iter, 2, &s, -1); if (s != NULL) navigation_context (treeview, s); /* FIXME */ @@ -1067,8 +1038,8 @@ treeview = GTK_TREE_VIEW (glade_xml_get_widget (gui.xml, "userlist")); - if (gui.server_tree->current_path != NULL) - navigation_model_path_deref (gui.server_tree->model, gui.server_tree->current_path); + if (gui.server_tree->current_rowref != NULL) + navigation_model_rowref_deref (gui.server_tree->current_rowref); /* If find bar is open, hide it */ find_bar_close (FIND_BAR (gui.find_bar)); @@ -1078,14 +1049,17 @@ */ if (gtk_tree_selection_get_selected (treeselection, &model, &iter) && gui.current_session) { GtkWidget *menuitem; + GtkTreePath *path; - /* Update current_path. */ - if (gui.server_tree->current_path) { - gtk_tree_path_free (gui.server_tree->current_path); + path = gtk_tree_model_get_path (model, &iter); + + /* Update current_rowref. */ + if (gui.server_tree->current_rowref) { + gtk_tree_row_reference_free (gui.server_tree->current_rowref); } - gui.server_tree->current_path = gtk_tree_model_get_path (model, &iter); - navigation_model_path_ref (gui.tree_model, gui.server_tree->current_path); + gui.server_tree->current_rowref = gtk_tree_row_reference_new(model, path); + navigation_model_rowref_ref (gui.server_tree->current_rowref); gtk_tree_selection_get_selected (treeselection, &model, &iter); /* Get the session for the new selection. */ @@ -1154,16 +1128,21 @@ static void row_expanded (GtkTreeView *treeview, GtkTreeIter *iter, GtkTreePath *path, gpointer user_data) { + GtkTreePath *current_path; + + current_path = gtk_tree_row_reference_get_path (NAVTREE (treeview)->current_rowref); + /* If we had something selected before the row was collapsed make sure it gets selected. */ - if (NAVTREE (treeview)->current_path && gtk_tree_path_is_ancestor (path, NAVTREE (treeview)->current_path)) { + if (current_path && gtk_tree_path_is_ancestor (path, current_path)) { GtkTreeSelection *selection; selection = gtk_tree_view_get_selection (treeview); g_signal_handler_block ((gpointer) selection, NAVTREE (treeview)->selection_changed_id); - gtk_tree_selection_select_path (selection, NAVTREE (treeview)->current_path); + gtk_tree_selection_select_path (selection, current_path); g_signal_handler_unblock ((gpointer) selection, NAVTREE (treeview)->selection_changed_id); } navigation_tree_update_refs (NAVTREE (treeview)); + gtk_tree_path_free(current_path); } static void @@ -1438,36 +1417,52 @@ } void -navigation_model_path_ref (NavModel * model, GtkTreePath * path) +navigation_model_rowref_ref (GtkTreeRowReference * rowref) { gint ref_count; - GtkTreeIter iter; - GtkTreePath *unsorted = gtk_tree_model_sort_convert_path_to_child_path (GTK_TREE_MODEL_SORT (model->sorted), path); + GtkTreeIter iter, childiter; + GtkTreeModel *model, *childmodel; + GtkTreePath *path; - gtk_tree_model_get_iter (GTK_TREE_MODEL (model->store), &iter, unsorted); - gtk_tree_model_get (GTK_TREE_MODEL (model->store), &iter, 5, &ref_count, -1); - gtk_tree_store_set (model->store, &iter, 5, ref_count + 1, -1); + model = gtk_tree_row_reference_get_model (rowref); + path = gtk_tree_row_reference_get_path (rowref); + childmodel = gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT (model)); - gtk_tree_path_free (unsorted); + gtk_tree_model_get_iter (model, &iter, path); + gtk_tree_model_get (model, &iter, 5, &ref_count, -1); + gtk_tree_model_sort_convert_iter_to_child_iter (GTK_TREE_MODEL_SORT (model), &childiter, &iter); + gtk_tree_store_set (GTK_TREE_STORE (childmodel), &childiter, 5, ref_count + 1, -1); + gtk_tree_path_free (path); } void -navigation_model_path_deref (NavModel * model, GtkTreePath * path) +navigation_model_rowref_deref (GtkTreeRowReference *rowref) { gint ref_count; GtkTreeIter iter; - GtkTreePath *unsorted = gtk_tree_model_sort_convert_path_to_child_path (GTK_TREE_MODEL_SORT (model->sorted), path); + GtkTreeModel *model; + GtkTreePath *path; - if (gtk_tree_model_get_iter (GTK_TREE_MODEL (model->store), &iter, unsorted) == FALSE) { - g_critical ("path is invalid in navigation_model_path_deref\n"); + model = gtk_tree_row_reference_get_model (rowref); + path = gtk_tree_row_reference_get_path (rowref); + + if (gtk_tree_model_get_iter (model, &iter, path) == FALSE) { + g_critical ("path is invalid in navigation_model_rowref_deref\n"); return; } - gtk_tree_model_get (GTK_TREE_MODEL (model->store), &iter, 5, &ref_count, -1); - if (ref_count > 0) - gtk_tree_store_set (model->store, &iter, 5, ref_count - 1, -1); + gtk_tree_model_get (model, &iter, 5, &ref_count, -1); - gtk_tree_path_free (unsorted); + if (ref_count > 0) { + GtkTreeModel *childmodel; + GtkTreeIter childiter; + + childmodel = gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT (model)); + gtk_tree_model_sort_convert_iter_to_child_iter (GTK_TREE_MODEL_SORT (model), &childiter, &iter); + gtk_tree_store_set (GTK_TREE_STORE (childmodel), &childiter, 5, ref_count - 1, -1); + } + + gtk_tree_path_free (path); } void Index: ChangeLog =================================================================== --- ChangeLog (revision 10171) +++ ChangeLog (working copy) @@ -1,3 +1,10 @@ +Tue Feb 21 23:27:00 GMT 2006 Daniel Drake + + * src/fe-gnome/navigation-tree.h: + * src/fe-gnome/navigation-tree.c: + - Convert current_path into current_rowref which should fix reference + counting. (# + Mon Feb 20 17:49:38 MST 2006 David Trowbridge * plugins/notification/notification.c: