Bug 25293

This has been discussed in this thread as gnome-shell needs this. It can start handling the channel itself and then give it back to Empathy.

http://lists.freedesktop.org/archives/telepathy/2010-November/005061.html

The idea was to implement this:

ChannelDispatcher.GiveBack(o: Account, ao: Channels,
        x: User_Action_Time, s: Next_Handler, a{sv}: Hints) -> nothing

It has been suggested to return a ChannelRequest but I'm not convinced it buys us anything.

Here we go: http://git.collabora.co.uk/?p=user/cassidy/telepathy-spec;a=shortlog;h=refs/heads/re-handle-25293

I've finally choosed RedispatchChannels() as suggested by Sjoerd, I think that's clearer.

I started implementing this in bug #33271

It seems to work fine (in tests), the only issue is that currently the client getting the channel being re-dispatched doesn't have a way to get Hints.
That's because the only way to get it in HandleChannels() is throught the ChannelRequest() and I don't create a new CR when redispatching.

I guess we could create one, but that sounds a bit like an abuse of the first goal of ChannelRequest tbh. Maybe instead we could add a new key Handler_Info to pass the redispatching hints?

Note from our quick discussion on IRC.

Sjoerd suggested that instead of raising NotCapable, MC would succeed and re-call HandleChannels() on the caller. That would make things easier for the caller as it could assume after calling RedispatchChannels() that it is not longer handling the channel. (except if it passes invalid arguments or a channel it's not handling but in that case that's his fault).

If we go for that approach, should RedispatchChannels() return before or after HandleChannels() has been called?


I was also wondering if it would be useful for the new Handler to know which was the previous handler before him. We could do that by adding a key to Handler_Info if needed.

HTML: http://people.freedesktop.org/~gdesmott/telepathy-spec-re_handle_25293/spec/Channel_Dispatcher.html

I added RedEnsureChannel()
http://people.freedesktop.org/~gdesmott/telepathy-spec-re_handle_25293/spec/Channel_Dispatcher.html#Method:ReEnsureChannel

I didn't include a Hints parameter as the Handler won't have any way to get it. We could solve that by adding Hints to Handler_Info as suggested above.

Looks good to me. I don't think Hints are really needed, the use-case for this is, prod the current channel not to modify/update/put in extra hints..

If we find a usecase for it we can do a ReEnsureChannelWithHints later.

What about ReDispatchChannels()? Should we remove the Hints argument as well?

Actually, what's the point to pass the Account to ReDispatchChannels() and ReEnsureChannel()? MC should already know it from the channel(s).

ReDispatchChannels
------------------

I think something like DelegateChannels() would be a better name. It doesn't actually send the channel through the whole dispatcher again—or does it? Do Observers get the channel again? Is a ChannelRequest created? I guess you think this is an open question too. :)

(In reply to comment #9)
> Actually, what's the point to pass the Account to ReDispatchChannels() and
> ReEnsureChannel()? MC should already know it from the channel(s).

I kind of agree. If ReDispatchChannels() is plural, then why shouldn't we be able to redispatch channels from different accounts? This would also make it marginally easier to use.

    The time at which user action occurred, or 0 if this channel request is for
    some reason not involving user action.

What channel request?

    Either the well-known bus name (starting with
    org.freedesktop.Telepathy.Client.) of the preferred new handler for these
    channels, or an empty string to indicate that any handler would be
    acceptable. The behaviour and rationale are the same as for the
    corresponding parameter to CreateChannelWithHints.

If I pass the empty string for the preferred handler, is it okay for MC to give me the channel right back again (if I'm the best match)?

I think this method needs an example of how it would be used.

    Additional information about the channels request, which will be used as
    the value for the resulting request's Hints property.

What channel request? :)


ReEnsureChannel
---------------

Again, I think it would be better to have a different method name to more clearly signal the intent of this method. How about EvokeChannel()? EvokeHandlerForChannel()? KickHandler()? SummonHandler()?

I think it's inconsistent that one of these methods can take hints and stuff, bu the other cannot. Since they both end up being a call to HandleChannels() on some handler…

(Also I wonder how many handlers will deal correctly with being given 5 text channels at once... ;-))

(In reply to comment #10)
> ReDispatchChannels
> ------------------
> 
> I think something like DelegateChannels() would be a better name. It doesn't
> actually send the channel through the whole dispatcher again—or does it? Do
> Observers get the channel again? Is a ChannelRequest created? I guess you think
> this is an open question too. :)

I'd say 'no' to all of these questions, so yeah DelegateChannels() sounds like
a better name. I renamed it.

> (In reply to comment #9)
> > Actually, what's the point to pass the Account to ReDispatchChannels() and
> > ReEnsureChannel()? MC should already know it from the channel(s).
> 
> I kind of agree. If ReDispatchChannels() is plural, then why shouldn't we be
> able to redispatch channels from different accounts? This would also make it
> marginally easier to use.

Ok, I removed the Account.

>     The time at which user action occurred, or 0 if this channel request is for
>     some reason not involving user action.
> 
> What channel request?

fixed.

>     Either the well-known bus name (starting with
>     org.freedesktop.Telepathy.Client.) of the preferred new handler for these
>     channels, or an empty string to indicate that any handler would be
>     acceptable. The behaviour and rationale are the same as for the
>     corresponding parameter to CreateChannelWithHints.
> 
> If I pass the empty string for the preferred handler, is it okay for MC to give
> me the channel right back again (if I'm the best match)?

I'd say no as the goal is explicitely to pass the channel to another client.
What do you think?

> I think this method needs an example of how it would be used.

I'll add one once we agree on the API.

>     Additional information about the channels request, which will be used as
>     the value for the resulting request's Hints property.
> 
> What channel request? :)

Right. This move us back to the question "Should we allow to pass Hints" ?
If we want to (which is probably a good idea) and we don't create a
ChannelRequest (which I also think is a good idea) then our only option is to
add a 'Hints' (a(a{sv}) key to Handler_Info.

> ReEnsureChannel
> ---------------
> 
> Again, I think it would be better to have a different method name to more
> clearly signal the intent of this method. How about EvokeChannel()?
> EvokeHandlerForChannel()? KickHandler()? SummonHandler()?

PresentChannel()?

> I think it's inconsistent that one of these methods can take hints and stuff,
> bu the other cannot. Since they both end up being a call to HandleChannels() on
> some handler…

If we go for the 'Hints' key in Handler_Info approach we can easily add one.

I updated:
http://git.collabora.co.uk/?p=user/cassidy/telepathy-spec;a=shortlog;h=refs/heads/re-handle-25293
http://people.freedesktop.org/~gdesmott/telepathy-spec-re_handle_25293/spec/Channel_Dispatcher.html#Method:DelegateChannels

(In reply to comment #11)
> (In reply to comment #10)
> > If I pass the empty string for the preferred handler, is it okay for MC to give
> > me the channel right back again (if I'm the best match)?
> 
> I'd say no as the goal is explicitely to pass the channel to another client.
> What do you think?

I agree. I think the method call should fail if the preferred handler doesn't exist, or if you pass "" and there are no handlers beside yourself.

> >     Additional information about the channels request, which will be used as
> >     the value for the resulting request's Hints property.
> > 
> > What channel request? :)
> 
> Right. This move us back to the question "Should we allow to pass Hints" ?
> If we want to (which is probably a good idea) and we don't create a
> ChannelRequest (which I also think is a good idea) then our only option is to
> add a 'Hints' (a(a{sv}) key to Handler_Info.

I don't really like this because it means that every implementation of HandleChannels() will have to look in two different places for hints. It won't be right by default.

I don't think making a ChannelRequest spring up would be so bad. It's a bit of an abuse, but…

I'm undecided about which I think would be worse.

> > ReEnsureChannel
> > ---------------
> > 
> > Again, I think it would be better to have a different method name to more
> > clearly signal the intent of this method. How about EvokeChannel()?
> > EvokeHandlerForChannel()? KickHandler()? SummonHandler()?
> 
> PresentChannel()?

Someone got upset last time someone suggested “Present”—which is why I didn't suggest it—but given that this is what this method is *for* I think it's reasonable.

> > I think it's inconsistent that one of these methods can take hints and stuff,
> > bu the other cannot. Since they both end up being a call to HandleChannels() on
> > some handler…
> 
> If we go for the 'Hints' key in Handler_Info approach we can easily add one.

And if we go for “create a ChannelRequest”, then we can add one too. :)

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > If I pass the empty string for the preferred handler, is it okay for MC to give
> > > me the channel right back again (if I'm the best match)?
> > 
> > I'd say no as the goal is explicitely to pass the channel to another client.
> > What do you think?
> 
> I agree. I think the method call should fail if the preferred handler doesn't
> exist, or if you pass "" and there are no handlers beside yourself.

Cool, that's already what the spec says (Not Capable error).

> > >     Additional information about the channels request, which will be used as
> > >     the value for the resulting request's Hints property.
> > > 
> > > What channel request? :)
> > 
> > Right. This move us back to the question "Should we allow to pass Hints" ?
> > If we want to (which is probably a good idea) and we don't create a
> > ChannelRequest (which I also think is a good idea) then our only option is to
> > add a 'Hints' (a(a{sv}) key to Handler_Info.
> 
> I don't really like this because it means that every implementation of
> HandleChannels() will have to look in two different places for hints. It won't
> be right by default.
> 
> I don't think making a ChannelRequest spring up would be so bad. It's a bit of
> an abuse, but…
> 
> I'm undecided about which I think would be worse.

I'm still not convinced we should have a ChannelRequest; that's a re-dispatch
request, not a channel one. And then, should DelegateChannels() return a CR
and the dispatching be delayed until we call CR.Proceed()?
If we don't, we have 2 different way to use CRs.

> > > ReEnsureChannel
> > > ---------------
> > > 
> > > Again, I think it would be better to have a different method name to more
> > > clearly signal the intent of this method. How about EvokeChannel()?
> > > EvokeHandlerForChannel()? KickHandler()? SummonHandler()?
> > 
> > PresentChannel()?
> 
> Someone got upset last time someone suggested “Present”—which is why I didn't
> suggest it—but given that this is what this method is *for* I think it's
> reasonable.

PresentChannel() it is then: changed.

> > > I think it's inconsistent that one of these methods can take hints and stuff,
> > > bu the other cannot. Since they both end up being a call to HandleChannels() on
> > > some handler…
> > 
> > If we go for the 'Hints' key in Handler_Info approach we can easily add one.
> 
> And if we go for “create a ChannelRequest”, then we can add one too. :)

Right.

So I think the last thing to decide is if we go for a CR or not.

Don't use CR
============

- Current approach in my branch/html file.
- Make things easier to use.
- The main (only) problem is how to pass the Hints to the Handler. Using
Handler_Info is handy but means we have 2 code paths to find Hints (sadface).

Use CR
======
- DelegateChannels() and PresentChannel() returns a CR.
- We have to change the description of ChannelRequel.
- Should user call CR.Proceed()?
- What would contain CR.Requests? The properties of the channel(s)?

Humm I thought about a potential issue with DelegateChannels(). Handlers are
supposed to announce the channels they are handling in Handler.HandledChannels [1].

In practice clients don't have to care as TpBaseClient (or the tp-qt4
equivalent) transparently does it for them. We'll have to make sure that
TpBaseClient is informed when a channel is delegated as it'll have to remove
the channel from the HandledChannels list.
Maybe we should announce that on D-Bus? Another option would be to hook the
tp-glib DelegateChannels API with TpBaseClient but that sounds pretty fragile
a bit hacky to me.

[1] http://telepathy.freedesktop.org/spec/Client_Handler.html#Property:HandledChannels

(In reply to comment #14)
> Maybe we should announce that on D-Bus? Another option would be to hook the
> tp-glib DelegateChannels API with TpBaseClient

I did that in http://cgit.collabora.co.uk/git/user/cassidy/telepathy-glib/log/?h=re-dispatch-34610 and it worked pretty well (I can now move channels from the Shell to Empathy) so I think that's the way to go.


As the main issue with the spec was to retrive Hints argument, I just removed them. We don't need it for now and I'd rather not stay blocked on this.

New branch: http://cgit.collabora.co.uk/git/user/cassidy/telepathy-spec/log/?id=refs/heads/re-handle-25293

HTML:
http://people.freedesktop.org/~gdesmott/telepathy-spec-re_handle_25293/spec/Channel_Dispatcher.html#Method:DelegateChannels

+        with a Requested_Properties which would result in ensuring
+        Channel.</p>

Generally speaking I mark up argument names with <var/>. But obvs. not a merge blocker.

Yus yus let's do this.

Merged to master ! Will be in 0.23.1 (I think).