Browser scripts (chrome) may be loaded more than once

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sfoster, Assigned: sfoster)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
When a single script provides more than one lazy-loadable symbol/module, the way scripts are loaded in browser-scripts causes the script to be loaded and interpreted each time. This is unnecessary overhead and results in loss of state and unpredictable behavior as objects are redefined.

This issue was observed with chrome://browser/content/TopSites.js, which provides TopSitesView, TopSitesSnappedView and TopSitesStartView - but applies equally to dowloads.js and others (present and future).
(Assignee)

Comment 1

6 years ago
This issue is currently blocking testing and getting deterministic behavior from TopSites pin/unpin behavior.
Assignee: nobody → sfoster
Blocks: 80870
Hardware: x86_64 → x86
(Assignee)

Comment 2

6 years ago
First pass at a fix. I use a simple object lookup to stash the sandbox/context objects keyed by the script path. This allows us to skip the loadSubScript call for a script that is already loaded. 

The side-effect is that now all the contexts are in scope in browser-scripts.js - previously those were scoped to the defineLazyGetter callback. I'm not sure if this has any consequences.
Attachment #719042 - Flags: review?(mbrubeck)
(Assignee)

Updated

6 years ago
Blocks: 808770
No longer blocks: 80870
Comment on attachment 719042 [details] [diff] [review]
Add a lookup for browser-scripts to allow loading a script only once

Thanks!
Attachment #719042 - Flags: review?(mbrubeck) → review+
(Assignee)

Comment 4

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/31fce81d3394
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Reopening until this lands in mozilla-central.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/31fce81d3394
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.