Firefox Sync not working until manually visiting

RESOLVED FIXED in Firefox 63



8 months ago
8 months ago


(Reporter: ragimiri, Assigned: lina)



62 Branch
Firefox 64
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox62 wontfix, firefox63 fixed, firefox64 fixed)



(2 attachments)



8 months ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180920131237

Steps to reproduce:

1. start Firefox

2. in hamburger menu check there is icon and name of Firefox account

3. wait for automatic sync or run it manually

Actual results:

Sync is not successful.

When I click on Firefox account icon and visit I can run sync manually without problem.

Expected results:

Sync should work without manually visiting
>ERROR	Sync encountered a login error
Component: Untriaged → Sync
Did you reset your master password or did this just start at some point ?
Flags: needinfo?(ragimiri)

Comment 3

8 months ago
I think the error is a side effect of switching over to `defineLazyGlobalGetters`.

`Cu.importGlobalProperties(["fetch"])` imports `fetch`, `Headers`, `Request`, and `Response` into the global scope (, but the lazy getter only runs when accessing the `fetch` symbol ( )...which means, if we're the first caller to import `fetch`, we'll call `_buildHeaders`, then throw because `importGlobalProperties` hasn't run yet, so `Headers` isn't in scope.
Blocks: 1464548
Ever confirmed: true
Flags: needinfo?(ragimiri)

Comment 4

8 months ago
Some global imports, like `fetch`, expose multiple symbols. This patch
ensures we can define lazy getters for those symbols, by mapping the
extra symbol name to the main import name.

Comment 5

8 months ago
Pushed by
Add support for importing extra symbols to `defineLazyGlobalGetters`. r=kmag

Comment 6

8 months ago
I think this is too late for 62, but 63 is merging to release in 3 weeks.
Assignee: nobody → lina

Comment 7

8 months ago
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64

Comment 8

8 months ago
Comment on attachment 9013780 [details]
Add support for importing extra symbols to `defineLazyGlobalGetters`.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1464548

User impact if declined: Sync will continuously fail if we hit the edge case in comment 3.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This is tricky to reproduce manually and in xpcshell, because it requires loading Sync before any other module that imports the `fetch` function. Typically, I think Activity Stream loads first, so most folks won't see this bug...but I'm not sure that order is deterministic.

However, this is a small patch, and I've verified it works locally by removing the `if` check in,178, then running the Sync tests.

String changes made/needed:
Attachment #9013780 - Flags: approval-mozilla-beta?
Comment on attachment 9013780 [details]
Add support for importing extra symbols to `defineLazyGlobalGetters`.

Minimal patch fixing a 63 Sync regression, uplift approved for 63 beta 12, thanks.
Attachment #9013780 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.