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

[bugzilla-daemon at freedesktop.org]

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

--- Comment #8 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
(In reply to comment #7)
>  - tp_dbus_connection_try_register_object() is starting to be really long
> and pretty sequential. I would like to factor out at least 3 functions:
> dup_skeletons_from_dbus_object(); dup_skeletons_from_svc_interfaces(); and
> export_skeletons().

Fine, I'll do that.

>  - _register_object should check for GDBusObjectSkeleton instead of just
> GDBusObject iface. You're not going to get GDBusInterfaceSkeletons out of a
> GDBusObjectProxy. I would warn early if it's not a skeleton instead of
> pretending everything is fine.

Rationale: if we check for it being a GDBusObject containing
GDBusInterfaceSkeletons instead of a GDBusObjectSkeleton, then it is easy to
use a reimplementation of the GDBusObject interface.

Possible uses include:

* an object that has-a (as opposed to is-a) GDBusObjectSkeleton, which can
easily implement GDBusObject by proxing the GDBusObjectSkeleton's interface

* retrofitting GDBusObject functionality onto an object whose superclass cannot
change without breaking ABI

I think I'd be OK with adding a g_return_if_fail (!G_IS_DBUS_OBJECT (obj) ||
G_IS_DBUS_OBJECT_SKELETON (obj)) for now, so that every GDBusObject must also
be a GDBusObjectSkeleton, but leaving the underlying implementation capable of
dealing with any GDBusObject<GDBusInterfaceSkeleton> so that it's easy to relax
the check in future. Does that sound OK?

>  - "TpSvcInterfaceSkeleton: move to the -dbus library" --> Hmm, that seems
> pretty complex juggling with the properties mixin. I would move the whole
> TpDBusPropertiesMixin to -core since ultimately we won't need it.

When we spoke about this on IRC I said "yes, good point, we can do that".
Unfortunately, now I've remembered why I did it this way: TpDBusPropertiesMixin
depends on TP_ERROR, which is a symbol in the main library.

I would prefer not to have to bring back the -core library (3-level hierarchy)
- there is a nonzero cost to library proliferation, and two levels ought to be
enough.

We could just change the errors to be in the org.freedesktop.DBus.Error domain
(probably InvalidArgs like GDBus uses), and patch the CMs' and MC's tests to
cope - would that be OK?

>  - _tp_g_dbus_object_dup_interface_names(): I would take a varargs of
> interfaces to ignore (or const gchar * const *ignore). Having just 2 is
> arbitrary and "skip_type" is fine for a channel but TpBaseConnection abuse
> of it for Requests.

Yeah, that did occur to me. I'll do that.

>  - "tp_base_connection_change_status: emit status-changed before
> StatusChanged": Yes, and the update_rcc_property(self); should be moved
> earlier as well for the same reason.

OK.

>  - "tp_base_connection_set_self_handle: consolidate GObject setters": I
> don't think it is needed, the gdbus-codegen emits PropertiesChanged in an
> idle callback and aggregates changes. It's a bit complex with threading,
> though... do you confirm the codegen is fine for our usage?

No idea. I'll look.

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