From bb62fac38a3b956dad12ff3c91e71763d06808a0 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 19 May 2011 18:14:24 +0100 Subject: [PATCH 3/4] 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 --- src/mcd-dispatch-operation.c | 28 ++++++++++++++++++---------- src/mcd-dispatcher.c | 22 +++++++++++----------- src/mcd-handler-map.c | 3 +++ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/mcd-dispatch-operation.c b/src/mcd-dispatch-operation.c index adcecab..0283a17 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 0b7de91..613ded1 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 d750006..63094b3 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, -- 1.7.5.4