If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

browser-syncui.js triggers as lazy getter for resource://services-sync/status.js during shutdown

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Sync
P2
normal
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: florian, Assigned: eoger)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
Not a giant perf impact (9ms in my profile), but this doesn't seem intended.

This is when accessing Weave.Status at http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/browser/base/content/browser-syncui.js#68

Of course this is on a profile where sync isn't configured, otherwise I assume these status.js and browserid_identity.js modules would already be loaded.

See the profile at https://perfht.ml/2nABiet

Comment 1

6 months ago
It looks like this should be referencing this.weaveService.ready, as already done a few lines up in that function.

Updated

6 months ago
Assignee: nobody → eoger
Priority: -- → P2
Comment hidden (mozreview-request)

Comment 3

5 months ago
mozreview-review
Comment on attachment 8858914 [details]
Bug 1355481 - Avoid initializing Sync on window unload if Sync is not configured.

https://reviewboard.mozilla.org/r/130904/#review133636
Attachment #8858914 - Flags: review?(markh) → review+

Comment 4

5 months ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e5e754f197cd
Avoid initializing Sync on window unload if Sync is not configured. r=markh
Backed out for breaking many tests:

https://hg.mozilla.org/integration/autoland/rev/b708d01388e3862be454e28985755687fe2bb0d4

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=e5e754f197cd5f1f0bc3c4179c9f1236b3b43c48&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=92380435&repo=autoland

[task 2017-04-18T15:33:23.675276Z] 15:33:23     INFO - GECKO(1239) | TEST-UNEXPECTED-FAIL | unknown test url | uncaught exception - TypeError: this.weaveService is undefined at onUnload@chrome://browser/content/browser-syncui.js:70:1
[task 2017-04-18T15:33:23.676853Z] 15:33:23     INFO - GECKO(1239) | EventListener.handleEvent*init@chrome://browser/content/browser-syncui.js:64:5
[task 2017-04-18T15:33:23.676933Z] 15:33:23     INFO - GECKO(1239) | _delayedStartup@chrome://browser/content/browser.js:1510:5
[task 2017-04-18T15:33:23.677771Z] 15:33:23     INFO - GECKO(1239) | EventListener.handleEvent*onLoad@chrome://browser/content/browser.js:1239:5
[task 2017-04-18T15:33:23.679405Z] 15:33:23     INFO - GECKO(1239) | onload@chrome://browser/content/browser.xul:1:1
[task 2017-04-18T15:33:23.679495Z] 15:33:23     INFO - GECKO(1239) | JavaScript error: chrome://browser/content/browser-syncui.js, line 70: TypeError: this.weaveService is undefined
Flags: needinfo?(eoger)
Comment hidden (mozreview-request)
(Assignee)

Comment 7

5 months ago
I got bit by the |this| scoping, sorry about this.
(Assignee)

Updated

5 months ago
Flags: needinfo?(eoger)

Comment 8

5 months ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/533c5fc7c41a
Avoid initializing Sync on window unload if Sync is not configured. r=markh

Comment 9

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/533c5fc7c41a
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.