[gtk+] GtkPathBar: Centralize handling of outstanding cancellables

[Federico Mena Quintero <federico src gnome org>]

commit 559d32420c21a5888d0cc37169157e91780b6c9d
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri Oct 27 17:04:02 2017 -0500

    GtkPathBar: Centralize handling of outstanding cancellables
    
    The path bar would crash if we disposed it before all pending I/O
    operations had finished.  Now we remember all the outstanding
    operations directly in the GtkPathBarPrivate, and deal with them
    consistently.

 gtk/gtkpathbar.c |  135 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 107 insertions(+), 28 deletions(-)
---
diff --git a/gtk/gtkpathbar.c b/gtk/gtkpathbar.c
index e58927f..77fb804 100644
--- a/gtk/gtkpathbar.c
+++ b/gtk/gtkpathbar.c
@@ -45,6 +45,30 @@ struct _GtkPathBarPrivate
   GFile *home_file;
   GFile *desktop_file;
 
+  /* List of running GCancellable.  When we cancel one, we remove it from this list.
+   * The pathbar cancels all outstanding cancellables when it is disposed.
+   *
+   * In code that queues async I/O operations:
+   *
+   *   - Obtain a cancellable from the async I/O APIs, and call add_cancellable().
+   *
+   * To cancel a cancellable:
+   *
+   *   - Call cancel_cancellable().
+   *
+   * In async I/O callbacks:
+   *
+   *   - Check right away if g_cancellable_is_cancelled():  if true, just
+   *     g_object_unref() the cancellable and return early (also free your
+   *     closure data if you have one).
+   *
+   *   - If it was not cancelled, call cancellable_async_done().  This will
+   *     unref the cancellable and unqueue it from the pathbar's outstanding
+   *     cancellables.  Do your normal work to process the async result and free
+   *     your closure data if you have one.
+   */
+  GList *cancellables;
+
   GCancellable *get_info_cancellable;
 
   GIcon *root_icon;
@@ -168,6 +192,52 @@ static void gtk_path_bar_scroll_controller_scroll (GtkEventControllerScroll *scr
                                                    GtkPathBar               *path_bar);
 
 static void
+add_cancellable (GtkPathBar   *path_bar,
+                GCancellable *cancellable)
+{
+  g_assert (g_list_find (path_bar->priv->cancellables, cancellable) == NULL);
+  path_bar->priv->cancellables = g_list_prepend (path_bar->priv->cancellables, cancellable);
+}
+
+static void
+drop_node_for_cancellable (GtkPathBar *path_bar,
+                          GCancellable *cancellable)
+{
+  GList *node;
+
+  node = g_list_find (path_bar->priv->cancellables, cancellable);
+  g_assert (node != NULL);
+  node->data = NULL;
+  path_bar->priv->cancellables = g_list_delete_link (path_bar->priv->cancellables, node);
+}
+
+static void
+cancel_cancellable (GtkPathBar   *path_bar,
+                   GCancellable *cancellable)
+{
+  drop_node_for_cancellable (path_bar, cancellable);
+  g_cancellable_cancel (cancellable);
+}
+
+static void
+cancellable_async_done (GtkPathBar   *path_bar,
+                       GCancellable *cancellable)
+{
+  drop_node_for_cancellable (path_bar, cancellable);
+  g_object_unref (cancellable);
+}
+
+static void
+cancel_all_cancellables (GtkPathBar *path_bar)
+{
+  while (path_bar->priv->cancellables)
+    {
+      GCancellable *cancellable = path_bar->priv->cancellables->data;
+      cancel_cancellable (path_bar, cancellable);
+    }
+}
+
+static void
 on_slider_unmap (GtkWidget  *widget,
                 GtkPathBar *path_bar)
 {
@@ -207,6 +277,7 @@ gtk_path_bar_init (GtkPathBar *path_bar)
   gtk_style_context_add_class (context, GTK_STYLE_CLASS_LINKED);
 
   path_bar->priv->get_info_cancellable = NULL;
+  path_bar->priv->cancellables = NULL;
 
   path_bar->priv->scroll_controller =
     gtk_event_controller_scroll_new (GTK_WIDGET (path_bar),
@@ -282,6 +353,7 @@ gtk_path_bar_finalize (GObject *object)
 
   path_bar = GTK_PATH_BAR (object);
 
+  cancel_all_cancellables (path_bar);
   gtk_path_bar_stop_scrolling (path_bar);
 
   g_list_free (path_bar->priv->button_list);
@@ -323,9 +395,8 @@ gtk_path_bar_dispose (GObject *object)
 
   remove_settings_signal (path_bar, gtk_widget_get_screen (GTK_WIDGET (object)));
 
-  if (path_bar->priv->get_info_cancellable)
-    g_cancellable_cancel (path_bar->priv->get_info_cancellable);
   path_bar->priv->get_info_cancellable = NULL;
+  cancel_all_cancellables (path_bar);
 
   G_OBJECT_CLASS (gtk_path_bar_parent_class)->dispose (object);
 }
@@ -1203,18 +1274,21 @@ set_button_image_get_info_cb (GCancellable *cancellable,
   GIcon *icon;
   struct SetButtonImageData *data = user_data;
 
-  if (cancellable != data->button_data->cancellable)
-    goto out;
-
-  data->button_data->cancellable = NULL;
-
-  if (!data->button_data->button)
+  if (cancelled)
     {
-      g_free (data->button_data);
-      goto out;
+      g_free (data);
+      g_object_unref (cancellable);
+      return;
     }
 
-  if (cancelled || error)
+  g_assert (GTK_IS_PATH_BAR (data->path_bar));
+  g_assert (G_OBJECT (data->path_bar)->ref_count > 0);
+
+  g_assert (cancellable == data->button_data->cancellable);
+  cancellable_async_done (data->path_bar, cancellable);
+  data->button_data->cancellable = NULL;
+
+  if (error)
     goto out;
 
   icon = g_file_info_get_symbolic_icon (info);
@@ -1238,7 +1312,6 @@ set_button_image_get_info_cb (GCancellable *cancellable,
 
 out:
   g_free (data);
-  g_object_unref (cancellable);
 }
 
 static void
@@ -1280,7 +1353,9 @@ set_button_image (GtkPathBar *path_bar,
       data->button_data = button_data;
 
       if (button_data->cancellable)
-       g_cancellable_cancel (button_data->cancellable);
+       {
+         cancel_cancellable (path_bar, button_data->cancellable);
+       }
 
       button_data->cancellable =
         _gtk_file_system_get_info (path_bar->priv->file_system,
@@ -1288,6 +1363,7 @@ set_button_image (GtkPathBar *path_bar,
                                   "standard::symbolic-icon",
                                   set_button_image_get_info_cb,
                                   data);
+      add_cancellable (path_bar, button_data->cancellable);
       break;
 
     case DESKTOP_BUTTON:
@@ -1302,7 +1378,9 @@ set_button_image (GtkPathBar *path_bar,
       data->button_data = button_data;
 
       if (button_data->cancellable)
-       g_cancellable_cancel (button_data->cancellable);
+       {
+         cancel_cancellable (path_bar, button_data->cancellable);
+       }
 
       button_data->cancellable =
         _gtk_file_system_get_info (path_bar->priv->file_system,
@@ -1310,6 +1388,7 @@ set_button_image (GtkPathBar *path_bar,
                                   "standard::symbolic-icon",
                                   set_button_image_get_info_cb,
                                   data);
+      add_cancellable (path_bar, button_data->cancellable);
       break;
 
     case NORMAL_BUTTON:
@@ -1330,10 +1409,7 @@ button_data_free (ButtonData *button_data)
 
   button_data->button = NULL;
 
-  if (button_data->cancellable)
-    g_cancellable_cancel (button_data->cancellable);
-  else
-    g_free (button_data);
+  g_free (button_data);
 }
 
 static const char *
@@ -1429,7 +1505,6 @@ make_directory_button (GtkPathBar  *path_bar,
   file_is_hidden = !! file_is_hidden;
   /* Is it a special button? */
   button_data = g_new0 (ButtonData, 1);
-
   button_data->type = find_button_type (path_bar, file);
   button_data->button = gtk_toggle_button_new ();
   atk_obj = gtk_widget_get_accessible (button_data->button);
@@ -1613,20 +1688,21 @@ gtk_path_bar_get_info_callback (GCancellable *cancellable,
   const gchar *display_name;
   gboolean is_hidden;
 
-  if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-    return;
-
-  if (cancellable != file_info->path_bar->priv->get_info_cancellable)
+  if (cancelled)
     {
       gtk_path_bar_set_file_finish (file_info, FALSE);
       g_object_unref (cancellable);
       return;
     }
 
-  g_object_unref (cancellable);
+  g_assert (GTK_IS_PATH_BAR (file_info->path_bar));
+  g_assert (G_OBJECT (file_info->path_bar)->ref_count > 0);
+
+  g_assert (cancellable == file_info->path_bar->priv->get_info_cancellable);
+  cancellable_async_done (file_info->path_bar, cancellable);
   file_info->path_bar->priv->get_info_cancellable = NULL;
 
-  if (cancelled || !info)
+  if (!info)
     {
       gtk_path_bar_set_file_finish (file_info, FALSE);
       return;
@@ -1638,7 +1714,7 @@ gtk_path_bar_get_info_callback (GCancellable *cancellable,
   button_data = make_directory_button (file_info->path_bar, display_name,
                                        file_info->file,
                                       file_info->first_directory, is_hidden);
-  g_object_unref (file_info->file);
+  g_clear_object (&file_info->file);
 
   file_info->new_buttons = g_list_prepend (file_info->new_buttons, button_data);
 
@@ -1668,6 +1744,7 @@ gtk_path_bar_get_info_callback (GCancellable *cancellable,
                               "standard::display-name,standard::is-hidden,standard::is-backup",
                               gtk_path_bar_get_info_callback,
                               file_info);
+  add_cancellable (file_info->path_bar, file_info->path_bar->priv->get_info_cancellable);
 }
 
 void
@@ -1693,7 +1770,9 @@ _gtk_path_bar_set_file (GtkPathBar *path_bar,
   info->parent_file = g_file_get_parent (info->file);
 
   if (path_bar->priv->get_info_cancellable)
-    g_cancellable_cancel (path_bar->priv->get_info_cancellable);
+    {
+      cancel_cancellable (path_bar, path_bar->priv->get_info_cancellable);
+    }
 
   path_bar->priv->get_info_cancellable =
     _gtk_file_system_get_info (path_bar->priv->file_system,
@@ -1701,7 +1780,7 @@ _gtk_path_bar_set_file (GtkPathBar *path_bar,
                                "standard::display-name,standard::is-hidden,standard::is-backup",
                                gtk_path_bar_get_info_callback,
                                info);
-
+  add_cancellable (path_bar, path_bar->priv->get_info_cancellable);
 }
 
 /* FIXME: This should be a construct-only property */