[Bug 29218] TpStreamTube - high level stream tube API

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |guillaume.desmottes at collabo
                   |                            |ra.co.uk, sjoerd at luon.net

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-08-06 04:37:54 PDT ---
(In reply to comment #5)
> I'm not convinced by this channel-view approach. That means that in practice
> any application will have to create and prepare an extra object per channel
> when receiving them.

It was initially a subclass; Danielle, Sjoerd and I argued about this in the
office for quite some time, and ended up preferring the "view" design.

The problem with subclassing TpChannel (as is done in telepathy-qt4, for
instance) is that it makes it harder to have application-specific subclasses.
If you want to make (say) a MaemoTpChannel with extra stuff bolted on (see
librtcom-telepathy-glib for motivation), you end up needing to duplicate the
class hierarchy.

Because GObject doesn't do multiple inheritance, you'd also end up with
MaemoTextChannel and MaemoTubeChannel (or whatever) not having a common
ancestor other than TpChannel (mitigated by the fact that you could give them a
common GInterface).

Another problem with subclassing TpChannel is that everything that constructs a
TpChannel of unspecified type behind the scenes will need some sort of channel
factory which knows about all the TpChannel subclasses, which damages the
ability to add channel types in a loosely-coupled way.

If we add a channel factory, we need to be careful to have it not be a piece of
global state. If two components of the same app both try to set the channel
factory to give them their favourite component-specific subclasses, we need to
avoid having one of them be overridden by the other and not get the subclass it
asked for. As usual, the nastiest case is two independent Telepathy plugins
dlopened by a non-Telepathy app.

A further problem is that it's easy to get your assumptions violated by not
having enough information at construction time. If you assume that every Text
channel will be a TpTextChannel, and you ever create a TpChannel whose type
isn't known yet, then you lose.

TpChannelView avoids that by requiring that the TpChannel has the core feature
prepared, i.e. it knows its own type already.

As Danielle described it at the time (I haven't actually reviewed this branch),
TpStreamTube doesn't do async preparation anyway, so the "double preparation"
situation doesn't arise - you do make an async call to offer/accept the tube,
but that doesn't really make sense to turn into a Feature, so it's unavoidable.

Some unfortunate cases where we *would* have the double-preparation you mention
are when TpTextChannelView downloads the pending message queue, or when
TpCallChannelView gets the initial call state.

> So, why not create subclass of TpChannel for high level API? We could add API
> to TpBaseClient to say we are interested in those and even API to pass our own
> factory if we want to create crazy subclass ourself.

One interesting approach might be to have each place that creates channels
behind the scenes take a GType as a parameter. If you already know you want
stream tubes, tell it TP_TYPE_STREAM_TUBE_CHANNEL; if you're handling channels
in a generic way, TP_TYPE_CHANNEL should be sufficient.

(Or, we could define an object or callback that is a channel factory, and use
that, but that requires that the factory knows about every subclass, which
makes me sad from a loosely-coupled-objects point of view.)

TpBaseClient would have to do this via tp_base_client_set_channel_gtype (TpBC
*, GType), but that's not a problem; subclasses of TpBaseClient could call it
in constructed(), and TpSimpleHandler etc. could let the caller call it at the
same time that they add their channel filters.

Where do we make new channels behind the scenes? At the moment I think it's:

* TpBaseClient - easy, add another method as above
* TpChannelDispatchOperation when it fetches the Channels D-Bus property - type
or factory could be passed through from TpBaseClient when using one, but if you
create a TpChannelDispatchOperation yourself, you're on your own

and we'll soon add:

* Guillaume's channel-requesting API from Bug #13422, which is secretly a
TpBaseClient internally

If we want to go back to subclassing, having weighed up advantages and
disadvantages, I wonder whether we need the API in Bug #13422 to be
encapsulated in an object (rather than being a standalone async call), so that
we have an object on which to tweak "preferences" like which GType you want to
end up with (without adding a million parameters to the initial async call,
which doesn't scale).

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