[Bug 77189] [next] make TpBaseConnection GVariant-based

[bugzilla-daemon at freedesktop.org]

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

--- Comment #30 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
(In reply to comment #29)
>  - 0b57d9e66ca25a5e6684751a23a88b91e719e3c6 can be removed since it's
> replaced by 22ed228270e7e29e7ede985a198d5c675464be51

I don't think an extra static assertion hurts, tbh.

>  - I have to admit I'm totally lost with TpDBusPropertiesMixin.

It implements the properties of all TpSvc* interfaces. It does not implement
the properties of non-TpSvc* interfaces.

Here is how it normally works:

* method call comes in
* dispatched to a GDBusInterfaceSkeleton
* is it a TpSvcInterfaceSkeleton?
  - yes:
    * is it a property Get/Set/GetAll?
      - yes:
        * does it have a TpDBusPropertiesMixin?
          - yes: call into it
          - no: method call fails
      - no: call into TpSvcWhateverClass vfuncs
  - no: call into GDBusInterfaceSkeleton vfuncs

If the object implements TpSvcDBusProperties - for which the only remaining use
is McdDBusProp - then that whole logic is bypassed and we use the
TpSvcDBusProperties vtable instead. In the case of McdDBusProp, the object
doesn't have a TpDBusPropertiesMixin either.

>  - I have to admit I'm totally lost with TpDBusPropertiesMixin. We never
> implement TpSvcDBusProperties anymore, right?

Wrong. Mission Control has its own implementation of TpSvcDBusProperties,
because it wants to be able to set properties that are read-only at the D-Bus
level, at (or in fact just after) construct time, as if they were
read/write/construct-only, by using their D-Bus names (and without duplicating
their setter logic).

Eventually, I would like MC to do something better than McdDBusProp, but now is
not the time.

I think TpSvcDBusProperties should be a normal interface (for symmetry with its
client-side equivalent, if nothing else) but nothing in telepathy-glib should
implement it, and eventually, nothing in *Telepathy* should implement it.

> We could use the "invisible mixin" in all CMs to kill
> tp_dbus_properties_mixin_class_init() and all code paths where offset!=0?
> I'm happier now that it is in -dbus because it should really be considered
> deprecated and we should try to remove it completely from all CMs on the
> long term.

Feel free to clone a bug. I think I might actually have eradicated offset != 0
while updating the CMs (it seemed cleaner to use the invisible mixin than to
include telepathy-glib-dbus.h in their headers) but this is way off the
critical path for a clean and future-proof libtelepathy-glib-1.so API.

>  - tbh, I don't really like TpPreseceMixin using qdata to find the skeleton,
> I was happy to remove that kind of hack from TpBaseContactList. But if you
> prefer that way, I've no strong argument to stop you from doing it. Note
> that the TpBaseConnectionPrivate struct can move back to the .c instead of
> -internal.h now if you want.

I do prefer that: I think it's better than having individual interfaces' mixins
tightly-coupled to the base class. In particular, if we release 1.0 with an
unintrospectable TpPresenceMixin, we can deprecate it and add TpPresenceMixin2
without leaving detritus in TpBaseConnection.

Yes, I would like to move the private struct back to the .c at some point, but
it isn't critical-path.

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