file-browser: remove dummy child last

Previously, when the virtual root that has any number of non-dummy
children is removed from under gedit, depending on the order of calls
to `on_directory_monitor_event()`, a segmentation fault could occur in GTK.

The reason for this is helpfully noted by GTK:

  (gedit:71019): Gtk-CRITICAL **: 19:33:58.168: ../gtk/gtk/gtktreeview.c:6711
                 (validate_visible_area): assertion `has_next' failed.
  There is a disparity between the internal view of the GtkTreeView,
  and the GtkTreeModel.  This generally means that the model has changed
  without letting the view know.  Any display from now on is likely to
  be incorrect.

Due to this disparity `gedit_file_browser_store_get_value()` cannot
succeed at a later point, effectively returning an invalid GValue to
GTK, causing the crash in the end.

That disparity is caused by the fact that when a non-dummy node
is removed, then `model_check_dummy()` is called on its parent
(if there is one). Now, when `model_remove_node_children()` is
removing the children of the virtual root, its first child
will be a dummy node, and that is removed without a problem.
However, the removal of the second child (the first non-dummy one)
will result in a call to `model_check_dummy()`, which will create
a new dummy node because the "original" dummy node has *just*
been removed in the previous iteration. In the last iteration
the `model_check_dummy()` call will see that there are no other
children of the parent, and hence will insert the dummy node
into the GtkTreeModel. And this extra insertion causes the
disparity between GTK's and gedit's state.

Fix that by removing dummy child last. By not removing the dummy
child first, it is guaranteed that `model_check_dummy()` will
*not* create a new dummy node - although it will still insert it
into the GtkTreeModel when removing the last non-dummy, but that
is not an issue since the `model_remove_node()` takes care of
removing the dummy node from the model shortly. It also guarantees
that all children of a node will be removed after a call to
`model_remove_node_children()`, and this makes it possible
to simplify `model_clear()` as it no longer needs to concern
itself with the leftover dummy node.

(Hopefully) fixes #356

1 files changed, 12 insertions, 20 deletions

diff --git a/plugins/filebrowser/gedit-file-browser-store.c b/plugins/filebrowser/gedit-file-browser-store.c
index abb0f7d15..b4050724e 100644
--- a/plugins/filebrowser/gedit-file-browser-store.c
+++ b/plugins/filebrowser/gedit-file-browser-store.c
@@ -1449,6 +1449,7 @@ model_remove_node_children (GeditFileBrowserStore *model,
 {
 	FileBrowserNodeDir *dir;
 	GtkTreePath *path_child;
+	FileBrowserNode *child;
 	GSList *list;
 
 	if (node == NULL || !NODE_IS_DIR (node))
@@ -1477,10 +1478,19 @@ model_remove_node_children (GeditFileBrowserStore *model,
 
 	list = g_slist_copy (dir->children);
 
-	for (GSList *item = list; item; item = item->next)
-		model_remove_node (model, (FileBrowserNode *)(item->data), path_child, free_nodes);
+	for (GSList *item = g_slist_next (list); item; item = item->next)
+	{
+		child = item->data;
+		g_assert (!NODE_IS_DUMMY (child));
+		model_remove_node (model, child, path_child, free_nodes);
+	}
 
 	g_slist_free (list);
+
+	child = dir->children->data;
+	g_assert (NODE_IS_DUMMY (child));
+	model_remove_node (model, child, path_child, free_nodes);
+
 	gtk_tree_path_free (path_child);
 }
 
@@ -1555,24 +1565,6 @@ model_clear (GeditFileBrowserStore *model,
 
 	model_remove_node_children (model, model->priv->virtual_root, path, free_nodes);
 	gtk_tree_path_free (path);
-
-	/* Remove the dummy if there is one */
-	if (model->priv->virtual_root)
-	{
-		FileBrowserNodeDir *dir = FILE_BROWSER_NODE_DIR (model->priv->virtual_root);
-
-		if (dir->children != NULL)
-		{
-			FileBrowserNode *dummy = (FileBrowserNode *)(dir->children->data);
-
-			if (NODE_IS_DUMMY (dummy) && model_node_visibility (model, dummy))
-			{
-				path = gtk_tree_path_new_first ();
-				row_deleted (model, dummy, path);
-				gtk_tree_path_free (path);
-			}
-		}
-	}
 }
 
 static void