Bug 65131 - Gabble 0.17.3 does not interoperate with GMail web UI
Summary: Gabble 0.17.3 does not interoperate with GMail web UI
Status: RESOLVED FIXED
Alias: None
Product: Wocky
Classification: Unclassified
Component: General (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on: 65311
Blocks: 65290
  Show dependency treegraph
 
Reported: 2013-05-29 14:12 UTC by Simon McVittie
Modified: 2013-06-04 11:42 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Require GLib 2.32 (3.67 KB, patch)
2013-05-29 14:13 UTC, Simon McVittie
Details | Splinter Review
Acknowledge Jingle IQs in a way that the Google webmail client likes (6.03 KB, patch)
2013-05-29 14:14 UTC, Simon McVittie
Details | Splinter Review
[1 v2] Require GLib 2.32 (4.74 KB, patch)
2013-05-30 16:28 UTC, Simon McVittie
Details | Splinter Review
[2 v2] Acknowledge Jingle IQs in a way that the Google webmail client likes (6.11 KB, patch)
2013-05-30 16:30 UTC, Simon McVittie
Details | Splinter Review
[Gabble] Update Wocky for fd.o #65131, and test it (1.84 KB, patch)
2013-05-30 18:35 UTC, Simon McVittie
Details | Splinter Review
hack: don't send telephone-event PTs to Google webmail (1.35 KB, patch)
2013-05-30 19:32 UTC, Simon McVittie
Details | Splinter Review
WockyJingleFactory: ref session while calling parse() (2.56 KB, patch)
2013-06-03 10:08 UTC, Simon McVittie
Details | Splinter Review
WockyJingleSession: check more preconditions in public API (9.00 KB, patch)
2013-06-03 10:13 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2013-05-29 14:12:28 UTC
I've recently been testing calls between GMail's web UI (in Chrome on Windows 7, if that matters) and Empathy. After a bit of Wocky hacking, they can be made to work.
Comment 1 Simon McVittie 2013-05-29 14:13:49 UTC
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.
Comment 2 Simon McVittie 2013-05-29 14:14:22 UTC
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 3 Xavier Claessens 2013-05-30 11:07:58 UTC
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 4 Xavier Claessens 2013-05-30 11:09:39 UTC
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?
Comment 5 Simon McVittie 2013-05-30 16:28:10 UTC
Created attachment 80051 [details] [review]
[1 v2] Require GLib 2.32
Comment 6 Simon McVittie 2013-05-30 16:30:36 UTC
Created attachment 80052 [details] [review]
[2 v2] Acknowledge Jingle IQs in a way that the Google webmail  client likes
Comment 7 Simon McVittie 2013-05-30 18:35:16 UTC
Created attachment 80062 [details] [review]
[Gabble] Update Wocky for fd.o #65131, and test it
Comment 8 Xavier Claessens 2013-05-30 19:21:49 UTC
+1
Comment 9 Simon McVittie 2013-05-30 19:32:39 UTC
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 10 Simon McVittie 2013-05-30 19:35:59 UTC
Comment on attachment 80051 [details] [review]
[1 v2] Require GLib 2.32

merged, thanks
Comment 11 Simon McVittie 2013-05-30 19:36:09 UTC
Comment on attachment 80052 [details] [review]
[2 v2] Acknowledge Jingle IQs in a way that the Google webmail  client likes

merged, thanks
Comment 12 Simon McVittie 2013-05-30 19:36:22 UTC
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
Comment 13 Simon McVittie 2013-06-03 10:08:32 UTC
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.
Comment 14 Simon McVittie 2013-06-03 10:13:12 UTC
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 15 Simon McVittie 2013-06-03 19:14:48 UTC
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.
Comment 16 Xavier Claessens 2013-06-04 09:40:58 UTC
attachment #80196 [details] [review] and attachment #80197 [details] [review] looks good to me.
Comment 17 Simon McVittie 2013-06-04 11:42:49 UTC
Fixed in git for gabble 0.17.5, thanks.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.