[SM52 2/2] Update to Spidermonkey 52 APIs.

No particularly noteworthy changes, as most complex API changes were
already supported in SM45 and done.
The addition of JSStructuredCloneData allows to remove our custom class.

Changes:
- InformalValueTypeName is back in the API, so remove our
implementation.
- Stop using JSRuntime entirely in favour of JSContext*
- JSPropertyDescriptor is renamed.
- CompartmentOptions are tweaked slightly (no functional changes)
- JS::Construct - API update.
- JSClass split - API update.
- A js.msg error message was removed, so we had to use a different one.
- Tests fix: fix comparison of union instances
- Disable warning in spidermonkey Vector.h
- Update error reporting to SM52 (minor API updates)
- Ignore warnings about unused return values (would come from OOM, which
isn't recoverable)

Most of the patching was done by Itms.

Tested by: Stan, Freagarach
Fixes #4893

Differential Revision: https://code.wildfiregames.com/D3095
This was SVN commit r24203.

source/scriptinterface/NativeWrapperDefns.h

@@ -180,7 +180,7 @@ bool ScriptInterface::CallFunction(JS::HandleValue val, const char* name, R& ret
 	ScriptRequest rq(this);
 	JS::RootedValue jsRet(rq.cx);
 	JS::AutoValueVector argv(rq.cx);
-	argv.resize(sizeof...(Ts));
+	(void)argv.resize(sizeof...(Ts));
 	AssignOrToJSValHelper<0>(rq, argv, params...);
 	if (!CallFunction_(val, name, argv, &jsRet))
 		return false;
@@ -193,7 +193,7 @@ bool ScriptInterface::CallFunction(JS::HandleValue val, const char* name, JS::Ro
 	ScriptRequest rq(this);
 	JS::MutableHandle<R> jsRet(ret);
 	JS::AutoValueVector argv(rq.cx);
-	argv.resize(sizeof...(Ts));
+	(void)argv.resize(sizeof...(Ts));
 	AssignOrToJSValHelper<0>(rq, argv, params...);
 	return CallFunction_(val, name, argv, jsRet);
 }
@@ -203,7 +203,7 @@ bool ScriptInterface::CallFunction(JS::HandleValue val, const char* name, JS::Mu
 {
 	ScriptRequest rq(this);
 	JS::AutoValueVector argv(rq.cx);
-	argv.resize(sizeof...(Ts));
+	(void)argv.resize(sizeof...(Ts));
 	AssignOrToJSValHelper<0>(rq, argv, params...);
 	return CallFunction_(val, name, argv, ret);
 }
@@ -215,7 +215,7 @@ bool ScriptInterface::CallFunctionVoid(JS::HandleValue val, const char* name, co
 	ScriptRequest rq(this);
 	JS::RootedValue jsRet(rq.cx);
 	JS::AutoValueVector argv(rq.cx);
-	argv.resize(sizeof...(Ts));
+	(void)argv.resize(sizeof...(Ts));
 	AssignOrToJSValHelper<0>(rq, argv, params...);
 	return CallFunction_(val, name, argv, &jsRet);
 }

source/scriptinterface/ScriptContext.cpp

@@ -25,7 +25,7 @@
 #include "scriptinterface/ScriptInterface.h"
 
 
-void GCSliceCallbackHook(JSRuntime* UNUSED(rt), JS::GCProgress progress, const JS::GCDescription& UNUSED(desc))
+void GCSliceCallbackHook(JSContext* UNUSED(cx), JS::GCProgress progress, const JS::GCDescription& UNUSED(desc))
 {
 	/*
 	 * During non-incremental GC, the GC is bracketed by JSGC_CYCLE_BEGIN/END
@@ -69,7 +69,7 @@ void GCSliceCallbackHook(JSRuntime* UNUSED(rt), JS::GCProgress progress, const J
 	if (progress == JS::GCProgress::GC_CYCLE_BEGIN)
 		printf("starting cycle ===========================================\n");
 
-	const char16_t* str = desc.formatMessage(rt);
+	const char16_t* str = desc.formatMessage(cx);
 	int len = 0;
 
 	for(int i = 0; i < 10000; i++)
@@ -103,35 +103,32 @@ ScriptContext::ScriptContext(int contextSize, int heapGrowthBytesGCTrigger):
 {
 	ENSURE(ScriptEngine::IsInitialised() && "The ScriptEngine must be initialized before constructing any ScriptContexts!");
 
-	m_rt = JS_NewRuntime(contextSize, JS::DefaultNurseryBytes, nullptr);
-	ENSURE(m_rt); // TODO: error handling
+	m_cx = JS_NewContext(contextSize, JS::DefaultNurseryBytes, nullptr);
+	ENSURE(m_cx); // TODO: error handling
 
-	JS::SetGCSliceCallback(m_rt, GCSliceCallbackHook);
+	ENSURE(JS::InitSelfHostedCode(m_cx));
 
-	JS_SetGCParameter(m_rt, JSGC_MAX_MALLOC_BYTES, m_ContextSize);
-	JS_SetGCParameter(m_rt, JSGC_MAX_BYTES, m_ContextSize);
-	JS_SetGCParameter(m_rt, JSGC_MODE, JSGC_MODE_INCREMENTAL);
+	JS::SetGCSliceCallback(m_cx, GCSliceCallbackHook);
+
+	JS_SetGCParameter(m_cx, JSGC_MAX_MALLOC_BYTES, m_ContextSize);
+	JS_SetGCParameter(m_cx, JSGC_MAX_BYTES, m_ContextSize);
+	JS_SetGCParameter(m_cx, JSGC_MODE, JSGC_MODE_INCREMENTAL);
 
 	// The whole heap-growth mechanism seems to work only for non-incremental GCs.
 	// We disable it to make it more clear if full GCs happen triggered by this JSAPI internal mechanism.
-	JS_SetGCParameter(m_rt, JSGC_DYNAMIC_HEAP_GROWTH, false);
+	JS_SetGCParameter(m_cx, JSGC_DYNAMIC_HEAP_GROWTH, false);
 
-	JS_SetErrorReporter(m_rt, ScriptException::ErrorReporter);
-
-	m_cx = JS_NewContext(m_rt, STACK_CHUNK_SIZE);
-	ENSURE(m_cx); // TODO: error handling
-
-	JS_SetOffthreadIonCompilationEnabled(m_rt, true);
+	JS_SetOffthreadIonCompilationEnabled(m_cx, true);
 
 	// For GC debugging:
 	// JS_SetGCZeal(m_cx, 2, JS_DEFAULT_ZEAL_FREQ);
 
 	JS_SetContextPrivate(m_cx, nullptr);
 
-	JS_SetGlobalJitCompilerOption(m_rt, JSJITCOMPILER_ION_ENABLE, 1);
-	JS_SetGlobalJitCompilerOption(m_rt, JSJITCOMPILER_BASELINE_ENABLE, 1);
+	JS_SetGlobalJitCompilerOption(m_cx, JSJITCOMPILER_ION_ENABLE, 1);
+	JS_SetGlobalJitCompilerOption(m_cx, JSJITCOMPILER_BASELINE_ENABLE, 1);
 
-	JS::RuntimeOptionsRef(m_cx)
+	JS::ContextOptionsRef(m_cx)
 		.setExtraWarnings(true)
 		.setWerror(false)
 		.setStrictMode(true);
@@ -144,8 +141,6 @@ ScriptContext::~ScriptContext()
 	ENSURE(ScriptEngine::IsInitialised() && "The ScriptEngine must be active (initialized and not yet shut down) when destroying a ScriptContext!");
 
 	JS_DestroyContext(m_cx);
-	JS_DestroyRuntime(m_rt);
-
 	ScriptEngine::GetSingleton().UnRegisterContext(m_cx);
 }
 
@@ -165,7 +160,7 @@ void ScriptContext::MaybeIncrementalGC(double delay)
 {
 	PROFILE2("MaybeIncrementalGC");
 
-	if (JS::IsIncrementalGCEnabled(m_rt))
+	if (JS::IsIncrementalGCEnabled(m_cx))
 	{
 		// The idea is to get the heap size after a completed GC and trigger the next GC when the heap size has
 		// reached m_LastGCBytes + X.
@@ -184,7 +179,7 @@ void ScriptContext::MaybeIncrementalGC(double delay)
 
 		m_LastGCCheck = timer_Time();
 
-		int gcBytes = JS_GetGCParameter(m_rt, JSGC_BYTES);
+		int gcBytes = JS_GetGCParameter(m_cx, JSGC_BYTES);
 
 #if GC_DEBUG_PRINT
 			std::cout << "gcBytes: " << gcBytes / 1024 << " KB" << std::endl;
@@ -201,10 +196,10 @@ void ScriptContext::MaybeIncrementalGC(double delay)
 		// Run an additional incremental GC slice if the currently running incremental GC isn't over yet
 		// ... or
 		// start a new incremental GC if the JS heap size has grown enough for a GC to make sense
-		if (JS::IsIncrementalGCInProgress(m_rt) || (gcBytes - m_LastGCBytes > m_HeapGrowthBytesGCTrigger))
+		if (JS::IsIncrementalGCInProgress(m_cx) || (gcBytes - m_LastGCBytes > m_HeapGrowthBytesGCTrigger))
 		{
 #if GC_DEBUG_PRINT
-			if (JS::IsIncrementalGCInProgress(m_rt))
+			if (JS::IsIncrementalGCInProgress(m_cx))
 				printf("An incremental GC cycle is in progress. \n");
 			else
 				printf("GC needed because JSGC_BYTES - m_LastGCBytes > m_HeapGrowthBytesGCTrigger \n"
@@ -218,13 +213,13 @@ void ScriptContext::MaybeIncrementalGC(double delay)
 			// fast enough.
 			if (gcBytes > m_ContextSize / 2)
 			{
-				if (JS::IsIncrementalGCInProgress(m_rt))
+				if (JS::IsIncrementalGCInProgress(m_cx))
 				{
 #if GC_DEBUG_PRINT
 					printf("Finishing incremental GC because gcBytes > m_ContextSize / 2. \n");
 #endif
 					PrepareCompartmentsForIncrementalGC();
-					JS::FinishIncrementalGC(m_rt, JS::gcreason::REFRESH_FRAME);
+					JS::FinishIncrementalGC(m_cx, JS::gcreason::REFRESH_FRAME);
 				}
 				else
 				{
@@ -240,23 +235,23 @@ void ScriptContext::MaybeIncrementalGC(double delay)
 #if GC_DEBUG_PRINT
 						printf("Running full GC because gcBytes > m_ContextSize / 2. \n");
 #endif
-						JS_GC(m_rt);
+						JS_GC(m_cx);
 					}
 				}
 			}
 			else
 			{
 #if GC_DEBUG_PRINT
-				if (!JS::IsIncrementalGCInProgress(m_rt))
+				if (!JS::IsIncrementalGCInProgress(m_cx))
 					printf("Starting incremental GC \n");
 				else
 					printf("Running incremental GC slice \n");
 #endif
 				PrepareCompartmentsForIncrementalGC();
-				if (!JS::IsIncrementalGCInProgress(m_rt))
-					JS::StartIncrementalGC(m_rt, GC_NORMAL, JS::gcreason::REFRESH_FRAME, GCSliceTimeBudget);
+				if (!JS::IsIncrementalGCInProgress(m_cx))
+					JS::StartIncrementalGC(m_cx, GC_NORMAL, JS::gcreason::REFRESH_FRAME, GCSliceTimeBudget);
 				else
-					JS::IncrementalGCSlice(m_rt, JS::gcreason::REFRESH_FRAME, GCSliceTimeBudget);
+					JS::IncrementalGCSlice(m_cx, JS::gcreason::REFRESH_FRAME, GCSliceTimeBudget);
 			}
 			m_LastGCBytes = gcBytes;
 		}
@@ -265,10 +260,10 @@ void ScriptContext::MaybeIncrementalGC(double delay)
 
 void ScriptContext::ShrinkingGC()
 {
-	JS_SetGCParameter(m_rt, JSGC_MODE, JSGC_MODE_COMPARTMENT);
-	JS::PrepareForFullGC(m_rt);
-	JS::GCForReason(m_rt, GC_SHRINK, JS::gcreason::REFRESH_FRAME);
-	JS_SetGCParameter(m_rt, JSGC_MODE, JSGC_MODE_INCREMENTAL);
+	JS_SetGCParameter(m_cx, JSGC_MODE, JSGC_MODE_ZONE);
+	JS::PrepareForFullGC(m_cx);
+	JS::GCForReason(m_cx, GC_SHRINK, JS::gcreason::REFRESH_FRAME);
+	JS_SetGCParameter(m_cx, JSGC_MODE, JSGC_MODE_INCREMENTAL);
 }
 
 void ScriptContext::PrepareCompartmentsForIncrementalGC() const

source/scriptinterface/ScriptContext.h

@@ -23,16 +23,14 @@
 
 #include <sstream>
 
-constexpr int STACK_CHUNK_SIZE = 8192;
-
 // Those are minimal defaults. The runtime for the main game is larger and GCs upon a larger growth.
 constexpr int DEFAULT_CONTEXT_SIZE = 16 * 1024 * 1024;
 constexpr int DEFAULT_HEAP_GROWTH_BYTES_GCTRIGGER = 2 * 1024 * 1024;
 
 /**
- * Abstraction around a SpiderMonkey JSRuntime/JSContext.
+ * Abstraction around a SpiderMonkey JSContext.
  *
- * A single ScriptContext, with the associated runtime and context,
+ * A single ScriptContext, with the associated context,
  * should only be used on a single thread.
  *
  * (One means to share data between threads and contexts is to create
@@ -76,12 +74,10 @@ public:
 	void RegisterCompartment(JSCompartment* cmpt);
 	void UnRegisterCompartment(JSCompartment* cmpt);
 
-	JSRuntime* GetJSRuntime() const { return m_rt; }
-
 	/**
 	 * GetGeneralJSContext returns the context without starting a GC request and without
 	 * entering any compartment. It should only be used in specific situations, such as
-	 * creating a new compartment, or as an unsafe alternative to GetJSRuntime.
+	 * creating a new compartment, or when initializing a persistent rooted.
 	 * If you need the compartmented context of a ScriptInterface, you should create a
 	 * ScriptRequest and use the context from that.
 	 */
@@ -89,7 +85,6 @@ public:
 
 private:
 
-	JSRuntime* m_rt;
 	JSContext* m_cx;
 
 	void PrepareCompartmentsForIncrementalGC() const;

source/scriptinterface/ScriptConversions.cpp

@@ -28,28 +28,7 @@
 #define FAIL(msg) STMT(LOGERROR(msg); return false)
 
 // Implicit type conversions often hide bugs, so warn about them
-#define WARN_IF_NOT(c, v) STMT(if (!(c)) { JS_ReportWarning(rq.cx, "Script value conversion check failed: %s (got type %s)", #c, InformalValueTypeName(v)); })
-
-// TODO: SpiderMonkey: Follow upstream progresses about JS_InformalValueTypeName in the API
-// https://bugzilla.mozilla.org/show_bug.cgi?id=1285917
-static const char* InformalValueTypeName(const JS::Value& v)
-{
-	if (v.isObject())
-		return "object";
-	if (v.isString())
-		return "string";
-	if (v.isSymbol())
-		return "symbol";
-	if (v.isNumber())
-		return "number";
-	if (v.isBoolean())
-		return "boolean";
-	if (v.isNull())
-		return "null";
-	if (v.isUndefined())
-		return "undefined";
-	return "value";
-}
+#define WARN_IF_NOT(c, v) STMT(if (!(c)) { JS_ReportWarningUTF8(rq.cx, "Script value conversion check failed: %s (got type %s)", #c, JS::InformalValueTypeName(v)); })
 
 template<> bool ScriptInterface::FromJSVal<bool>(const ScriptRequest& rq, JS::HandleValue v, bool& out)
 {

source/scriptinterface/ScriptExceptions.cpp

@@ -73,10 +73,9 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
 		msg << " line " << report->lineno << "\n";
 	}
 
-	// TODO SM52:
-	// msg << report->message();
+	msg << report->message().c_str();
 
-	JS::RootedObject stackObj(rq.cx, ExceptionStackOrNull(rq.cx, excnObj));
+	JS::RootedObject stackObj(rq.cx, ExceptionStackOrNull(excnObj));
 	JS::RootedValue stackVal(rq.cx, JS::ObjectOrNullValue(stackObj));
 	if (!stackVal.isNull())
 	{
@@ -97,55 +96,10 @@ bool ScriptException::CatchPending(const ScriptRequest& rq)
 	return true;
 }
 
-void ScriptException::ErrorReporter(JSContext* cx, const char* message, JSErrorReport* report)
-{
-	if (!ScriptInterface::GetScriptInterfaceAndCBData(cx))
-	{
-		LOGERROR("Javascript - Out of Memory error");
-		return;
-	}
-	ScriptRequest rq(*ScriptInterface::GetScriptInterfaceAndCBData(cx)->pScriptInterface);
-
-	std::stringstream msg;
-	bool isWarning = JSREPORT_IS_WARNING(report->flags);
-	msg << (isWarning ? "JavaScript warning: " : "JavaScript error: ");
-	if (report->filename)
-	{
-		msg << report->filename;
-		msg << " line " << report->lineno << "\n";
-	}
-
-	msg << message;
-
-	// If there is an exception, then print its stack trace
-	JS::RootedValue excn(rq.cx);
-	if (JS_GetPendingException(rq.cx, &excn) && excn.isObject())
-	{
-		JS::RootedValue stackVal(rq.cx);
-		JS::RootedObject excnObj(rq.cx, &excn.toObject());
-		JS_GetProperty(rq.cx, excnObj, "stack", &stackVal);
-
-		std::string stackText;
-		ScriptInterface::FromJSVal(rq, stackVal, stackText);
-
-		std::istringstream stream(stackText);
-		for (std::string line; std::getline(stream, line);)
-			msg << "\n  " << line;
-	}
-
-	if (isWarning)
-		LOGWARNING("%s", msg.str().c_str());
-	else
-		LOGERROR("%s", msg.str().c_str());
-
-	// When running under Valgrind, print more information in the error message
-	//	VALGRIND_PRINTF_BACKTRACE("->");
-}
-
 void ScriptException::Raise(const ScriptRequest& rq, const char* format, ...)
 {
 	va_list ap;
 	va_start(ap, format);
-	JS_ReportError(rq.cx, format, ap);
+	JS_ReportErrorUTF8(rq.cx, format, ap);
 	va_end(ap);
 }

source/scriptinterface/ScriptExceptions.h

@@ -39,8 +39,6 @@ bool IsPending(const ScriptRequest& rq);
  */
 bool CatchPending(const ScriptRequest& rq);
 
-void ErrorReporter(JSContext* rt, const char* message, JSErrorReport* report);
-
 /**
  * Raise a JS exception from C++ code.
  * This is only really relevant in JSNative functions that don't use ObjectOpResult,

source/scriptinterface/ScriptInterface.cpp

@@ -94,8 +94,7 @@ ScriptRequest::~ScriptRequest()
 namespace
 {
 
-JSClass global_class = {
-	"global", JSCLASS_GLOBAL_FLAGS,
+JSClassOps global_classops = {
 	nullptr, nullptr,
 	nullptr, nullptr,
 	nullptr, nullptr, nullptr,
@@ -103,6 +102,10 @@ JSClass global_class = {
 	JS_GlobalObjectTraceHook
 };
 
+JSClass global_class = {
+	"global", JSCLASS_GLOBAL_FLAGS, &global_classops
+};
+
 // Functions in the global namespace:
 
 bool print(JSContext* cx, uint argc, JS::Value* vp)
@@ -319,12 +322,15 @@ bool ScriptInterface::MathRandom(double& nbr)
 }
 
 ScriptInterface_impl::ScriptInterface_impl(const char* nativeScopeName, const shared_ptr<ScriptContext>& context) :
-	m_context(context), m_cx(context->GetGeneralJSContext()), m_glob(context->GetJSRuntime()), m_nativeScope(context->GetJSRuntime())
+	m_context(context), m_cx(context->GetGeneralJSContext()), m_glob(context->GetGeneralJSContext()), m_nativeScope(context->GetGeneralJSContext())
 {
-	JS::CompartmentOptions opt;
-	opt.setVersion(JSVERSION_LATEST);
+	JS::CompartmentCreationOptions creationOpt;
 	// Keep JIT code during non-shrinking GCs. This brings a quite big performance improvement.
-	opt.setPreserveJitCode(true);
+	creationOpt.setPreserveJitCode(true);
+	JS::CompartmentBehaviors behaviors;
+	behaviors.setVersion(JSVERSION_LATEST);
+
+	JS::CompartmentOptions opt(creationOpt, behaviors);
 
 	JSAutoRequest rq(m_cx);
 	m_glob = JS_NewGlobalObject(m_cx, &global_class, nullptr, JS::OnNewGlobalHookOption::FireOnNewGlobalHook, opt);
@@ -446,9 +452,9 @@ void ScriptInterface::Register(const char* name, JSNative fptr, size_t nargs) co
 	m->Register(name, fptr, (uint)nargs);
 }
 
-JSRuntime* ScriptInterface::GetJSRuntime() const
+JSContext* ScriptInterface::GetGeneralJSContext() const
 {
-	return m->m_context->GetJSRuntime();
+	return m->m_context->GetGeneralJSContext();
 }
 
 shared_ptr<ScriptContext> ScriptInterface::GetContext() const
@@ -560,7 +566,7 @@ bool ScriptInterface::SetGlobal_(const char* name, JS::HandleValue value, bool r
 		return false;
 	if (found)
 	{
-		JS::Rooted<JSPropertyDescriptor> desc(rq.cx);
+		JS::Rooted<JS::PropertyDescriptor> desc(rq.cx);
 		if (!JS_GetOwnPropertyDescriptor(rq.cx, global, name, &desc))
 			return false;
 
@@ -971,15 +977,7 @@ std::string ScriptInterface::ToString(JS::MutableHandleValue obj, bool pretty) c
 		Stringifier str;
 		JS::RootedValue indentVal(rq.cx, JS::Int32Value(2));
 
-		// Temporary disable the error reporter, so we don't print complaints about cyclic values
-		JSErrorReporter er = JS_SetErrorReporter(GetJSRuntime(), nullptr);
-
-		bool ok = JS_Stringify(rq.cx, obj, nullptr, indentVal, &Stringifier::callback, &str);
-
-		// Restore error reporter
-		JS_SetErrorReporter(GetJSRuntime(), er);
-
-		if (ok)
+		if (JS_Stringify(rq.cx, obj, nullptr, indentVal, &Stringifier::callback, &str))
 			return str.stream.str();
 
 		// Drop exceptions raised by cyclic values before trying something else
@@ -994,48 +992,37 @@ std::string ScriptInterface::ToString(JS::MutableHandleValue obj, bool pretty) c
 	return utf8_from_wstring(source);
 }
 
-JS::Value ScriptInterface::CloneValueFromOtherCompartment(const ScriptInterface& otherCompartment, JS::HandleValue val) const
+JS::Value ScriptInterface::CloneValueFromOtherCompartment(const ScriptInterface& otherCompartment, JS::HandleValue val, bool sameThread) const
 {
 	PROFILE("CloneValueFromOtherCompartment");
 	ScriptRequest rq(this);
 	JS::RootedValue out(rq.cx);
-	shared_ptr<StructuredClone> structuredClone = otherCompartment.WriteStructuredClone(val);
+	ScriptInterface::StructuredClone structuredClone = otherCompartment.WriteStructuredClone(val, sameThread);
 	ReadStructuredClone(structuredClone, &out);
 	return out.get();
 }
 
-ScriptInterface::StructuredClone::StructuredClone() :
-	m_Data(NULL), m_Size(0)
-{
-}
-
-ScriptInterface::StructuredClone::~StructuredClone()
-{
-	if (m_Data)
-		JS_ClearStructuredClone(m_Data, m_Size, NULL, NULL);
-}
-
-shared_ptr<ScriptInterface::StructuredClone> ScriptInterface::WriteStructuredClone(JS::HandleValue v) const
+ScriptInterface::StructuredClone ScriptInterface::WriteStructuredClone(JS::HandleValue v, bool sameThread) const
 {
 	ScriptRequest rq(this);
-	u64* data = NULL;
-	size_t nbytes = 0;
-	if (!JS_WriteStructuredClone(rq.cx, v, &data, &nbytes, NULL, NULL, JS::UndefinedHandleValue))
+
+	JS::StructuredCloneScope scope = sameThread ? JS::StructuredCloneScope::SameProcessSameThread : JS::StructuredCloneScope::SameProcessDifferentThread;
+	ScriptInterface::StructuredClone ret(new JSStructuredCloneData(scope));
+	JS::CloneDataPolicy policy;
+
+	if (!JS_WriteStructuredClone(rq.cx, v, ret.get(), scope, policy, nullptr, nullptr, JS::UndefinedHandleValue))
 	{
 		debug_warn(L"Writing a structured clone with JS_WriteStructuredClone failed!");
 		ScriptException::CatchPending(rq);
-		return shared_ptr<StructuredClone>();
+		return ScriptInterface::StructuredClone();
 	}
 
-	shared_ptr<StructuredClone> ret(new StructuredClone);
-	ret->m_Data = data;
-	ret->m_Size = nbytes;
 	return ret;
 }
 
-void ScriptInterface::ReadStructuredClone(const shared_ptr<ScriptInterface::StructuredClone>& ptr, JS::MutableHandleValue ret) const
+void ScriptInterface::ReadStructuredClone(const ScriptInterface::StructuredClone& ptr, JS::MutableHandleValue ret) const
 {
 	ScriptRequest rq(this);
-	if (!JS_ReadStructuredClone(rq.cx, ptr->m_Data, ptr->m_Size, JS_STRUCTURED_CLONE_VERSION, ret, NULL, NULL))
+	if (!JS_ReadStructuredClone(rq.cx, *ptr, JS_STRUCTURED_CLONE_VERSION, ptr->scope(), ret, nullptr, nullptr))
 		ScriptException::CatchPending(rq);
 }

source/scriptinterface/ScriptInterface.h

@@ -119,7 +119,14 @@ public:
 	void SetCallbackData(void* pCBData);
 	static CmptPrivate* GetScriptInterfaceAndCBData(JSContext* cx);
 
-	JSRuntime* GetJSRuntime() const;
+	/**
+	 * GetGeneralJSContext returns the context without starting a GC request and without
+	 * entering the ScriptInterface compartment. It should only be used in specific situations,
+	 * for instance when initializing a persistent rooted.
+	 * If you need the compartmented context of the ScriptInterface, you should create a
+	 * ScriptInterface::Request and use the context from that.
+	 */
+	JSContext* GetGeneralJSContext() const;
 	shared_ptr<ScriptContext> GetContext() const;
 
 	/**
@@ -289,14 +296,6 @@ public:
 	 */
 	bool LoadGlobalScriptFile(const VfsPath& path) const;
 
-	/**
-	 * Construct a new value (usable in this ScriptInterface's compartment) by cloning
-	 * a value from a different compartment.
-	 * Complex values (functions, XML, etc) won't be cloned correctly, but basic
-	 * types and cyclic references should be fine.
-	 */
-	JS::Value CloneValueFromOtherCompartment(const ScriptInterface& otherCompartment, JS::HandleValue val) const;
-
 	/**
 	 * Convert a JS::Value to a C++ type. (This might trigger GC.)
 	 */
@@ -331,18 +330,18 @@ public:
 	 * We wrap them in shared_ptr so memory management is automatic and
 	 * thread-safe.
 	 */
-	class StructuredClone
-	{
-		NONCOPYABLE(StructuredClone);
-	public:
-		StructuredClone();
-		~StructuredClone();
-		u64* m_Data;
-		size_t m_Size;
-	};
+	using StructuredClone = shared_ptr<JSStructuredCloneData>;
 
-	shared_ptr<StructuredClone> WriteStructuredClone(JS::HandleValue v) const;
-	void ReadStructuredClone(const shared_ptr<StructuredClone>& ptr, JS::MutableHandleValue ret) const;
+	StructuredClone WriteStructuredClone(JS::HandleValue v, bool sameThread) const;
+	void ReadStructuredClone(const StructuredClone& ptr, JS::MutableHandleValue ret) const;
+
+	/**
+	 * Construct a new value (usable in this ScriptInterface's compartment) by cloning
+	 * a value from a different compartment.
+	 * Complex values (functions, XML, etc) won't be cloned correctly, but basic
+	 * types and cyclic references should be fine.
+	 */
+	JS::Value CloneValueFromOtherCompartment(const ScriptInterface& otherCompartment, JS::HandleValue val, bool sameThread) const;
 
 	/**
 	 * Retrieve the private data field of a JSObject that is an instance of the given JSClass.

source/scriptinterface/ScriptStats.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2015 Wildfire Games.
+/* Copyright (C) 2020 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -84,28 +84,28 @@ CStr CScriptStatsTable::GetCellText(size_t row, size_t col)
 	{
 		if (col == 0)
 			return "max nominal heap bytes";
-		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetJSRuntime(), JSGC_MAX_BYTES);
+		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetGeneralJSContext(), JSGC_MAX_BYTES);
 		return CStr::FromUInt(n);
 	}
 	case Row_MaxMallocBytes:
 	{
 		if (col == 0)
 			return "max JS_malloc bytes";
-		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetJSRuntime(), JSGC_MAX_MALLOC_BYTES);
+		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetGeneralJSContext(), JSGC_MAX_MALLOC_BYTES);
 		return CStr::FromUInt(n);
 	}
 	case Row_Bytes:
 	{
 		if (col == 0)
 			return "allocated bytes";
-		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetJSRuntime(), JSGC_BYTES);
+		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetGeneralJSContext(), JSGC_BYTES);
 		return CStr::FromUInt(n);
 	}
 	case Row_NumberGC:
 	{
 		if (col == 0)
 			return "number of GCs";
-		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetJSRuntime(), JSGC_NUMBER);
+		uint32_t n = JS_GetGCParameter(m_ScriptInterfaces.at(col-1).first->GetGeneralJSContext(), JSGC_NUMBER);
 		return CStr::FromUInt(n);
 	}
 	default:

source/scriptinterface/ScriptTypes.h

@@ -55,6 +55,8 @@
 #if MSC_VERSION
 // reduce the warning level for the SpiderMonkey headers
 # pragma warning(push, 1)
+// ignore C4291 in <mozilla/Vector.h>
+# pragma warning(disable: 4291)
 #endif
 
 #include "jspubtd.h"

source/scriptinterface/tests/test_ScriptInterface.h

@@ -40,7 +40,7 @@ public:
 		ScriptInterface script("Test", "Test", g_ScriptContext);
 		TestLogger logger;
 		TS_ASSERT(!script.LoadScript(L"test.js", "1+"));
-		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "ERROR: JavaScript error: test.js line 1\nSyntaxError: expected expression, got end of script");
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "ERROR: JavaScript error: test.js line 1\nexpected expression, got end of script");
 	}
 
 	void test_loadscript_strict_warning()
@@ -57,7 +57,7 @@ public:
 		ScriptInterface script("Test", "Test", g_ScriptContext);
 		TestLogger logger;
 		TS_ASSERT(!script.LoadScript(L"test.js", "with(1){}"));
-		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "ERROR: JavaScript error: test.js line 1\nSyntaxError: strict mode code may not contain \'with\' statements");
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "ERROR: JavaScript error: test.js line 1\nstrict mode code may not contain \'with\' statements");
 	}
 
 	void test_clone_basic()
@@ -72,7 +72,7 @@ public:
 		{
 			ScriptRequest rq2(script2);
 
-			JS::RootedValue obj2(rq2.cx, script2.CloneValueFromOtherCompartment(script1, obj1));
+			JS::RootedValue obj2(rq2.cx, script2.CloneValueFromOtherCompartment(script1, obj1, true));
 
 			std::string source;
 			TS_ASSERT(script2.CallFunction(obj2, "toSource", source));
@@ -94,7 +94,7 @@ public:
 		{
 			ScriptRequest rq2(script2);
 
-			JS::RootedValue obj2(rq2.cx, script2.CloneValueFromOtherCompartment(script1, obj1));
+			JS::RootedValue obj2(rq2.cx, script2.CloneValueFromOtherCompartment(script1, obj1, true));
 
 			std::string source;
 			TS_ASSERT(script2.CallFunction(obj2, "toSource", source));
@@ -114,7 +114,7 @@ public:
 
 		{
 			ScriptRequest rq2(script2);
-			JS::RootedValue obj2(rq2.cx, script2.CloneValueFromOtherCompartment(script1, obj1));
+			JS::RootedValue obj2(rq2.cx, script2.CloneValueFromOtherCompartment(script1, obj1, true));
 
 			// Use JSAPI function to check if the values of the properties "a", "b" are equals a.x[0]
 			JS::RootedValue prop_a(rq2.cx);
@@ -125,8 +125,8 @@ public:
 			TS_ASSERT(prop_a.isObject());
 			TS_ASSERT(prop_b.isObject());
 			TS_ASSERT(script2.GetProperty(prop_a, "0", &prop_x1));
-			TS_ASSERT_EQUALS(prop_x1.get(), prop_a.get());
-			TS_ASSERT_EQUALS(prop_x1.get(), prop_b.get());
+			TS_ASSERT(prop_x1.get() == prop_a.get());
+			TS_ASSERT(prop_x1.get() == prop_b.get());
 		}
 	}