[gedit] file-browser: remove dummy child last

[Jeremy Bicha <jbicha src gnome org>]

commit b626a0cc0ed74a080f75e7bb3823ec86f7494fa5
Author: Barnabás Pőcze <pobrn protonmail com>
Date:   Wed Jul 28 19:01:24 2021 +0200

    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

 plugins/filebrowser/gedit-file-browser-store.c | 32 ++++++++++----------------
 1 file 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