[PATCH 4/5] core: ensure correct cancel notification

[Guillaume Emont <gemont igalia com>]

This commit ensures that the callback (or the last callback in the case of
browse, search and query) is always called with
GRL_CORE_ERROR_OPERATION_CANCELLED set if the operation has been cancelled
(both in the cases where the source implements ->cancel() and where it
doesn't).
---
 src/grl-media-source.c |  103 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/src/grl-media-source.c b/src/grl-media-source.c
index 4463c63..0504188 100644
--- a/src/grl-media-source.c
+++ b/src/grl-media-source.c
@@ -501,8 +501,14 @@ browse_idle (gpointer user_data)
   if (!operation_is_cancelled (bs->source, bs->browse_id)) {
     GRL_MEDIA_SOURCE_GET_CLASS (bs->source)->browse (bs->source, bs);
   } else {
+    GError *error;
     GRL_DEBUG ("  operation was cancelled");
-    bs->callback (bs->source, bs->browse_id, NULL, 0, bs->user_data, NULL);
+    error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                         "Operation was cancelled");
+    /* Note: at this point, bs->callback should not be the user-provided
+     * callback, but rather browse_result_relay_cb() */
+    bs->callback (bs->source, bs->browse_id, NULL, 0, bs->user_data, error);
+    g_error_free (error);
   }
   return FALSE;
 }
@@ -516,8 +522,12 @@ search_idle (gpointer user_data)
   if (!operation_is_cancelled (ss->source, ss->search_id)) {
     GRL_MEDIA_SOURCE_GET_CLASS (ss->source)->search (ss->source, ss);
   } else {
+    GError *error;
     GRL_DEBUG ("  operation was cancelled");
-    ss->callback (ss->source, ss->search_id, NULL, 0, ss->user_data, NULL);
+    error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                         "Operation was cancelled");
+    ss->callback (ss->source, ss->search_id, NULL, 0, ss->user_data, error);
+    g_error_free (error);
   }
   return FALSE;
 }
@@ -530,8 +540,12 @@ query_idle (gpointer user_data)
   if (!operation_is_cancelled (qs->source, qs->query_id)) {
     GRL_MEDIA_SOURCE_GET_CLASS (qs->source)->query (qs->source, qs);
   } else {
+    GError *error;
     GRL_DEBUG ("  operation was cancelled");
-    qs->callback (qs->source, qs->query_id, NULL, 0, qs->user_data, NULL);
+    error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                         "Operation was cancelled");
+    qs->callback (qs->source, qs->query_id, NULL, 0, qs->user_data, error);
+    g_error_free (error);
   }
   return FALSE;
 }
@@ -544,8 +558,12 @@ metadata_idle (gpointer user_data)
   if (!operation_is_cancelled (ms->source, ms->metadata_id)) {
     GRL_MEDIA_SOURCE_GET_CLASS (ms->source)->metadata (ms->source, ms);
   } else {
+    GError *error;
     GRL_DEBUG ("  operation was cancelled");
-    ms->callback (ms->source, ms->media, ms->user_data, NULL);
+    error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                         "Operation was cancelled");
+    ms->callback (ms->source, ms->media, ms->user_data, error);
+    g_error_free (error);
   }
   return FALSE;
 }
@@ -643,6 +661,15 @@ browse_result_relay_idle (gpointer user_data)
     cancelled = TRUE;
   }
   if (!cancelled || bri->remaining == 0) {
+    if (cancelled) {
+      /* Last callback call for a cancelled operation, the cancelled error
+       * takes precedence, because the caller shouldn't care about other errors
+       * if it called _cancel(). */
+      if (bri->error)
+        g_error_free (bri->error);
+      bri->error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                           "Operation was cancelled");
+    }
     bri->user_callback (bri->source,
 			bri->browse_id,
 			bri->media,
@@ -659,7 +686,8 @@ browse_result_relay_idle (gpointer user_data)
     set_operation_finished (bri->source, bri->browse_id);
   }
 
-  /* We copy the error if we do idle relay, we have to free it here */
+  /* We copy the error if we do idle relay or might have created one above in
+   * some cancellation cases, we have to free it here */
   if (bri->error) {
     g_error_free (bri->error);
   }
@@ -810,12 +838,27 @@ browse_result_relay_cb (GrlMediaSource *source,
     bri->chained = brc->chained;
     g_idle_add (browse_result_relay_idle, bri);
   } else {
+    gboolean should_free_error = FALSE;
+    GError *_error = (GError *)error;
+    if (remaining == 0 && operation_is_cancelled (source, browse_id)) {
+      /* last callback call for a cancelled operation */
+      /* if the plugin already set an error, we don't care because we're
+       * cancelled */
+      _error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                           "Operation was cancelled");
+      /* Yet, we should free the error we just created (if we didn't create it,
+       * the plugin owns it) */
+      should_free_error = TRUE;
+    }
     brc->user_callback (source,
 			browse_id,
 			media,
 			remaining,
 			brc->user_data,
-			error);
+			_error);
+    if (should_free_error && _error)
+      g_error_free (_error);
+
     if (remaining == 0 && !brc->chained) {
       /* This is the last post-processing callback, so we can remove
 	 the operation state data here */
@@ -890,6 +933,8 @@ metadata_result_relay_cb (GrlMediaSource *source,
 			  gpointer user_data,
 			  const GError *error)
 {
+  gboolean should_free_error = FALSE;
+  GError *_error = (GError *)error;
   GRL_DEBUG ("metadata_result_relay_cb");
 
   struct MetadataRelayCb *mrc;
@@ -900,8 +945,21 @@ metadata_result_relay_cb (GrlMediaSource *source,
                           grl_metadata_source_get_id (GRL_METADATA_SOURCE (source)));
   }
 
+  if (operation_is_cancelled (source, mrc->spec->metadata_id)) {
+    /* if the plugin already set an error, we don't care because we're
+     * cancelled */
+    _error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                          "Operation was cancelled");
+    /* yet, we should free the error we just created (if we didn't create it,
+     * the plugin owns it) */
+    should_free_error = TRUE;
+  }
+
   mrc->user_callback (source, media, mrc->user_data, error);
 
+  if (should_free_error && _error)
+    g_error_free (_error);
+
   g_object_unref (mrc->spec->source);
   if (mrc->spec->media) {
     /* Can be NULL if getting metadata for root category */
@@ -1073,16 +1131,30 @@ full_resolution_done_cb (GrlMetadataSource *source,
 	 was cancelled and this is the one with remaining == 0*/
       if (GPOINTER_TO_UINT (ctl_info->next_index->data) == cb_info->remaining
 	  || cancelled) {
+  GError *_error = (GError *)error;
+  gboolean should_free_error;
 	/* Notice we pass NULL as error on purpose
 	   since the result is valid even if the full-resolution failed */
 	guint remaining = cb_info->remaining;
+  if (cancelled && remaining==0
+      && !g_error_matches (_error, GRL_CORE_ERROR,
+                           GRL_CORE_ERROR_OPERATION_CANCELLED)) {
+    /* We are cancelled and this is the last callback, cancelled error need to
+     * be set */
+    _error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                          "Operation was cancelled");
+    should_free_error = TRUE;
+  }
 	GRL_DEBUG ("  Result is in sort order, emitting (%d)", remaining);
 	ctl_info->user_callback (cb_info->source,
 				 cb_info->browse_id,
 				 media,
 				 cb_info->remaining,
 				 ctl_info->user_data,
-				 error);
+				 _error);
+  if (should_free_error && _error)
+    g_error_free (_error);
+
 	ctl_info->next_index = g_list_delete_link (ctl_info->next_index,
 						   ctl_info->next_index);
 	/* Now that we have emitted the next result, check if we
@@ -1199,10 +1271,25 @@ metadata_full_resolution_done_cb (GrlMetadataSource *source,
   }
 
   if (cb_info->pending_callbacks == 0) {
+    GError *_error = (GError *)error;
+    gboolean should_free_error = FALSE;
+    if (operation_is_cancelled (GRL_MEDIA_SOURCE (source),
+                                cb_info->ctl_info->metadata_id)) {
+      /* if the plugin already set an error, we don't care because we're
+       * cancelled */
+      _error = g_error_new (GRL_CORE_ERROR, GRL_CORE_ERROR_OPERATION_CANCELLED,
+                           "Operation was cancelled");
+      /* Yet, we should free the error we just created (if we didn't create it,
+       * the plugin owns it) */
+      should_free_error = TRUE;
+    }
     cb_info->user_callback (cb_info->source,
 			    media,
 			    cb_info->user_data,
-			    NULL);
+			    _error);
+
+    if (should_free_error && _error)
+      g_error_free (_error);
 
     free_source_map_list (cb_info->ctl_info->source_map_list);
     g_free (cb_info->ctl_info);
-- 
1.7.1