Closed Bug 1355481 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Sync, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: florian, Assigned: eoger)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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
It looks like this should be referencing this.weaveService.ready, as already done a few lines up in that function.
Assignee: nobody → eoger
Priority: -- → P2
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+
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)
I got bit by the |this| scoping, sorry about this.
Flags: needinfo?(eoger)
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
https://hg.mozilla.org/mozilla-central/rev/533c5fc7c41a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.