Merge branch 'local-pending-flicker2' into telepathy-gabble-0.12telepathy-gabble-0.12

2 files changed, 82 insertions, 6 deletions

diff --git a/src/roster.c b/src/roster.c
index 5656662a5..4fd9c74af 100644
--- a/src/roster.c
+++ b/src/roster.c
@@ -1233,12 +1233,17 @@ process_roster (
         case GABBLE_ROSTER_SUBSCRIPTION_FROM:
         case GABBLE_ROSTER_SUBSCRIPTION_BOTH:
           if (google_roster &&
-              /* Don't hide contacts from stored if they're remote pending.
-               * This works around Google Talk flickering ask="subscribe"
-               * when you try to subscribe to someone; see
-               * test-google-roster.py.
+              /* Don't hide contacts from stored if they're pending.
+               * This works around two Google Talk issues:
+               * - When you try to subscribe to someone, you get a flickering
+               *   ask="subscribe";
+               * - When somebody tries to subscribe to you, you get a presence
+               *   with type="subscribe" followed by a roster update with
+               *   subscribe="none".
+               * See test-google-roster.py for more details.
                */
               item->subscribe != TP_SUBSCRIPTION_STATE_ASK &&
+              item->publish != TP_SUBSCRIPTION_STATE_ASK &&
               !_google_roster_item_should_keep (jid, item))
             {
               tp_handle_set_remove (changed, handle);
diff --git a/tests/twisted/roster/test-google-roster.py b/tests/twisted/roster/test-google-roster.py
index 630e3c314..3773377df 100644
--- a/tests/twisted/roster/test-google-roster.py
+++ b/tests/twisted/roster/test-google-roster.py
@@ -131,8 +131,9 @@ def test_inital_roster(q, bus, conn, stream):
 
 def test_flickering(q, bus, conn, stream, subscribe):
     """
-    Google's server is buggy, and subscription state transitions "flicker"
-    sometimes. Here, we test that Gabble is suppressing the flickers.
+    Google's server is buggy; when asking to subscribe to somebody, the
+    subscription state transitions "flicker" sometimes. Here, we test that
+    Gabble is suppressing the flickers.
     """
 
     self_handle = conn.GetSelfHandle()
@@ -264,6 +265,75 @@ def test_flickering(q, bus, conn, stream, subscribe):
     sync_dbus(bus, q, conn)
     q.unforbid_events(change_events)
 
+def test_local_pending(q, bus, conn, stream, subscribe):
+    """
+    When somebody asks to subscribe to us, Google sends the subscription
+    request and then a roster update saying there is no subscription.
+    This causes the contact to appear in local pending and then disappear.
+    Here, we test that Gabble is suppressing the flickers.
+    """
+
+    contact = 'alice@foo.com'
+    handle = conn.RequestHandles(cs.HT_CONTACT, [contact])[0]
+
+    # Alice asks to subscribes to us
+    presence = domish.Element(('jabber:client', 'presence'))
+    presence['from'] = contact
+    presence['type'] = 'subscribe'
+    stream.send(presence)
+
+    q.expect_many(
+        EventPattern('dbus-signal', signal='MembersChanged',
+            args=['', [], [], [handle], [], handle, cs.GC_REASON_NONE],
+            predicate=is_publish),
+        EventPattern('dbus-signal', signal='ContactsChanged',
+            args=[{handle: (cs.SUBSCRIPTION_STATE_NO,
+                cs.SUBSCRIPTION_STATE_ASK, '')}, []]),
+        )
+
+    # Now we send the spurious roster update with subscribe="none" and verify
+    # that nothing happens to her publish state in reaction to that
+    change_event = EventPattern('dbus-signal', signal='MembersChanged',
+        predicate=is_publish)
+    q.forbid_events([change_event])
+
+    iq = make_set_roster_iq(stream, 'test@localhost/Resource', contact,
+            "none", False)
+    stream.send(iq)
+
+    sync_stream(q, stream)
+    sync_dbus(bus, q, conn)
+    q.unforbid_events([change_event])
+
+    # Now we cancel alice's subscription request and verify that if the
+    # redundant IQ is sent again, it's safely handled
+    presence = domish.Element(('jabber:client', 'presence'))
+    presence['from'] = contact
+    presence['type'] = 'unsubscribe'
+    stream.send(presence)
+
+    q.expect_many(
+        EventPattern('dbus-signal', signal='MembersChanged',
+            args=['', [], [handle], [], [], handle, cs.GC_REASON_NONE],
+            predicate=is_publish),
+        EventPattern('dbus-signal', signal='ContactsChanged',
+            args=[{handle: (cs.SUBSCRIPTION_STATE_NO,
+                cs.SUBSCRIPTION_STATE_REMOVED_REMOTELY, '')}, []]),
+        )
+
+    # Now we send a roster roster update with subscribe="none" again (which
+    # doesn't change anything, it just confirms what we already knew) and
+    # verify that nothing happens to her publish state in reaction to that.
+    q.forbid_events([change_event])
+
+    iq = make_set_roster_iq(stream, 'test@localhost/Resource', contact,
+            "none", False)
+    stream.send(iq)
+
+    sync_stream(q, stream)
+    sync_dbus(bus, q, conn)
+    q.unforbid_events([change_event])
+
 # This event is forbidden in all of the deny tests!
 remove_events = [
     EventPattern('stream-iq', query_ns=ns.ROSTER,
@@ -520,6 +590,7 @@ def test(q, bus, conn, stream):
     publish, subscribe, stored, deny = test_inital_roster(q, bus, conn, stream)
 
     test_flickering(q, bus, conn, stream, subscribe)
+    test_local_pending(q, bus, conn, stream, subscribe)
     test_deny_simple(q, bus, conn, stream, stored, deny)
     test_deny_overlap_one(q, bus, conn, stream, subscribe, stored, deny)
     test_deny_overlap_two(q, bus, conn, stream,