[Bug 29973] TpClientChannelFactory interface and TpDefaultChannelFactory

[bugzilla-daemon at freedesktop.org]

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

--- Comment #19 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-10-11 02:33:16 PDT ---
(In reply to comment #18)
> > +tp_base_client_set_channel_factory
> > +tp_base_client_get_channel_factory
> 
> Hmm. Does this mean that if you want to have control over both TpChannel and,
> say, TpChannelDispatchOperation, you're going to have to call both
> set_channel_factory and set_cdo_factory?

Humm, we could have a set_proxy_factory that will be used for all proxy
creation in TpBaseClient. But that implies that if we add more type of proxy
later, we'll have to check that the factory implement the new interface before
using it (which is fine). set_factory will take a GObject * (or gpointer?) as
argument.

If you think it's fine, then I'm happy to do the change. I think it makes sense
actually, custom factory can easily implement more than one interface if
needed.

> > + * The current version of #TpAutomaticProxyFactory guarantees to create the
> > + * following objects:
> > + *  - if channel is of type TP_IFACE_CHANNEL_TYPE_STREAM_TUBE, a
> > + *  #TpStreamTubeChannel
> > + *  - for all the other channel types, a #TpChannel
> 
> The constant should have a %.

fixed.

> This is a little vague about what happens now vs. what we guarantee in future.
> Perhaps this:
> 
>     #TpAutomaticProxyFactory will currently create #TpChannel objects
>     as follows:
> 
>     - a #TpStreamTubeChannel, if the channel is of type
>       %TP_IFACE_CHANNEL_TYPE_STREAM_TUBE;
>     - a plain #TpChannel, otherwise
> 
>     It is guaranteed that the objects returned by future versions
>     will be either the class that is currently used, or a more specific
>     subclass of that class.
> 
>     TpProxy subclasses other than TpChannel are not currently supported.

updated.

> Should we have a tp_automatic_proxy_factory_dup()? I expect it'll be quite
> commonly-used.

Should I replace _new() by _dup() and so define this object as a singleton? I
guess that's ok

> I think there should be a "see also" link to #TpBasicProxyFactory, and vice
> versa.

done.

> TpBasicProxyFactory should indicate its intended future behaviour:
> 
>     This factory implements the #TpClientChannelFactory interface to create
>     plain #TpChannel objects. Unlike #TpAutomaticProxyFactory, it will
>     not create higher-level subclasses like #TpStreamTubeChannel.
> 
>     TpProxy subclasses other than TpChannel are not currently supported.

done.

> > +struct _TpClientChannelFactoryInterface {
> > +    GTypeInterface parent;
> > ...
> > +    /*<private>*/
> > +    GCallback _padding[7];
> 
> The GObject people tell me padding isn't actually necessary for a GInterface,
> because they're assumed to be variable-size.

removed.

> >  #include <telepathy-glib/stream-tube-channel.h>
> >  #include <telepathy-glib/stream-tube-connection.h>
> > +#include <telepathy-glib/client-channel-factory.h>
> > +#include <telepathy-glib/automatic-proxy-factory.h>
> > +#include <telepathy-glib/basic-proxy-factory.h>
> 
> I'd prefer these in alphabetical order.

sorted.

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