[telepathy-gabble/master] Only send <gone/> if we've sent another state
Will Thompson
will.thompson at collabora.co.uk
Tue Jun 2 10:32:33 PDT 2009
This prevents opening a channel and immediately closing it from sending
a spurious <gone/> notification to the peer. Fixes: fd.o#18951.
---
src/im-channel.c | 32 +++++++++++++----
tests/twisted/text/test-chat-state.py | 61 +++++++++++++++++++++++++++-----
2 files changed, 75 insertions(+), 18 deletions(-)
diff --git a/src/im-channel.c b/src/im-channel.c
index 767ab0d..09c949a 100644
--- a/src/im-channel.c
+++ b/src/im-channel.c
@@ -105,6 +105,13 @@ struct _GabbleIMChannelPrivate
gchar *peer_jid;
gboolean send_nick;
+ /* FALSE unless at least one chat state notification has been sent; <gone/>
+ * will only be sent when the channel closes if this is TRUE. This prevents
+ * opening a channel and closing it immediately sending a spurious <gone/> to
+ * the peer.
+ */
+ gboolean send_gone;
+
gboolean closed;
gboolean dispose_has_run;
};
@@ -393,12 +400,17 @@ emit_closed_and_send_gone (GabbleIMChannel *self)
GabbleIMChannelPrivate *priv = self->priv;
GabblePresence *presence;
- presence = gabble_presence_cache_get (priv->conn->presence_cache,
- priv->handle);
+ if (priv->send_gone)
+ {
+ presence = gabble_presence_cache_get (priv->conn->presence_cache,
+ priv->handle);
- if (presence && (presence->caps & PRESENCE_CAP_CHAT_STATES))
- gabble_message_util_send_chat_state (G_OBJECT (self), priv->conn,
- 0, TP_CHANNEL_CHAT_STATE_GONE, priv->peer_jid, NULL);
+ if (presence && (presence->caps & PRESENCE_CAP_CHAT_STATES))
+ gabble_message_util_send_chat_state (G_OBJECT (self), priv->conn,
+ 0, TP_CHANNEL_CHAT_STATE_GONE, priv->peer_jid, NULL);
+
+ priv->send_gone = FALSE;
+ }
DEBUG ("Emitting Closed");
tp_svc_channel_emit_closed (self);
@@ -486,6 +498,7 @@ _gabble_im_channel_send_message (GObject *object,
if (presence && (presence->caps & PRESENCE_CAP_CHAT_STATES))
{
state = TP_CHANNEL_CHAT_STATE_ACTIVE;
+ priv->send_gone = TRUE;
}
/* We don't support providing successful delivery reports. */
@@ -798,10 +811,13 @@ gabble_im_channel_set_chat_state (TpSvcChannelInterfaceChatState *iface,
g_set_error (&error, TP_ERRORS, TP_ERROR_INVALID_ARGUMENT,
"you may not explicitly set the Gone state");
}
+ else if (gabble_message_util_send_chat_state (G_OBJECT (self), priv->conn,
+ 0, state, priv->peer_jid, &error))
+ {
+ priv->send_gone = TRUE;
+ }
- if (error != NULL ||
- !gabble_message_util_send_chat_state (G_OBJECT (self), priv->conn,
- 0, state, priv->peer_jid, &error))
+ if (error != NULL)
{
dbus_g_method_return_error (context, error);
g_error_free (error);
diff --git a/tests/twisted/text/test-chat-state.py b/tests/twisted/text/test-chat-state.py
index 0efbc28..51d382a 100644
--- a/tests/twisted/text/test-chat-state.py
+++ b/tests/twisted/text/test-chat-state.py
@@ -6,11 +6,22 @@ channels.
from twisted.words.xish import domish
-from servicetest import call_async, assertEquals, wrap_channel
+from servicetest import call_async, assertEquals, wrap_channel, EventPattern
from gabbletest import exec_test, make_result_iq, sync_stream, make_presence
import constants as cs
import ns
+def check_state_notification(elem, name):
+ assertEquals('message', elem.name)
+ assertEquals('normal', elem['type'])
+
+ children = list(elem.elements())
+ assert len(children) == 1, elem.toXml()
+ notification = children[0]
+
+ assert notification.name == name, notification.toXml()
+ assert notification.uri == ns.CHAT_STATES, notification.toXml()
+
def test(q, bus, conn, stream):
conn.Connect()
q.expect('dbus-signal', signal='StatusChanged', args=[0, 1])
@@ -26,7 +37,7 @@ def test(q, bus, conn, stream):
cs.TARGET_HANDLE: foo_handle,
})[0]
chan = wrap_channel(bus.get_object(conn.bus_name, path), 'Text',
- ['ChatState'])
+ ['ChatState', 'Destroyable'])
presence = make_presence('foo at bar.com/Foo', status='hello')
c = presence.addElement(('http://jabber.org/protocol/caps', 'c'))
@@ -80,14 +91,7 @@ def test(q, bus, conn, stream):
call_async(q, chan.ChatState, 'SetChatState', cs.CHAT_STATE_COMPOSING)
stream_message = q.expect('stream-message')
- elem = stream_message.stanza
- assert elem.name == 'message'
- assert elem['type'] == 'normal'
- chrilden = list(elem.elements())
- assert len(chrilden) == 1, elem.toXml()
- composing = chrilden[0]
- assert composing.name == 'composing', composing.toXml()
- assert composing.uri == ns.CHAT_STATES, composing.toXml()
+ check_state_notification(stream_message.stanza, 'composing')
# XEP 0085:
# every content message SHOULD contain an <active/> notification.
@@ -115,6 +119,43 @@ def test(q, bus, conn, stream):
assert len(filter(is_body, children)) == 1, elem.toXml()
assert len(filter(is_active, children)) == 1, elem.toXml()
+ # Close the channel without acking the received message. The peer should
+ # get a <gone/> notification, and the channel should respawn.
+ chan.Close()
+
+ gone, _, _ = q.expect_many(
+ EventPattern('stream-message'),
+ EventPattern('dbus-signal', signal='Closed'),
+ EventPattern('dbus-signal', signal='NewChannel'),
+ )
+ check_state_notification(gone.stanza, 'gone')
+
+ # Reusing the proxy object because we happen to know it'll be at the same
+ # path...
+
+ # Destroy the channel. The peer shouldn't get a <gone/> notification, since
+ # we already said we were gone and haven't sent them any messages to the
+ # contrary.
+ es = [EventPattern('stream-message')]
+ q.forbid_events(es)
+
+ chan.Destroyable.Destroy()
+ sync_stream(q, stream)
+
+ # Make the channel anew.
+ path = conn.Requests.CreateChannel(
+ { cs.CHANNEL_TYPE: cs.CHANNEL_TYPE_TEXT,
+ cs.TARGET_HANDLE_TYPE: cs.HT_CONTACT,
+ cs.TARGET_HANDLE: foo_handle,
+ })[0]
+ chan = wrap_channel(bus.get_object(conn.bus_name, path), 'Text',
+ ['ChatState', 'Destroyable'])
+
+ # Close it immediately; the peer should again not get a <gone/>
+ # notification, since we haven't sent any notifications on that channel.
+ chan.Close()
+ sync_stream(q, stream)
+
conn.Disconnect()
q.expect('dbus-signal', signal='StatusChanged', args=[2, 1])
--
1.5.6.5
More information about the telepathy-commits
mailing list