commit 63333e6ccd4c9887f9b7a64646257016ba70da8c
parent 3cfcdb4caa93d913389f2cca9b76e32d58f44070
Author: Tooru Fujisawa <arai_a@mac.com>
Date: Mon, 20 Oct 2025 09:06:25 +0000
Bug 1991358 - Part 2: Remove ScriptLoadRequest::mScriptForCache. r=nbp
Differential Revision: https://phabricator.services.mozilla.com/D266619
Diffstat:
4 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/js/loader/ModuleLoadRequest.cpp b/js/loader/ModuleLoadRequest.cpp
@@ -61,6 +61,14 @@ ModuleLoadRequest::ModuleLoadRequest(
MOZ_ASSERT(mLoader);
}
+ModuleLoadRequest::~ModuleLoadRequest() {
+ MOZ_ASSERT(!mReferrerScript);
+ MOZ_ASSERT(!mModuleRequestObj);
+ MOZ_ASSERT(mPayload.isUndefined());
+
+ DropJSObjects(this);
+}
+
nsIGlobalObject* ModuleLoadRequest::GetGlobalObject() {
return mLoader->GetGlobalObject();
}
diff --git a/js/loader/ModuleLoadRequest.h b/js/loader/ModuleLoadRequest.h
@@ -11,6 +11,7 @@
#include "ScriptLoadRequest.h"
#include "ModuleLoaderBase.h"
#include "mozilla/Assertions.h"
+#include "mozilla/HoldDropJSObjects.h"
#include "js/RootingAPI.h"
#include "js/Value.h"
#include "nsURIHashKey.h"
@@ -27,11 +28,7 @@ class ModuleLoaderBase;
// multiple imports of the same module.
class ModuleLoadRequest final : public ScriptLoadRequest {
- ~ModuleLoadRequest() {
- MOZ_ASSERT(!mReferrerScript);
- MOZ_ASSERT(!mModuleRequestObj);
- MOZ_ASSERT(mPayload.isUndefined());
- }
+ ~ModuleLoadRequest();
ModuleLoadRequest(const ModuleLoadRequest& aOther) = delete;
ModuleLoadRequest(ModuleLoadRequest&& aOther) = delete;
diff --git a/js/loader/ScriptLoadRequest.cpp b/js/loader/ScriptLoadRequest.cpp
@@ -11,7 +11,6 @@
#include "mozilla/dom/ScriptLoadContext.h"
#include "mozilla/dom/WorkerLoadContext.h"
#include "mozilla/dom/ScriptSettings.h"
-#include "mozilla/HoldDropJSObjects.h"
#include "mozilla/StaticPrefs_dom.h"
#include "mozilla/Unused.h"
#include "mozilla/Utf8.h" // mozilla::Utf8Unit
@@ -70,20 +69,22 @@ NS_INTERFACE_MAP_END
NS_IMPL_CYCLE_COLLECTING_ADDREF(ScriptLoadRequest)
NS_IMPL_CYCLE_COLLECTING_RELEASE(ScriptLoadRequest)
-NS_IMPL_CYCLE_COLLECTION_CLASS(ScriptLoadRequest)
-
-NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ScriptLoadRequest)
- NS_IMPL_CYCLE_COLLECTION_UNLINK(mFetchOptions, mOriginPrincipal, mBaseURL,
- mLoadedScript, mLoadContext)
- tmp->mScriptForCache = nullptr;
-NS_IMPL_CYCLE_COLLECTION_UNLINK_END
-
-NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ScriptLoadRequest)
- NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFetchOptions, mLoadContext, mLoadedScript)
-NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+// ScriptLoadRequest can be accessed from multiple threads.
+//
+// For instance, worker script loader passes the ScriptLoadRequest to
+// the main thread to perform the actual load.
+// Even while it's handled by the main thread, the ScriptLoadRequest is
+// the target of the worker thread's cycle collector.
+//
+// Fields that can be modified by the main thread shouldn't be touched by
+// the cycle collection.
+//
+// NOTE: nsIURI and nsIPrincipal doesn't have to be touched here because
+// they cannot be a part of cycle.
+NS_IMPL_CYCLE_COLLECTION(ScriptLoadRequest, mFetchOptions, mLoadedScript,
+ mLoadContext)
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ScriptLoadRequest)
- NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mScriptForCache)
NS_IMPL_CYCLE_COLLECTION_TRACE_END
ScriptLoadRequest::ScriptLoadRequest(
@@ -108,7 +109,7 @@ ScriptLoadRequest::ScriptLoadRequest(
}
}
-ScriptLoadRequest::~ScriptLoadRequest() { DropJSObjects(this); }
+ScriptLoadRequest::~ScriptLoadRequest() {}
void ScriptLoadRequest::SetReady() {
MOZ_ASSERT(!IsFinished());
@@ -245,10 +246,7 @@ void ScriptLoadRequest::SetPendingFetchingError() {
void ScriptLoadRequest::MarkScriptForCache(JSScript* aScript) {
MOZ_ASSERT(!IsModuleRequest());
- MOZ_ASSERT(!mScriptForCache);
MarkForCache();
- mScriptForCache = aScript;
- HoldJSObjects(this);
}
static bool IsInternalURIScheme(nsIURI* uri) {
diff --git a/js/loader/ScriptLoadRequest.h b/js/loader/ScriptLoadRequest.h
@@ -302,8 +302,7 @@ class ScriptLoadRequest : public nsISupports,
// This fits the condition for the caching (e.g. file size, fetch count).
PassedCondition,
- // This is marked for encoding, with setting sufficient input,
- // e.g. mScriptForCache for script.
+ // This is marked for encoding.
MarkedForCache,
};
CachingPlan mDiskCachingPlan = CachingPlan::Uninitialized;
@@ -356,12 +355,6 @@ class ScriptLoadRequest : public nsISupports,
// would share the same loaded script.
RefPtr<LoadedScript> mLoadedScript;
- // Holds the top-level JSScript that corresponds to the current source, once
- // it is parsed, and marked to be saved in the bytecode cache.
- //
- // NOTE: This field is not used for ModuleLoadRequest.
- JS::Heap<JSScript*> mScriptForCache;
-
// LoadContext for augmenting the load depending on the loading
// context (DOM, Worker, etc.)
RefPtr<LoadContextBase> mLoadContext;