A further problem with dynamically inserted import maps was discovered where
sometimes module scripts would never execute. This happens when the script is
still being fetched when the import map is added.
To fix this, cancel all fetching module preloads as well when an import map is
registered and remove them from the module map. In theory this shouldn't be
necessary but I wasn't able to make the tests pass without this step.
For simplicity also remove all module preloads from the scriptloader's list of
preload requests rather than detecting and ignoring them later on.
Differential Revision: https://phabricator.services.mozilla.com/D204202
Currently we record the URI of each fetching modules and any addtional requests
that are blocked waiting on that fetch. This adds the fetching module request
itself.
This is needed by the next patch.
Differential Revision: https://phabricator.services.mozilla.com/D204201
The initial problem reported in the bug is that we try to speculatively load a
bareword import before the import map is dynamically inserted that makes that
load valid. The load fails and then we cache the failure in the module map.
The more general problem is that import maps change the locations that imports
are resolved to so if when there is a dynamically inserted import map
speculative preload may load and cache the wrong things.
This patch fixes this problem by removing any preloaded modules from the module
map when an import map is registered.
Previously we used to do something like this but it was changed because I
wasn't confident that it wouldn't remove too much. However it appears that this
is necessary to handle this situation, so it's implemented here but with more
checks that it only removes preloaded modules. This is handled by adding extra
flags where necessary so we have the information on hand to check.
I've made these diagnostic asserts so that this actually gets check in real use.
Differential Revision: https://phabricator.services.mozilla.com/D202611
A further problem with dynamically inserted import maps was discovered where
sometimes module scripts would never execute. This happens when the script is
still being fetched when the import map is added.
To fix this, cancel all fetching module preloads as well when an import map is
registered and remove them from the module map. In theory this shouldn't be
necessary but I wasn't able to make the tests pass without this step.
For simplicity also remove all module preloads from the scriptloader's list of
preload requests rather than detecting and ignoring them later on.
Differential Revision: https://phabricator.services.mozilla.com/D204202
Currently we record the URI of each fetching modules and any addtional requests
that are blocked waiting on that fetch. This adds the fetching module request
itself.
This is needed by the next patch.
Differential Revision: https://phabricator.services.mozilla.com/D204201
The initial problem reported in the bug is that we try to speculatively load a
bareword import before the import map is dynamically inserted that makes that
load valid. The load fails and then we cache the failure in the module map.
The more general problem is that import maps change the locations that imports
are resolved to so if when there is a dynamically inserted import map
speculative preload may load and cache the wrong things.
This patch fixes this problem by removing any preloaded modules from the module
map when an import map is registered.
Previously we used to do something like this but it was changed because I
wasn't confident that it wouldn't remove too much. However it appears that this
is necessary to handle this situation, so it's implemented here but with more
checks that it only removes preloaded modules. This is handled by adding extra
flags where necessary so we have the information on hand to check.
I've made these diagnostic asserts so that this actually gets check in real use.
Differential Revision: https://phabricator.services.mozilla.com/D202611
A further problem with dynamically inserted import maps was discovered where
sometimes module scripts would never execute. This happens when the script is
still being fetched when the import map is added.
To fix this, cancel all fetching module preloads as well when an import map is
registered and remove them from the module map. In theory this shouldn't be
necessary but I wasn't able to make the tests pass without this step.
For simplicity also remove all module preloads from the scriptloader's list of
preload requests rather than detecting and ignoring them later on.
Differential Revision: https://phabricator.services.mozilla.com/D204202
Currently we record the URI of each fetching modules and any addtional requests
that are blocked waiting on that fetch. This adds the fetching module request
itself.
This is needed by the next patch.
Differential Revision: https://phabricator.services.mozilla.com/D204201
The initial problem reported in the bug is that we try to speculatively load a
bareword import before the import map is dynamically inserted that makes that
load valid. The load fails and then we cache the failure in the module map.
The more general problem is that import maps change the locations that imports
are resolved to so if when there is a dynamically inserted import map
speculative preload may load and cache the wrong things.
This patch fixes this problem by removing any preloaded modules from the module
map when an import map is registered.
Previously we used to do something like this but it was changed because I
wasn't confident that it wouldn't remove too much. However it appears that this
is necessary to handle this situation, so it's implemented here but with more
checks that it only removes preloaded modules. This is handled by adding extra
flags where necessary so we have the information on hand to check.
I've made these diagnostic asserts so that this actually gets check in real use.
Differential Revision: https://phabricator.services.mozilla.com/D202611
On the main thread, single mozJSModuleLoader instance is shared across all
loads in non-shared global, with resetting the internal state after importing
a module graph.
NonSharedGlobalSyncModuleLoaderScope manages the lifetime of each usage.
Import into the same non-shared global can be nested, but import into the
different non-shared gloobal is not allowed while other import for non-shared
global is ongoing.
Differential Revision: https://phabricator.services.mozilla.com/D199456
In non-shared global, the sync load is performed by separate SyncModuleLoader,
with the following algorithm:
1. copy all loaded modules from async module loader, so that they can be
used in the module graph in SyncModuleLoader
2. import a module graph
3. move all modules to async module loader
ModuleLoaderBase::CopyModulesTo is for step 1, and
ModuleLoaderBase::MoveModulesTo is for step 3.
The consumer should assert that there's no fetching modules.
Differential Revision: https://phabricator.services.mozilla.com/D199455
In order to perform sync load in arbitrary global, add
ModuleLoaderBase::mOverriddenBy field, and let
ModuleLoaderBase::GetCurrentModuleLoader return the overridden loader,
so that the module loading can be overridden by SyncModuleLoader.
AutoOverrideModuleLoader manages the lifetime of the override.
Differential Revision: https://phabricator.services.mozilla.com/D199454
On the main thread, single mozJSModuleLoader instance is shared across all
loads in non-shared global, with resetting the internal state after importing
a module graph.
NonSharedGlobalSyncModuleLoaderScope manages the lifetime of each usage.
Import into the same non-shared global can be nested, but import into the
different non-shared gloobal is not allowed while other import for non-shared
global is ongoing.
Differential Revision: https://phabricator.services.mozilla.com/D199456
In non-shared global, the sync load is performed by separate SyncModuleLoader,
with the following algorithm:
1. copy all loaded modules from async module loader, so that they can be
used in the module graph in SyncModuleLoader
2. import a module graph
3. move all modules to async module loader
ModuleLoaderBase::CopyModulesTo is for step 1, and
ModuleLoaderBase::MoveModulesTo is for step 3.
The consumer should assert that there's no fetching modules.
Differential Revision: https://phabricator.services.mozilla.com/D199455
In order to perform sync load in arbitrary global, add
ModuleLoaderBase::mOverriddenBy field, and let
ModuleLoaderBase::GetCurrentModuleLoader return the overridden loader,
so that the module loading can be overridden by SyncModuleLoader.
AutoOverrideModuleLoader manages the lifetime of the override.
Differential Revision: https://phabricator.services.mozilla.com/D199454
We are looking into caching loaded script in memory. To do so we need something
to cache.
At the moment, the `ScriptLoadRequest` structure hold all the fields which are
loaded, and used before executing JavaScript code. Then, the `ScriptLoadRequest`
is not guaranteed to out-live the first execution.
Therefore, we have to move fields out of the `ScriptLoadRequest` such that they
can later be used by any caching mechanism. The `LoadedScript` is the closest
existing structure which exists which fit the description.
This patch moves fields out of the ScriptLoadRequest into the `LoadedScript`,
which already has a `LoadedScript` field.
The `LoadedScript` field is initialized sooner, when the `ScriptLoadRequest` is
created, to be subsituted later by a real cache implementation. At the moment
the function `ScriptLoadRequest::NoCacheEntryFound` is used as a placeholder to
change the state of the `ScriptLoadRequest` from `CheckingCache` to `Fetching`.
Existing initializations are replaced by assertions to fail in debug build if
the current patch does not reproduce the expected state properly.
The `LoadedScript` get fields such as the source text, the text length, the
bytecode buffer (which also contains SRI), and the offset at which the bytecode
starts within the bytecode buffer. As these fields are no longer reachable by
name, multiple accessors are added to work-around the issue. Using this as an
opportunity to add extra assertions as part of these accessors.
A new class named `LoadedScriptDelegate` is added to re-add, by inheritance, all
the accessors which used to be part of `ScriptLoadRequest` as methods which are
delegating to the field which is holding the `LoadedScript`. This class is using
templates to avoid virtual inheritance which might hinder inlining, especially
since `ScriptLoadRequest` cannot be made final, as `ModuleLoadRequest` extends
it.
The `ScriptFetchOptions` structure is moved to its own file to solve C++ include
issues.
Differential Revision: https://phabricator.services.mozilla.com/D163615
We are looking into caching loaded script in memory. To do so we need something
to cache.
At the moment, the `ScriptLoadRequest` structure hold all the fields which are
loaded, and used before executing JavaScript code. Then, the `ScriptLoadRequest`
is not guaranteed to out-live the first execution.
Therefore, we have to move fields out of the `ScriptLoadRequest` such that they
can later be used by any caching mechanism. The `LoadedScript` is the closest
existing structure which exists which fit the description.
This patch moves fields out of the ScriptLoadRequest into the `LoadedScript`,
which already has a `LoadedScript` field.
The `LoadedScript` field is initialized sooner, when the `ScriptLoadRequest` is
created, to be subsituted later by a real cache implementation. At the moment
the function `ScriptLoadRequest::NoCacheEntryFound` is used as a placeholder to
change the state of the `ScriptLoadRequest` from `CheckingCache` to `Fetching`.
Existing initializations are replaced by assertions to fail in debug build if
the current patch does not reproduce the expected state properly.
The `LoadedScript` get fields such as the source text, the text length, the
bytecode buffer (which also contains SRI), and the offset at which the bytecode
starts within the bytecode buffer. As these fields are no longer reachable by
name, multiple accessors are added to work-around the issue. Using this as an
opportunity to add extra assertions as part of these accessors.
A new class named `LoadedScriptDelegate` is added to re-add, by inheritance, all
the accessors which used to be part of `ScriptLoadRequest` as methods which are
delegating to the field which is holding the `LoadedScript`. This class is using
templates to avoid virtual inheritance which might hinder inlining, especially
since `ScriptLoadRequest` cannot be made final, as `ModuleLoadRequest` extends
it.
The `ScriptFetchOptions` structure is moved to its own file to solve C++ include
issues.
Differential Revision: https://phabricator.services.mozilla.com/D163615
In Gecko we use a LoadedScript pointer as the referencing private for dynamic
imports. This is passed through the JS engine as a private value, which doesn't
care what it means. Currently we store this in the module loader as a JS::Value,
but we could just as well unpack it since we know what it is. That lets use a
RefPtr which also keeps it alive and will let use remove some manual reference
counting.
We also don't need to pass it to the CreateDynamicImport method twice.
This change makes it the responsibility of the module loader for keeping the
referencing private alive until FinishDynamicModuleImport is called.
Differential Revision: https://phabricator.services.mozilla.com/D196974
Based on the explanation on the bug, the solution would seem to be to call the
ModuleLoaderBase::OnModuleLoadComplete hook from a microtask. This causes a ton
of test failures.
Instead this patch changes the DOM script loader to implement this hook with a
runnable. This fixes the problem (verified manually) and all tests pass.
(Replacing the runnable with a microtask just in the DOM script loader also
does not work and results in many test failures. I'm still trying to understand
why.)
The changes to ContinueDynamicImport are removed because they are also on this
path and we don't need a microtask as well.
This takes us closer to the state before bug 1842798 landed when all the module
loader used MozPromise internally and completions like this happened via
runnables.
I was not able to immediately come up with a test for this.
Differential Revision: https://phabricator.services.mozilla.com/D185830
According to the spec this should happen as the result of resolving a promise
(see ContinueDynamicImport). If this happens synchronously it's possible for
the importing module to still be in the evaluating state when trying to
instantiate and evaluate the dynamically imported module which breaks the
module loader invariants.
Differential Revision: https://phabricator.services.mozilla.com/D183876
This replaces the use of a promise to wait for all imports of a module to load
with a counter of reminaing imports in the parent and a pointer to the parent
that is waiting in the child. The parent request is updated immediately rather
than by dispatching a runnable.
Differential Revision: https://phabricator.services.mozilla.com/D183273
