1104279 - Direct FxA Hello calls don't connect after you log in

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

If on browser A I log into FxA after starting in a logged-out state, and then have another browser B try to make an FxA call to the user logged into browser A, no incoming call indicator appears, and eventually the caller times out.  A can call B via FxA, however.

Also, if I log out, and then log in (without quitting the browser), we have the same problem.

It does work if I start the browser after shutting it down while logged in.
I haven't tested every variation of this, just the ones mentioned.

Occurs on Nightly/36 and Aurora 35, but not on Beta/34.

Comment 1

[Mike de Boer [:mikedeboer]]

This could be another fallout of bug 1074663, which introduced LoopCalls.jsm.

Comment 2

[Mark Banner (:standard8)]

I think there may be a similar issue with rooms as well, although bug 1102432 is making it harder to tell.

Comment 3

[u279076]

[Tracking Requested - why for this release]: this is a regression which blocks Firefox Hello in Firefox 35.

Comment 4

[:shell escalante]

needs owner

Comment 5

[Maire Reavy [:mreavy]]

Thanks, Paul!

Comment 7

[Mark Banner (:standard8)]

Attachment: Direct FxA Hello calls don't connect after you log in. Avoid registering with the loop server before we've finished fxa registration.

One-liner fix, to ensure we've finished FxA login before running registration with the loop-server.

I think we really ought to try to get a unit test for this, but if its not easy, I suggest we spin it out to another bug so that we can get the simple fix landed.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/fx-team/rev/b38a659f3d0d

Comment 9

[Paul Kerr [:pkerr]]

Attachment: Part 1: Direct FxA Hello calls don't connect after you log in. Avoid registering with the loop server before we've finished fxa registration

Comment 10

[Paul Kerr [:pkerr]]

Attachment: Part 2: Add sequence verification to logInToFxa test

Comment 11

[Paul Kerr [:pkerr]]

Attachment: Part 2: Add sequence verification to logInToFxa test

Comment 12

[Paul Kerr [:pkerr]]

Comment on attachment 8539374 [details] [diff] [review]
Part 2: Add sequence verification to logInToFxa test

Note, the first part has already been landed on fx-team. So, if using the latest fx-team, do not apply the first patch (or ignore the rejected single line change).

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8539374 [details] [diff] [review]
Part 2: Add sequence verification to logInToFxa test

Review of attachment 8539374 [details] [diff] [review]:
-----------------------------------------------------------------

r=dmose, once the two comments added are addressed.

::: browser/components/loop/test/mochitest/loop_fxa.sjs
@@ +200,5 @@
>                                               request.bodyInputStream.available());
>    let payload = JSON.parse(body);
> +  if (payload.simplePushURL == "https://localhost/pushUrl/fxa" ||
> +      payload.simplePushURLs.calls == "https://localhost/pushUrl/fxa-calls" ||
> +      payload.simplePushURLs.rooms == "https://localhost/pushUrl/fxa-rooms") {

It'd be great if you could somehow encapsulate this in a function named something like isFxARequest so that the intent is more clear.

@@ +209,5 @@
> +        response.write("401 Missing Hawk Authorization header");
> +        return;
> +      } else if (!getSharedState("/fxa-oauth/token")) {
> +        response.setStatusLine(request.httpVersion, 409, "Wrong State");
> +        response.write("409 complete OAuth before attempting registration");

Please file any appropriate bug on the real loop-server having issues in this situation, and maybe set a blocking? flag on it.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8539366 [details] [diff] [review]
Part 1: Direct FxA Hello calls don't connect after you log in. Avoid registering with the loop server before we've finished fxa registration

Approval Request Comment
[Feature/regressing bug #]: N/A (FxA)

[User impact if declined]: Inability to receive calls without stopping the browser and restarting (BAD)

[Describe test coverage new/current, TBPL]: on fxteam; will merge to m-c soon.  Will verify before landing (I've already verified on fxteam)

[Risks and why]: Low risk, simple fix

[String/UUID change made/needed]: none

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8539374 [details] [diff] [review]
Part 2: Add sequence verification to logInToFxa test

Approval Request Comment
[Feature/regressing bug #]: N/A (FxA)

[User impact if declined]: Lack of test coverage for this bug

[Describe test coverage new/current, TBPL]: will land shortly.  Will wait for green on m-c before uplift.

[Risks and why]: test-only change, no risk to shipping code

[String/UUID change made/needed]: none

Comment 16

[Paul Kerr [:pkerr]]

Attachment: Part 2: Add sequence verification to logInToFxa test

incorporate review feedback

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b38a659f3d0d

Comment 18

[Maire Reavy [:mreavy]]

Reopening so that the second patch (the tests) will land on m-c.

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c02c3b49ce95

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c02c3b49ce95

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/59089cd08413

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8a7b2ce0aeaf

Comment 23

[Randell Jesup [:jesup] (needinfo me)]

Missed part 1:
https://hg.mozilla.org/releases/mozilla-aurora/rev/172c154b8d05
https://hg.mozilla.org/releases/mozilla-beta/rev/9d2cf73901c8

Comment 24

[u279076]

Florin, since this was a regression, can you please make sure this gets tested as part of the next Beta sign off?

Comment 25

[Cornel Ionce [:noni] [Hubs QA]]

Confirming that this issue is now fixed on Mac OS X 10.9.5, Ubuntu 12.04 32-bit and Windows 7 x64.
The user is now able to receive calls without needing to restart the browser after log in, as mentioned in the description.
Also, exploratory testing was performed around this fix to make sure no other issues regressed.

Tested with:
Latest Nightly, build ID: 20141230030214
Firefox 36.0a2, build ID: 20141229004004
Firefox 25 beta 8, build ID: 20141229214612