See bug #39544 for a related bug on whitespace pings.

http://cgit.collabora.com/git/user/bari/wocky.git/log/?h=whitespace-ping contains the fix I decided to implement after talking with Will. http://cgit.collabora.com/git/user/bari/telepathy-gabble.git/log/?h=whitespace-ping is the tiny corresponding Gabble branch.

The patch removes the code for proper XMPP pings and make WockyPing accepts only C2S porters.
I'm not entirely sure about this solution as pings could potentially be useful for direct pings to contacts and because we usually have base Porters, so you end up having cast code like you can see in the Gabble branch.

There is another problem, one of the 2 ping tests in wocky doesn't work anymore. I have no idea how to fix it as what we do is:
1. Have 2 different connections
2. Get the first one to send a ping using WockyPing
3. Check the second one gets a XMPP ping
To make it work we would need to change 3 to expect a whitespace ping instead, but I don't see how as the parser doesn't really care about the whitespaces.

You're missing docstrings for wocky_c2s_porter_send_whitespace_ping_async and _finish.

You may be interested in wocky_implement_finish_void for future reference.

+typedef struct
+{
+  WockyC2SPorter *self;
+  GSimpleAsyncResult *result;
+} WhitespacePingData;

+  data->self = g_object_ref (self);
+  data->result = g_simple_async_result_new (G_OBJECT (self),
+    callback, user_data, wocky_c2s_porter_send_whitespace_ping_async);

This structure is pointless. GSimpleAsyncResult holds a ref to its source object: you can get it with g_async_result_get_source_object(). (Danger: that function returns a ref.)

The only functional issue with this branch is: I think you need to add a guard to wocky_c2s_porter_send_async() to make sure it doesn't call wocky_xmpp_connection_send_stanza_async while a wocky_xmpp_connection_send_whitespace_ping_async call is ongoing. Otherwise the call to wocky_xmpp_connection_send_stanza_async will fail and then the whole connection will get blown away.

It already has a guard for g_queue_get_length (priv->sending_queue) == 1 — I think you need to add && !priv->sending_whitespace_ping there; and a call to send_head_stanza in the success path of send_whitespace_ping_cb().

Updated, can you please check again?

(In reply to comment #2)
> The patch removes the code for proper XMPP pings and make WockyPing accepts
> only C2S porters.
> I'm not entirely sure about this solution as pings could potentially be useful
> for direct pings to contacts and because we usually have base Porters, so you
> end up having cast code like you can see in the Gabble branch.

How about this? Did you look at http://cgit.collabora.com/git/user/bari/telepathy-gabble.git/log/?h=whitespace-ping ?

(In reply to comment #4)
> Updated, can you please check again?

I think you need to unref res_out in send_whitespace_ping_cb.

Consider this situation:

• sending_queue is empty
• something calls wocky_c2s_porter_send_whitespace_ping_async(); it calls wocky_xmpp_connection_send_whitespace_ping_async() and sets sending_whitespace_ping = TRUE;
• something else calls wocky_porter_send_async(); it adds an element to sending_queue but doesn't try to send it because sending_whitespace_ping;
• send_whitespace_ping_cb is called; it sets sending_whitespace_ping = FALSE but doesn't flush the queue afaict? I think it should call send_head_stanza() on the success path.

On the tests: interesting choice to use wocky_test_stream_set_direct_read_callback not a signal. Not really a criticism  as such.

> 
> (In reply to comment #2)
> > The patch removes the code for proper XMPP pings and make WockyPing accepts
> > only C2S porters.
> > I'm not entirely sure about this solution as pings could potentially be useful
> > for direct pings to contacts and because we usually have base Porters, so you
> > end up having cast code like you can see in the Gabble branch.
> 
> How about this? Did you look at
> http://cgit.collabora.com/git/user/bari/telepathy-gabble.git/log/?h=whitespace-ping
> ?

I think the cast is fine, inasmuch as the other places we assume the same thing are fine… I don't really like it but I don't think it's a blocker.

(In reply to comment #5)
> On the tests: interesting choice to use
> wocky_test_stream_set_direct_read_callback not a signal. Not really a criticism
>  as such.

I did that because the input stream class is not public and I find it weird to connect to a signal that we are not supposed to know about.

Fixed the issues you found, plus another similar problem I found. We also need to close the connection after sending the ping if you are waiting to do so.

I pushed some "fixup!" commits as they are small and, I think, easy to review for you. If you prefer me to squash them together just ask.

(In reply to comment #7)
> Fixed the issues you found, plus another similar problem I found. We also need
> to close the connection after sending the ping if you are waiting to do so.

Great catches.

Looks good, ship it.

Pushed to wocky master and gabble master.