Commit graph

498 commits

Author SHA1 Message Date
Yulia Startsev
dba0139599 Bug 1792984 - Introduce asserts r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D158425
2022-10-13 07:19:25 +00:00
Yulia Startsev
75e1b76e92 Bug 1792984 - Introduce aggressive cancellation r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D158558
2022-10-13 07:19:24 +00:00
Yulia Startsev
f593e162fd Bug 1784481 - Use ScriptLoaderInterface method for filling compile options; r=jonco
Depends on D147314

Differential Revision: https://phabricator.services.mozilla.com/D147315
2022-10-04 16:01:29 +00:00
Yulia Startsev
7bf63f5988 Bug 1784481 - Use ScriptLoaderInterface methods for reporting errors; r=jonco
Fill out the ReportErrorToConsole method, and add documentation for ReportWarningToConsole

Depends on D147313

Differential Revision: https://phabricator.services.mozilla.com/D147314
2022-10-04 16:01:29 +00:00
Yulia Startsev
0dd649a319 Bug 1784481 - Implement ScriptLoaderInterface Skeleton in WorkerScriptLoader; r=jonco
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
2022-10-04 16:01:28 +00:00
Yulia Startsev
5ad6240ade Bug 1786571 - Do not call LoadingFinished from handlers when cancelled; r=asuth
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
2022-09-29 16:28:30 +00:00
Jens Stutte
ecb5da0fbc Bug 1786571 - Add IsCancelled checks to NetworkLoadHandler::OnStreamComplete and have a GetCancelResult r=dom-worker-reviewers,asuth
Differential Revision: https://phabricator.services.mozilla.com/D157307
2022-09-20 05:58:45 +00:00
Cosmin Sabou
9281cd6961 Backed out changeset 374cb9c09095 (bug 1786571) for causing several service_worker related regressions. 2022-09-16 18:27:46 +03:00
Jens Stutte
d4688e2641 Bug 1786571 - Add IsCancelled checks to NetworkLoadHandler::OnStreamComplete and have a GetCancelResult r=dom-worker-reviewers,asuth
Differential Revision: https://phabricator.services.mozilla.com/D157307
2022-09-16 03:53:53 +00:00
Noemi Erli
0123802021 Backed out changeset b246c998fb8f (bug 1786571) for causing ScriptLoader failures CLOSED TREE 2022-09-15 14:20:23 +03:00
Jens Stutte
a5bd0870c0 Bug 1786571 - Add IsCancelled checks to NetworkLoadHandler::OnStreamComplete and have a GetCancelResult r=dom-worker-reviewers,asuth
Differential Revision: https://phabricator.services.mozilla.com/D157307
2022-09-15 10:37:50 +00:00
Yulia Startsev
dbb424426a Bug 1783190 - Introduce lock for cancellation in case of race;r=jstutte
Differential Revision: https://phabricator.services.mozilla.com/D156943
2022-09-09 13:22:42 +00:00
Yulia Startsev
0f84338a77 Bug 1784482 - Implement method for single script loading; r=asuth
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
2022-09-08 17:13:28 +00:00
Yulia Startsev
53ed8260e3 Bug 1783190 - In the case of cancellation, cleanup cache; r=asuth
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
2022-09-08 17:13:27 +00:00
Yulia Startsev
4a09a4f2aa Bug 1784482 - Move shutdown operation to be always after ProcessRequests; r=asuth
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
2022-09-08 17:13:27 +00:00
Yulia Startsev
55e47a4cc9 Bug 1784482 - Do not rely on ScriptLoadRequestList in the main thread; r=asuth
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
2022-09-08 17:13:27 +00:00
Yulia Startsev
9601e09968 Bug 1784482 - Move creation of ScriptLoadRequests into own functions; r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D147318
2022-09-08 17:13:26 +00:00
Butkovits Atila
4b03a9e7f7 Backed out 6 changesets (bug 1783190, bug 1784482) for causing crashes at mozilla::dom::Promise::MaybeReject(nsresult)]. CLOSED TREE
Backed out changeset 3bbb7bedfa0a (bug 1784482)
Backed out changeset a67c04d83558 (bug 1784482)
Backed out changeset 593c1d517530 (bug 1783190)
Backed out changeset 25311707288f (bug 1784482)
Backed out changeset 13e623b3779f (bug 1784482)
Backed out changeset f1f81f81af17 (bug 1784482)
2022-09-07 16:12:45 +03:00
Yulia Startsev
8c7b1e9f2d Bug 1784482 - Implement method for single script loading; r=asuth
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
2022-09-07 09:37:40 +00:00
Yulia Startsev
35baf41dd2 Bug 1783190 - In the case of cancellation, cleanup cache; r=asuth
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
2022-09-07 09:37:39 +00:00
Yulia Startsev
119c3f9a63 Bug 1784482 - Move shutdown operation to be always after ProcessRequests; r=asuth
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
2022-09-07 09:37:39 +00:00
Yulia Startsev
4e067f14e8 Bug 1784482 - Do not rely on ScriptLoadRequestList in the main thread; r=asuth
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
2022-09-07 09:37:38 +00:00
Yulia Startsev
0d822c68c2 Bug 1784482 - Move creation of ScriptLoadRequests into own functions; r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D147318
2022-09-07 09:37:38 +00:00
Yulia Startsev
e48cee9f1d Bug 1784476 - Move ClientInfo to WorkerLoadContext; r=asuth
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
2022-08-24 13:38:34 +00:00
Yulia Startsev
24c8e12590 Bug 1784476 - Move all required information for constructing ScriptLoadRequests to WorkerScriptLoader constructor; r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D147316
2022-08-24 13:38:34 +00:00
Yulia Startsev
eeadeb575f Bug 1784457 - Implement IsTopLevel in place of IsMainWorkerModule; r=asuth
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
2022-08-22 16:22:04 +00:00
Yulia Startsev
415c74a6cd Bug 1784457 - Split GetBaseURI into two functions; r=asuth
`GetBaseURI` will implement the virtual method `GetBaseURI` from ScriptLoaderInterface
(https://searchfox.org/mozilla-central/rev/6ec440e105c2b75d5cae9d34f957a2f85a106d54/js/loader/ModuleLoaderBase.h#64)
-- but the behavior needed there is only ever for child modules. By splitting this, we remove
contextual information which is only used for the initial script.

Differential Revision: https://phabricator.services.mozilla.com/D154520
2022-08-22 16:22:04 +00:00
Yulia Startsev
472e3c8cf5 Bug 1784457 - Remove unused mLoadingWorkerScript atomic boolean from WorkerPrivate; r=asuth
After investigating this a bit, I found that this wasn't used. It seems safe to remove.

Differential Revision: https://phabricator.services.mozilla.com/D154519
2022-08-22 16:22:04 +00:00
Yulia Startsev
09daba2264 Bug 1783190 - Replace WorkerPrivate usage on CacheLoadHandler with ThreadSafeWorkerRef; r=asuth
Cleanup, Optional, Same as the NetworkLoadHandler.

Differential Revision: https://phabricator.services.mozilla.com/D154385
2022-08-22 11:43:02 +00:00
Yulia Startsev
6b185b9350 Bug 1783190 - Replace WorkerPrivate usage with ThreadSafeWorkerRef usage; r=asuth
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
2022-08-22 11:43:02 +00:00
Yulia Startsev
e6dd426bff Bug 1783190 - Hold a ThreadSafeWorkerRef on WorkerScriptLoader; r=asuth
This addresses part of the issue, by holding a strong ref until we shutdown, so that we do not end
up in a situation where the worker closes before we finish cleanup.

Differential Revision: https://phabricator.services.mozilla.com/D154381
2022-08-22 11:43:01 +00:00
Yulia Startsev
3bd594dbd5 Bug 1779762 - Reimplement mLoadingFinished check; r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D152181
2022-07-19 16:38:15 +00:00
Yulia Startsev
ccd1b4e8bd Bug 1742438 - Part 13: Use ScriptLoadRequest's mURL instead of WorkerLoadContext's; r=asuth,jonco,nchevobbe
Differential Revision: https://phabricator.services.mozilla.com/D146459
2022-07-14 17:07:29 +00:00
Yulia Startsev
0cfc4b16d0 Bug 1742438 - Part 12: Remove WorkerLoadContext mSourceMapURL and use ScriptLoadRequest mSourceMapURL; r=asuth
The simplest change: these two fields were identical across the representations

Differential Revision: https://phabricator.services.mozilla.com/D146184
2022-07-14 17:07:29 +00:00
Yulia Startsev
68fc4d0318 Bug 1742438 - Part 11: Replace Finished() check with IsAwaitingPromise() and move state change to worker thread; r=asuth,jonco
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
2022-07-14 17:07:28 +00:00
Yulia Startsev
5552f1227e Bug 1742438 - Part 10: Remove mExecutionResult; r=asuth
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
2022-07-14 17:07:28 +00:00
Yulia Startsev
9681fade05 Bug 1742438 - Part 9: Replace mLoadingFinished with State::Ready in ScriptLoadRequest; r=asuth
The "mLoadingFinished" state that we are using a boolean for can be represented by State::Ready in
ScriptLoadRequest.

Differential Revision: https://phabricator.services.mozilla.com/D146181
2022-07-14 17:07:28 +00:00
Yulia Startsev
d82f1773f0 Bug 1742438 - Part 8: Use mScriptData instead of custom load context field; r=arai,asuth
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
2022-07-14 17:07:27 +00:00
Yulia Startsev
de63d5eeab Bug 1742438 - Part 5: Rename ScriptLoadInfo to WorkerLoadContext; r=asuth
Differential Revision: https://phabricator.services.mozilla.com/D146177
2022-07-14 17:07:26 +00:00
Yulia Startsev
adbf7d814a Bug 1742438 - Part 3: Split ScriptLoadInfo into ScriptLoadRequest and ScriptLoadInfo (as a LoadContext) classes; r=asuth,jonco
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
2022-07-14 17:07:25 +00:00
Yulia Startsev
c7e0fc2f1f Bug 1742438 - Part 2: Rename ScriptLoadInfo references from {a,m}LoadInfo to {a,m}Request; r=asuth
Doing the rename here so it doesn't make it confusing to review.

Differential Revision: https://phabricator.services.mozilla.com/D146174
2022-07-14 17:07:25 +00:00
Yulia Startsev
1ade00f949 Bug 1742438 - Part 1: Use ScriptLoadRequestList and have ScriptLoadInfo inherit from ScriptLoadRequest; r=asuth
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
2022-07-14 17:07:24 +00:00
Yulia Startsev
047056ed6c Bug 1742438 - Prep Part 9: Encode URIs on worker script loader creation; r=asuth
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
2022-07-14 17:07:23 +00:00
Yulia Startsev
21d0d4b5bc Bug 1742438 - Prep Part 8: Implement all scripts executed on WorkerScriptLoader; r=asuth
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
2022-07-14 17:07:23 +00:00
Yulia Startsev
d4eb919786 Bug 1742438 - Prep Part 7: Move list management to the worker side; r=asuth
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
2022-07-14 17:07:23 +00:00
Yulia Startsev
244367a543 Bug 1742438 - Prep Part 6: CacheCreator is managed by ScriptLoadInfo, not WorkerScriptLoader; r=asuth
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
2022-07-14 17:07:22 +00:00
Yulia Startsev
419c62b9c5 Bug 1742438 - Prep Part 5: Remove mChannel from ScriptLoadInfo; r=asuth
This is mostly a freebee: Since we no longer use mChannel for cancellation, it can disappear.

This cancellation pattern was introduced in [1]. Part of the goal as I understand was to prevent a
copy[2]. If we want to eagerly cancel the channel, we can achieve in using OnIncrementalData. Happy
to discuss this approach.

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=649537
[2]: https://bugzilla.mozilla.org/show_bug.cgi?id=931249#c34

Differential Revision: https://phabricator.services.mozilla.com/D145447
2022-07-14 17:07:22 +00:00
Yulia Startsev
d2a6e2fd19 Bug 1742438 - Prep Part 4: Change cancellation to not rely on iterating over mLoadingRequests r=asuth
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
2022-07-14 17:07:21 +00:00
Yulia Startsev
390e9c33ef Bug 1742438 - Prep Part 3: Make Executable list of ScriptLoadInfos into a list of references; r=asuth
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
2022-07-14 17:07:21 +00:00
Yulia Startsev
ba227096e2 Bug 1742438 - Prep Part 2: Create an array of references for ScriptLoadInfos; r=asuth
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
2022-07-14 17:07:21 +00:00