845863 - Browser scripts (chrome) may be loaded more than once

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Sam Foster [:sfoster] (he/him)]

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).

Comment 1

[Sam Foster [:sfoster] (he/him)]

This issue is currently blocking testing and getting deterministic behavior from TopSites pin/unpin behavior.

Comment 2

[Sam Foster [:sfoster] (he/him)]

Attachment: Add a lookup for browser-scripts to allow loading a script only once

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.

Comment 3

[Matt Brubeck (:mbrubeck)]

Comment on attachment 719042 [details] [diff] [review]
Add a lookup for browser-scripts to allow loading a script only once

Thanks!

Comment 4

[Sam Foster [:sfoster] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31fce81d3394

Comment 5

[Matt Brubeck (:mbrubeck)]

Reopening until this lands in mozilla-central.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/31fce81d3394