We don't need the mExecutionScheduled state, as the mLoadingFinished state was only used to
determine if the promise for a given request had resolved. In fact -- we already know that it is in
a possibly resolved state when we call MaybeExecuteFinishedScripts. So, we can remove this state,
and only have the meaningful check (whether or not a promise on the service worker case hasn't
resolved yet) instead.
Differential Revision: https://phabricator.services.mozilla.com/D146183
This field is no longer necessary, as we are removing executed scripts from our list of scripts to
execute, so we cannot enter a state where something may be executed twice.
Differential Revision: https://phabricator.services.mozilla.com/D146182
This is the most substantial change in the transition from ScriptLoadInfo to ScriptLoadRequest with
regards to data representation. ScriptLoadRequests can have their data incrementally loaded,
so it is already fully decoded and ready to go by the time that we create the source buffer for
worker scripts. This simplifies some of the code, and we can add incremental loading when we are ready.
Differential Revision: https://phabricator.services.mozilla.com/D146180
Here we split ScriptLoadInfo's representation -- we are distinguishing between "what is being loaded"
and "how it is being loaded in a worker context". Requesting review from asuth for the workers side
of things, and jonco for js/loader.
Differential Revision: https://phabricator.services.mozilla.com/D146175
This section of the patch queue starts the migration from ScriptLoadInfo to
ScriptLoadRequest/LoadContext pairs.
We will be making use of the ModuleLoader, and in the future we will have a unified ScriptLoader
implementation (currently, the skeleton for this is in ScriptLoaderInterface). ScriptLoadRequest has
been rewritten to be generic to all contexts, with a companion object "LoadContext" that will handle
specialized behavior required to load the request. That is exactly the case we have with workers,
most of the fields are generic with a couple of specialized fields associated with service workers.
Quick itemization of what will happen
* Patches 1-5: Focuses on getting things into place without using them, we rely on
ScriptLoadInfo (later renamed to WorkerLoadContext).
* Patches 6-7: This is a refactoring of what will be a shared data structure for decoding scripts.
As we will be using requests, we can use this pretty much in the form it exists in on the DOM, and
this cleanup makes it a bit nicer for the DOM as well (in my opinion anyway).
* Patches 8-12: We migrate all shared data to the standard utilization used by other loaders (DOM
and component loader). The biggest pieces is `.mScriptData` transitioning from a stream of bits
that haven't been decoded, to a decoded source text that can be used directly. This was enabled by
patches 6-7.
* The final patches are small cleanups and documentation.
Differential Revision: https://phabricator.services.mozilla.com/D146173
This step is not strictly necessary, and if this is too ugly we can think of something else. The
reason for this change is that ScriptLoadRequests are initialized by a URI, and the owner of these
objects will be the worker thread. However, the current strategy requires that we create the URI
only on the main thread. We always have this information however, when we create the loader, so
there isn't any reason to defer this step until we bounce back into the main thread.
This is another spot where we can reduce reliance on the main thread, which is an eventual goal, so
it seems like this change is an improvement even outside of the goal of moving to ScriptLoadRequests
as a representation.
An alternative approach here would be to initialized the nsIURI ahead of time, and pass it with the
script url to the initialization of the WorkerScriptLoader. However, this isn't ideal as _ideally_
we would remove the text url all together, but this is necessary for errors. We can do this if we
handle workers and main thread differently (and maybe it is worthwhile) as only the main thread
needs encoding information.
Differential Revision: https://phabricator.services.mozilla.com/D146170
Cleanup: tracking if the scripts have been executed can
now be done on the worker side without information from the main thread. This also introduces a name
change: We are really interested in whether the scripts are all totally finished whatever they were
doing, not if they are ready to be run.
Differential Revision: https://phabricator.services.mozilla.com/D145450
This is the last piece that we need in order to move ahead with managing the lists by the worker
thread. This patch moves the last iteration over mLoadingRequests over to the worker thread.
Differential Revision: https://phabricator.services.mozilla.com/D145449
This follows up the cancellation requirements -- CacheCreator is a main thread only datastructure,
which must be cleaned up once the last ScriptLoadInfo is dispatched. Previously, we determined this
via iteration. Now, we manage it as part of ScriptLoadInfo, and release our reference to it when we
dispatch a given ScriptLoadInfo. Later, once we transition to ScriptLoadRequest and LoadContexts,
the cache information can live on a ServiceWorkerLoadContext[1]. This will help use differentiate
between regular worker loader environments and service worker environments.
[1]: https://searchfox.org/mozilla-central/rev/997a56b018662e2940c99bbaf57a6ac9d1aa5422/js/loader/LoadContextBase.h#22-34
Differential Revision: https://phabricator.services.mozilla.com/D145448
The joys of multi-threaded programming:
As you likely noticed, we now have list modification where previously we didn't.
We need move all list modification on the worker side, not the main thread side.
The only reason this isn't showing up on thread sanitizer now, is because the main
thread is acting as the owner of the the lists.
In order to implement Worker ownership, we need to remove all list traversal from the main thread
runnable. There are two significant cases in which we traverse lists: Cancellation, and Dispatching.
Here we handle part of the story for cancellation.
The strategy we take is fundamentally the same as in the DOM Script Loader -- We use the
"mCanceledMainThread" flag to indicate to our load handlers that the thread has been cancelled.
This is taken directly from the DOM loader [1] approach.
[1]: https://searchfox.org/mozilla-central/rev/997a56b018662e2940c99bbaf57a6ac9d1aa5422/dom/script/ScriptLoadHandler.cpp#128-132
Differential Revision: https://phabricator.services.mozilla.com/D145446
Much like Part 2, we are replacing the prior Span structure with a list of pointers. This also
reflects the ultimate ScriptLoadRequestList implementation: We will be modifying lists, moving the
head of one list to the tail of the other, in order to preserve the ImportScripts invariant around
post order == evaluation order.
Differential Revision: https://phabricator.services.mozilla.com/D145443
This patch introduces a temporary array for the purposes of this refactoring. First, I'll
demonstrate the new loading strategy using the nsTArray datastructure, before moving everything over
to a ScriptLoadRequestList and ScriptLoadRequest datastructure. The impact of this change will only
exist for the course of this bug.
Differential Revision: https://phabricator.services.mozilla.com/D145442
The goal of this patch stack is to transition from using the ScriptLoadInfo to represent script
requests for workers, to ScriptLoadRequest and its partner datastructure ScriptLoadRequestList.
There are a number of differences between these two datastructures, but they fundamentally represent
the same thing: a request which is either about to be loaded, or about to be executed.
To get there, in a reasonable manner, is a bit tricky. ScriptLoadRequest et all are refcounted, and
restrict which thread can delete or add them. The decision I've made in this transformation is to
make the Worker thread the home of ScriptLoadRequests. This means that we have to change how we are
doing things on the WorkerScriptLoader so that the MainThread is largely unaware of the
ScriptLoadRequestList, and it only operates in terms of ScriptLoadRequests (or, ScriptLoadInfos as
they are known for now)
The following need to be changed:
* We need to prepare everything to operate on pointers (Prep Patches 1-3)
* Cancellation must no longer iterate over a list (Prep patches 4-8, removing the mChannel reference
is a bonus)
* List management must only be done on the Worker side (Prep Patches 9-10)
This first patch transitions from using the ScriptLoadInfo directly to passing around references.
You will find a similar explaination for the actual transition in the patches that follow the prep
patches. This is bundled in the same bug as the goal here remains the same.
Differential Revision: https://phabricator.services.mozilla.com/D145441
This patch queue splits up dom/workers/ScriptLoader.cpp into several files, with the goal of better
readability and improved comprehension of the relationships between the classes in question. Classes
that can be treated as "intrinsic" to the ScriptLoader (such as the private runnables
ScriptLoaderRunnable and ScriptExecutorRunnable) will remain in ScriptLoader.cpp, as will functions
and classes that currently make up the interface of this file (ChannelGetterRunnable,
ChannelFromScriptURL{MainThread,WorkerThread}). All other classes will be moved out and put under a
`workerinternals::loader` namespace because I couldn't think of anything better.
However, for this first patch: We are moving ScriptLoadInfo to a mozilla::dom level. This is because
this class will play the same role as ScriptLoadContext[1], but for workers.
[1]: https://searchfox.org/mozilla-central/rev/997a56b018662e2940c99bbaf57a6ac9d1aa5422/dom/script/ScriptLoadContext.h
Differential Revision: https://phabricator.services.mozilla.com/D144839
The next three patches focus on removing direct usages of private fields by friend classes in
WorkerScriptLoader. This is an attempt to make it clearer what is truly private and what isn't.
Mostly for sanity.
Differential Revision: https://phabricator.services.mozilla.com/D145084
Small cleanup: GetBaseURI is one of the methods of a ScriptLoadInterface. This also makes it easier
to split these into files (done separately)
Differential Revision: https://phabricator.services.mozilla.com/D144835
This makes the relationship between these classes and the dom ScriptLoadHandler clearer. Once we
move to ScriptLoadRequests, these three classes will share a loadhandler class which will take care
of script decoding.
Differential Revision: https://phabricator.services.mozilla.com/D145063
Similar to what was done with the prior patch, this can be described as the companion method to
loading from the network in the LoaderListener, with special requirements in terms of how it populates the loaded request
data and updates the WorkerPrivate. At some point, it may make sense to have the service worker loader as a child class of the
main worker loader, but that won't be done here (I hope anyway).
Differential Revision: https://phabricator.services.mozilla.com/D144838
We don't have a correlation to these methods in the ScriptLoadHandler on the DOM side. For now,
rename them to make the difference clear.
Differential Revision: https://phabricator.services.mozilla.com/D144837
The LoaderListener is currently used to forward events to WorkerScriptLoader. It fills a similar
role to the ScriptLoadHandler on the DOM both for OnStartRequest and OnStreamComplete. In the case
of the DOM loader, the responsibility for performing certain channel related tasks is left to the
ScriptLoadHandler, in particular the decoding of the string into UTF8 or UTF16.
While the decoding is embedded in the onStreamCompleteInternal method, it is not a 1:1 mapping.
Some of the responsibilities handled in the `OnStreamCompleteInternal` method are instead handled in
`ScriptLoader::PrepareLoadedRequest`, such as populating the SourceMaps field, managing the channel,
etc are normally handled within the ScriptLoader.
However, in the case of the WorkerScriptLoader we have two paths to reach a completed load:
* from the network
* from the cache
These have different requirements in terms of preparing the loaded request, for example the cache
does not forward a SourceMap.
In the interest of not complicating things too much, for now the method is moved wholesale into
LoaderListener. In a separate PR I will handle renaming. The next step will be to do a similar move
for the CacheScriptLoader.
Differential Revision: https://phabricator.services.mozilla.com/D144836
This just brings us a bit closer to the DOM pattern of Script Loading. One difference is that we
evaluate a source buffer instead of a JSScript, so rather than "Execute", we will still ahandle data
using "Evaluate". Maybe later we can make these two methods on the SpiderMonkey side a bit more
descriptive. For now this gets us pretty close to the goal of "if you know the DOM script loader, you know the
Worker Script loader" and vice-versa.
Differential Revision: https://phabricator.services.mozilla.com/D145065
This is mostly to bring us in line with the naming of everything else, and also make it clear that
we are crossing a thread boundary.
Differential Revision: https://phabricator.services.mozilla.com/D144990
Continuation of integrating methods from the ScriptExecutorRunnable into the WorkerScriptLoader. In this
case, the primarily function being moved here is the batch evaluation of scripts. This roughly
aligns with the ProcessPendingRequests. The function has been renamed to reflect the similarity of
responsibilities between the two classes.
As a driveby change -- the prerun function now only has one responsibility, and it is related to
loading so this has been moved and named.
Differential Revision: https://phabricator.services.mozilla.com/D144961
Part 1 of integrating the operations that the ScriptExecutorRunnable was doing on the
WorkerScriptLoader into the WorkerScriptLoader itself. This first pass moves some of the smaller,
contained functions over to the WorkerScriptLoader: The shutdown, evaluate and logging methods.
Differential Revision: https://phabricator.services.mozilla.com/D144327
This encompasses the dispatching of the loader runnable as part of the api of WorkerScriptLoader.
The same pattern will be used for the ScriptExecutorRunnable. We might be able to just use an NS
runnable method instead here, if that is preferred.
Differential Revision: https://phabricator.services.mozilla.com/D144991
We are gradually moving closer towards a ScriptLoader implementation based on the DOM ScriptLoader.
The patch is done in three parts:
* WorkerScriptLoader adjustments (WSL Parts 1-6)
* LoaderHandler class adjustments (LoadHandler Parts 1-4)
* Cleanups
The class ScriptLoaderRunnable was being passed between runnables to access information
that was shared. That role will now be done by the WorkerScriptLoader which will eventually inherit
from ScriptLoaderInterface. This is the basis for building up the Module Loader implementation for
workers. This first patch splits the responsibility of the runnable and the data holding class.
In the next few patches, methods from the ScriptExecutorRunnable will also be moved into this class,
and the runnables themselves will no longer hold execution information. Instead, they will be used
as dispatchers to have the work run in the correct place. In the next step, the same pattern will be
done with the ScriptExecutorRunnable.
Differential Revision: https://phabricator.services.mozilla.com/D144326
aLoadInfosAlreadyExecuted is used to track two pieces of state, but we are doing iteration to do
this. Instead, we can track the same state by two booleans: We know all scripts are ready to execute
if we are at the end of the mLoadInfos list. We know that we have failed if we record if any script
fails on mScriptLoader.
Depends on D143621
Differential Revision: https://phabricator.services.mozilla.com/D144276
This removes the pref `security.csp.enable` and amends various callers
in dom/, which no longer have to take this pref into consideration.
Furthermore, we can remove the test in dom/base/test/browser_bug593387.js
The test used the pref to test that external content can be embedded into
about:plugins, which is historic baggage from a previous architecture of
said page that we no longer require. It's also an anti-pattern that we
do not want to support any longer. In fact, the test had to jump through
additional hoops to make that work at all.
Differential Revision: https://phabricator.services.mozilla.com/D138661
