TpBaseConnection: allow interfaces to be added in status-changed

This is necessary for Gabble's use of TpBaseContactList: it wants to
add the Blocking interface on connections to Google servers (or
in principle, servers implementing XEP-0016, XEP-0159, XEP-0161
or XEP-0191, if someone adds a client-side implementation of those).

Also delay _tp_gdbus_connection_set_status() until just before we
emit StatusChanged on D-Bus, so that we follow this good pattern:

* update internal state, so that we will not give contradictory answers
  if a signal handler calls accessors
* emit GLib signals, so that in-process stuff gets a chance
  to react before we signal changes to D-Bus (for instance, adding
  interfaces)
* finally, update D-Bus properties and emit D-Bus signals

Reviewed-by: Xavier Claessens <xavier.claessens@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=77189

2 files changed, 17 insertions, 3 deletions

diff --git a/telepathy-glib/base-connection-internal.h b/telepathy-glib/base-connection-internal.h
index 2062d2b27..aff75abf5 100644
--- a/telepathy-glib/base-connection-internal.h
+++ b/telepathy-glib/base-connection-internal.h
@@ -73,6 +73,9 @@ struct _TpBaseConnectionPrivate
   gboolean been_constructed;
   /* TRUE if on D-Bus */
   gboolean been_registered;
+  /* TRUE if CONNECTED has been signalled to D-Bus, so it's too late to
+   * manipulate the list of interfaces */
+  gboolean been_connected;
 
   /* g_strdup (unique name) => owned ClientData struct */
   GHashTable *clients;
diff --git a/telepathy-glib/base-connection.c b/telepathy-glib/base-connection.c
index 76cd50f53..bde77e6a8 100644
--- a/telepathy-glib/base-connection.c
+++ b/telepathy-glib/base-connection.c
@@ -658,7 +658,7 @@ tp_base_connection_interface_changed_cb (TpBaseConnection *self,
 
   g_assert (user_data == INTERFACE_ADDED || user_data == INTERFACE_REMOVED);
 
-  if (self->priv->status == TP_CONNECTION_STATUS_CONNECTED)
+  if (self->priv->been_connected)
     {
       WARNING ("Adding or removing Connection interfaces after CONNECTED "
           "status has been reached is not supported. "
@@ -1810,11 +1810,22 @@ tp_base_connection_change_status (TpBaseConnection *self,
     }
 
   DEBUG("emitting status-changed to %u, for reason %u", status, reason);
-  _tp_gdbus_connection_set_status (self->priv->connection_skeleton, status);
+
   /* Emit status-changed before sending the D-Bus signal, because in practice
    * that's what happened in telepathy-glib 0.x, as demonstrated by Gabble's
-   * regression tests failing otherwise. */
+   * regression tests failing otherwise. Also, connection manager modules
+   * (in particular the TpBaseContactList) need this opportunity to
+   * staple on additional interfaces when we go CONNECTED. */
   g_signal_emit (self, signals[STATUS_CHANGED], 0, status, reason);
+
+  /* Handlers for the status-changed signal is the last possible point at
+   * which we are allowed to add interfaces. Once we've signalled CONNECTED
+   * to D-Bus, it's too late. */
+  if (status == TP_CONNECTION_STATUS_CONNECTED)
+    priv->been_connected = TRUE;
+
+  /* Tell D-Bus what we've done. */
+  _tp_gdbus_connection_set_status (self->priv->connection_skeleton, status);
   _tp_gdbus_connection_emit_status_changed (self->priv->connection_skeleton,
       status, reason);