menus don't show sync status until too long after startup

RESOLVED FIXED in Firefox 59

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dietrich, Assigned: markh)

Tracking

unspecified
Firefox 59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
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!
Reporter

Comment 1

2 years ago
More info:

* 347 tabs
* about:support report: https://pastebin.mozilla.org/9029769
Assignee

Comment 2

2 years ago
How long does it take for you? We recently optimized this for minimal startup cost but it shouldn't take "too long".
Reporter

Comment 3

2 years ago
Just timed it, about 40 seconds.
We may want to add a 5 or 10 sec timeout in the initialization code [0], what do you think Florian?

[0] http://searchfox.org/mozilla-central/source/browser/base/content/browser.js#1705-1708
Flags: needinfo?(florian)
(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 [0],
> what do you think Florian?
> 
> [0]
> 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.
Flags: needinfo?(florian)
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.
Reporter

Comment 7

2 years ago
(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.
Assignee

Comment 8

2 years ago
(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.
Assignee

Comment 10

2 years ago
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
Assignee: nobody → eoger
Priority: -- → P1
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.
Flags: needinfo?(dietrich)
Deprioritized. Can't work on this until we hear back from reporter.
Priority: P1 → P2
Reporter

Comment 14

2 years ago
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.
Flags: needinfo?(dietrich)
Assignee: eoger → nobody
Assignee

Updated

2 years ago
Flags: needinfo?(markh)
Assignee

Comment 15

2 years ago
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.
Flags: needinfo?(markh)
Assignee

Comment 16

2 years ago
Not blocking for the profile is a simple fix.
Assignee: nobody → markh
Comment hidden (mozreview-request)

Comment 18

2 years ago
mozreview-review
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+

Comment 19

2 years ago
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/0e26ab46cbb7
never block the UI waiting for the FxA profile. r=eoger
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 23

2 years ago
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/36187d67ca82
never block the UI waiting for the FxA profile. r=eoger

Comment 24

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/36187d67ca82
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Assignee

Updated

2 years ago
Flags: needinfo?(markh)
You need to log in before you can comment on or make changes to this bug.