Closed Bug 1390478 Opened 3 years ago Closed 3 years ago
menus don't show sync status until too long after startup
59 bytes, text/x-review-board-request
57.0a1 (2017-08-15) (64-bit) Mac OS X STR: 1. start Firefox 2. open the cheeseburger menu Expected: Says I'm signed into Sync. Actual: Doesn't. Later it does, though!
More info: * 347 tabs * about:support report: https://pastebin.mozilla.org/9029769
How long does it take for you? We recently optimized this for minimal startup cost but it shouldn't take "too long".
Just timed it, about 40 seconds.
We may want to add a 5 or 10 sec timeout in the initialization code , what do you think Florian?  http://searchfox.org/mozilla-central/source/browser/base/content/browser.js#1705-1708
(In reply to Dietrich Ayala (:dietrich) from comment #3) > Just timed it, about 40 seconds. Is it the time it takes for the CPU usage to go down after restoring all your tabs? (In reply to Edouard Oger [:eoger] from comment #4) > We may want to add a 5 or 10 sec timeout in the initialization code , > what do you think Florian? > >  > http://searchfox.org/mozilla-central/source/browser/base/content/browser. > js#1705-1708 That would be ok. But I wonder if we should just force the initialization when someone clicks the hamburger button.
So it turns out it is a bit more complicated than this, in UIState.jsm _refreshFxAState does a profile HTTP call. The UI won't be updated until that HTTP call finishes and I'm guessing this is what happened for :dietrich. There are some solutions here: - We can either update the UI 2 times unconditionally (once after reading the FxA data, then after the profile calls eventually succeed) - We could also put a Promise timeout on that HTTP request (with Promise.race(), 5 sec max for example). If we are out of time, though luck, update the UI and try next time. - We could implement some kind of hybrid of the two previous solutions: if we get a Promise timeout, update the UI and update the UI when the profile HTTP request is done. It requires more refactoring, and I'm not sure it is really worth it.
(In reply to Florian Quèze [:florian] [:flo] from comment #5) > (In reply to Dietrich Ayala (:dietrich) from comment #3) > > Just timed it, about 40 seconds. > > Is it the time it takes for the CPU usage to go down after restoring all > your tabs? No way! These days it's WAY faster than that :D (In reply to Edouard Oger [:eoger] from comment #6) > > The UI won't be updated until that HTTP call finishes This seems like a problematic pattern.
(In reply to Edouard Oger [:eoger] from comment #6) > So it turns out it is a bit more complicated than this, in UIState.jsm > _refreshFxAState does a profile HTTP call. > The UI won't be updated until that HTTP call finishes and I'm guessing this > is what happened for :dietrich. Don't we cache the profile these days and immediately use the cached version, kicking off a background request to update the profile if it is stale?
I don't think we cache the profile on disk though, so we have to do a request.
I think we do via http://searchfox.org/mozilla-central/source/services/fxaccounts/FxAccounts.jsm#1635 - my signedInUser.json certainly has it.
I have created a Nightly build that logs what is happening when we refresh the sync UI state. If you can reproduce the problem reliability, could you run the build until the hamburger menu gets updated and copy-paste the content of the browser console somewhere? Here is a direct link to the build: https://queue.taskcluster.net/v1/task/RzlS6upcRUqQsZC1DIW1Bg/runs/0/artifacts/public/build/target.dmg from https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c123ee7f8f887eb92b616004273319324391b12
Here's an up-to-date build: https://queue.taskcluster.net/v1/task/IkRK8i-eR8C6dQ3L5jn0_w/runs/0/artifacts/public/build/target.dmg If you have time to try it Dietrich.
Deprioritized. Can't work on this until we hear back from reporter.
Priority: P1 → P2
I don't have time to dig into this, and also the test build is no longer there. But you shouldn't need me in order to fix this. As long as your code is racing the user vs the http request completing, anyone on a slow network can experience this to varying degrees. Loads of networking throttling tools are available for creating these conditions. It also sounds like, per comments #8 - #10, that this is only happening because of maybe some other underlying bug that probably needs to be looked at.
We do block on the profile request if we don't have a cached copy, which is an obvious improvement we could make. However, it does seem unlikely we'd hit this in practice any time other than the user signing up.
Not blocking for the profile is a simple fix.
Assignee: nobody → markh
Comment on attachment 8935580 [details] Bug 1390478 - never block the UI waiting for the FxA profile. https://reviewboard.mozilla.org/r/206460/#review212470 LGTM, thanks!
Attachment #8935580 - Flags: review?(eoger) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/0e26ab46cbb7 never block the UI waiting for the FxA profile. r=eoger
Backed out changeset 0e26ab46cbb7 (bug 1390478) for Xpcshell failure in services/fxaccounts/tests/xpcshell/test_profile.js on a CLOSED TREE https://treeherder.mozilla.org/logviewer.html#?job_id=152345824&repo=autoland&lineNumber=7994 https://hg.mozilla.org/integration/autoland/rev/d063918c34f8d803d00821e6b0676f1501a25f97 https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0e26ab46cbb7a6cff4fc129bf11c3d3cd86b6ab5&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=152345824
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/36187d67ca82 never block the UI waiting for the FxA profile. r=eoger
You need to log in before you can comment on or make changes to this bug.