[gtk+/wip/quartzwork: 2/9] Fix GtkApplicationWindow action group implementation

[Ryan Lortie <desrt src gnome org>]

commit 130ff2a3fcab77f88dd28855c2a0045258d2015a
Author: Ryan Lortie <desrt desrt ca>
Date:   Mon Dec 16 12:25:54 2013 -0500

    Fix GtkApplicationWindow action group implementation
    
    GtkApplicationWindow frees its internal action group on dispose for the
    usual reasons: to avoid the possibility of reference cycles caused by
    actions referring back to the window again.
    
    Unfortunately, if it happens to be inside of a GtkActionMuxer at the
    time that it is disposed, it will (eventually) be removed from the muxer
    after it has been disposed.  Removing an action group from a muxer
    involves a call to g_action_group_list_actions() which will crash
    because the internal action group to which we normally delegate the call
    has been freed.
    
    A future patch that reworks the quartz menu code will introduce a use of
    GtkActionMuxer in a way that causes exactly this problem.
    
    We can guard against the problem in a number of ways.
    
    First, we can avoid the entire situation by ensuring that we are removed
    from the muxer before we destroy the action group.  To this end, we
    delay destruction of the action group until after the chain-up to the
    dispose of GtkWindow (which is where the window is removed from the
    GtkApplication).
    
    Secondly, we can add checks to each of our GActionGroup and GActionMap
    implementation functions to check that the internal action group is
    still alive before we attempt to delegate to it.
    
    We have to be careful, though: because our _list_actions() call will
    suddenly be returning an empty list, people watching the group from
    outside will have expected to see "action-removed" calls for the
    now-missing items.  Make sure we send those. but only if someone is
    watching.

 gtk/gtkapplicationwindow.c |   77 ++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 74 insertions(+), 3 deletions(-)
---
diff --git a/gtk/gtkapplicationwindow.c b/gtk/gtkapplicationwindow.c
index 5cbe71e..df337c0 100644
--- a/gtk/gtkapplicationwindow.c
+++ b/gtk/gtkapplicationwindow.c
@@ -389,6 +389,10 @@ gtk_application_window_list_actions (GActionGroup *group)
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (group);
 
+  /* may be NULL after dispose has run */
+  if (!window->priv->actions)
+    return g_new0 (char *, 0 + 1);
+
   return g_action_group_list_actions (G_ACTION_GROUP (window->priv->actions));
 }
 
@@ -403,6 +407,9 @@ gtk_application_window_query_action (GActionGroup        *group,
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (group);
 
+  if (!window->priv->actions)
+    return FALSE;
+
   return g_action_group_query_action (G_ACTION_GROUP (window->priv->actions),
                                       action_name, enabled, parameter_type, state_type, state_hint, state);
 }
@@ -414,7 +421,10 @@ gtk_application_window_activate_action (GActionGroup *group,
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (group);
 
-  return g_action_group_activate_action (G_ACTION_GROUP (window->priv->actions), action_name, parameter);
+  if (!window->priv->actions)
+    return;
+
+  g_action_group_activate_action (G_ACTION_GROUP (window->priv->actions), action_name, parameter);
 }
 
 static void
@@ -424,7 +434,10 @@ gtk_application_window_change_action_state (GActionGroup *group,
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (group);
 
-  return g_action_group_change_action_state (G_ACTION_GROUP (window->priv->actions), action_name, state);
+  if (!window->priv->actions)
+    return;
+
+  g_action_group_change_action_state (G_ACTION_GROUP (window->priv->actions), action_name, state);
 }
 
 static GAction *
@@ -433,6 +446,9 @@ gtk_application_window_lookup_action (GActionMap  *action_map,
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (action_map);
 
+  if (!window->priv->actions)
+    return NULL;
+
   return g_action_map_lookup_action (G_ACTION_MAP (window->priv->actions), action_name);
 }
 
@@ -442,6 +458,9 @@ gtk_application_window_add_action (GActionMap *action_map,
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (action_map);
 
+  if (!window->priv->actions)
+    return;
+
   g_action_map_add_action (G_ACTION_MAP (window->priv->actions), action);
 }
 
@@ -451,6 +470,9 @@ gtk_application_window_remove_action (GActionMap  *action_map,
 {
   GtkApplicationWindow *window = GTK_APPLICATION_WINDOW (action_map);
 
+  if (!window->priv->actions)
+    return;
+
   g_action_map_remove_action (G_ACTION_MAP (window->priv->actions), action_name);
 }
 
@@ -740,10 +762,59 @@ gtk_application_window_dispose (GObject *object)
 
   g_clear_object (&window->priv->app_menu_section);
   g_clear_object (&window->priv->menubar_section);
-  g_clear_object (&window->priv->actions);
 
   G_OBJECT_CLASS (gtk_application_window_parent_class)
     ->dispose (object);
+
+  /* We do this below the chain-up above to give us a chance to be
+   * removed from the GtkApplication (which is done in the dispose
+   * handler of GtkWindow).
+   *
+   * That reduces our chances of being watched as a GActionGroup from a
+   * muxer constructed by GtkApplication.  Even still, it's
+   * theoretically possible that someone else could be watching us.
+   * Therefore, we have to take care to ensure that we don't violate our
+   * obligations under the interface of GActionGroup.
+   *
+   * The easiest thing is just for us to act as if all of the actions
+   * suddenly disappeared.
+   */
+  if (window->priv->actions)
+    {
+      gchar **action_names;
+      guint signal;
+      gint i;
+
+      /* Only send the remove signals if someone is listening */
+      signal = g_signal_lookup ("action-removed", G_TYPE_ACTION_GROUP);
+      if (signal && g_signal_has_handler_pending (window, signal, 0, TRUE))
+        /* need to send a removed signal for each action */
+        action_names = g_action_group_list_actions (G_ACTION_GROUP (window->priv->actions));
+      else
+        /* don't need to send signals: nobody is watching */
+        action_names = NULL;
+
+      /* Free the group before sending the signals for two reasons:
+       *
+       *  1) we want any incoming calls to see an empty group
+       *
+       *  2) we don't want signal handlers that trigger in response to
+       *     the action-removed signals that we're firing to attempt to
+       *     modify the action group in a way that may cause it to fire
+       *     additional signals (which we would then propagate)
+       */
+      g_object_unref (window->priv->actions);
+      window->priv->actions = NULL;
+
+      /* It's safe to send the signals now, if we need to. */
+      if (action_names)
+        {
+          for (i = 0; action_names[i]; i++)
+            g_action_group_action_removed (G_ACTION_GROUP (window), action_names[i]);
+
+          g_strfreev (action_names);
+        }
+    }
 }
 
 static void