[Bug 76111] [next] Make TpProxy subclasses API more coherent

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> ---
(In reply to comment #2)
> 1) In next it would be nice to have the factory as the only top-level
> singleton. That means that we could replace tp_a_m_dup() by
> tp_client_factory_dup() + tp_client_factory_dup_am(). I think we would need
> tp_client_factory_set_default() for apps that want to create their factory
> subclass and set it as default. Maybe we can keep tp_a_m_dup() as helper
> that dup the default factory then dup the factory's am.

Agreed, I never liked having 2 kind of top level objects anyway.

> 2) In next it would be nice if TpProxy:factory becomes a mandatory
> construct-only property. tp_proxy_constructed() should assert that's
> non-null and app can then rely on tp_proxy_get_factory() to never return
> NULL. That doesn't mean we want public tp_client_factory_dup_foo() for
> everything. For example TpCallContent is created only internally by
> TpCallChannel so _tp_call_content_new() can simply pass the channel's
> factory to g_object_new().

I like the idea but smcv seemed to be keen on still being able to create
proxies behind the factory's back.

> 3) dup_features() vfuncs are probably useless. TpClientFactory subclasses
> could just call _add_foo_features() from its _constructed() method. One
> rational for those vfunc is that TpTextChannel doesn't need to get
> TpCallChannel features. But TpProxy will probably nicely filter that nicely
> anyway.

Not sure. Maybe?

> 4) Some create() vfunc could be retracted. Their only purpose is to let
> create subclasses, and in practice we do that only for TpChannel. Maybe we
> should just forget about subclassing TpAccount, TpConnection and TpContact
> since nobody did it anyway and that would simplify a bit of code.

I guess if we let enough padding we could always add them later if needed. I
suspect MC is the only who may ever need subclassing something else than
channels.

> (In reply to comment #0)
> > Created internally by tp-glib without using TpClientFactory
> > -----------------------------------------------------------
> > - TpCallContent
> > - TpCallStream
> 
> Uniqueness is already guaranteed by TpCallChannel, so it's fine like that. A
> trivial patch (even in master) would be to pass the channel's factory in the
> g_object_new() properties to get closer to the goal "every TpProxy has a
> factory".

Good point. I'll do that.

> > - TpClient: actually it doesn't seem to be instantiated at all by tp-glib,
> > only in its tests
> 
> Not even sure why we have that in tp-glib API at all, maybe it should be
> moved to MC since it's the only user. I think MC will always be the special
> case where relying on the lowlevel dbus API will always be needed anyway.

Because we generate low level D-Bus API for those and so need a TpProxy
subclass.

> > Can be created by user using a specific API
> > -------------------------------------------
> > - TpProtocol: but bug#75881 is going to move it to the factory
> > - TpTLSCertificate: I guess it could use the factory instead
> 
> I think those need public API to be created from a factory. No need of vfunc
> for them though.

Agreed.

> > Should these 2 use the factory? I guess we could?
> > - TpChannelDispatcher
> 
> Do we need TpChannelDispatcher constructor to be public? I would consider
> that object to be created internally in tp-glib only, no?

Clients may need it to use tp_channel_dispatcher_present_channel_async().

> > - TpLogger
> 
> Same case as TpAM, I would have tp_client_factory_dup_logger() and maybe
> keep tp_logger_dup() as an helper that call tp_client_factory_dup_logger()
> on the default factory.

Agreed.

> > - TpConnectionManager: as I said on bug#75881 the connection manager is not
> > considered as a "top level object". Maybe it should be?
> 
> The factory could gain a CM-name -> TpConnectionManager table just like you
> did for TpProtocol.

I guess yeah.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.