[telepathy-gabble/master] Respond to removal of last content with terminate

Will Thompson will.thompson at collabora.co.uk
Sat Apr 18 05:57:31 PDT 2009


XEP-0166 says (in a footnote!) that if your peer sends a content-remove
that leaves the session empty, you should send a session-terminate.

Making this work is a matter of deleting some code guarding against it
happening, and then deleting some other code (which never ran because of
the aforementioned guard) which was intended to implement it, but in
fact was redundant and crashed if allowed to run because the same "no
more contents ⇒ terminate" logic is also in content_removed_cb (which is
responsible for terminating the session rather than sending a
content-remove for the last content).

Deleting code is good, right?
---
 src/jingle-session.c                               |   17 -----------
 .../twisted/jingle/test-content-adding-removal.py  |   30 +++++++++++++++++--
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/jingle-session.c b/src/jingle-session.c
index 9b44143..002b49e 100644
--- a/src/jingle-session.c
+++ b/src/jingle-session.c
@@ -690,7 +690,6 @@ static void
 _each_content_remove (GabbleJingleSession *sess, GabbleJingleContent *c,
     LmMessageNode *content_node, GError **error)
 {
-  GabbleJingleSessionPrivate *priv = sess->priv;
   const gchar *name = lm_message_node_get_attribute (content_node, "name");
 
   if (c == NULL)
@@ -698,14 +697,6 @@ _each_content_remove (GabbleJingleSession *sess, GabbleJingleContent *c,
       SET_BAD_REQ ("content called \"%s\" doesn't exist", name);
       return;
     }
-
-  /* FIXME: do we treat this as a session terminate instead of error? */
-  if (g_hash_table_size (priv->contents) == 1)
-    {
-      SET_BAD_REQ ("unable to remove the last content in a session");
-      return;
-    }
-
   gabble_jingle_content_remove (c, FALSE);
 }
 
@@ -835,15 +826,7 @@ static void
 on_content_remove (GabbleJingleSession *sess, LmMessageNode *node,
     GError **error)
 {
-  GabbleJingleSessionPrivate *priv = sess->priv;
-
   _foreach_content (sess, node, _each_content_remove, error);
-
-  if (g_hash_table_size (priv->contents) == 0)
-    {
-      gabble_jingle_session_terminate (sess,
-          TP_CHANNEL_GROUP_CHANGE_REASON_NONE, NULL);
-    }
 }
 
 static void
diff --git a/tests/twisted/jingle/test-content-adding-removal.py b/tests/twisted/jingle/test-content-adding-removal.py
index b85c469..e5b975c 100644
--- a/tests/twisted/jingle/test-content-adding-removal.py
+++ b/tests/twisted/jingle/test-content-adding-removal.py
@@ -15,7 +15,13 @@ from jingletest2 import (
     )
 import constants as cs
 
-def test(jp, q, bus, conn, stream):
+def gabble_terminates(jp, q, bus, conn, stream):
+    test(jp, q, bus, conn, stream, False)
+
+def peer_terminates(jp, q, bus, conn, stream):
+    test(jp, q, bus, conn, stream, True)
+
+def test(jp, q, bus, conn, stream, peer_removes_final_content):
     jt = JingleTest2(jp, conn, q, stream, 'test at localhost', 'foo at bar.com/Foo')
     jt.prepare()
 
@@ -115,8 +121,23 @@ def test(jp, q, bus, conn, stream):
         jp.match_jingle_action(x.query, 'content-remove'))
     stream.send(make_result_iq(stream, e.stanza))
 
-    # Then we remove the second stream, which terminates the session
-    chan.StreamedMedia.RemoveStreams([stream2_id])
+    if peer_removes_final_content:
+        # The peer removes the final countdo^W content. From a footnote (!) in
+        # XEP 0166:
+        #     If the content-remove results in zero content definitions for the
+        #     session, the entity that receives the content-remove SHOULD send
+        #     a session-terminate action to the other party (since a session
+        #     with no content definitions is void).
+        # So, Gabble should respond to the content-remove with a
+        # session-terminate.
+        node = jp.SetIq(jt.peer, jt.jid, [
+            jp.Jingle(jt.sid, jt.peer, 'content-remove', [
+                jp.Content(c['name'], c['creator'], c['senders'], []) ]) ])
+        stream.send(jp.xml(node))
+    else:
+        # The Telepathy client removes the second stream; Gabble should
+        # terminate the session rather than sending a content-remove.
+        chan.StreamedMedia.RemoveStreams([stream2_id])
 
     st, closed = q.expect_many(
         EventPattern('stream-iq', iq_type='set', predicate=lambda x:
@@ -135,5 +156,6 @@ def test(jp, q, bus, conn, stream):
 
 
 if __name__ == '__main__':
-    test_dialects(test, [JingleProtocol015, JingleProtocol031])
+    test_dialects(gabble_terminates, [JingleProtocol015, JingleProtocol031])
+    test_dialects(peer_terminates, [JingleProtocol015, JingleProtocol031])
 
-- 
1.5.6.5




More information about the telepathy-commits mailing list