Bug 25286 - requirement for Client.Handler.HandledChannels is onerous
Summary: requirement for Client.Handler.HandledChannels is onerous
Status: RESOLVED WONTFIX
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-25 14:30 UTC by Danielle Madeley
Modified: 2010-06-09 05:50 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Danielle Madeley 2009-11-25 14:30:49 UTC
Client.Handler.HandledChannels states that:

"The value of this property SHOULD be the same for all Client instances that share a unique bus name, and SHOULD include all channels that are being handled, even if they were conceptually handled by a different Client instance."

This requirement is overly onerous and could prove impossible to implement using Handler helpers in the case where a user is mixing bindings (e.g. a combination of TpHandler and Tp::AbstractClient) or mixing helper classes and hand-tooled classes.

Rob proposes that each Handler should announce a set of channels it thinks it is handling and the Channel Dispatcher can union this set together for a unique name, as it sees fit.

Channels belonging to transient Handlers can be migrated to a more long-running Handler internally within the client, and needn't be covered by the spec.
Comment 1 Simon McVittie 2009-11-26 03:29:10 UTC
(In reply to comment #0)
> Client.Handler.HandledChannels states that:
> 
> "The value of this property SHOULD be the same for all Client instances that
> share a unique bus name, and SHOULD include all channels that are being
> handled, even if they were conceptually handled by a different Client
> instance."

Note that this is a SHOULD (a recommendation), not a MUST (a requirement).

> This requirement is overly onerous and could prove impossible to implement
> using Handler helpers in the case where a user is mixing bindings (e.g. a
> combination of TpHandler and Tp::AbstractClient)

In practice this doesn't happen, because QtDBus and dbus-python use private D-Bus connections, and dbus-java, ndesk-dbus (CLR/Mono/.net) and dbus-ruby are reimplementations. As such, they have a parallel bus connection, with its own unique name.

So, in practice, dbus-glib will only ever share connections with itself or with raw libdbus (or possibly with the Perl and/or PHP D-Bus bindings, I haven't investigated those).

> or mixing helper classes and hand-tooled classes.

That's a valid concern, but I think it'd be acceptable if telepathy-glib violates that recommendation when mixed with lower-level APIs.

> Rob proposes that each Handler should announce a set of channels it thinks it
> is handling and the Channel Dispatcher can union this set together for a unique
> name, as it sees fit.

The intention of that part of the spec was to make it clear that this is how we expect "good" Telepathy applications to behave at a conceptual level. If a client pops up an extra Client name (a transient BypassApproval Handler for text chat associated with the VoIP call it's handling, perhaps), then drops that Client name when it's no longer necessary, it is highly desirable for the Text channel not to be closed by ChannelDispatcher crash-handling.

As a side-effect, if bindings make an effort to meet that recommendation, it's not necessary to have C/C++/... API to "hand off" channels between handlers with the same unique name, which I think is desirable. We want this to "just work" rather than requiring an explicit hand-off.

I don't think implementing that semantic is actually very onerous - telepathy-qt4 does it. All you need is a hidden object per DBusConnection (or equivalently, per (unique name, bus) tuple), tied on with dbus_connection_set_data, which stores a list of TpChannel instances considered to be handled by that unique name; when a channel is invalidated, drop it, and when told about a TpChannel by a TpHandler, add it.

(This is a little harder in telepathy-qt4, which doesn't expose DBusConnection, but still possible by keying a mapping by the "name" of each QDBusConnection.)

On the mailing list you proposed an API like:

         GList *tp_handler_get_handled_channels (TpHandler *);
         tp_handler_handling_channel (TpHandler *, TpChannel *);

However, that can't work between TpHandler instances that live on different unique names, because that would require telling the ChannelDispatcher what you've done (otherwise, its crash handling would close the channel at the wrong time). I don't think it's wise to have C API to pass channels between handlers until the ChannelDispatcher has been enhanced to make that actually work in all cases, and I'll clone this bug to represent that.
Comment 2 Simon McVittie 2009-11-26 04:47:06 UTC
(In reply to comment #1)
> Note that this is a SHOULD (a recommendation), not a MUST (a requirement).

Something I forgot to say in this version of my comment[1]: as a result of this not being a requirement, ChannelDispatcher implementations can't assume it, and so have to look at all the Handler heads and union the results anyway. I believe Mission Control 5 does this right.

[1] I hate the collision between ^W for "delete word" and for "close tab" :-/
Comment 3 Simon McVittie 2009-11-26 04:49:38 UTC
(In reply to comment #1)
> I don't think it's wise to have C API to pass channels between handlers
> until the ChannelDispatcher has been enhanced to make that actually work in all
> cases, and I'll clone this bug to represent that.

That's Bug #25293.
Comment 4 Simon McVittie 2010-06-09 05:50:49 UTC
telepathy-glib and telepathy-qt4 both have helper code that implement this for you, so I think this can now be WONTFIX.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.