Bug 31001 - channel client factory: add API for TpChannel preparation
Summary: channel client factory: add API for TpChannel preparation
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Guillaume Desmottes
QA Contact: Telepathy bugs list
URL: http://git.Collabora.co.uk/?p=user/ca...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2010-10-20 05:24 UTC by Guillaume Desmottes
Modified: 2010-10-26 04:13 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Guillaume Desmottes 2010-10-20 05:24:35 UTC
On bug #29973 we considered 2 options regarding preparations of extra features:

A) Add a prepare async API on TpClientChannelFactoryInterface doing the preparation of a TpChannel

b) Add API to get a list of features to be prepared and let the user (TpBaseClient basically) doing the preparation.

The consensus seems to go for B) as TpBaseClient already has code to prepare the channels.
If that's ok I'll do the following changes:

-  Add GQuark *features tp_client_channel_factory_get_features (TpClientChannelFactoryInterface *self, TpChannel *self) implemented using a virtual function returning CORE as default implementation.
Or maybe we should return a more flexible structur to make it easier to merge it with TpBaseClient's features?

- Modify TpBaseClient to prepare the union of its features and the ones returned by _get_features(). I guess that's fine if we ask to prepare twice the same feature.

- Implement _get_features () inTpAutomaticProxyFactory  by returning CORE + TP_CHANNEL_FEATURE_GROUP if the channel implements Group.

- I'm not sure if we want to add API on TpClientChannelFactoryInterface to add features? It seems weird to add features on a factory which already created channel. And clients can still add feature on TpBaseClient or create their own factory if needed.
Comment 1 Simon McVittie 2010-10-20 05:42:25 UTC
(In reply to comment #0)
> -  Add GQuark *features tp_client_channel_factory_get_features
> (TpClientChannelFactoryInterface *self, TpChannel *self) implemented using a
> virtual function returning CORE as default implementation.
> Or maybe we should return a more flexible structur to make it easier to merge
> it with TpBaseClient's features?

I'm not sure what structure we'd return; perhaps a GArray?

> - Modify TpBaseClient to prepare the union of its features and the ones
> returned by _get_features(). I guess that's fine if we ask to prepare twice the
> same feature.

It should be OK, yes; preparing one "batch" which may contain duplicates is probably a little more efficient than two overlapping "batches".

> - Implement _get_features () inTpAutomaticProxyFactory  by returning CORE +
> TP_CHANNEL_FEATURE_GROUP if the channel implements Group.

IMO there's no need for this: just include TP_CHANNEL_FEATURE_GROUP unconditionally, and have the guarantee be "well, we tried".

> - I'm not sure if we want to add API on TpClientChannelFactoryInterface to add
> features? It seems weird to add features on a factory which already created
> channel. And clients can still add feature on TpBaseClient or create their own
> factory if needed.

Yes, I think leaving this out is the conservative route. You could always have an EmpathyProxyFactory subclass of TpAutomaticProxyFactory which just overrides this method, if you wanted.
Comment 3 Simon McVittie 2010-10-21 06:11:18 UTC
> + * Once a channel has be created by a factory using

has been created

> + * Returns: (transfer container): a newly allocated #GArray

I think containers of integers are usually said to be "transfer full", since for non-freeable contents it's equivalent to "transfer container" anyway, and "full" is simpler to explain.

I think the name should say _dup_ (and the same for the underlying virtual method).

> +array_contain_feature (GArray *features,

array_contains_feature, and @features should be const
Comment 4 Guillaume Desmottes 2010-10-21 06:25:08 UTC
All fixed.
Comment 5 Simon McVittie 2010-10-21 06:27:27 UTC
go go go
Comment 6 Guillaume Desmottes 2010-10-21 06:29:10 UTC
Merged to master; will be in 0.13.3


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.