Closed Bug 1104279 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect

Tracking

(firefox34 unaffected, firefox35+ verified, firefox36 verified, firefox37 verified)

VERIFIED FIXED
mozilla37
Iteration:
37.1
Tracking Status
firefox34 --- unaffected
firefox35 + verified
firefox36 --- verified
firefox37 --- verified
Blocking Flags:
backlog Fx35+

People

(Reporter: jesup, Assigned: pkerr)

References

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

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.
This could be another fallout of bug 1074663, which introduced LoopCalls.jsm.
Severity: normal → major
OS: Linux → All
Priority: -- → P1
Hardware: x86_64 → All
I think there may be a similar issue with rooms as well, although bug 1102432 is making it harder to tell.
backlog: --- → Fx35?
backlog: Fx35? → Fx35+
[Tracking Requested - why for this release]: this is a regression which blocks Firefox Hello in Firefox 35.
needs owner
Flags: needinfo?(mreavy)
Assignee: nobody → pkerr
Thanks, Paul!
Flags: needinfo?(mreavy)
Duplicate of this bug: 1109674
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.
Attachment #8538797 - Flags: review?(pkerr)
Attachment #8538797 - Flags: review?(pkerr) → review+
Attachment #8538797 - Attachment is obsolete: true
Attachment #8539366 - Flags: review+
Attachment #8539372 - Attachment is obsolete: true
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).
Attachment #8539374 - Flags: review?(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.
Attachment #8539374 - Flags: review?(dmose) → review+
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
Attachment #8539366 - Flags: approval-mozilla-beta?
Attachment #8539366 - Flags: approval-mozilla-aurora?
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
Attachment #8539374 - Flags: approval-mozilla-beta?
Attachment #8539374 - Flags: approval-mozilla-aurora?
incorporate review feedback
Attachment #8539374 - Attachment is obsolete: true
Attachment #8539374 - Flags: approval-mozilla-beta?
Attachment #8539374 - Flags: approval-mozilla-aurora?
Attachment #8539547 - Flags: review+
Attachment #8539547 - Flags: approval-mozilla-beta?
Attachment #8539547 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/b38a659f3d0d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Reopening so that the second patch (the tests) will land on m-c.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/c02c3b49ce95
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Attachment #8539366 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8539547 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8539366 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8539547 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Florin, since this was a regression, can you please make sure this gets tested as part of the next Beta sign off?
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
Flags: in-testsuite+
Iteration: --- → 37.1
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
Status: RESOLVED → VERIFIED
Flags: needinfo?(florin.mezei)
You need to log in before you can comment on or make changes to this bug.