Wait that we have received the disco reply on our bare jid

We have to block the transition to the CONNECTED state until we received the
reply of the disco on our bare jid as this can change some immutable
properties or interfaces on the Connection object.

3 files changed, 80 insertions, 52 deletions

diff --git a/src/connection.c b/src/connection.c
index 8e4686c28..66ede1684 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -236,6 +236,10 @@ struct _GabbleConnectionPrivate
   /* timer used when trying to properly disconnect */
   guint disconnect_timer;
 
+  /* Number of things we are waiting for before changing the connection status
+   * to connected */
+  guint waiting_connected;
+
   gboolean closing;
   /* gobject housekeeping */
   gboolean dispose_has_run;
@@ -1452,6 +1456,7 @@ static LmHandlerResult connection_iq_unknown_cb (LmMessageHandler *,
     LmConnection *, LmMessage *, gpointer);
 static void connection_disco_cb (GabbleDisco *, GabbleDiscoRequest *,
     const gchar *, const gchar *, LmMessageNode *, GError *, gpointer);
+static void decrement_waiting_connected (GabbleConnection *connection);
 
 static void
 remote_closed_cb (WockyPorter *porter,
@@ -1686,7 +1691,7 @@ bare_jid_disco_cb (GabbleDisco *disco,
         }
     }
 
-  /* FIXME: add OLPC iface if PEP is supported? */
+  decrement_waiting_connected (conn);
 }
 
 /**
@@ -1694,8 +1699,8 @@ bare_jid_disco_cb (GabbleDisco *disco,
  *
  * Stage 2 of connecting, this function is called once the connect operation
  * has finished. It checks if the connection succeeded, creates and starts
- * the WockyPorter, then sends a discovery request to find the
- * server's features.
+ * the WockyPorter, then sends two discovery requests to find the
+ * server's features (one to the server and one to our bare jid).
  */
 static void
 connector_connected (GabbleConnection *self,
@@ -1790,6 +1795,7 @@ connector_connected (GabbleConnection *self,
   /* set initial capabilities */
   gabble_connection_refresh_capabilities (self, NULL);
 
+  /* Disco server features */
   if (!gabble_disco_request_with_timeout (self->disco, GABBLE_DISCO_TYPE_INFO,
                                           priv->stream_server, NULL,
                                           disco_reply_timeout,
@@ -1815,7 +1821,13 @@ connector_connected (GabbleConnection *self,
           error->message);
 
       g_error_free (error);
+
+      tp_base_connection_change_status ((TpBaseConnection *) self,
+          TP_CONNECTION_STATUS_DISCONNECTED,
+          TP_CONNECTION_STATUS_REASON_NETWORK_ERROR);
     }
+
+  self->priv->waiting_connected = 2;
 }
 
 static void
@@ -1931,7 +1943,7 @@ disconnect_callbacks (TpBaseConnection *base)
  *
  * Stage 1 is _gabble_connection_connect calling wocky_connector_connect_async
  * Stage 2 is connector_connected initiating service discovery
- * Stage 3 is connection_disco_cb advertising initial presence, requesting
+ * Stage 3 is set_status_to_connected advertising initial presence, requesting
  *   the roster and setting the CONNECTED state
  */
 static gboolean
@@ -2541,6 +2553,14 @@ connection_iq_unknown_cb (LmMessageHandler *handler,
   return LM_HANDLER_RESULT_REMOVE_MESSAGE;
 }
 
+/**
+ * connection_disco_cb
+ *
+ * Stage 3 of connecting, this function is called once all the events we were
+ * waiting for happened.
+ * It sends the user's initial presence to the server, marking them as
+ * available, and requests the roster.
+ */
 static void
 set_status_to_connected (GabbleConnection *conn)
 {
@@ -2587,14 +2607,15 @@ ERROR:
       TP_CONNECTION_STATUS_REASON_NETWORK_ERROR);
 }
 
-/**
- * connection_disco_cb
- *
- * Stage 3 of connecting, this function is called by GabbleDisco after the
- * result of the non-blocking server feature discovery call is known. It sends
- * the user's initial presence to the server, marking them as available,
- * and requests the roster.
- */
+static void
+decrement_waiting_connected (GabbleConnection *conn)
+{
+  conn->priv->waiting_connected--;
+
+  if (conn->priv->waiting_connected == 0)
+    set_status_to_connected (conn);
+}
+
 static void
 connection_disco_cb (GabbleDisco *disco,
                      GabbleDiscoRequest *request,
@@ -2673,7 +2694,7 @@ connection_disco_cb (GabbleDisco *disco,
       DEBUG ("set features flags to %d", conn->features);
     }
 
-  set_status_to_connected (conn);
+  decrement_waiting_connected (conn);
   return;
 
 ERROR:
diff --git a/tests/twisted/gabbletest.py b/tests/twisted/gabbletest.py
index e00cd84c8..0c0c46dbe 100644
--- a/tests/twisted/gabbletest.py
+++ b/tests/twisted/gabbletest.py
@@ -341,27 +341,34 @@ class BaseXmlStream(xmlstream.XmlStream):
     def _cb_authd(self, _):
         # called when stream is authenticated
         self.addObserver(
-            "/iq/query[@xmlns='http://jabber.org/protocol/disco#info']",
+            "/iq[@to='localhost']/query[@xmlns='http://jabber.org/protocol/disco#info']",
             self._cb_disco_iq)
+        self.addObserver(
+            "/iq[@to='test@localhost']/query[@xmlns='http://jabber.org/protocol/disco#info']",
+            self._cb_bare_jid_disco_iq)
         self.event_func(servicetest.Event('stream-authenticated'))
 
     def _cb_disco_iq(self, iq):
-        if iq.getAttribute('to') == 'localhost':
-            # add PEP support
-            # This is actually wrong. PEP support should be advertised when
-            # discoing user's bare JID, not the server. Lot of old ejabberd
-            # versions still behave this way though.
-            nodes = xpath.queryForNodes(
-                "/iq/query[@xmlns='http://jabber.org/protocol/disco#info']",
-                iq)
-            query = nodes[0]
-            identity = query.addElement('identity')
-            identity['category'] = 'pubsub'
-            identity['type'] = 'pep'
-
-            iq['type'] = 'result'
-            iq['from'] = iq['to']
-            self.send(iq)
+        # add PEP support
+        # This is actually wrong. PEP support should be advertised when
+        # discoing user's bare JID, not the server. Lot of old ejabberd
+        # versions still behave this way though.
+        nodes = xpath.queryForNodes(
+            "/iq/query[@xmlns='http://jabber.org/protocol/disco#info']",
+            iq)
+        query = nodes[0]
+        identity = query.addElement('identity')
+        identity['category'] = 'pubsub'
+        identity['type'] = 'pep'
+
+        iq['type'] = 'result'
+        iq['from'] = iq['to']
+        self.send(iq)
+
+    def _cb_bare_jid_disco_iq(self, iq):
+        iq['type'] = 'result'
+        iq['from'] = iq['to']
+        self.send(iq)
 
     def onDocumentEnd(self):
         self.event_func(servicetest.Event('stream-closed'))
diff --git a/tests/twisted/pep-support.py b/tests/twisted/pep-support.py
index 60c47d5af..b97a27f81 100644
--- a/tests/twisted/pep-support.py
+++ b/tests/twisted/pep-support.py
@@ -46,22 +46,8 @@ def test_no_pep(q, bus, conn, stream):
 def test_pep(q, bus, conn, stream):
     conn.Connect()
 
-    e = q.expect('stream-iq', iq_type='get', to='test@localhost',
-            query_ns=ns.DISCO_INFO)
-
-    iq = e.stanza
-    nodes = xpath.queryForNodes(
-    "/iq/query[@xmlns='http://jabber.org/protocol/disco#info']", iq)
-    query = nodes[0]
-    identity = query.addElement('identity')
-    identity['category'] = 'pubsub'
-    identity['type'] = 'pep'
-
-    iq['type'] = 'result'
-    iq['from'] = 'test@localhost'
-    stream.send(iq)
-
-    sync_stream(q, stream)
+    q.expect('dbus-signal', signal='StatusChanged',
+        args=[cs.CONN_STATUS_CONNECTED, cs.CSR_REQUESTED])
 
     call_async(q, conn.Location, 'SetLocation', {
         'lat': 0.0,
@@ -75,16 +61,30 @@ def test_pep(q, bus, conn, stream):
     acknowledge_iq(stream, event.stanza)
     q.expect('dbus-return', method='SetLocation')
 
-class MyXmppStream(BaseXmlStream):
+class PEPinJidDiscoXmppStream(BaseXmlStream):
     version = (1, 0)
 
     def _cb_disco_iq(self, iq):
-        if iq.getAttribute('to') == 'localhost':
-            iq['type'] = 'result'
-            iq['from'] = 'localhost'
-            self.send(iq)
+        # Server disco doesn't contain PEP
+        iq['type'] = 'result'
+        iq['from'] = 'localhost'
+        self.send(iq)
+
+    def _cb_bare_jid_disco_iq(self, iq):
+        # ...but our own jid disco does
+        nodes = xpath.queryForNodes(
+            "/iq/query[@xmlns='http://jabber.org/protocol/disco#info']",
+            iq)
+        query = nodes[0]
+        identity = query.addElement('identity')
+        identity['category'] = 'pubsub'
+        identity['type'] = 'pep'
+
+        iq['type'] = 'result'
+        iq['from'] = iq['to']
+        self.send(iq)
 
 if __name__ == '__main__':
     exec_test(test_legacy)
     exec_test(test_no_pep, protocol=GoogleXmlStream)
-    exec_test(test_pep, protocol=MyXmppStream)
+    exec_test(test_pep, protocol=PEPinJidDiscoXmppStream)