[Bug 48210] Could TpBaseChannel be more flexible?

[bugzilla-daemon at freedesktop.org]

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

--- Comment #10 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2012-06-04 09:37:49 PDT ---
> + * // destoyed() must have been called; forget this channel

"destroyed"

reopened[_with_requested] needs to re-register the object on D-Bus immediately
after emitting closed, I think:

  if (!self->priv->registered)
    tp_base_channel_register (self);

Otherwise, it can't implement any methods during its second life, and any
signals that it emits during its second life will not reach D-Bus.

This is sufficiently subtle that I would like a regression test in
telepathy-glib.

+ * channel_closed_cb (TpBaseChannel *chan,
+ * TpChannelManager *manager)
+ * {
+ * MyChannelManager *self = MY_CHANNEL_MANAGER (manager);
+ * TpHandle handle = tp_base_channel_get_target_handle (chan);
+ *
+ * tp_channel_manager_emit_channel_closed (manager,
+ * TP_EXPORTABLE_CHANNEL (chan));

I think this also emits ChannelClosed twice in this situation, which isn't
hugely harmful but seems non-ideal:

disappear()
    -> emits closed
        -> dbus-glib handler emits Closed on D-Bus
        -> channel manager handler emits ChannelClosed on D-Bus
    -> unregisters from bus

reopened()
    -> emits closed
        -> no dbus-glib handler because it was unregistered and has
           not yet re-registered
        -> channel manager handler emits ChannelClosed on D-Bus again (!)
        -> channel manager emits NewChannel, NewChannels
    -> synchronously (so you can't miss any messages)
       re-registers on bus (when you implement that)

Two possible solutions, depending how much subtlety you're OK with piling onto
the channel manager:

* require the channel manager to do:

    if (tp_base_channel_is_registered)
      tp_channel_manager_emit_channel_closed (...)

* make tp_channel_manager_emit_channel_closed check for
  TP_IS_BASE_CHANNEL and, if it is, suppress the signal

The former is more subtlety in CMs, the latter is arguably a layering
violation.

For next, I'd welcome patches to move some of the sublety into the
TpExportableChannel GInterface: perhaps it could grow channel-destroyed,
channel-disappeared and channel-reopened signals, and TpChannelManager could
become responsible for emitting the channel's D-Bus closed signal at the same
time it emits ChannelClosed?

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