fd.o #29022: if a channel can't be handled, dispatch it anyway, to run observers

Arguably, we should have some way that approvers can opt-in to being told
about undispatchable channels that they might want to Claim, but that
requires spec, whereas we can fix this straight away.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=29022

3 files changed, 32 insertions, 21 deletions

diff --git a/src/mcd-dispatch-operation.c b/src/mcd-dispatch-operation.c
index e8823a1a..04db4afe 100644
--- a/src/mcd-dispatch-operation.c
+++ b/src/mcd-dispatch-operation.c
@@ -514,6 +514,18 @@ _mcd_dispatch_operation_check_client_locks (McdDispatchOperation *self)
         return;
     }
 
+    /* If there are no potential handlers, the story ends here: we don't
+     * want to run approvers in this case */
+    if (self->priv->possible_handlers == NULL)
+    {
+        GError incapable = { TP_ERRORS, TP_ERROR_NOT_CAPABLE,
+            "No possible handlers, giving up" };
+
+        DEBUG ("%s", incapable.message);
+        _mcd_dispatch_operation_close_as_undispatchable (self, &incapable);
+        return;
+    }
+
     approval = g_queue_peek_head (self->priv->approvals);
 
     /* if we've been claimed, respond, then do not call HandleChannels */
@@ -1063,12 +1075,6 @@ mcd_dispatch_operation_constructor (GType type, guint n_params,
     if (!priv->client_registry || !priv->handler_map)
         goto error;
 
-    if (priv->possible_handlers == NULL && !priv->observe_only)
-    {
-        g_critical ("!observe_only => possible_handlers must not be NULL");
-        goto error;
-    }
-
     if (priv->needs_approval && priv->observe_only)
     {
         g_critical ("observe_only => needs_approval must not be TRUE");
@@ -1438,9 +1444,6 @@ _mcd_dispatch_operation_new (McdClientRegistry *client_registry,
 {
     gpointer *obj;
 
-    /* possible-handlers is only allowed to be NULL if we're only observing */
-    g_return_val_if_fail (possible_handlers != NULL || observe_only, NULL);
-
     /* If we're only observing, then the channels were requested "behind MC's
      * back", so they can't need approval (i.e. observe_only implies
      * !needs_approval) */
@@ -1812,8 +1815,13 @@ _mcd_dispatch_operation_handlers_can_bypass_approval (
     if (_mcd_dispatch_operation_is_internal (self))
         return TRUE;
 
+    /* special case: we don't have any handlers at all, so we don't want
+     * approval - we're just going to fail */
+    if (self->priv->possible_handlers == NULL)
+        return TRUE;
+
     for (iter = self->priv->possible_handlers;
-         iter != NULL && *iter != NULL;
+         *iter != NULL;
          iter++)
     {
         McdClientProxy *handler = _mcd_client_registry_lookup (
diff --git a/src/mcd-dispatcher.c b/src/mcd-dispatcher.c
index 7b16f6a9..70ca7752 100644
--- a/src/mcd-dispatcher.c
+++ b/src/mcd-dispatcher.c
@@ -415,7 +415,6 @@ _mcd_dispatcher_enter_state_machine (McdDispatcher *dispatcher,
     g_return_if_fail (channels != NULL);
     g_return_if_fail (MCD_IS_CHANNEL (channels->data));
     g_return_if_fail (requested || !only_observe);
-    g_return_if_fail (possible_handlers != NULL || only_observe);
 
     account = mcd_channel_get_account (channels->data);
     if (G_UNLIKELY (!account))
@@ -1435,9 +1434,8 @@ _mcd_dispatcher_take_channels (McdDispatcher *dispatcher, GList *channels,
     {
         if (channels->next == NULL)
         {
-            DEBUG ("One channel, which cannot be handled");
-            _mcd_channel_undispatchable (channels->data);
-            g_list_free (channels);
+            DEBUG ("One channel, which cannot be handled - making a CDO "
+                   "anyway, to get Observers run");
         }
         else
         {
@@ -1451,6 +1449,8 @@ _mcd_dispatcher_take_channels (McdDispatcher *dispatcher, GList *channels,
                 _mcd_dispatcher_take_channels (dispatcher, list, requested,
                                                FALSE);
             }
+
+            return;
         }
     }
     else
@@ -1458,15 +1458,15 @@ _mcd_dispatcher_take_channels (McdDispatcher *dispatcher, GList *channels,
         DEBUG ("%s handler(s) found, dispatching %u channels",
                internal_request ? "internal" : "possible",
                g_list_length (channels));
+    }
 
-        for (list = channels; list != NULL; list = list->next)
-            _mcd_channel_set_status (MCD_CHANNEL (list->data),
-                                     MCD_CHANNEL_STATUS_DISPATCHING);
+    for (list = channels; list != NULL; list = list->next)
+        _mcd_channel_set_status (MCD_CHANNEL (list->data),
+                                 MCD_CHANNEL_STATUS_DISPATCHING);
 
-        _mcd_dispatcher_enter_state_machine (dispatcher, channels,
-            (const gchar * const *) possible_handlers, requested, FALSE);
-        g_list_free (channels);
-    }
+    _mcd_dispatcher_enter_state_machine (dispatcher, channels,
+        (const gchar * const *) possible_handlers, requested, FALSE);
+    g_list_free (channels);
 
     g_strfreev (possible_handlers);
 }
diff --git a/src/mcd-handler-map.c b/src/mcd-handler-map.c
index d750006f..63094b39 100644
--- a/src/mcd-handler-map.c
+++ b/src/mcd-handler-map.c
@@ -460,6 +460,9 @@ _mcd_handler_map_get_channel_account (McdHandlerMap *self,
         channel_path);
 }
 
+/*
+ * Record that MC itself is handling this channel, internally.
+ */
 void
 _mcd_handler_map_set_channel_handled_internally (McdHandlerMap *self,
                                                  TpChannel *channel,