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
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
In order to use the WorkerLoadContext as a handle for ScriptLoadRequests in the main thread portion
of worker loading, we need to make it thread safe. Which means, it cannot be CC'd, and we need to
manually break cycles. This patch does the first portion of this work, which is inroduce two new
loadContextBase classes, that are either cc'd or not cc'd. For the DOM and mozJSComponent loader, we
continue to use a CC'd LoadContextBase. Workers however now have a non-cc'd variant.
Differential Revision: https://phabricator.services.mozilla.com/D160333
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 relates to 1784482, in that it allows modules to pass through the scriptloader independently.
This was a small bug that I caught in the wpt tests. I didn't have a better segment to put it under
so it is here.
Depends on D154382
Differential Revision: https://phabricator.services.mozilla.com/D147325
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 relates to 1784482, in that it allows modules to pass through the scriptloader independently.
This was a small bug that I caught in the wpt tests. I didn't have a better segment to put it under
so it is here.
Depends on D154382
Differential Revision: https://phabricator.services.mozilla.com/D147325
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
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
Cleanup, optional. Repeating the same work on the NetWorkLoaderHandler as on the WorkerScriptLoader.
Previously, this was considered "safe" because the assumption is that the NetworkLoadHandler is
shorter lived than the WorkerScriptLoader. Rather than assuming this, if we end up in a situation
where this does out-live the WorkerScriptLoader, then we will end up leaking.
Differential Revision: https://phabricator.services.mozilla.com/D154384
Cleanup, optional. It seems strange to have two ways to access the WorkerPrivate, and
ThreadSafeWorkerRef seems like the more reasonable choice.
Differential Revision: https://phabricator.services.mozilla.com/D154383
