[telepathy-gabble/master] Send <hold/> if necessary after session-initiate
Will Thompson
will.thompson at collabora.co.uk
Tue Jun 2 04:01:13 PDT 2009
If a call is put on hold before it's been initiated, Gabble should send
a <hold/> notification when the session-initiate is acked. This patch
makes this work both in the case where the GabbleJingleSession already
exists (but isn't initiated) when the call is placed on hold, and when
the session hasn't even been created yet (by setting the current hold
state on the session when it is created).
---
src/jingle-factory.c | 20 +++++--
src/jingle-factory.h | 7 ++-
src/jingle-session.c | 53 ++++++++++++++++-
src/jingle-session.h | 10 ++-
src/media-channel-hold.c | 6 +-
src/media-channel.c | 3 +-
tests/twisted/jingle/hold-audio.py | 109 +++++++++++++++++++++++++++++++++--
tests/twisted/jingle/hold-av.py | 21 +++++--
8 files changed, 197 insertions(+), 32 deletions(-)
diff --git a/src/jingle-factory.c b/src/jingle-factory.c
index 064f3b7..59b72d6 100644
--- a/src/jingle-factory.c
+++ b/src/jingle-factory.c
@@ -87,7 +87,10 @@ struct _GabbleJingleFactoryPrivate
static LmHandlerResult jingle_cb (LmMessageHandler *handler,
LmConnection *lmconn, LmMessage *message, gpointer user_data);
static GabbleJingleSession *create_session (GabbleJingleFactory *fac,
- const gchar *sid, TpHandle peer, const gchar *peer_resource);
+ const gchar *sid,
+ TpHandle peer,
+ const gchar *peer_resource,
+ gboolean local_hold);
static void session_terminated_cb (GabbleJingleSession *sess,
gboolean local_terminator, TpChannelGroupChangeReason reason,
@@ -717,7 +720,7 @@ jingle_cb (LmMessageHandler *handler,
goto REQUEST_ERROR;
}
new_session = TRUE;
- sess = create_session (self, sid, 0, NULL);
+ sess = create_session (self, sid, 0, NULL, FALSE);
g_object_set (sess, "dialect", dialect, NULL);
}
@@ -757,7 +760,10 @@ REQUEST_ERROR:
*/
static GabbleJingleSession *
create_session (GabbleJingleFactory *fac,
- const gchar *sid, TpHandle peer, const gchar *peer_resource)
+ const gchar *sid,
+ TpHandle peer,
+ const gchar *peer_resource,
+ gboolean local_hold)
{
GabbleJingleFactoryPrivate *priv = fac->priv;
GabbleJingleSession *sess;
@@ -777,7 +783,7 @@ create_session (GabbleJingleFactory *fac,
}
sess = gabble_jingle_session_new (priv->conn, sid_, local_initiator, peer,
- peer_resource);
+ peer_resource, local_hold);
g_signal_connect (sess, "terminated", (GCallback) session_terminated_cb, fac);
@@ -789,9 +795,11 @@ create_session (GabbleJingleFactory *fac,
GabbleJingleSession *
gabble_jingle_factory_create_session (GabbleJingleFactory *fac,
- TpHandle peer, const gchar *peer_resource)
+ TpHandle peer,
+ const gchar *peer_resource,
+ gboolean local_hold)
{
- return create_session (fac, NULL, peer, peer_resource);
+ return create_session (fac, NULL, peer, peer_resource, local_hold);
}
void
diff --git a/src/jingle-factory.h b/src/jingle-factory.h
index b2109c9..9af5452 100644
--- a/src/jingle-factory.h
+++ b/src/jingle-factory.h
@@ -140,8 +140,11 @@ GType gabble_jingle_factory_lookup_transport (GabbleJingleFactory *self,
void _jingle_factory_unregister_session (GabbleJingleFactory *factory,
const gchar *sid);
-GabbleJingleSession *gabble_jingle_factory_create_session (GabbleJingleFactory
- *fac, TpHandle peer, const gchar *peer_resource);
+GabbleJingleSession *gabble_jingle_factory_create_session (
+ GabbleJingleFactory *fac,
+ TpHandle peer,
+ const gchar *peer_resource,
+ gboolean local_hold);
typedef void (*GabbleJingleFactoryRelaySessionCb) (GPtrArray *relays,
gpointer user_data);
diff --git a/src/jingle-session.c b/src/jingle-session.c
index e34158a..bdf7715 100644
--- a/src/jingle-session.c
+++ b/src/jingle-session.c
@@ -63,6 +63,7 @@ enum
PROP_LOCAL_INITIATOR,
PROP_STATE,
PROP_DIALECT,
+ PROP_LOCAL_HOLD,
PROP_REMOTE_HOLD,
PROP_REMOTE_RINGING,
LAST_PROPERTY
@@ -88,6 +89,8 @@ struct _GabbleJingleSessionPrivate
gboolean locally_accepted;
gboolean locally_terminated;
+ gboolean local_hold;
+
gboolean remote_hold;
gboolean remote_ringing;
@@ -137,6 +140,8 @@ static JingleAction allowed_actions[MAX_JINGLE_STATES][MAX_ACTIONS_PER_STATE] =
{ JINGLE_ACTION_UNKNOWN }
};
+static void gabble_jingle_session_send_held (GabbleJingleSession *sess);
+
static void
gabble_jingle_session_init (GabbleJingleSession *obj)
{
@@ -229,6 +234,9 @@ gabble_jingle_session_get_property (GObject *object,
case PROP_DIALECT:
g_value_set_uint (value, priv->dialect);
break;
+ case PROP_LOCAL_HOLD:
+ g_value_set_boolean (value, priv->local_hold);
+ break;
case PROP_REMOTE_HOLD:
g_value_set_boolean (value, priv->remote_hold);
break;
@@ -279,6 +287,24 @@ gabble_jingle_session_set_property (GObject *object,
g_free (priv->peer_resource);
priv->peer_resource = g_value_dup_string (value);
break;
+ case PROP_LOCAL_HOLD:
+ {
+ gboolean local_hold = g_value_get_boolean (value);
+
+ if (priv->local_hold != local_hold)
+ {
+ priv->local_hold = local_hold;
+
+ if (priv->state >= JS_STATE_PENDING_INITIATED &&
+ priv->state < JS_STATE_ENDED)
+ gabble_jingle_session_send_held (sess);
+
+ /* else, we'll send this in set_state when we move to PENDING_INITIATED or
+ * better.
+ */
+ }
+ break;
+ }
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
g_assert_not_reached ();
@@ -291,7 +317,8 @@ gabble_jingle_session_new (GabbleConnection *connection,
const gchar *session_id,
gboolean local_initiator,
TpHandle peer,
- const gchar *peer_resource)
+ const gchar *peer_resource,
+ gboolean local_hold)
{
return g_object_new (GABBLE_TYPE_JINGLE_SESSION,
"session-id", session_id,
@@ -299,6 +326,7 @@ gabble_jingle_session_new (GabbleConnection *connection,
"local-initiator", local_initiator,
"peer", peer,
"peer-resource", peer_resource,
+ "local-hold", local_hold,
NULL);
}
@@ -382,6 +410,11 @@ gabble_jingle_session_class_init (GabbleJingleSessionClass *cls)
G_PARAM_STATIC_BLURB);
g_object_class_install_property (object_class, PROP_DIALECT, param_spec);
+ param_spec = g_param_spec_boolean ("local-hold", "Local hold",
+ "TRUE if we've placed the peer on hold", FALSE,
+ G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+ g_object_class_install_property (object_class, PROP_LOCAL_HOLD, param_spec);
+
param_spec = g_param_spec_boolean ("remote-hold", "Remote hold",
"TRUE if the peer has placed us on hold", FALSE,
G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
@@ -1788,6 +1821,12 @@ set_state (GabbleJingleSession *sess,
priv->state = state;
g_object_notify (G_OBJECT (sess), "state");
+ /* If we have an outstanding "you're on hold notification", send it */
+ if (priv->local_hold &&
+ state >= JS_STATE_PENDING_INITIATED &&
+ state < JS_STATE_ENDED)
+ gabble_jingle_session_send_held (sess);
+
/* if we or peer just initiated the session, set the session timer */
if ((priv->local_initiator && (state == JS_STATE_PENDING_INITIATE_SENT)) ||
(!priv->local_initiator && (state == JS_STATE_PENDING_INITIATED)))
@@ -2059,11 +2098,19 @@ gabble_jingle_session_send_rtp_info (GabbleJingleSession *sess,
gabble_jingle_session_send (sess, message, NULL, NULL);
}
+static void
+gabble_jingle_session_send_held (GabbleJingleSession *sess)
+{
+ const gchar *s = (sess->priv->local_hold ? "hold" : "unhold");
+
+ gabble_jingle_session_send_rtp_info (sess, s);
+}
+
void
-gabble_jingle_session_send_held (GabbleJingleSession *sess,
+gabble_jingle_session_set_local_hold (GabbleJingleSession *sess,
gboolean held)
{
- gabble_jingle_session_send_rtp_info (sess, (held ? "hold" : "unhold"));
+ g_object_set (sess, "local-hold", held, NULL);
}
gboolean
diff --git a/src/jingle-session.h b/src/jingle-session.h
index 870b716..73b0a73 100644
--- a/src/jingle-session.h
+++ b/src/jingle-session.h
@@ -80,8 +80,11 @@ struct _GabbleJingleSession {
};
GabbleJingleSession *gabble_jingle_session_new (GabbleConnection *connection,
- const gchar *session_id, gboolean local_initiator, TpHandle peer,
- const gchar *peer_resource);
+ const gchar *session_id,
+ gboolean local_initiator,
+ TpHandle peer,
+ const gchar *peer_resource,
+ gboolean local_hold);
const gchar * gabble_jingle_session_detect (LmMessage *message,
JingleAction *action, JingleDialect *dialect);
@@ -110,7 +113,8 @@ void gabble_jingle_session_send (GabbleJingleSession *sess,
JingleReplyHandler cb,
GObject *weak_object);
-void gabble_jingle_session_send_held (GabbleJingleSession *sess, gboolean held);
+void gabble_jingle_session_set_local_hold (GabbleJingleSession *sess,
+ gboolean held);
gboolean gabble_jingle_session_get_remote_hold (GabbleJingleSession *sess);
diff --git a/src/media-channel-hold.c b/src/media-channel-hold.c
index 233d892..27f8db2 100644
--- a/src/media-channel-hold.c
+++ b/src/media-channel-hold.c
@@ -78,7 +78,7 @@ stream_hold_state_changed (GabbleMediaStream *stream G_GNUC_UNUSED,
priv->hold_state = TP_LOCAL_HOLD_STATE_UNHELD;
if (priv->session != NULL)
- gabble_jingle_session_send_held (priv->session, FALSE);
+ gabble_jingle_session_set_local_hold (priv->session, FALSE);
break;
@@ -171,7 +171,7 @@ stream_hold_state_changed (GabbleMediaStream *stream G_GNUC_UNUSED,
/* Tell the peer what's happened. */
if (priv->session != NULL)
- gabble_jingle_session_send_held (priv->session, FALSE);
+ gabble_jingle_session_set_local_hold (priv->session, FALSE);
}
tp_svc_channel_interface_hold_emit_hold_state_changed (self,
@@ -248,7 +248,7 @@ gabble_media_channel_request_hold (TpSvcChannelInterfaceHold *iface,
}
if (priv->hold_state == TP_LOCAL_HOLD_STATE_UNHELD && session != NULL)
- gabble_jingle_session_send_held (session, TRUE);
+ gabble_jingle_session_set_local_hold (session, TRUE);
priv->hold_state = TP_LOCAL_HOLD_STATE_PENDING_HOLD;
}
diff --git a/src/media-channel.c b/src/media-channel.c
index f7b83d8..3248470 100644
--- a/src/media-channel.c
+++ b/src/media-channel.c
@@ -256,6 +256,7 @@ static void
create_session (GabbleMediaChannel *chan, TpHandle peer)
{
GabbleMediaChannelPrivate *priv = chan->priv;
+ gboolean local_hold = (priv->hold_state != TP_LOCAL_HOLD_STATE_UNHELD);
g_assert (priv->session == NULL);
@@ -263,7 +264,7 @@ create_session (GabbleMediaChannel *chan, TpHandle peer)
priv->session = g_object_ref (
gabble_jingle_factory_create_session (priv->conn->jingle_factory,
- peer, NULL));
+ peer, NULL, local_hold));
_latch_to_session (chan);
}
diff --git a/tests/twisted/jingle/hold-audio.py b/tests/twisted/jingle/hold-audio.py
index 61b161c..d7a4a84 100644
--- a/tests/twisted/jingle/hold-audio.py
+++ b/tests/twisted/jingle/hold-audio.py
@@ -4,6 +4,7 @@ Test the Hold API.
from gabbletest import make_result_iq, sync_stream
from servicetest import (
+ assertEquals, sync_dbus,
make_channel_proxy, call_async, EventPattern, wrap_channel,
)
import constants as cs
@@ -24,6 +25,47 @@ def test(jp, q, bus, conn, stream):
chan = wrap_channel(bus.get_object(conn.bus_name, path), 'StreamedMedia',
['Hold'])
+ # These are 0- (for old dialects) or 1- (for new dialects) element lists
+ # that can be splatted into expect_many with *
+ hold_event = jp.rtp_info_event_list("hold")
+ unhold_event = jp.rtp_info_event_list("unhold")
+
+ # Before we have any streams, GetHoldState returns Unheld and unhold is a
+ # no-op.
+ assertEquals((cs.HS_UNHELD, cs.HSR_NONE), chan.Hold.GetHoldState())
+ chan.Hold.RequestHold(False)
+
+ # Before we have any streams, RequestHold(True) should work; because there
+ # are no streams, it should take effect at once. It certainly shouldn't
+ # send anything to the peer.
+ q.forbid_events(hold_event)
+ q.forbid_events(unhold_event)
+
+ call_async(q, chan.Hold, 'RequestHold', True)
+ q.expect('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_PENDING_HOLD, cs.HSR_REQUESTED])
+ q.expect('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_HELD, cs.HSR_REQUESTED])
+ assertEquals((cs.HS_HELD, cs.HSR_REQUESTED), chan.Hold.GetHoldState())
+
+ # If we unhold, it should succeed immediately again, because there are no
+ # resources to reclaim.
+ call_async(q, chan.Hold, 'RequestHold', False)
+ q.expect('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_PENDING_UNHOLD, cs.HSR_REQUESTED])
+ q.expect('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_UNHELD, cs.HSR_REQUESTED])
+ assertEquals((cs.HS_UNHELD, cs.HSR_REQUESTED), chan.Hold.GetHoldState())
+
+ # Put the call back on hold ...
+ call_async(q, chan.Hold, 'RequestHold', True)
+ q.expect('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_PENDING_HOLD, cs.HSR_REQUESTED])
+ q.expect('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_HELD, cs.HSR_REQUESTED])
+ assertEquals((cs.HS_HELD, cs.HSR_REQUESTED), chan.Hold.GetHoldState())
+
+ # ... and request a stream.
chan.StreamedMedia.RequestStreams(handle, [cs.MEDIA_STREAM_TYPE_AUDIO])
# S-E gets notified about new session handler, and calls Ready on it
@@ -37,23 +79,76 @@ def test(jp, q, bus, conn, stream):
stream_handler = make_channel_proxy(conn, e.args[0], 'Media.StreamHandler')
- stream_handler.NewNativeCandidate("fake", jt.get_remote_transports_dbus())
+ # Syncing here to make sure SetStreamHeld happens after Ready...
+ sync_dbus(bus, q, conn)
stream_handler.Ready(jt.get_audio_codecs_dbus())
+ stream_handler.NewNativeCandidate("fake", jt.get_remote_transports_dbus())
stream_handler.StreamState(cs.MEDIA_STREAM_STATE_CONNECTED)
- e = q.expect('stream-iq', predicate=lambda e:
- jp.match_jingle_action(e.query, 'session-initiate'))
+ # Now Gabble tells the streaming implementation to go on hold (because it
+ # said it was Ready), and the session is initiated.
+ e = q.expect_many(
+ EventPattern('dbus-signal', signal='SetStreamHeld', args=[True]),
+ EventPattern('stream-iq',
+ predicate=jp.action_predicate('session-initiate')),
+ )[1]
+
+ # Ensure that if Gabble sent the <hold/> stanza too early it's already
+ # arrived.
+ sync_stream(q, stream)
+ q.unforbid_events(hold_event)
+
stream.send(make_result_iq(stream, e.stanza))
+ # We've acked the s-i, so we do speak Jingle; Gabble should send the
+ # <hold/> notification.
+ q.expect_many(*hold_event)
+
+ # The call's still on hold, both before and after the streaming
+ # implementation says it's okay with that (re-entering PENDING_HOLD seems
+ # silly).
+ assertEquals((cs.HS_HELD, cs.HSR_REQUESTED), chan.Hold.GetHoldState())
+ stream_handler.HoldState(True)
+ assertEquals((cs.HS_HELD, cs.HSR_REQUESTED), chan.Hold.GetHoldState())
+
+ # The peer answers the call; they're still on hold.
jt.set_sid_from_initiate(e.query)
jt.accept()
q.expect('stream-iq', iq_type='result')
- # These are 0- (for old dialects) or 1- (for new dialects) element lists
- # that can be splatted into expect_many with *
- hold_event = jp.rtp_info_event_list("hold")
- unhold_event = jp.rtp_info_event_list("unhold")
+ assertEquals((cs.HS_HELD, cs.HSR_REQUESTED), chan.Hold.GetHoldState())
+
+ # Now we decide we do actually want to speak to them, and unhold.
+ # Ensure that if Gabble sent the <unhold/> stanza too early it's already
+ # arrived.
+ sync_stream(q, stream)
+ q.forbid_events(unhold_event)
+
+ call_async(q, chan.Hold, 'RequestHold', False)
+ q.expect_many(
+ EventPattern('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_PENDING_UNHOLD, cs.HSR_REQUESTED]),
+ EventPattern('dbus-signal', signal='SetStreamHeld', args=[False]),
+ EventPattern('dbus-return', method='RequestHold', value=()),
+ )
+
+ # Ensure that if Gabble sent the <unhold/> stanza too early it's already
+ # arrived.
+ sync_stream(q, stream)
+ q.unforbid_events(unhold_event)
+
+ call_async(q, stream_handler, 'HoldState', False)
+ q.expect_many(
+ EventPattern('dbus-return', method='HoldState', value=()),
+ EventPattern('dbus-signal', signal='HoldStateChanged',
+ args=[cs.HS_UNHELD, cs.HSR_REQUESTED]),
+ *unhold_event
+ )
+
+ # Hooray! Now let's check that Hold works properly once the call's fully
+ # established.
+
# ---- Test 1: GetHoldState returns unheld and unhold is a no-op ----
hold_state = chan.Hold.GetHoldState()
diff --git a/tests/twisted/jingle/hold-av.py b/tests/twisted/jingle/hold-av.py
index a3cad19..098729a 100644
--- a/tests/twisted/jingle/hold-av.py
+++ b/tests/twisted/jingle/hold-av.py
@@ -13,6 +13,16 @@ import constants as cs
from jingletest2 import JingleTest2, test_all_dialects
def test(jp, q, bus, conn, stream):
+ # These are 0- (for old dialects) or 1- (for new dialects) element lists
+ # that can be splatted into expect_many with *
+ hold_event = jp.rtp_info_event_list("hold")
+ unhold_event = jp.rtp_info_event_list("unhold")
+
+ # Let's forbid them until we're ready to start holding, to check that
+ # Gabble doesn't send spurious notifications.
+ q.forbid_events(hold_event)
+ q.forbid_events(unhold_event)
+
jt = JingleTest2(jp, conn, q, stream, 'test at localhost', 'foo at bar.com/Foo')
jt.prepare()
@@ -77,17 +87,16 @@ def test(jp, q, bus, conn, stream):
q.expect('stream-iq', iq_type='result')
- # These are 0- (for old dialects) or 1- (for new dialects) element lists
- # that can be splatted into expect_many with *
- hold_event = jp.rtp_info_event_list("hold")
- unhold_event = jp.rtp_info_event_list("unhold")
-
# ---- Test 1: GetHoldState returns unheld and unhold is a no-op ----
hold_state = chan.Hold.GetHoldState()
assert hold_state[0] == cs.HS_UNHELD, hold_state
chan.Hold.RequestHold(False)
+ # We're about to start holding, so remove the ban on <hold/>.
+ sync_stream(q, stream)
+ q.unforbid_events(hold_event)
+
# ---- Test 2: successful hold ----
call_async(q, chan.Hold, 'RequestHold', True)
@@ -120,8 +129,6 @@ def test(jp, q, bus, conn, stream):
# ---- Test 4: successful unhold ----
- q.forbid_events(unhold_event)
-
call_async(q, chan.Hold, 'RequestHold', False)
q.expect_many(
EventPattern('dbus-signal', signal='HoldStateChanged',
--
1.5.6.5
More information about the telepathy-commits
mailing list