[Bug 24120] Refactor the McdDispatcher

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24120





--- Comment #26 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-10-20 09:48:13 PST ---
(In reply to comment #25)
> dr4.4-fix-reinvocation:
> 
> I'm suspicious of the new logic which invokes the first handler object on the
> unique name that originally handled the channel that matches the filter. Could
> the McdHandlerMap remember which well-known name handled the channel as well?
> Maybe MC could try to use that if it still exists, and fall back to the
> first-matching-one behaviour?

In principle, yes it could. I'm assuming here that "multi-headed" clients will
always have to have some centralized map from object path to GtkWindow (or
whatever), in order to reply to Get("HandledChannels") correctly, and so it
doesn't really matter which head we call - they'll all have an early-return
guard, "if already handling this, bump the window into the foreground and
return".

There are two ways that the new logic could pick the wrong head from such a
hydra:

* The handler has put up an extra head that matches the channel better; that
new head catches the channel, instead of it going to the less specific head
(this would happen if the fake Empathy in dispatcher/capture-bundle.py had a
channel redispatched to it)

* The handler previously had an extra head that matched the channel better, but
it has cut off that head (I can't think of a real-world use case for this); due
to the fallback behaviour you propose, your logic and mine are effectively
equivalent here

To what extent do you consider the former case to be a problem? I could try to
code up this functionality if you think it's a blocker, or just file a bug
about it.

> +        DEBUG ("Handler %s does not exist in client registry, not "
> +               "reinvoking", possible_handlers[0]);
> +        mcd_dispatcher_finish_reinvocation (request);
> +        return;
> +    }
> 
> should goto finally;

Good catch, minor memory leak, fixed in the branch (I haven't rebased
subsequent branches).

> +mcd_dispatcher_finish_reinvocation (McdChannel *request)
>  {
> +    _mcd_channel_set_status (request, MCD_CHANNEL_STATUS_DISPATCHED);
> 
> Why is that necessary? We only enter this code path in the first place if the
> status is Dispatched.

You'd think... but no. We have two McdChannel objects. After the renaming done
at the beginning of this chapter of the branch, they are named consistently as
'channel' and 'request'. The situation is:

* 'channel' exists already
* 'request' (which is an McdChannel with its "really a ChannelRequest" hat on)
is an EnsureChannel() call that returned the same object-path that 'channel'
has
* 'channel' has been DISPATCHED
* _mcd_channel_copy_details copies the TpChannel from 'channel' to 'request',
but nothing else
* reinvocation runs
* 'request' now needs to be set to DISPATCHED

So, I think I was right the first time.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.