Skip to content

Commit 557139c

Browse files
edusperoniedusperoni
committed
feat: accept platform options under android, deprecate androidPriority
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.
1 parent 60d03e9 commit 557139c

6 files changed

Lines changed: 306 additions & 50 deletions

File tree

‎test-app/app/src/main/assets/app/mainpage.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ require("./tests/testWebAssembly");
2727
require("./tests/testEventLoop");
2828
require("./tests/testMultithreadedJavascript");
2929
require("./tests/testWorkerTerminateDuringLoad");
30+
require("./tests/testWorkerOptions");
3031
require("./tests/testInterfaceDefaultMethods");
3132
require("./tests/testInterfaceStaticMethods");
3233
require("./tests/testMetadata");

‎test-app/app/src/main/assets/app/tests/testWorkerOptions.js‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
describe("Worker platform options", function () {
2+
var entry = "./workerOptionsPriorityWorker.js";
3+
4+
// Jasmine arms a spec's async timeout before calling it, so the interval
5+
// has to be raised ahead of the spec, not inside it. A thread niced down
6+
// to 19 boots a whole isolate on whatever CPU is left over; on a loaded
7+
// host that has taken well over 10 s.
8+
var originalTimeout;
9+
beforeEach(function () {
10+
originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
11+
jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000;
12+
});
13+
afterEach(function () {
14+
jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout;
15+
});
16+
17+
var reportPriority = function (options, done, check) {
18+
var worker = options === undefined ? new Worker(entry) : new Worker(entry, options);
19+
var settled = false;
20+
var finish = function () {
21+
if (settled) {
22+
return;
23+
}
24+
settled = true;
25+
worker.terminate();
26+
done();
27+
};
28+
// A throw inside either handler must still settle the spec and
29+
// terminate the worker; Jasmine only guards the spec body itself.
30+
worker.onmessage = function (msg) {
31+
try {
32+
check(msg.data.priority);
33+
} finally {
34+
finish();
35+
}
36+
};
37+
worker.onerror = function (e) {
38+
try {
39+
expect(String(e && e.message ? e.message : e)).toBe("<no worker error>");
40+
} finally {
41+
finish();
42+
}
43+
};
44+
};
45+
46+
// Only the non-negative nice values are asserted exactly: lowering a
47+
// thread's nice value needs a privilege the app may not hold, so the
48+
// negative names are covered below by starting a worker instead.
49+
var priorities = [
50+
["lowest", 19],
51+
["background", 10],
52+
["lessFavorable", 1],
53+
["default", 0]
54+
];
55+
56+
priorities.forEach(function (pair) {
57+
it("runs the worker thread at " + pair[0] + " priority", function (done) {
58+
reportPriority({ android: { priority: pair[0] } }, done, function (priority) {
59+
expect(priority).toBe(pair[1]);
60+
});
61+
});
62+
});
63+
64+
it("accepts a negative priority name", function () {
65+
var worker;
66+
expect(function () {
67+
worker = new Worker(entry, { android: { priority: "urgentAudio" } });
68+
}).not.toThrow();
69+
worker.terminate();
70+
});
71+
72+
it("accepts a raw nice value", function (done) {
73+
reportPriority({ android: { priority: 12 } }, done, function (priority) {
74+
expect(priority).toBe(12);
75+
});
76+
});
77+
78+
it("clamps a nice value above the kernel range", function (done) {
79+
reportPriority({ android: { priority: 100 } }, done, function (priority) {
80+
expect(priority).toBe(19);
81+
});
82+
});
83+
84+
it("still honors the deprecated androidPriority option", function (done) {
85+
reportPriority({ androidPriority: "lowest" }, done, function (priority) {
86+
expect(priority).toBe(19);
87+
});
88+
});
89+
90+
it("prefers android.priority over androidPriority when both are given", function (done) {
91+
reportPriority({ android: { priority: "default" }, androidPriority: "lowest" }, done,
92+
function (priority) {
93+
expect(priority).toBe(0);
94+
});
95+
});
96+
97+
it("ignores unknown keys inside android", function (done) {
98+
reportPriority({ android: { priority: "lowest", somethingElse: 42 } }, done,
99+
function (priority) {
100+
expect(priority).toBe(19);
101+
});
102+
});
103+
104+
it("starts a worker given no options at all", function (done) {
105+
reportPriority(undefined, done, function (priority) {
106+
expect(typeof priority).toBe("number");
107+
});
108+
});
109+
110+
it("treats android: null like an absent android", function (done) {
111+
reportPriority({ android: null, androidPriority: "lowest" }, done, function (priority) {
112+
expect(priority).toBe(19);
113+
});
114+
});
115+
116+
it("propagates the error thrown by an option getter", function () {
117+
var boom = new Error("boom");
118+
var options = new Proxy({}, {
119+
get: function (target, key) {
120+
if (key === "android") {
121+
throw boom;
122+
}
123+
return undefined;
124+
}
125+
});
126+
var thrown;
127+
try {
128+
new Worker(entry, options);
129+
} catch (e) {
130+
thrown = e;
131+
}
132+
expect(thrown).toBe(boom);
133+
});
134+
135+
it("throws a TypeError when android is not an object", function () {
136+
expect(function () {
137+
new Worker(entry, { android: 42 });
138+
}).toThrowError(TypeError, /"android"/);
139+
});
140+
141+
it("throws a TypeError for an unknown android.priority", function () {
142+
expect(function () {
143+
new Worker(entry, { android: { priority: "highest" } });
144+
}).toThrowError(TypeError, /"android\.priority"/);
145+
});
146+
147+
it("throws a TypeError for an android.priority that is neither a name nor a number",
148+
function () {
149+
expect(function () {
150+
new Worker(entry, { android: { priority: {} } });
151+
}).toThrowError(TypeError, /"android\.priority"/);
152+
});
153+
});

‎test-app/app/src/main/assets/app/tests/workerOptionsPriorityWorker.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Entry for testWorkerOptions: reports the nice value the runtime gave this
2+
// worker's thread, which is the only observable effect of the
3+
// `android.priority` option.
4+
postMessage({ priority: android.os.Process.getThreadPriority(android.os.Process.myTid()) });

‎test-app/runtime/src/main/cpp/CallbackHandlers.cpp‎

Lines changed: 133 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,72 +1106,152 @@ jobjectArray CallbackHandlers::GetJavaStringArray(JEnv &env, int length) {
11061106
return (jobjectArray) env.NewGlobalRef(tmpArr);
11071107
}
11081108

1109+
namespace {
1110+
1111+
const int kDefaultWorkerPriority = 10; // android.os.Process.THREAD_PRIORITY_BACKGROUND
1112+
1113+
const char *const kWorkerPriorityNames =
1114+
"'lowest', 'background', 'lessFavorable', 'default', 'moreFavorable', "
1115+
"'foreground', 'display', 'urgentDisplay', 'video', 'audio', 'urgentAudio' "
1116+
"or a number between -20 and 19";
1117+
1118+
// The android.os.Process THREAD_PRIORITY_* nice values, under their camelCase
1119+
// names; anything else is a caller error.
1120+
bool MapWorkerPriorityName(const std::string &name, int &priority) {
1121+
if (name == "lowest") {
1122+
priority = 19;
1123+
} else if (name == "background") {
1124+
priority = 10;
1125+
} else if (name == "lessFavorable") {
1126+
priority = 1;
1127+
} else if (name == "default") {
1128+
priority = 0;
1129+
} else if (name == "moreFavorable") {
1130+
priority = -1;
1131+
} else if (name == "foreground") {
1132+
priority = -2;
1133+
} else if (name == "display") {
1134+
priority = -4;
1135+
} else if (name == "urgentDisplay") {
1136+
priority = -8;
1137+
} else if (name == "video") {
1138+
priority = -10;
1139+
} else if (name == "audio") {
1140+
priority = -16;
1141+
} else if (name == "urgentAudio") {
1142+
priority = -19;
1143+
} else {
1144+
return false;
1145+
}
1146+
return true;
1147+
}
1148+
1149+
// Nice values outside the kernel's range are clamped rather than rejected:
1150+
// a caller asking for "as low as possible" gets it.
1151+
int ClampWorkerPriority(Local<Context> context, Local<Value> value) {
1152+
int priority = value->Int32Value(context).FromMaybe(kDefaultWorkerPriority);
1153+
if (priority < -20) {
1154+
return -20;
1155+
}
1156+
if (priority > 19) {
1157+
return 19;
1158+
}
1159+
return priority;
1160+
}
1161+
1162+
// Carries a real TypeError instance so `catch (e) { e instanceof TypeError }`
1163+
// holds in JS; NewThreadCallback's catch block rethrows it unchanged.
1164+
[[noreturn]] void ThrowWorkerOptionTypeError(Isolate *isolate, const std::string &message) {
1165+
Local<Value> error = Exception::TypeError(ArgConverter::ConvertToV8String(isolate, message));
1166+
throw NativeScriptException(isolate, error, message);
1167+
}
1168+
1169+
// Reads `key` from `object`. A false return means the getter threw: the
1170+
// exception is already pending on the isolate and construction must stop
1171+
// without running anything else on it.
1172+
bool ReadWorkerOption(Isolate *isolate, Local<Context> context, Local<Object> object,
1173+
const char *key, Local<Value> &out) {
1174+
return object->Get(context, ArgConverter::ConvertToV8String(isolate, key)).ToLocal(&out);
1175+
}
1176+
11091177
/*
1110-
* Resolves the `androidPriority` Worker option to an android.os.Process
1111-
* thread priority (nice value). Accepts the THREAD_PRIORITY_* names in
1112-
* camelCase or a raw nice value clamped to [-20, 19].
1113-
* Defaults to THREAD_PRIORITY_BACKGROUND (10), the previously hardcoded value.
1178+
* Resolves the Worker thread priority to an android.os.Process nice value from
1179+
* `android.priority`, falling back to the deprecated top-level
1180+
* `androidPriority`. Defaults to THREAD_PRIORITY_BACKGROUND (10).
1181+
* Returns false when an option getter threw (see ReadWorkerOption).
11141182
*/
1115-
static int GetWorkerThreadPriority(Isolate *isolate, Local<Context> context,
1116-
const v8::FunctionCallbackInfo<v8::Value> &args) {
1117-
const int defaultPriority = 10; // android.os.Process.THREAD_PRIORITY_BACKGROUND
1183+
bool GetWorkerThreadPriority(Isolate *isolate, Local<Context> context,
1184+
const v8::FunctionCallbackInfo<v8::Value> &args, int &priority) {
1185+
priority = kDefaultWorkerPriority;
11181186

11191187
if (args.Length() < 2 || !args[1]->IsObject()) {
1120-
return defaultPriority;
1188+
return true;
11211189
}
11221190

11231191
auto options = args[1].As<Object>();
1124-
Local<Value> value;
1125-
if (!options->Get(context, ArgConverter::ConvertToV8String(isolate, "androidPriority"))
1126-
.ToLocal(&value) ||
1127-
value->IsNullOrUndefined()) {
1128-
return defaultPriority;
1192+
bool resolved = false;
1193+
1194+
Local<Value> androidVal;
1195+
if (!ReadWorkerOption(isolate, context, options, "android", androidVal)) {
1196+
return false;
11291197
}
1198+
if (!androidVal->IsNullOrUndefined()) {
1199+
if (!androidVal->IsObject()) {
1200+
ThrowWorkerOptionTypeError(isolate, "Worker option \"android\" must be an object.");
1201+
}
11301202

1131-
if (value->IsNumber()) {
1132-
int priority = value->Int32Value(context).FromMaybe(defaultPriority);
1133-
if (priority < -20) {
1134-
priority = -20;
1135-
} else if (priority > 19) {
1136-
priority = 19;
1203+
Local<Value> priorityVal;
1204+
if (!ReadWorkerOption(isolate, context, androidVal.As<Object>(), "priority", priorityVal)) {
1205+
return false;
1206+
}
1207+
if (!priorityVal->IsUndefined()) {
1208+
if (priorityVal->IsNumber()) {
1209+
priority = ClampWorkerPriority(context, priorityVal);
1210+
} else if (!priorityVal->IsString() ||
1211+
!MapWorkerPriorityName(
1212+
ArgConverter::ConvertToString(priorityVal.As<String>()), priority)) {
1213+
ThrowWorkerOptionTypeError(
1214+
isolate, std::string("Worker option \"android.priority\" must be one of ") +
1215+
kWorkerPriorityNames + ".");
1216+
}
1217+
resolved = true;
11371218
}
1138-
return priority;
11391219
}
11401220

1141-
if (value->IsString()) {
1142-
auto name = ArgConverter::ConvertToString(value.As<String>());
1143-
if (name == "lowest") {
1144-
return 19;
1145-
} else if (name == "background") {
1146-
return 10;
1147-
} else if (name == "lessFavorable") {
1148-
return 1;
1149-
} else if (name == "default") {
1150-
return 0;
1151-
} else if (name == "moreFavorable") {
1152-
return -1;
1153-
} else if (name == "foreground") {
1154-
return -2;
1155-
} else if (name == "display") {
1156-
return -4;
1157-
} else if (name == "urgentDisplay") {
1158-
return -8;
1159-
} else if (name == "video") {
1160-
return -10;
1161-
} else if (name == "audio") {
1162-
return -16;
1163-
} else if (name == "urgentAudio") {
1164-
return -19;
1165-
}
1221+
Local<Value> legacyVal;
1222+
if (!ReadWorkerOption(isolate, context, options, "androidPriority", legacyVal)) {
1223+
return false;
1224+
}
1225+
if (legacyVal->IsNullOrUndefined()) {
1226+
return true;
1227+
}
1228+
1229+
static std::once_flag warnedDeprecated;
1230+
std::call_once(warnedDeprecated, []() {
1231+
DEBUG_WRITE_FORCE("NativeScript: the Worker option \"androidPriority\" is deprecated. "
1232+
"Use \"android\": { \"priority\": ... } instead.");
1233+
});
1234+
1235+
if (resolved) {
1236+
return true;
1237+
}
1238+
1239+
if (legacyVal->IsNumber()) {
1240+
priority = ClampWorkerPriority(context, legacyVal);
1241+
return true;
1242+
}
1243+
if (legacyVal->IsString() &&
1244+
MapWorkerPriorityName(ArgConverter::ConvertToString(legacyVal.As<String>()), priority)) {
1245+
return true;
11661246
}
11671247

11681248
throw NativeScriptException(
1169-
"Invalid value for the Worker 'androidPriority' option. Expected one of: "
1170-
"'lowest', 'background', 'lessFavorable', 'default', 'moreFavorable', "
1171-
"'foreground', 'display', 'urgentDisplay', 'video', 'audio', 'urgentAudio' "
1172-
"or a number between -20 and 19.");
1249+
std::string("Invalid value for the Worker 'androidPriority' option. Expected one of: ") +
1250+
kWorkerPriorityNames + ".");
11731251
}
11741252

1253+
} // namespace
1254+
11751255
void CallbackHandlers::NewThreadCallback(const v8::FunctionCallbackInfo<v8::Value> &args) {
11761256
try {
11771257
if (!args.IsConstructCall()) {
@@ -1226,7 +1306,10 @@ void CallbackHandlers::NewThreadCallback(const v8::FunctionCallbackInfo<v8::Valu
12261306
throw NativeScriptException("Worker constructor expects a string URL or URL object.");
12271307
}
12281308

1229-
int priority = GetWorkerThreadPriority(isolate, context, args);
1309+
int priority;
1310+
if (!GetWorkerThreadPriority(isolate, context, args, priority)) {
1311+
return;
1312+
}
12301313

12311314
// An http(s) entry has no filesystem form to validate or to resolve
12321315
// against the caller's directory: it is already absolute, and the

‎test-app/runtime/src/main/cpp/NativeScriptException.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,13 @@ NativeScriptException::NativeScriptException(const string& message,
204204
m_message(message),
205205
m_stackTrace(stackTrace) {}
206206

207+
NativeScriptException::NativeScriptException(Isolate* isolate,
208+
Local<Value> error,
209+
const string& message)
210+
: m_javascriptException(MakeOwnedPersistent(isolate, error)),
211+
m_javaException(JniLocalRef()),
212+
m_message(message) {}
213+
207214
NativeScriptException::NativeScriptException(TryCatch& tc,
208215
const string& message)
209216
: m_javaException(JniLocalRef()) {

0 commit comments

Comments
 (0)