[PATCH 3/3] core: ensure correct cancel notification for browse

[Guillaume Emont <gemont igalia com>]

---
 src/grl-media-source.c |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/grl-media-source.c b/src/grl-media-source.c
index f0ac763..8351462 100644
--- a/src/grl-media-source.c
+++ b/src/grl-media-source.c
@@ -500,8 +500,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;
 }
@@ -642,6 +648,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,
@@ -658,7 +673,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);
   }
@@ -809,12 +825,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 = 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 (otherwise, 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 */
-- 
1.7.1