Bug 65131

Created attachment 79970 [details] [review]
Require GLib 2.32

This avoids compilation failing because the new(ish) g_thread_new
wasn't available in our target version. telepathy-gabble indirectly
depends on GLib 2.32 anyway, via telepathy-glib 0.20.

Created attachment 79971 [details] [review]
Acknowledge Jingle IQs in a way that the Google webmail  client likes

The Google webmail client currently starts calls like this:

    <iq type="set" ...>
        <jingle xmlns="urn:xmpp:jingle:1'"
          action="session-initiate" ...>...</jingle>
        <session xmlns="http://www.google.com/session"
          type="initiate" ...>...</session>
    </iq>

(This isn't valid XMPP Core, because 'An IQ stanza of type "get"
or "set" MUST contain exactly one child element', but we can
tolerate it.)

When called, it also echoes the contents of the sender's IQ back to the
sender. It appears that this is how, when it makes an outgoing call,
it determines which dialect the recipient wants to use: the recipient
echoes the appropriate child element.

Comment on attachment 79971 [details] [review]
Acknowledge Jingle IQs in a way that the Google webmail  client likes

Review of attachment 79971 [details] [review]:
-----------------------------------------------------------------

::: wocky/wocky-jingle-session.c
@@ +2565,5 @@
> +              wocky_porter_send (self->priv->porter, reply);
> +              g_object_unref (reply);
> +            }
> +        }
> +    }

an early return is missing here AFAIK.

Comment on attachment 79970 [details] [review]
Require GLib 2.32

Review of attachment 79970 [details] [review]:
-----------------------------------------------------------------

wocky.pc.in still has: Requires.private: glib-2.0 >= 2.16, gobject-2.0 >= 2.16, gio-2.0 >= 2.26

Shouldn't it be bumped to .32 as well?

Created attachment 80051 [details] [review]
[1 v2] Require GLib 2.32

Created attachment 80052 [details] [review]
[2 v2] Acknowledge Jingle IQs in a way that the Google webmail  client likes

Created attachment 80062 [details] [review]
[Gabble] Update Wocky for fd.o #65131, and test it

+1

Created attachment 80065 [details] [review]
hack: don't send telephone-event PTs to Google webmail

On one OS I've tried (but not on Debian for some reason...),
we get 'incompatible-parameters' from the Google web UI if
we tell them we support telephone events. Your guess is
as good as mine...

I doubt this should actually be merged, but it's a start.

---

This applies to session-initiate from us to Google, i.e. we initiate the call.

Comment on attachment 80051 [details] [review]
[1 v2] Require GLib 2.32

merged, thanks

Comment on attachment 80052 [details] [review]
[2 v2] Acknowledge Jingle IQs in a way that the Google webmail  client likes

merged, thanks

Comment on attachment 80062 [details] [review]
[Gabble] Update Wocky for fd.o #65131, and test it

this part fixed in git for 0.17.5, thanks

Created attachment 80196 [details] [review]
WockyJingleFactory: ref session while calling parse()

wocky_jingle_session_parse() advances the session's state
machine. In particular, it may cause termination, which
causes the session to be removed from the factory's
hash table, which may cause its last ref to be released.

Until recently, this would have gone unnoticed, but
wocky_jingle_session_acknowledge_iq() now emits a signal
from the session (to check whether it has the "is Google
webmail" quirk), and that causes a check that it is in fact
still a valid session object.

Also correct a misleading comment spotted while debugging
this: priv->sessions owns both key and sess.

---

Unfortunately, the patches I already merged for this caused a regression, which I caught while trying to make Gabble's tests a little more reliable. This seems to fix it.

Created attachment 80197 [details] [review]
WockyJingleSession: check more preconditions in public  API

Some were already checked, but as assertions: this is
(now) public library API, so downgrade to g_return_if_fail().

Comment on attachment 80065 [details] [review]
hack: don't send telephone-event PTs to Google webmail

A better workaround (I think), in Farstream instead of Wocky, is now available on Bug #65311.

attachment #80196 [details] [review] and attachment #80197 [details] [review] looks good to me.

Fixed in git for gabble 0.17.5, thanks.