Closed Bug 1761938 Opened 2 months ago Closed 1 month ago

Use separate module loaders for dynamic module import in web extension content scripts

Categories

(WebExtensions :: General, task, P2)

task

Tracking

(firefox101 fixed)

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: jonco, Assigned: jonco, NeedInfo)

Details

Attachments

(7 files)

Bug 1536094 implemented dynamic module import for web extension content scripts by using the main web page's module map to track loaded modules. This works by keying the modules off the global as well as the module URI for content scripts.

This works well but leads to some complexity because the script loader and module loader are associated with more than one global and it's not always clear what the correct global to use is. Ideally we would have a separate module map per content script (which would be associated with a single global).

The plan in this bug is to use a separate module loader per content script and remove the keying off the global object. The module loader can have a field for its global rather than querying it via a method call.

Smaug, does this sound reasonable to you? I'm unsure of the relative lifetime of content script globals WRT the script loader. The script loader will need to hold a list of module loaders for content scripts. Would I need to add something to clean these up at some point?

Flags: needinfo?(bugs)

I don't know about the lifetime of web extensions stuff.
zombie might know that better.

Flags: needinfo?(bugs) → needinfo?(tomica)

But in general, using separate module map does sound reasonable to me.

(In reply to Olli Pettay [:smaug] from comment #3)
OK great. Questions about content script globals notwithstanding, I'll put the patches up for review.

Tidyup. There's no reason for this to be static if it takes a pointer to the instance.

Assignee: nobody → jcoppeard
Status: NEW → ASSIGNED

The script loader will need to deal with requests from more than one module
loader so this adds methods to the request which dispatch to the correct
loader.

Depends on D142827

This gives the DOM module loader a kind and records content script module
loaders in an array on the script loader.

Depends on D142828

Since content scripts have separate module loaders there's no need to key the
module map on the global any more.

Depends on D142829

This gives the module loader a field for the current global, since a module map
is only associated with a single global.

This adds a method to Document to tell the script loader when its global
changes. I'm not sure of when we do this exactly.

Depends on D142830

This removes the global object field from the script load context and gets the
global from the script loader / module loader where necessary.

Depends on D142832

Severity: -- → N/A
Priority: -- → P2
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/da0c4a2767c5
Part 0: Make ModuleLoaderBase::ResolveModuleSpecifier an instance method r=yulia
https://hg.mozilla.org/integration/autoland/rev/bedefc75fd2a
Part 1: Call into the module loader via the module load request object r=yulia
https://hg.mozilla.org/integration/autoland/rev/7c175beb4616
Part 2: Give each web extension content script global its own module loader r=smaug,yulia
https://hg.mozilla.org/integration/autoland/rev/667fc1b891ee
Part 3: Remove global from the module map key and use URI only r=yulia
https://hg.mozilla.org/integration/autoland/rev/3334d5729ed4
Part 4: Give module loaders a global object field r=smaug,yulia
https://hg.mozilla.org/integration/autoland/rev/721780e3ab66
Part 5: Use module loader's global where possible and remove GetGlobalForRequest from script loader interface r=yulia
https://hg.mozilla.org/integration/autoland/rev/5aa276c140f6
Part 6: Remove global from script load context r=smaug,yulia
You need to log in before you can comment on or make changes to this bug.