Closed
Bug 1495723
Opened 6 years ago
Closed 6 years ago
Firefox Sync not working until manually visiting accounts.firefox.com
Categories
(Firefox :: Sync, defect)
Tracking
()
RESOLVED
FIXED
Firefox 64
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox62 | --- | wontfix |
firefox63 | --- | fixed |
firefox64 | --- | fixed |
People
(Reporter: ragimiri, Assigned: lina)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
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 accounts.firefox.com I can run sync manually without problem.
Expected results:
Sync should work without manually visiting accounts.firefox.com.
Comment 2•6 years ago
|
||
Did you reset your master password or did this just start at some point ?
Flags: needinfo?(ragimiri)
Assignee | ||
Comment 3•6 years 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 (https://searchfox.org/mozilla-central/rev/6ddb5fb144993fb5de044e2e8d900d7643b98a4d/js/xpconnect/src/Sandbox.cpp#348-350), but the lazy getter only runs when accessing the `fetch` symbol (https://searchfox.org/mozilla-central/rev/6ddb5fb144993fb5de044e2e8d900d7643b98a4d/js/xpconnect/loader/XPCOMUtils.jsm#173-182 )...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.
Assignee | ||
Comment 4•6 years 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.
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1fa05a1ba762
Add support for importing extra symbols to `defineLazyGlobalGetters`. r=kmag
Assignee | ||
Comment 6•6 years ago
|
||
I think this is too late for 62, but 63 is merging to release in 3 weeks.
Assignee: nobody → lina
status-firefox62:
--- → wontfix
status-firefox63:
--- → affected
status-firefox64:
--- → affected
Comment 7•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Assignee | ||
Comment 8•6 years 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 https://searchfox.org/mozilla-central/rev/6ddb5fb144993fb5de044e2e8d900d7643b98a4d/js/xpconnect/loader/XPCOMUtils.jsm#176,178, then running the Sync tests.
String changes made/needed:
Attachment #9013780 -
Flags: approval-mozilla-beta?
Comment 9•6 years ago
|
||
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+
Comment 10•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Keywords: regression
Updated•6 years ago
|
status-firefox-esr60:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•