From 557139c2d23baa1ee44216d1ed0ceb83ade1990c Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 11 Sep 2026 17:52:12 -0300 Subject: [PATCH 1/2] feat: accept platform options under android, deprecate androidPriority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Worker options now carry Android-specific settings in an `android` namespace object, so future platform options have a place to live instead of accumulating as `androidSomething` keys on the top level: new Worker("./w.js", { android: { priority: "lowest" } }) `android.priority` is validated strictly — a non-object `android`, or a priority that is neither one of the camelCase THREAD_PRIORITY_* names nor a nice value, throws a TypeError — while unknown keys inside `android` are ignored so later options can be added without breaking older runtimes. `androidPriority` keeps its current behavior and logs a one-time per-process deprecation warning; `android.priority` takes precedence when both are given. An option getter that throws now stops construction rather than being swallowed into the default priority, and an option error reaches JS as a real TypeError instance so `e instanceof TypeError` holds. --- test-app/app/src/main/assets/app/mainpage.js | 1 + .../assets/app/tests/testWorkerOptions.js | 153 +++++++++++++++ .../app/tests/workerOptionsPriorityWorker.js | 4 + .../runtime/src/main/cpp/CallbackHandlers.cpp | 183 +++++++++++++----- .../src/main/cpp/NativeScriptException.cpp | 7 + .../src/main/cpp/NativeScriptException.h | 8 + 6 files changed, 306 insertions(+), 50 deletions(-) create mode 100644 test-app/app/src/main/assets/app/tests/testWorkerOptions.js create mode 100644 test-app/app/src/main/assets/app/tests/workerOptionsPriorityWorker.js diff --git a/test-app/app/src/main/assets/app/mainpage.js b/test-app/app/src/main/assets/app/mainpage.js index 1d5d9a027..0473d207f 100644 --- a/test-app/app/src/main/assets/app/mainpage.js +++ b/test-app/app/src/main/assets/app/mainpage.js @@ -27,6 +27,7 @@ require("./tests/testWebAssembly"); require("./tests/testEventLoop"); require("./tests/testMultithreadedJavascript"); require("./tests/testWorkerTerminateDuringLoad"); +require("./tests/testWorkerOptions"); require("./tests/testInterfaceDefaultMethods"); require("./tests/testInterfaceStaticMethods"); require("./tests/testMetadata"); diff --git a/test-app/app/src/main/assets/app/tests/testWorkerOptions.js b/test-app/app/src/main/assets/app/tests/testWorkerOptions.js new file mode 100644 index 000000000..f15708e0b --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/testWorkerOptions.js @@ -0,0 +1,153 @@ +describe("Worker platform options", function () { + var entry = "./workerOptionsPriorityWorker.js"; + + // Jasmine arms a spec's async timeout before calling it, so the interval + // has to be raised ahead of the spec, not inside it. A thread niced down + // to 19 boots a whole isolate on whatever CPU is left over; on a loaded + // host that has taken well over 10 s. + var originalTimeout; + beforeEach(function () { + originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000; + }); + afterEach(function () { + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); + + var reportPriority = function (options, done, check) { + var worker = options === undefined ? new Worker(entry) : new Worker(entry, options); + var settled = false; + var finish = function () { + if (settled) { + return; + } + settled = true; + worker.terminate(); + done(); + }; + // A throw inside either handler must still settle the spec and + // terminate the worker; Jasmine only guards the spec body itself. + worker.onmessage = function (msg) { + try { + check(msg.data.priority); + } finally { + finish(); + } + }; + worker.onerror = function (e) { + try { + expect(String(e && e.message ? e.message : e)).toBe(""); + } finally { + finish(); + } + }; + }; + + // Only the non-negative nice values are asserted exactly: lowering a + // thread's nice value needs a privilege the app may not hold, so the + // negative names are covered below by starting a worker instead. + var priorities = [ + ["lowest", 19], + ["background", 10], + ["lessFavorable", 1], + ["default", 0] + ]; + + priorities.forEach(function (pair) { + it("runs the worker thread at " + pair[0] + " priority", function (done) { + reportPriority({ android: { priority: pair[0] } }, done, function (priority) { + expect(priority).toBe(pair[1]); + }); + }); + }); + + it("accepts a negative priority name", function () { + var worker; + expect(function () { + worker = new Worker(entry, { android: { priority: "urgentAudio" } }); + }).not.toThrow(); + worker.terminate(); + }); + + it("accepts a raw nice value", function (done) { + reportPriority({ android: { priority: 12 } }, done, function (priority) { + expect(priority).toBe(12); + }); + }); + + it("clamps a nice value above the kernel range", function (done) { + reportPriority({ android: { priority: 100 } }, done, function (priority) { + expect(priority).toBe(19); + }); + }); + + it("still honors the deprecated androidPriority option", function (done) { + reportPriority({ androidPriority: "lowest" }, done, function (priority) { + expect(priority).toBe(19); + }); + }); + + it("prefers android.priority over androidPriority when both are given", function (done) { + reportPriority({ android: { priority: "default" }, androidPriority: "lowest" }, done, + function (priority) { + expect(priority).toBe(0); + }); + }); + + it("ignores unknown keys inside android", function (done) { + reportPriority({ android: { priority: "lowest", somethingElse: 42 } }, done, + function (priority) { + expect(priority).toBe(19); + }); + }); + + it("starts a worker given no options at all", function (done) { + reportPriority(undefined, done, function (priority) { + expect(typeof priority).toBe("number"); + }); + }); + + it("treats android: null like an absent android", function (done) { + reportPriority({ android: null, androidPriority: "lowest" }, done, function (priority) { + expect(priority).toBe(19); + }); + }); + + it("propagates the error thrown by an option getter", function () { + var boom = new Error("boom"); + var options = new Proxy({}, { + get: function (target, key) { + if (key === "android") { + throw boom; + } + return undefined; + } + }); + var thrown; + try { + new Worker(entry, options); + } catch (e) { + thrown = e; + } + expect(thrown).toBe(boom); + }); + + it("throws a TypeError when android is not an object", function () { + expect(function () { + new Worker(entry, { android: 42 }); + }).toThrowError(TypeError, /"android"/); + }); + + it("throws a TypeError for an unknown android.priority", function () { + expect(function () { + new Worker(entry, { android: { priority: "highest" } }); + }).toThrowError(TypeError, /"android\.priority"/); + }); + + it("throws a TypeError for an android.priority that is neither a name nor a number", + function () { + expect(function () { + new Worker(entry, { android: { priority: {} } }); + }).toThrowError(TypeError, /"android\.priority"/); + }); +}); diff --git a/test-app/app/src/main/assets/app/tests/workerOptionsPriorityWorker.js b/test-app/app/src/main/assets/app/tests/workerOptionsPriorityWorker.js new file mode 100644 index 000000000..fa358271d --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/workerOptionsPriorityWorker.js @@ -0,0 +1,4 @@ +// Entry for testWorkerOptions: reports the nice value the runtime gave this +// worker's thread, which is the only observable effect of the +// `android.priority` option. +postMessage({ priority: android.os.Process.getThreadPriority(android.os.Process.myTid()) }); diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index cff979ba0..4f3e94ca2 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -1106,72 +1106,152 @@ jobjectArray CallbackHandlers::GetJavaStringArray(JEnv &env, int length) { return (jobjectArray) env.NewGlobalRef(tmpArr); } +namespace { + +const int kDefaultWorkerPriority = 10; // android.os.Process.THREAD_PRIORITY_BACKGROUND + +const char *const kWorkerPriorityNames = + "'lowest', 'background', 'lessFavorable', 'default', 'moreFavorable', " + "'foreground', 'display', 'urgentDisplay', 'video', 'audio', 'urgentAudio' " + "or a number between -20 and 19"; + +// The android.os.Process THREAD_PRIORITY_* nice values, under their camelCase +// names; anything else is a caller error. +bool MapWorkerPriorityName(const std::string &name, int &priority) { + if (name == "lowest") { + priority = 19; + } else if (name == "background") { + priority = 10; + } else if (name == "lessFavorable") { + priority = 1; + } else if (name == "default") { + priority = 0; + } else if (name == "moreFavorable") { + priority = -1; + } else if (name == "foreground") { + priority = -2; + } else if (name == "display") { + priority = -4; + } else if (name == "urgentDisplay") { + priority = -8; + } else if (name == "video") { + priority = -10; + } else if (name == "audio") { + priority = -16; + } else if (name == "urgentAudio") { + priority = -19; + } else { + return false; + } + return true; +} + +// Nice values outside the kernel's range are clamped rather than rejected: +// a caller asking for "as low as possible" gets it. +int ClampWorkerPriority(Local context, Local value) { + int priority = value->Int32Value(context).FromMaybe(kDefaultWorkerPriority); + if (priority < -20) { + return -20; + } + if (priority > 19) { + return 19; + } + return priority; +} + +// Carries a real TypeError instance so `catch (e) { e instanceof TypeError }` +// holds in JS; NewThreadCallback's catch block rethrows it unchanged. +[[noreturn]] void ThrowWorkerOptionTypeError(Isolate *isolate, const std::string &message) { + Local error = Exception::TypeError(ArgConverter::ConvertToV8String(isolate, message)); + throw NativeScriptException(isolate, error, message); +} + +// Reads `key` from `object`. A false return means the getter threw: the +// exception is already pending on the isolate and construction must stop +// without running anything else on it. +bool ReadWorkerOption(Isolate *isolate, Local context, Local object, + const char *key, Local &out) { + return object->Get(context, ArgConverter::ConvertToV8String(isolate, key)).ToLocal(&out); +} + /* - * Resolves the `androidPriority` Worker option to an android.os.Process - * thread priority (nice value). Accepts the THREAD_PRIORITY_* names in - * camelCase or a raw nice value clamped to [-20, 19]. - * Defaults to THREAD_PRIORITY_BACKGROUND (10), the previously hardcoded value. + * Resolves the Worker thread priority to an android.os.Process nice value from + * `android.priority`, falling back to the deprecated top-level + * `androidPriority`. Defaults to THREAD_PRIORITY_BACKGROUND (10). + * Returns false when an option getter threw (see ReadWorkerOption). */ -static int GetWorkerThreadPriority(Isolate *isolate, Local context, - const v8::FunctionCallbackInfo &args) { - const int defaultPriority = 10; // android.os.Process.THREAD_PRIORITY_BACKGROUND +bool GetWorkerThreadPriority(Isolate *isolate, Local context, + const v8::FunctionCallbackInfo &args, int &priority) { + priority = kDefaultWorkerPriority; if (args.Length() < 2 || !args[1]->IsObject()) { - return defaultPriority; + return true; } auto options = args[1].As(); - Local value; - if (!options->Get(context, ArgConverter::ConvertToV8String(isolate, "androidPriority")) - .ToLocal(&value) || - value->IsNullOrUndefined()) { - return defaultPriority; + bool resolved = false; + + Local androidVal; + if (!ReadWorkerOption(isolate, context, options, "android", androidVal)) { + return false; } + if (!androidVal->IsNullOrUndefined()) { + if (!androidVal->IsObject()) { + ThrowWorkerOptionTypeError(isolate, "Worker option \"android\" must be an object."); + } - if (value->IsNumber()) { - int priority = value->Int32Value(context).FromMaybe(defaultPriority); - if (priority < -20) { - priority = -20; - } else if (priority > 19) { - priority = 19; + Local priorityVal; + if (!ReadWorkerOption(isolate, context, androidVal.As(), "priority", priorityVal)) { + return false; + } + if (!priorityVal->IsUndefined()) { + if (priorityVal->IsNumber()) { + priority = ClampWorkerPriority(context, priorityVal); + } else if (!priorityVal->IsString() || + !MapWorkerPriorityName( + ArgConverter::ConvertToString(priorityVal.As()), priority)) { + ThrowWorkerOptionTypeError( + isolate, std::string("Worker option \"android.priority\" must be one of ") + + kWorkerPriorityNames + "."); + } + resolved = true; } - return priority; } - if (value->IsString()) { - auto name = ArgConverter::ConvertToString(value.As()); - if (name == "lowest") { - return 19; - } else if (name == "background") { - return 10; - } else if (name == "lessFavorable") { - return 1; - } else if (name == "default") { - return 0; - } else if (name == "moreFavorable") { - return -1; - } else if (name == "foreground") { - return -2; - } else if (name == "display") { - return -4; - } else if (name == "urgentDisplay") { - return -8; - } else if (name == "video") { - return -10; - } else if (name == "audio") { - return -16; - } else if (name == "urgentAudio") { - return -19; - } + Local legacyVal; + if (!ReadWorkerOption(isolate, context, options, "androidPriority", legacyVal)) { + return false; + } + if (legacyVal->IsNullOrUndefined()) { + return true; + } + + static std::once_flag warnedDeprecated; + std::call_once(warnedDeprecated, []() { + DEBUG_WRITE_FORCE("NativeScript: the Worker option \"androidPriority\" is deprecated. " + "Use \"android\": { \"priority\": ... } instead."); + }); + + if (resolved) { + return true; + } + + if (legacyVal->IsNumber()) { + priority = ClampWorkerPriority(context, legacyVal); + return true; + } + if (legacyVal->IsString() && + MapWorkerPriorityName(ArgConverter::ConvertToString(legacyVal.As()), priority)) { + return true; } throw NativeScriptException( - "Invalid value for the Worker 'androidPriority' option. Expected one of: " - "'lowest', 'background', 'lessFavorable', 'default', 'moreFavorable', " - "'foreground', 'display', 'urgentDisplay', 'video', 'audio', 'urgentAudio' " - "or a number between -20 and 19."); + std::string("Invalid value for the Worker 'androidPriority' option. Expected one of: ") + + kWorkerPriorityNames + "."); } +} // namespace + void CallbackHandlers::NewThreadCallback(const v8::FunctionCallbackInfo &args) { try { if (!args.IsConstructCall()) { @@ -1226,7 +1306,10 @@ void CallbackHandlers::NewThreadCallback(const v8::FunctionCallbackInfo error, + const string& message) + : m_javascriptException(MakeOwnedPersistent(isolate, error)), + m_javaException(JniLocalRef()), + m_message(message) {} + NativeScriptException::NativeScriptException(TryCatch& tc, const string& message) : m_javaException(JniLocalRef()) { diff --git a/test-app/runtime/src/main/cpp/NativeScriptException.h b/test-app/runtime/src/main/cpp/NativeScriptException.h index a38b14b1f..e322b2bc4 100644 --- a/test-app/runtime/src/main/cpp/NativeScriptException.h +++ b/test-app/runtime/src/main/cpp/NativeScriptException.h @@ -32,6 +32,14 @@ class NativeScriptException : public std::exception { NativeScriptException(const std::string& message, const std::string& stackTrace); + /* + * Generates a NativeScriptException carrying an already-built JS error + * value. ReThrowToV8 throws that value unchanged, so a TypeError reaches + * JS as a TypeError. + */ + NativeScriptException(v8::Isolate* isolate, v8::Local error, + const std::string& message); + /* * Generates a NativeScriptException with javascript error from TryCatch and a * prepend message if any From b35fd572cdfaac4a4bd2e4ffd46aaa586659e2b4 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 11 Sep 2026 20:39:22 -0300 Subject: [PATCH 2/2] fix: clamp numeric worker priorities before converting to int --- .../assets/app/tests/testWorkerOptions.js | 22 ++++++++++++ .../runtime/src/main/cpp/CallbackHandlers.cpp | 35 +++++++++++++------ 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/test-app/app/src/main/assets/app/tests/testWorkerOptions.js b/test-app/app/src/main/assets/app/tests/testWorkerOptions.js index f15708e0b..e2e0b906b 100644 --- a/test-app/app/src/main/assets/app/tests/testWorkerOptions.js +++ b/test-app/app/src/main/assets/app/tests/testWorkerOptions.js @@ -81,6 +81,22 @@ describe("Worker platform options", function () { }); }); + it("clamps a nice value past the range of a 32-bit integer", function (done) { + reportPriority({ android: { priority: 4294967295 } }, done, function (priority) { + expect(priority).toBe(19); + }); + }); + + // Asserted by starting the worker rather than by the reported nice value, + // for the same privilege reason as the negative names above. + it("accepts a nice value past the negative end of a 32-bit integer", function () { + var worker; + expect(function () { + worker = new Worker(entry, { android: { priority: -4294967296 } }); + }).not.toThrow(); + worker.terminate(); + }); + it("still honors the deprecated androidPriority option", function (done) { reportPriority({ androidPriority: "lowest" }, done, function (priority) { expect(priority).toBe(19); @@ -150,4 +166,10 @@ describe("Worker platform options", function () { new Worker(entry, { android: { priority: {} } }); }).toThrowError(TypeError, /"android\.priority"/); }); + + it("throws a TypeError for a NaN android.priority", function () { + expect(function () { + new Worker(entry, { android: { priority: NaN } }); + }).toThrowError(TypeError, /"android\.priority"/); + }); }); diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index 4f3e94ca2..381728a72 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -1146,17 +1146,23 @@ bool MapWorkerPriorityName(const std::string &name, int &priority) { return true; } -// Nice values outside the kernel's range are clamped rather than rejected: -// a caller asking for "as low as possible" gets it. -int ClampWorkerPriority(Local context, Local value) { - int priority = value->Int32Value(context).FromMaybe(kDefaultWorkerPriority); +// Nice values outside the kernel's range are clamped rather than rejected: a +// caller asking for "as low as possible" gets it. Clamping happens on the +// double, before any integer conversion - ToInt32 wraps modulo 2^32, which +// would turn a value past the range into an in-range one. NaN sits on no point +// of the scale and is left to the caller to accept or reject. +std::optional ClampWorkerPriority(Local value) { + double priority = value.As()->Value(); + if (std::isnan(priority)) { + return std::nullopt; + } if (priority < -20) { return -20; } if (priority > 19) { return 19; } - return priority; + return static_cast(priority); } // Carries a real TypeError instance so `catch (e) { e instanceof TypeError }` @@ -1205,15 +1211,22 @@ bool GetWorkerThreadPriority(Isolate *isolate, Local context, return false; } if (!priorityVal->IsUndefined()) { + std::optional resolvedPriority; if (priorityVal->IsNumber()) { - priority = ClampWorkerPriority(context, priorityVal); - } else if (!priorityVal->IsString() || - !MapWorkerPriorityName( - ArgConverter::ConvertToString(priorityVal.As()), priority)) { + resolvedPriority = ClampWorkerPriority(priorityVal); + } else if (priorityVal->IsString()) { + int named; + if (MapWorkerPriorityName(ArgConverter::ConvertToString(priorityVal.As()), + named)) { + resolvedPriority = named; + } + } + if (!resolvedPriority) { ThrowWorkerOptionTypeError( isolate, std::string("Worker option \"android.priority\" must be one of ") + kWorkerPriorityNames + "."); } + priority = *resolvedPriority; resolved = true; } } @@ -1237,7 +1250,9 @@ bool GetWorkerThreadPriority(Isolate *isolate, Local context, } if (legacyVal->IsNumber()) { - priority = ClampWorkerPriority(context, legacyVal); + // The deprecated option takes NaN as nice 0 (THREAD_PRIORITY_DEFAULT) + // rather than rejecting it. + priority = ClampWorkerPriority(legacyVal).value_or(0); return true; } if (legacyVal->IsString() &&