Closed Bug 982893 Opened 6 years ago Closed 6 years ago

attempt to avoid initializing Weave.Status from under updateUI doesn't work

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30

People

(Reporter: Gavin, Assigned: Gavin)

References

Details

Attachments

(1 file, 2 obsolete files)

There's code in browser-syncui's _loginFailed that checks service.ready before referencing Weave.Status, to try to avoid importing some Sync modules on startup.

However, the code in _needsSetup references Weave.Status unconditionally, so those modules get loaded anyways (_needsSetup and _loginFailed are both called by updateUI, which is called on window open via initUI).

The patch in bug 966342 that checked the user state also avoided initializing the service if it wasn't already, by checking service.ready before referencing Weave.Service.identity._signedInUser. But we can access _signedInUser via Weave.Status._authManager instead (which we are loading anyways), to avoid the issue of the menu being incorrect before Sync is initialized.
Attached patch patch (obsolete) — Splinter Review
* don't try to avoid loading Weave.Status modules on window open 
 * access the authManager via Weave.Status instead of Weave.Service, since Weave.Status will already be initialized in almost all cases.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #8390112 - Flags: review?(mhammond)
Attached patch tweaked patch (obsolete) — Splinter Review
Fixed the comment.
Attachment #8390112 - Attachment is obsolete: true
Attachment #8390112 - Flags: review?(mhammond)
Attachment #8390118 - Flags: review?(mhammond)
Attached patch tweaked patchSplinter Review
Fixed the comment as discussed IRL.
Attachment #8390161 - Flags: review?(mhammond)
Attachment #8390118 - Attachment is obsolete: true
Attachment #8390118 - Flags: review?(mhammond)
Attachment #8390161 - Flags: review?(mhammond) → review+
https://hg.mozilla.org/integration/fx-team/rev/8260554d3422

This is a trunk-only bug (relevant changes included in bug 966342's aurora patch)
Target Milestone: --- → Firefox 30
No longer blocks: 969593
https://hg.mozilla.org/mozilla-central/rev/8260554d3422
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.