nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1871075 - Part 1: Store referencing private as a pointer to the underlying LoadedScript r=yulia

Browse source 
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
This commit is contained in:
Jon Coppeard 2023-12-21 13:30:34 +00:00
parent 380a2947d1
commit f9febfda89
13 changed files with 27 additions and 29 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

5
dom/script/ModuleLoader.cpp
View file

@ -304,8 +304,7 @@ already_AddRefed<ModuleLoadRequest> ModuleLoader::CreateStaticImport(
already_AddRefed<ModuleLoadRequest> ModuleLoader::CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate, JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) {
JS::Handle<JSString*> aSpecifier, JS::Handle<JSObject*> aPromise) {
MOZ_ASSERT(aSpecifier);
MOZ_ASSERT(aPromise);
@ -357,7 +356,7 @@ already_AddRefed<ModuleLoadRequest> ModuleLoader::CreateDynamicImport(
/* is top level */ true, /* is dynamic import */
this, ModuleLoadRequest::NewVisitedSetForTopLevelImport(aURI), nullptr);
request->mDynamicReferencingPrivate = aReferencingPrivate;
request->mDynamicReferencingScript = aMaybeActiveScript;
request->mDynamicSpecifier = aSpecifier;
request->mDynamicPromise = aPromise;

1
dom/script/ModuleLoader.h
View file

@ -72,7 +72,6 @@ class ModuleLoader final : public JS::loader::ModuleLoaderBase {
// Create a module load request for a dynamic module import.
already_AddRefed<ModuleLoadRequest> CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate,
JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) override;

5
dom/workers/loader/WorkerModuleLoader.cpp
View file

@ -83,8 +83,7 @@ bool WorkerModuleLoader::CreateDynamicImportLoader() {
already_AddRefed<ModuleLoadRequest> WorkerModuleLoader::CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate, JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) {
JS::Handle<JSString*> aSpecifier, JS::Handle<JSObject*> aPromise) {
WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
if (!CreateDynamicImportLoader()) {
@ -143,7 +142,7 @@ already_AddRefed<ModuleLoadRequest> WorkerModuleLoader::CreateDynamicImport(
/* is top level */ true, /* is dynamic import */
this, ModuleLoadRequest::NewVisitedSetForTopLevelImport(aURI), nullptr);
request->mDynamicReferencingPrivate = aReferencingPrivate;
request->mDynamicReferencingScript = aMaybeActiveScript;
request->mDynamicSpecifier = aSpecifier;
request->mDynamicPromise = aPromise;

1
dom/workers/loader/WorkerModuleLoader.h
View file

@ -63,7 +63,6 @@ class WorkerModuleLoader : public JS::loader::ModuleLoaderBase {
already_AddRefed<ModuleLoadRequest> CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate,
JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) override;

3
dom/worklet/loader/WorkletModuleLoader.cpp
View file

@ -79,8 +79,7 @@ already_AddRefed<ModuleLoadRequest> WorkletModuleLoader::CreateStaticImport(
already_AddRefed<ModuleLoadRequest> WorkletModuleLoader::CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate, JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) {
JS::Handle<JSString*> aSpecifier, JS::Handle<JSObject*> aPromise) {
return nullptr;
}

1
dom/worklet/loader/WorkletModuleLoader.h
View file

@ -68,7 +68,6 @@ class WorkletModuleLoader : public JS::loader::ModuleLoaderBase {
already_AddRefed<JS::loader::ModuleLoadRequest> CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate,
JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) override;

2
js/loader/ImportMap.cpp
View file

@ -601,7 +601,7 @@ ResolveResult ImportMap::ResolveModuleSpecifier(ImportMap* aImportMap,
LOG(("ImportMap::ResolveModuleSpecifier specifier: %s",
NS_ConvertUTF16toUTF8(aSpecifier).get()));
nsCOMPtr<nsIURI> baseURL;
if (aScript) {
if (aScript && !aScript->IsEventScript()) {
baseURL = aScript->BaseURL();
} else {
baseURL = aLoader->GetBaseURI();

9
js/loader/ModuleLoadRequest.cpp
View file

@ -28,19 +28,20 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(ModuleLoadRequest)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ModuleLoadRequest,
ScriptLoadRequest)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader, mRootModule, mModuleScript, mImports,
mWaitingParentRequest)
mWaitingParentRequest,
mDynamicReferencingScript)
tmp->ClearDynamicImport();
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ModuleLoadRequest,
ScriptLoadRequest)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLoader, mRootModule, mModuleScript,
mImports, mWaitingParentRequest)
mImports, mWaitingParentRequest,
mDynamicReferencingScript)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(ModuleLoadRequest,
ScriptLoadRequest)
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDynamicReferencingPrivate)
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDynamicSpecifier)
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDynamicPromise)
NS_IMPL_CYCLE_COLLECTION_TRACE_END
@ -230,7 +231,7 @@ void ModuleLoadRequest::LoadFinished() {
}
void ModuleLoadRequest::ClearDynamicImport() {
mDynamicReferencingPrivate = JS::UndefinedValue();
mDynamicReferencingScript = nullptr;
mDynamicSpecifier = nullptr;
mDynamicPromise = nullptr;
}

3
js/loader/ModuleLoadRequest.h
View file

@ -18,6 +18,7 @@
namespace JS::loader {
class LoadedScript;
class ModuleScript;
class ModuleLoaderBase;
@ -163,7 +164,7 @@ class ModuleLoadRequest final : public ScriptLoadRequest {
RefPtr<VisitedURLSet> mVisitedSet;
// For dynamic imports, the details to pass to FinishDynamicImport.
JS::Heap<JS::Value> mDynamicReferencingPrivate;
RefPtr<LoadedScript> mDynamicReferencingScript;
JS::Heap<JSString*> mDynamicSpecifier;
JS::Heap<JSObject*> mDynamicPromise;
};

21
js/loader/ModuleLoaderBase.cpp
View file

@ -331,8 +331,8 @@ bool ModuleLoaderBase::HostImportModuleDynamically(
// Create a new top-level load request.
nsCOMPtr<nsIURI> uri = result.unwrap();
RefPtr<ModuleLoadRequest> request = loader->CreateDynamicImport(
aCx, uri, script, aReferencingPrivate, specifierString, aPromise);
RefPtr<ModuleLoadRequest> request =
loader->CreateDynamicImport(aCx, uri, script, specifierString, aPromise);
if (!request) {
// Throws TypeError if CreateDynamicImport returns nullptr.
@ -381,9 +381,6 @@ LoadedScript* ModuleLoaderBase::GetLoadedScriptOrNull(
}
auto* script = static_cast<LoadedScript*>(aReferencingPrivate.toPrivate());
if (script->IsEventScript()) {
return nullptr;
}
MOZ_ASSERT_IF(
script->IsModuleScript(),
@ -393,6 +390,14 @@ LoadedScript* ModuleLoaderBase::GetLoadedScriptOrNull(
return script;
}
JS::Value PrivateFromLoadedScript(LoadedScript* aScript) {
if (!aScript) {
return JS::UndefinedValue();
}
return JS::PrivateValue(aScript);
}
nsresult ModuleLoaderBase::StartModuleLoad(ModuleLoadRequest* aRequest) {
return StartOrRestartModuleLoad(aRequest, RestartRequest::No);
}
@ -769,7 +774,7 @@ ResolveResult ModuleLoaderBase::ResolveModuleSpecifier(
// Get the document's base URL if we don't have a referencing script here.
nsCOMPtr<nsIURI> baseURL;
if (aScript) {
if (aScript && !aScript->IsEventScript()) {
baseURL = aScript->BaseURL();
} else {
baseURL = GetBaseURI();
@ -1000,8 +1005,8 @@ void ModuleLoaderBase::FinishDynamicImport(
JSMSG_DYNAMIC_IMPORT_FAILED, url.get());
}
JS::Rooted<JS::Value> referencingScript(aCx,
aRequest->mDynamicReferencingPrivate);
JS::Rooted<JS::Value> referencingScript(
aCx, PrivateFromLoadedScript(aRequest->mDynamicReferencingScript));
JS::Rooted<JSString*> specifier(aCx, aRequest->mDynamicSpecifier);
JS::Rooted<JSObject*> promise(aCx, aRequest->mDynamicPromise);

1
js/loader/ModuleLoaderBase.h
View file

@ -232,7 +232,6 @@ class ModuleLoaderBase : public nsISupports {
// Called by HostImportModuleDynamically hook.
virtual already_AddRefed<ModuleLoadRequest> CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate,
JS::Handle<JSString*> aSpecifier, JS::Handle<JSObject*> aPromise) = 0;
// Check whether we can load a module. May return false with |aRvOut| set to

3
js/xpconnect/loader/ComponentModuleLoader.cpp
View file

@ -74,8 +74,7 @@ already_AddRefed<ModuleLoadRequest> ComponentModuleLoader::CreateStaticImport(
already_AddRefed<ModuleLoadRequest> ComponentModuleLoader::CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate, JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) {
JS::Handle<JSString*> aSpecifier, JS::Handle<JSObject*> aPromise) {
return nullptr; // Not yet implemented.
}

1
js/xpconnect/loader/ComponentModuleLoader.h
View file

@ -70,7 +70,6 @@ class ComponentModuleLoader : public JS::loader::ModuleLoaderBase {
already_AddRefed<ModuleLoadRequest> CreateDynamicImport(
JSContext* aCx, nsIURI* aURI, LoadedScript* aMaybeActiveScript,
JS::Handle<JS::Value> aReferencingPrivate,
JS::Handle<JSString*> aSpecifier,
JS::Handle<JSObject*> aPromise) override;