[gtksourceview/wip/chergert/vim] invert parent/child state ownership

[Christian Hergert <chergert src gnome org>]

commit fbb47dd717daf41ed0162b8db68b49c869a9052b
Author: Christian Hergert <chergert redhat com>
Date:   Fri Nov 5 16:00:31 2021 -0700

    invert parent/child state ownership
    
    the parent state now owns the child state. this still needs work to
    unparent the states instead of just g_clear_object().
    
    this is necessary so we don't leak, always go parent->child.

 gtksourceview/vim/gtk-source-vim-state.c | 185 +++++++++++++++++++++----------
 gtksourceview/vim/gtk-source-vim-state.h |   1 +
 2 files changed, 129 insertions(+), 57 deletions(-)
---
diff --git a/gtksourceview/vim/gtk-source-vim-state.c b/gtksourceview/vim/gtk-source-vim-state.c
index 07ba0263..14f86c9c 100644
--- a/gtksourceview/vim/gtk-source-vim-state.c
+++ b/gtksourceview/vim/gtk-source-vim-state.c
@@ -32,18 +32,45 @@
 
 typedef struct
 {
+       /* Owned reference to registers (usually set low in the stack) */
        GtkSourceVimState *registers;
+
+       /* Owned reference to the view (usually set low in the stack) */
+       GtkSourceView *view;
+
+       /* The name of the register set with "<name> */
+       const char *current_register;
+
+       /* Unowned parent pointer. The parent owns a reference to this
+        * instance of GtkSourceVimState.
+        */
        GtkSourceVimState *parent;
+
+       /* Unowned pointer to a child that has been pushed onto our
+        * stack of states. @child must exist within @children.
+        */
        GtkSourceVimState *child;
-       GtkSourceView     *view;
-       const char        *current_register;
 
-       int                count;
-       guint              column;
+       /* A queue of all our children, using @link of the children nodes
+        * to insert into the queue without extra allocations.
+        */
+       GQueue children;
+
+       /* The GList to be inserted into @children of the parent. */
+       GList link;
 
-       guint              count_set : 1;
-       guint              can_repeat : 1;
-       guint              column_set : 1;
+       /* A count if one has been associated with the state object */
+       int count;
+
+       /* The column we last were on. Usually this is just set by the
+        * Normal state but could also be set in others (like Visual).
+        */
+       guint column;
+
+       /* Various flags */
+       guint count_set : 1;
+       guint can_repeat : 1;
+       guint column_set : 1;
 } GtkSourceVimStatePrivate;
 
 G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE (GtkSourceVimState, gtk_source_vim_state, G_TYPE_OBJECT)
@@ -235,35 +262,28 @@ gtk_source_vim_state_dispose (GObject *object)
        GtkSourceVimState *self = (GtkSourceVimState *)object;
        GtkSourceVimStatePrivate *priv = gtk_source_vim_state_get_instance_private (self);
 
-       /* Cleanup children first so they can access the ancestors during
-        * destruction if necessary.
-        */
-       if (priv->child != NULL)
+       priv->current_register = NULL;
+
+       g_clear_weak_pointer (&priv->view);
+       g_clear_object (&priv->registers);
+
+       /* First remove the children from our list */
+       while (priv->children.length > 0)
        {
-               GtkSourceVimState *child = g_steal_pointer (&priv->child);
-               gtk_source_vim_state_set_parent (child, NULL);
+               GtkSourceVimState *child = g_queue_peek_head (&priv->children);
+               gtk_source_vim_state_unparent (child);
        }
 
-       /* This is extra protection to ensure that we don't have a dangling
-        * child pointer in the parent state. Generally, if
-        * gtk_source_vim_state_pop() is called properly this cannot happen
-        * as it will clear the child pointer. At that point only some
-        * states will choose to hold on to the child (such as Normal mode
-        * to allow for repeating the last command).
-        */
+       /* Now make sure we're unlinked from our parent */
        if (priv->parent != NULL)
        {
-               GtkSourceVimStatePrivate *parent_priv = gtk_source_vim_state_get_instance_private 
(priv->parent);
-
-               if (parent_priv->child == self)
-               {
-                       parent_priv->child = NULL;
-               }
+               gtk_source_vim_state_unparent (self);
        }
 
-       g_clear_weak_pointer (&priv->view);
-       g_clear_object (&priv->parent);
-       g_clear_object (&priv->registers);
+       g_assert (priv->parent == NULL);
+       g_assert (priv->children.length == 0);
+       g_assert (priv->children.head == NULL);
+       g_assert (priv->children.tail == NULL);
 
        G_OBJECT_CLASS (gtk_source_vim_state_parent_class)->dispose (object);
 }
@@ -355,6 +375,7 @@ gtk_source_vim_state_init (GtkSourceVimState *self)
 {
        GtkSourceVimStatePrivate *priv = gtk_source_vim_state_get_instance_private (self);
 
+       priv->link.data = self;
        priv->count = 1;
 }
 
@@ -491,6 +512,17 @@ gtk_source_vim_state_handle_event (GtkSourceVimState *self,
        return FALSE;
 }
 
+/**
+ * gtk_source_vim_state_push:
+ * @self: a #GtkSourceVimState
+ * @new_state: (transfer full): the new child state for @self
+ *
+ * Pushes @new_state as the current child of @self.
+ *
+ * This steals a reference from @new_state to simplify use.
+ * Remember to g_object_ref(new_state) if you need to keep
+ * a reference.
+ */
 void
 gtk_source_vim_state_push (GtkSourceVimState *self,
                            GtkSourceVimState *new_state)
@@ -504,23 +536,13 @@ gtk_source_vim_state_push (GtkSourceVimState *self,
        if (priv->child != NULL)
        {
                g_warning ("Attempt to push state %s onto %s when it already has a %s",
-                          G_OBJECT_TYPE_NAME (new_state),
-                          G_OBJECT_TYPE_NAME (self),
-                          G_OBJECT_TYPE_NAME (priv->child));
+                          G_OBJECT_TYPE_NAME (new_state),
+                          G_OBJECT_TYPE_NAME (self),
+                          G_OBJECT_TYPE_NAME (priv->child));
        }
 
-       /* Associate the state with our view and set @self as parent to
-        * hold the ref back to @self. We will not hold a reference to
-        * @new_state to avoid a circular reference.
-        */
-       g_object_set (new_state,
-                     "parent", self,
-                     "view", priv->view,
-                     NULL);
+       gtk_source_vim_state_set_parent (new_state, self);
 
-       /* Keep a pointer to the child. When the child is disposed, it will
-        * clear our priv->child field if it still matches @new_state.
-        */
        priv->child = new_state;
 
        if (GTK_SOURCE_VIM_STATE_GET_CLASS (self)->suspend)
@@ -532,6 +554,8 @@ gtk_source_vim_state_push (GtkSourceVimState *self,
        {
                GTK_SOURCE_VIM_STATE_GET_CLASS (new_state)->enter (new_state);
        }
+
+       g_object_unref (new_state);
 }
 
 void
@@ -548,18 +572,20 @@ gtk_source_vim_state_pop (GtkSourceVimState *self)
        parent = g_object_ref (priv->parent);
        parent_priv = gtk_source_vim_state_get_instance_private (parent);
 
-       if (GTK_SOURCE_VIM_STATE_GET_CLASS (self)->leave)
+       if (parent_priv->child == self)
        {
-               GTK_SOURCE_VIM_STATE_GET_CLASS (self)->leave (self);
+               parent_priv->child = NULL;
+       }
+       else
+       {
+               g_warning ("Attempt to pop state %s from %s but it is not current",
+                          G_OBJECT_TYPE_NAME (self),
+                          G_OBJECT_TYPE_NAME (parent));
        }
 
-       /* Clear the parent's child pointer, since we are no longer the
-        * active event delivery child. However, the parent can still keep
-        * a reference around elsewhere if they like for replaying things.
-        */
-       if (parent_priv->child == self)
+       if (GTK_SOURCE_VIM_STATE_GET_CLASS (self)->leave)
        {
-               parent_priv->child = NULL;
+               GTK_SOURCE_VIM_STATE_GET_CLASS (self)->leave (self);
        }
 
        if (GTK_SOURCE_VIM_STATE_GET_CLASS (parent)->resume)
@@ -568,7 +594,6 @@ gtk_source_vim_state_pop (GtkSourceVimState *self)
        }
 
        g_clear_object (&parent);
-       g_object_unref (self);
 }
 
 void
@@ -899,23 +924,69 @@ gtk_source_vim_state_set_count (GtkSourceVimState *self,
        priv->count_set = count != 0;
 }
 
+void
+gtk_source_vim_state_unparent (GtkSourceVimState *self)
+{
+       GtkSourceVimStatePrivate *priv = gtk_source_vim_state_get_instance_private (self);
+       GtkSourceVimStatePrivate *parent_priv;
+
+       g_return_if_fail (GTK_SOURCE_IS_VIM_STATE (self));
+       g_return_if_fail (priv->link.data == self);
+
+       if (priv->parent == NULL)
+       {
+               return;
+       }
+
+       parent_priv = gtk_source_vim_state_get_instance_private (priv->parent);
+       priv->parent = NULL;
+
+       if (parent_priv->child == self)
+       {
+               parent_priv->child = NULL;
+       }
+
+       g_queue_unlink (&parent_priv->children, &priv->link);
+
+       g_object_unref (self);
+}
+
 void
 gtk_source_vim_state_set_parent (GtkSourceVimState *self,
                                  GtkSourceVimState *parent)
 {
        GtkSourceVimStatePrivate *priv = gtk_source_vim_state_get_instance_private (self);
-       GtkSourceVimStatePrivate *parent_priv = gtk_source_vim_state_get_instance_private (parent);
+       GtkSourceVimStatePrivate *parent_priv;
 
        g_return_if_fail (GTK_SOURCE_IS_VIM_STATE (self));
        g_return_if_fail (!parent || GTK_SOURCE_IS_VIM_STATE (parent));
 
-       if (parent != NULL && parent_priv->child == self)
-               parent_priv->child = NULL;
+       if (priv->parent == parent)
+               return;
 
-       if (g_set_object (&priv->parent, parent))
+       g_object_ref (self);
+
+       if (priv->parent != NULL)
        {
-               g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_PARENT]);
+               gtk_source_vim_state_unparent (self);
        }
+
+       g_assert (priv->parent == NULL);
+       g_assert (priv->link.data == self);
+       g_assert (priv->link.next == NULL);
+       g_assert (priv->link.prev == NULL);
+
+       if (parent != NULL)
+       {
+               priv->parent = parent;
+               parent_priv = gtk_source_vim_state_get_instance_private (parent);
+               g_queue_push_tail_link (&parent_priv->children, &priv->link);
+               g_object_ref (self);
+       }
+
+       g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_PARENT]);
+
+       g_object_unref (self);
 }
 
 gboolean
diff --git a/gtksourceview/vim/gtk-source-vim-state.h b/gtksourceview/vim/gtk-source-vim-state.h
index 3332ff2c..73fa452b 100644
--- a/gtksourceview/vim/gtk-source-vim-state.h
+++ b/gtksourceview/vim/gtk-source-vim-state.h
@@ -62,6 +62,7 @@ struct _GtkSourceVimStateClass
 gboolean           gtk_source_vim_state_get_editable               (GtkSourceVimState *self);
 void               gtk_source_vim_state_set_parent                 (GtkSourceVimState *self,
                                                                     GtkSourceVimState *parent);
+void               gtk_source_vim_state_unparent                   (GtkSourceVimState *self);
 void               gtk_source_vim_state_push                       (GtkSourceVimState *self,
                                                                     GtkSourceVimState *new_state);
 void               gtk_source_vim_state_pop                        (GtkSourceVimState *self);