Previously, in the cancellation refactor, we removed knowledge of the list of scripts being loaded on the main thread. As a result of this, we could no longer clear the cache creator on the main thread after iterating over all requests. We can now do that once again, and it simplifies the code.
Differential Revision: https://phabricator.services.mozilla.com/D162220
The ScriptLoaderRunnable used to do double duty as both the runnable used to load a script, and as the data structure to hold on to loading information (what kind of worker, debugger status, etc, if we failed). The second half of these responsibilities remains with the WorkerScriptLoader. WorkerScriptLoader acts as a persistent data structure. ScriptLoaderRunnable is per batch of requests (such as ImportScripts, or a single module load). It works together with the ThreadSafeRequestHandle to load the script.
In the future, when we move to a single threaded implementation, ThreadSafeRequestHandle will likely be merged with WorkerLoadContext, but ScriptLoaderRunnable will stay as a datastructure to represent the batched request, and most of the existing fields (mSyncTarget, the request list) will stay. There is still some massaging to do here, but it gets closer to a final vision of how this should look.
Differential Revision: https://phabricator.services.mozilla.com/D162217
This was removed as part of an attempt to simplify worker cancellaton. However, this channel cancellation is important, as without it the timing changes when we finish a request. Without it, we continue to load a script, even after a worker has been shut down.
This tracking will help us reimplement cancellation.
Differential Revision: https://phabricator.services.mozilla.com/D162215
This introduces a new datastructure that wraps the ScriptLoadRequest. Previously, we were using the WorkerLoadContext to access the request, and we made it not cycle collected in order to make it safe across threads. This, of course, broke module behavior, as the cycle needs to be broken in more places, and things get rather complex.
In the spirit of making everyone's life easier, the ThreadSafeRequestHandle exists as a way to move the request to the main thread, operate on it, and then return it to the worker. We no longer need to track and free the WorkerLoadContext. Once the ThreadSafeRequestHandle is returned to the worker, we release the request, and the handle is empty. There is a possibility that this will cause issues, so we should keep an eye on this. Alternatively, we can never release the ScriptLoadRequest and let the ThreadSafeRequestHandle clean it up on destruction.
In the case that we somehow end up releasing on the main thread, we will send a runnable to release the request correctly on the worker.
Differential Revision: https://phabricator.services.mozilla.com/D162213
We will be using the mSyncLoopTarget for module promises, and for returning to the worker thread
when cleaning up for ThreadSafeRequestHandle.
Differential Revision: https://phabricator.services.mozilla.com/D162212
This patch doesn't change behavior; these APIs were already effectively
infallible, in the sense that it was impossible for them to return anything
other than NS_OK.
The actual API changes here are in xpcom/io/nsIPipe.idl and
xpcom/io/nsPipe3.cpp, and the rest of this patch is just updates to callsites,
removing code that handled failure-flavored return values from these functions.
(All of this removed code was already dead code, effectively.)
Differential Revision: https://phabricator.services.mozilla.com/D161947
I was over eager in introducing the strong pointers to WorkerLoadContext. It turns out that
previously when we were cycle collecting our ScriptLoadRequests, we were also cleaning up everything
related to WorkerLoadContext. Also, in an attempt to fix the cancellation for workers, We
accidentally stopped cleaning up the reference to the cache creator. This patch does the following:
1) cleans up the cache creator reference whenever we finish with a cache load handler. This is done
by calling Fail appropriately. This solves both bug 1798667 (we no longer reach NEW_URI if we fail)
and 1781295 (we no longer leak memory because things were not cleaned up properly).
2) Does no remove the back reference to WorkerLoadContext from the LoadRequest. It turns out that
this is sometimes necessary. There is a chance that we will accidently try to access the
WorkerLoadContext after cancellation. This actually causes problems later on in the module code.
Differential Revision: https://phabricator.services.mozilla.com/D161288
As WorkerLoadContexts now inherit from a non-CC'd loadContextBase, we have two outcomes.
1) we need to break cycles with ScriptLoadRequests manually, so that ScriptLoadRequests can be collected (ScriptLoadRequests must be CC'd).
2) we can now have refptrs to WorkerLoadContexts in the CacheLoadHandler and NetworkLoadHandler classes, and remove any remaining raw pointers to ScriptLoadRequest/WorkerLoadContext. There are cases where the NetworkLoadHandler or CacheLoadHandler might outlive the Worker Loader, so having refpointers here should help us recover in those cases.
Differential Revision: https://phabricator.services.mozilla.com/D160334
This introduces the basic skeleton to make WorkerScriptLoader a ScriptLoaderInterface.
ScriptLoaderInterface defines the methods that are shared between any given ScriptLoader (for
example the DOM script loader, the ComponentScriptLoader) and the ModuleLoader for a particular
component.
This patch also adds documentation to make the role and responsibilities of the
ScriptLoaderInterface clear.
Depends on D147321
Differential Revision: https://phabricator.services.mozilla.com/D147313
This largely keeps in tact what jstutte did. The initial crash was fixed by eagerly calling
LoadingFinished. The second crash is caused because we call it twice, and only in the service worker
case, where we call it once the promise rejects. Now, we check if we have cancelled, and if we have
then we don't call the scriptLoader methods from inside of the load handlers. LoadHandlers now only
use OnStreamComplete if they are "successful" -- that is, if they were not cancelled.
OnStreamComplete retains its assertion error in the case that something was cancelled and we somehow
ended up there. In a follow up, I will clean up the friend classes of the ScriptLoader so you can't
easily access these methods from the LoadHandlers.
Differential Revision: https://phabricator.services.mozilla.com/D158262
This will be used by child modules. This is currently not used, but it will be in modules. I can
move this over to the other bug, if necessary.
Depends on D147325
Differential Revision: https://phabricator.services.mozilla.com/D147321
This brings back the behavior to iterated over all of the load requests to cancel the cache
promise. I tried a couple of different ways of doing this, including deleting the cache, but this
seemed to be the cleanest way.
Depends on D147322
Differential Revision: https://phabricator.services.mozilla.com/D154382
This moves the shutdown operations out of the ScriptExecutorRunnable, and into something that can be
called independent. This does not change the behavior in this case, however it is important for
modules, which will have promises that resolve after the ScriptExecutorRunnable has closed.
Depends on D155231
Differential Revision: https://phabricator.services.mozilla.com/D147322
This enables us to send files to load that are not part of our executing scripts list. This unlocks
the ability to send single module scripts to be loaded without executing them. It also gives
us a way to get the list of files that are to be loaded.
Depends on D147318
Differential Revision: https://phabricator.services.mozilla.com/D155231
This will be used by child modules. This is currently not used, but it will be in modules. I can
move this over to the other bug, if necessary.
Depends on D147325
Differential Revision: https://phabricator.services.mozilla.com/D147321
This brings back the behavior to iterated over all of the load requests to cancel the cache
promise. I tried a couple of different ways of doing this, including deleting the cache, but this
seemed to be the cleanest way.
Depends on D147322
Differential Revision: https://phabricator.services.mozilla.com/D154382
This moves the shutdown operations out of the ScriptExecutorRunnable, and into something that can be
called independent. This does not change the behavior in this case, however it is important for
modules, which will have promises that resolve after the ScriptExecutorRunnable has closed.
Depends on D155231
Differential Revision: https://phabricator.services.mozilla.com/D147322
This enables us to send files to load that are not part of our executing scripts list. This unlocks
the ability to send single module scripts to be loaded without executing them. It also gives
us a way to get the list of files that are to be loaded.
Depends on D147318
Differential Revision: https://phabricator.services.mozilla.com/D155231
Previously, we had the client info for main scripts on the ScriptLoadInfo, and in the ScriptLoader
for non-main scripts (ImportScripts case). This was a bit confusing, so I moved it all to the ScriptLoader as in the
importScripts case, it is always recreated with each new load. However, for modules this is not
true. In order to make it persistant for main scripts and modules, as well as ensure that it has the
correct data, it should always live on the WorkerLoadContext.
Differential Revision: https://phabricator.services.mozilla.com/D147317
This implements the core of the change -- which is implementing an explicit IsTopLevel flag for
`WorkerLoadContexts`, and removing `mIsMainScript` from the loader. I've asked for some
clarification on this part of the spec from the whatwg editors, as I think this may be misnamed. It
is likely that what is meant here is `IsInitialScript` rather than `IsTopLevel` -- as there is a
note stating that this should be initialized with the agent cluster. Once this is clarified I may
update the name.
Differential Revision: https://phabricator.services.mozilla.com/D147319
