muc-manager: only expose the text MUC channel when necessary

Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk>

5 files changed, 193 insertions, 82 deletions

diff --git a/src/muc-channel.c b/src/muc-channel.c
index 2abc5907..eff9f72e 100644
--- a/src/muc-channel.c
+++ b/src/muc-channel.c
@@ -77,6 +77,7 @@ enum
     READY,
     JOIN_ERROR,
     NEW_TUBE,
+    APPEARED,
     LAST_SIGNAL
 };
 
@@ -103,6 +104,8 @@ struct _SalutMucChannelPrivate
   /* (gchar *) -> (SalutContact *) */
   GHashTable *senders;
 
+  gboolean autoclose;
+
   GHashTable *tubes;
 };
 
@@ -713,6 +716,15 @@ salut_muc_channel_class_init (SalutMucChannelClass *salut_muc_channel_class)
        * wants a value type, not an interface. */
       G_TYPE_NONE, 1, TP_TYPE_BASE_CHANNEL);
 
+  signals[APPEARED] = g_signal_new (
+      "appeared",
+      G_OBJECT_CLASS_TYPE (salut_muc_channel_class),
+      G_SIGNAL_RUN_LAST,
+      0,
+      NULL, NULL,
+      g_cclosure_marshal_VOID__VOID,
+      G_TYPE_NONE, 0);
+
   tp_group_mixin_class_init (object_class,
       G_STRUCT_OFFSET(SalutMucChannelClass, group_class),
       salut_muc_channel_add_member, NULL);
@@ -1257,6 +1269,17 @@ salut_muc_channel_received_stanza (GibberMucConnection *conn,
       return;
     }
 
+  /* are we actually hidden? */
+  if (!tp_base_channel_is_registered (base_chan))
+    {
+      DEBUG ("making MUC channel reappear!");
+      tp_base_channel_reopened_with_requested (base_chan, FALSE, from_handle);
+      g_signal_emit (self, signals[APPEARED], 0);
+    }
+
+  /* let's not autoclose now */
+  priv->autoclose = FALSE;
+
 #ifdef ENABLE_OLPC
   if (salut_connection_olpc_observe_muc_stanza (
           SALUT_CONNECTION (base_connection),
@@ -1364,11 +1387,43 @@ salut_muc_channel_disconnected (GibberTransport *transport, gpointer user_data)
   tp_base_channel_destroyed (TP_BASE_CHANNEL (self));
 }
 
+gboolean
+salut_muc_channel_can_be_closed (SalutMucChannel *self)
+{
+  if (self->priv->tubes == NULL)
+    return TRUE;
+
+  return (g_hash_table_size (self->priv->tubes) == 0);
+}
+
+gboolean
+salut_muc_channel_get_autoclose (SalutMucChannel *self)
+{
+  return self->priv->autoclose;
+}
+
+void
+salut_muc_channel_set_autoclose (SalutMucChannel *self,
+    gboolean autoclose)
+{
+  self->priv->autoclose = autoclose;
+}
+
 static void
 salut_muc_channel_close (TpBaseChannel *base)
 {
   SalutMucChannel *self = SALUT_MUC_CHANNEL (base);
 
+  /* if we have some tubes around then don't close yet and just
+   * disappear from the bus, faking having closed, otherwise
+   * cheerio! */
+  if (!salut_muc_channel_can_be_closed (self))
+    {
+      self->priv->autoclose = TRUE;
+      tp_base_channel_disappear (base);
+      return;
+    }
+
   salut_muc_channel_leave (self, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, "");
 
   tp_base_channel_destroyed (base);
diff --git a/src/muc-channel.h b/src/muc-channel.h
index f5992768..868e7af4 100644
--- a/src/muc-channel.h
+++ b/src/muc-channel.h
@@ -100,6 +100,13 @@ void salut_muc_channel_bytestream_offered (SalutMucChannel *self,
     GibberBytestreamIface *bytestream,
     WockyStanza *msg);
 
+void salut_muc_channel_set_autoclose (SalutMucChannel *chan,
+    gboolean autoclose);
+
+gboolean salut_muc_channel_get_autoclose (SalutMucChannel *chan);
+
+gboolean salut_muc_channel_can_be_closed (SalutMucChannel *chan);
+
 gboolean salut_muc_channel_is_ready (SalutMucChannel *self);
 
 G_END_DECLS
diff --git a/src/muc-manager.c b/src/muc-manager.c
index ca28004b..cf9c94b7 100644
--- a/src/muc-manager.c
+++ b/src/muc-manager.c
@@ -99,6 +99,9 @@ struct _SalutMucManagerPrivate
 #define SALUT_MUC_MANAGER_GET_PRIVATE(obj) \
   ((SalutMucManagerPrivate *) ((SalutMucManager *) obj)->priv)
 
+#define TUBE_TEXT_QUARK (g_quark_from_static_string (\
+          "salut-muc-manager-tube-text-channel"))
+
 static void
 salut_muc_manager_init (SalutMucManager *obj)
 {
@@ -412,12 +415,17 @@ muc_channel_closed_cb (SalutMucChannel *chan,
 {
   SalutMucManager *self = SALUT_MUC_MANAGER (user_data);
   SalutMucManagerPrivate *priv = SALUT_MUC_MANAGER_GET_PRIVATE (self);
+  TpBaseChannel *base = TP_BASE_CHANNEL (chan);
   TpHandle handle;
 
-  tp_channel_manager_emit_channel_closed_for_object (self,
-      TP_EXPORTABLE_CHANNEL (chan));
+  if (tp_base_channel_is_registered (base))
+    {
+      tp_channel_manager_emit_channel_closed_for_object (self,
+          TP_EXPORTABLE_CHANNEL (chan));
+    }
 
-  if (priv->text_channels)
+  if (tp_base_channel_is_destroyed (base)
+      && priv->text_channels)
     {
       g_object_get (chan, "handle", &handle, NULL);
       DEBUG ("Removing channel with handle %u", handle);
@@ -430,8 +438,27 @@ static void
 muc_channel_tube_closed_cb (SalutTubeIface *tube,
     SalutMucManager *mgr)
 {
+  SalutMucChannel *channel;
+
   tp_channel_manager_emit_channel_closed_for_object (mgr,
       TP_EXPORTABLE_CHANNEL (tube));
+
+  channel = g_object_get_qdata (G_OBJECT (tube), TUBE_TEXT_QUARK);
+  g_assert (channel != NULL);
+
+  if (salut_muc_channel_can_be_closed (channel)
+      && salut_muc_channel_get_autoclose (channel))
+    {
+      tp_base_channel_close (TP_BASE_CHANNEL (channel));
+    }
+}
+
+static void
+muc_channel_appeared_cb (SalutMucChannel *chan,
+    SalutMucManager *mgr)
+{
+  tp_channel_manager_emit_new_channel (mgr,
+      TP_EXPORTABLE_CHANNEL (chan), NULL);
 }
 
 static void
@@ -444,6 +471,9 @@ muc_channel_new_tube_cb (SalutMucChannel *channel,
 
   g_signal_connect (tube, "closed",
       G_CALLBACK (muc_channel_tube_closed_cb), mgr);
+
+  g_object_set_qdata (G_OBJECT (tube), TUBE_TEXT_QUARK,
+      channel);
 }
 
 static GibberMucConnection *
@@ -463,14 +493,17 @@ muc_channel_ready_cb (SalutMucChannel *chan,
     SalutMucManager *mgr)
 {
   SalutMucManagerPrivate *priv = SALUT_MUC_MANAGER_GET_PRIVATE (mgr);
-  GSList *satisfied;
   GSList *tube_channels;
   GSList *l;
 
-  /* announce the text channel finally */
-  satisfied = g_hash_table_lookup (priv->queued_requests, chan);
-  tp_channel_manager_emit_new_channel (mgr,
-      TP_EXPORTABLE_CHANNEL (chan), satisfied);
+  /* announce the text channel finally, but only if it is on the bus */
+  if (tp_base_channel_is_registered (TP_BASE_CHANNEL (chan)))
+    {
+      GSList *satisfied = g_hash_table_lookup (priv->queued_requests, chan);
+
+      tp_channel_manager_emit_new_channel (mgr,
+          TP_EXPORTABLE_CHANNEL (chan), satisfied);
+    }
   g_hash_table_remove (priv->queued_requests, chan);
 
   /* announce tube channels now */
@@ -538,7 +571,8 @@ salut_muc_manager_new_muc_channel (SalutMucManager *mgr,
                                    GibberMucConnection *connection,
                                    TpHandle initiator,
                                    gboolean new_connection,
-                                   gboolean requested)
+                                   gboolean requested,
+                                   gboolean initially_register)
 {
   SalutMucManagerPrivate *priv = SALUT_MUC_MANAGER_GET_PRIVATE(mgr);
   TpBaseConnection *base_connection = TP_BASE_CONNECTION(priv->connection);
@@ -563,10 +597,12 @@ salut_muc_manager_new_muc_channel (SalutMucManager *mgr,
       new_connection, requested);
   g_free (path);
 
-  tp_base_channel_register ((TpBaseChannel *) chan);
+  if (initially_register)
+    tp_base_channel_register ((TpBaseChannel *) chan);
 
   g_signal_connect (chan, "closed", G_CALLBACK (muc_channel_closed_cb), mgr);
   g_signal_connect (chan, "new-tube", G_CALLBACK (muc_channel_new_tube_cb), mgr);
+  g_signal_connect (chan, "appeared", G_CALLBACK (muc_channel_appeared_cb), mgr);
 
   g_hash_table_insert (priv->text_channels, GUINT_TO_POINTER (handle), chan);
 
@@ -659,13 +695,17 @@ salut_muc_manager_request_new_muc_channel (SalutMucManager *mgr,
 
   text_chan = salut_muc_manager_new_muc_channel (mgr, handle,
       connection, base_connection->self_handle, params == NULL,
-      requested);
+      requested, announce);
   r = salut_muc_channel_invited (text_chan,
         base_connection->self_handle, NULL, NULL);
   /* Inviting ourselves to a connected channel should always
    * succeed */
   g_assert (r);
 
+  /* only signal the creation of the text channel if we're asked for
+   * it to happen. note that in the case of announce=FALSE,
+   * muc_channel_ready_cb will still get fired, but because it isn't
+   * registered on the bus, it will not be signalled then either. */
   if (announce)
     {
       if (salut_muc_channel_is_ready (text_chan))
@@ -699,9 +739,6 @@ handle_tube_channel_request (SalutMucManager *self,
   SalutMucManagerPrivate *priv = SALUT_MUC_MANAGER_GET_PRIVATE (self);
   SalutMucChannel *text_chan;
   SalutTubeIface *new_channel;
-  GHashTable *channels;
-  GSList *request_tokens;
-  gboolean announce_text = FALSE;
 
   text_chan = g_hash_table_lookup (priv->text_channels,
       GUINT_TO_POINTER (handle));
@@ -712,8 +749,6 @@ handle_tube_channel_request (SalutMucManager *self,
       text_chan = salut_muc_manager_request_new_muc_channel (self,
           handle, NULL, FALSE, error);
 
-      announce_text = TRUE;
-
       if (text_chan == NULL)
         return FALSE;
     }
@@ -725,19 +760,29 @@ handle_tube_channel_request (SalutMucManager *self,
   g_signal_connect (new_channel, "closed",
       G_CALLBACK (muc_channel_tube_closed_cb), self);
 
-  /* announce channels */
-  channels = g_hash_table_new_full (g_direct_hash, g_direct_equal,
-      NULL, NULL);
+  g_object_set_qdata (G_OBJECT (new_channel), TUBE_TEXT_QUARK,
+      text_chan);
 
-  if (announce_text)
-    g_hash_table_insert (channels, text_chan, NULL);
+  if (salut_muc_channel_is_ready (text_chan))
+    {
+      GSList *request_tokens = g_slist_append (NULL, request_token);
+
+      tp_channel_manager_emit_new_channel (self,
+          TP_EXPORTABLE_CHANNEL (new_channel), request_tokens);
 
-  request_tokens = g_slist_prepend (NULL, request_token);
-  g_hash_table_insert (channels, new_channel, request_tokens);
-  tp_channel_manager_emit_new_channels (self, channels);
+      g_slist_free (request_tokens);
+    }
+  else
+    {
+      associate_channel_to_data (priv->text_needed_for_tube,
+          text_chan, new_channel);
+
+      /* we need to do this so when the muc channel is ready, the tube
+       * can be announced and satisfy this request */
+      associate_channel_to_data (priv->queued_requests,
+          new_channel, request_token);
+    }
 
-  g_hash_table_unref (channels);
-  g_slist_free (request_tokens);
   return TRUE;
 }
 
@@ -844,7 +889,8 @@ salut_muc_manager_request (SalutMucManager *self,
 
       if (text_chan != NULL)
         {
-          if (require_new)
+          if (require_new
+              && tp_base_channel_is_registered (TP_BASE_CHANNEL (text_chan)))
             {
               g_set_error (&error, TP_ERROR, TP_ERROR_NOT_AVAILABLE,
                   "That channel has already been created (or requested)");
@@ -852,8 +898,28 @@ salut_muc_manager_request (SalutMucManager *self,
             }
           else
             {
-              tp_channel_manager_emit_request_already_satisfied (self,
-                  request_token, TP_EXPORTABLE_CHANNEL (text_chan));
+              if (tp_base_channel_is_registered (TP_BASE_CHANNEL (text_chan)))
+                {
+                  tp_channel_manager_emit_request_already_satisfied (self,
+                      request_token, TP_EXPORTABLE_CHANNEL (text_chan));
+                }
+              else
+                {
+                  tp_base_channel_register (TP_BASE_CHANNEL (text_chan));
+
+                  if (salut_muc_channel_is_ready (text_chan))
+                    {
+                      GSList *tokens = g_slist_append (NULL, request_token);
+                      tp_channel_manager_emit_new_channel (self,
+                          TP_EXPORTABLE_CHANNEL (text_chan), tokens);
+                      g_slist_free (tokens);
+                    }
+                  else
+                    {
+                      associate_channel_to_data (priv->queued_requests,
+                          text_chan, request_token);
+                    }
+                }
             }
         }
       else
@@ -1035,7 +1101,7 @@ invite_stanza_callback (WockyPorter *porter,
         }
       /* Need to create a new one */
       chan = salut_muc_manager_new_muc_channel (self, room_handle,
-          connection, inviter_handle, FALSE, FALSE);
+          connection, inviter_handle, FALSE, FALSE, TRUE);
 
       tp_channel_manager_emit_new_channel (self, TP_EXPORTABLE_CHANNEL (chan),
           NULL);
diff --git a/tests/twisted/avahi/tubes/request-muc-tubes.py b/tests/twisted/avahi/tubes/request-muc-tubes.py
index ce54ab35..72ecbb19 100644
--- a/tests/twisted/avahi/tubes/request-muc-tubes.py
+++ b/tests/twisted/avahi/tubes/request-muc-tubes.py
@@ -61,25 +61,17 @@ def test(q, bus, conn):
 
     # text and tube channels are announced
     channels = new_sig.args[0]
-    assert len(channels) == 2
-    got_text, got_tube = False, False
-
-    for path, props in channels:
-        if props[CHANNEL_TYPE] == CHANNEL_TYPE_TEXT:
-            got_text = True
-            assert props[REQUESTED] == False
-        elif props[CHANNEL_TYPE] == CHANNEL_TYPE_STREAM_TUBE:
-            got_tube = True
-            assert path == tube_path
-            assert props == tube_props
-        else:
-            assert False
-
-        assert props[TARGET_HANDLE_TYPE] == HT_ROOM
-        assert props[TARGET_HANDLE] == handle
-        assert props[TARGET_ID] == 'my-second-room'
-        assert props[INITIATOR_HANDLE] == conn.GetSelfHandle()
-        assert props[INITIATOR_ID] == self_name
+    assert len(channels) == 1
+
+    path, props = channels[0]
+    assert props[CHANNEL_TYPE] == CHANNEL_TYPE_STREAM_TUBE
+    assert path == tube_path
+    assert props == tube_props
+    assert props[TARGET_HANDLE_TYPE] == HT_ROOM
+    assert props[TARGET_HANDLE] == handle
+    assert props[TARGET_ID] == 'my-second-room'
+    assert props[INITIATOR_HANDLE] == conn.GetSelfHandle()
+    assert props[INITIATOR_ID] == self_name
 
     # ensure the same channel
 #    yours, ensured_path, ensured_props = conn.Requests.EnsureChannel(
diff --git a/tests/twisted/avahi/tubes/two-muc-stream-tubes.py b/tests/twisted/avahi/tubes/two-muc-stream-tubes.py
index 51a7f880..82696335 100644
--- a/tests/twisted/avahi/tubes/two-muc-stream-tubes.py
+++ b/tests/twisted/avahi/tubes/two-muc-stream-tubes.py
@@ -80,49 +80,32 @@ def test(q, bus, conn):
         TARGET_ID: muc_name,
         STREAM_TUBE_SERVICE: 'test'})
 
-    e = q.expect('dbus-signal', signal='NewChannels',
-                 predicate=lambda e: len(e.args[0]) == 2)
+    e = q.expect('dbus-signal', signal='NewChannels')
     channels = e.args[0]
+    assert len(channels) == 1
 
     # get the list of all channels to check that newly announced ones are in it
     all_channels = conn.Properties.Get(CONN_IFACE_REQUESTS, 'Channels',
         byte_arrays=True)
 
-    got_text, got_tube = False, False
-    for path, props in channels:
-        if props[CHANNEL_TYPE] == CHANNEL_TYPE_TEXT:
-            got_text = True
-            assert props[REQUESTED] == False
-            text1 = wrap_channel(bus.get_object(conn.bus_name, path), 'Text')
-            txt_path = path
-        elif props[CHANNEL_TYPE] == CHANNEL_TYPE_STREAM_TUBE:
-            got_tube = True
-            assert props[REQUESTED] == True
-            assert props[INTERFACES] == [CHANNEL_IFACE_GROUP,
-                CHANNEL_IFACE_TUBE]
-            assert props[STREAM_TUBE_SERVICE] == 'test'
-
-            contact1_tube = wrap_channel(bus.get_object(conn.bus_name, path), 'StreamTube')
-            tube1_path = path
-        else:
-            assert False
-
-        assert props[INITIATOR_HANDLE] == conn1_self_handle
-        assert props[INITIATOR_ID] == contact1_name
-        assert props[TARGET_ID] == muc_name
-
-        assert (path, props) in all_channels, (path, props)
-
-    assert got_text
-    assert got_tube
+    path, props = channels[0]
+    assert props[CHANNEL_TYPE] == CHANNEL_TYPE_STREAM_TUBE
+    assert props[REQUESTED] == True
+    assert props[INTERFACES] == [CHANNEL_IFACE_GROUP,
+                                 CHANNEL_IFACE_TUBE]
+    assert props[STREAM_TUBE_SERVICE] == 'test'
+    assert props[INITIATOR_HANDLE] == conn1_self_handle
+    assert props[INITIATOR_ID] == contact1_name
+    assert props[TARGET_ID] == muc_name
+
+    assert (path, props) in all_channels, (path, props)
+
+    contact1_tube = wrap_channel(bus.get_object(conn.bus_name, path), 'StreamTube')
+    tube1_path = path
 
     state = contact1_tube.Properties.Get(CHANNEL_IFACE_TUBE, 'State')
     assert state == TUBE_CHANNEL_STATE_NOT_OFFERED
 
-    # added as member
-    q.expect('dbus-signal', signal='MembersChanged', path=txt_path,
-        args=['', [conn1_self_handle], [], [], [], conn1_self_handle, 0])
-
     call_async(q, contact1_tube.StreamTube, 'Offer',
             SOCKET_ADDRESS_TYPE_UNIX, dbus.ByteArray(server_socket_address),
             SOCKET_ACCESS_CONTROL_LOCALHOST, sample_parameters)
@@ -135,6 +118,14 @@ def test(q, bus, conn):
     state = contact1_tube.Properties.Get(CHANNEL_IFACE_TUBE, 'State')
     assert state == TUBE_CHANNEL_STATE_OPEN
 
+    # now let's get the text channel so we can invite contact2 using
+    # the utility t.invite_to_muc
+    _, path, _ = conn.Requests.EnsureChannel({
+            CHANNEL_TYPE: CHANNEL_TYPE_TEXT,
+            TARGET_HANDLE_TYPE: HT_ROOM,
+            TARGET_ID: muc_name})
+    text1 = wrap_channel(bus.get_object(conn.bus_name, path), 'Text')
+
     t.invite_to_muc(q, text1.Group, conn2, contact2_handle_on_conn1, contact1_handle_on_conn2)
 
     # tubes channel is created