feat(worker): accept platform options under ios, deprecate iosPriority (#470)

* feat(worker): accept platform options under ios, deprecate iosPriority

Worker options now carry iOS-specific settings in an `ios` namespace object:

    new Worker("./w.js", { ios: { priority: "userInitiated" } })

`ios.priority` is validated strictly — a non-object `ios`, a non-string
priority or an unrecognized priority name throws a TypeError — while unknown
keys inside `ios` are ignored so later options can be added without breaking
older runtimes.

`iosPriority` stays supported with its lenient behavior and logs a one-time
deprecation warning; `ios.priority` takes precedence when both are given.

The quality of service is carried as `std::optional<int>` so an explicit
"default" is distinguishable from no option at all.

* feat(worker): treat ios: null as an absent ios option

* test(worker): give low quality-of-service workers time to boot on a loaded host

* test(worker): stop waiting on a background-class thread to boot an isolate

* fix(worker): stop construction when an option getter throws

Author: edusperoni

NativeScript/runtime/DataWrapper.h

@@ -5,6 +5,7 @@
 
 #include <functional>
 #include <mutex>
+#include <optional>
 #include <thread>
 
 #include "Common.h"
@@ -548,7 +549,8 @@ class WorkerWrapper : public BaseDataWrapper {
   void DestroyInspector();
 
   void Start(std::shared_ptr<v8::Persistent<v8::Value>> poWorker,
-             std::function<v8::Isolate*()> func, int qualityOfService = -1);
+             std::function<v8::Isolate*()> func,
+             std::optional<int> qualityOfService = std::nullopt);
   void CallOnErrorHandlers(v8::TryCatch& tc);
   // Reports a rejected entry-evaluation promise. A rejection carries a reason
   // rather than a TryCatch, so it cannot go through CallOnErrorHandlers, but it

NativeScript/runtime/Worker.mm

@@ -1,6 +1,8 @@
 #include "Worker.h"
 #include <pthread.h>
 #include <functional>
+#include <mutex>
+#include <optional>
 #include "Caches.h"
 #include "Constants.h"
 #include "Helpers.h"
@@ -17,6 +19,96 @@
 
 std::vector<std::string> Worker::GlobalFunctions = {"postMessage", "close"};
 
+namespace {
+
+// The five names below are the whole public priority surface; anything else is
+// a caller error under `ios.priority` and ignored under the deprecated
+// `iosPriority`.
+bool MapPriorityName(const std::string& name, int& qos) {
+  if (name == "userInteractive") {
+    qos = NSQualityOfServiceUserInteractive;
+  } else if (name == "userInitiated") {
+    qos = NSQualityOfServiceUserInitiated;
+  } else if (name == "default") {
+    qos = NSQualityOfServiceDefault;
+  } else if (name == "utility") {
+    qos = NSQualityOfServiceUtility;
+  } else if (name == "background") {
+    qos = NSQualityOfServiceBackground;
+  } else {
+    return false;
+  }
+  return true;
+}
+
+// Carries a real TypeError instance so `catch (e) { e instanceof TypeError }`
+// holds in JS; the constructor's catch block rethrows it unchanged.
+[[noreturn]] void ThrowOptionTypeError(Isolate* isolate, const std::string& message) {
+  Local<Value> error = Exception::TypeError(tns::ToV8String(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 ReadOption(Isolate* isolate, Local<Context> context, Local<Object> object, const char* key,
+                Local<Value>& out) {
+  return object->Get(context, tns::ToV8String(isolate, key)).ToLocal(&out);
+}
+
+// Fills `qos` with the quality of service the caller asked for, or leaves it
+// empty for the operation queue's own default. Returns false when a getter
+// threw (see ReadOption).
+bool ParseQualityOfService(Isolate* isolate, Local<Context> context, Local<Object> options,
+                           std::optional<int>& qos) {
+  Local<Value> iosVal;
+  if (!ReadOption(isolate, context, options, "ios", iosVal)) {
+    return false;
+  }
+  if (!iosVal->IsNullOrUndefined()) {
+    if (!iosVal->IsObject()) {
+      ThrowOptionTypeError(isolate, "Worker option \"ios\" must be an object.");
+    }
+
+    Local<Value> priorityVal;
+    if (!ReadOption(isolate, context, iosVal.As<Object>(), "priority", priorityVal)) {
+      return false;
+    }
+    if (!priorityVal->IsUndefined()) {
+      int mapped;
+      if (!IsString(priorityVal) || !MapPriorityName(ToString(isolate, priorityVal), mapped)) {
+        ThrowOptionTypeError(isolate,
+                             "Worker option \"ios.priority\" must be one of \"userInteractive\", "
+                             "\"userInitiated\", \"default\", \"utility\" or \"background\".");
+      }
+      qos = mapped;
+    }
+  }
+
+  Local<Value> legacyVal;
+  if (!ReadOption(isolate, context, options, "iosPriority", legacyVal)) {
+    return false;
+  }
+  if (!legacyVal->IsUndefined()) {
+    static std::once_flag warnedDeprecated;
+    std::call_once(warnedDeprecated, []() {
+      Log(@"NativeScript: the Worker option \"iosPriority\" is deprecated. Use "
+          @"\"ios\": { \"priority\": ... } instead.");
+    });
+
+    int mapped;
+    // Lenient by contract: an unusable legacy value is ignored, never fatal.
+    if (!qos.has_value() && IsString(legacyVal) &&
+        MapPriorityName(ToString(isolate, legacyVal), mapped)) {
+      qos = mapped;
+    }
+  }
+
+  return true;
+}
+
+}  // namespace
+
 void Worker::Init(Isolate* isolate, Local<ObjectTemplate> globalTemplate) {
   Worker::Init(isolate, globalTemplate, Caches::Get(isolate)->isWorker);
 }
@@ -148,24 +240,10 @@ throw NativeScriptException(
       }
     }
 
-    int qos = -1;
+    std::optional<int> qos;
     if (info.Length() >= 2 && info[1]->IsObject()) {
-      Local<Object> options = info[1].As<Object>();
-      Local<Value> iosPriorityVal;
-      if (options->Get(context, tns::ToV8String(isolate, "iosPriority")).ToLocal(&iosPriorityVal) &&
-          IsString(iosPriorityVal)) {
-        std::string priority = ToString(isolate, iosPriorityVal);
-        if (priority == "userInteractive") {
-          qos = NSQualityOfServiceUserInteractive;
-        } else if (priority == "userInitiated") {
-          qos = NSQualityOfServiceUserInitiated;
-        } else if (priority == "default") {
-          qos = NSQualityOfServiceDefault;
-        } else if (priority == "utility") {
-          qos = NSQualityOfServiceUtility;
-        } else if (priority == "background") {
-          qos = NSQualityOfServiceBackground;
-        }
+      if (!ParseQualityOfService(isolate, context, info[1].As<Object>(), qos)) {
+        return;
       }
     }
 

NativeScript/runtime/WorkerWrapper.mm

@@ -73,16 +73,16 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function<void()> fn, bool a
 }
 
 void WorkerWrapper::Start(std::shared_ptr<Persistent<Value>> poWorker,
-                          std::function<Isolate*()> func, int qualityOfService) {
+                          std::function<Isolate*()> func, std::optional<int> qualityOfService) {
   this->poWorker_ = poWorker;
   this->workerId_ = nextId_.fetch_add(1, std::memory_order_relaxed) + 1;
 
   NSBlockOperation* op = [NSBlockOperation blockOperationWithBlock:^{
     this->BackgroundLooper(func);
   }];
 
-  if (qualityOfService >= 0) {
-    op.qualityOfService = static_cast<NSQualityOfService>(qualityOfService);
+  if (qualityOfService.has_value()) {
+    op.qualityOfService = static_cast<NSQualityOfService>(*qualityOfService);
   }
 
   [workers_ addOperation:op];

TestRunner/app/tests/WorkerOptionsTests.js

@@ -0,0 +1,139 @@
+describe("Worker platform options", function () {
+    var entry = "./workerOptions/qosWorker.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 utility thread boots
+    // a whole isolate under throttled CPU and I/O; on a contended host that has
+    // taken well over 10 s.
+    var originalTimeout;
+    beforeEach(function () {
+        originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
+        jasmine.DEFAULT_TIMEOUT_INTERVAL = 120000;
+    });
+    afterEach(function () {
+        jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout;
+    });
+
+    var reportQos = 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.qos);
+            } finally {
+                finish();
+            }
+        };
+        worker.onerror = function (e) {
+            try {
+                expect(String(e && e.message ? e.message : e)).toBe("<no worker error>");
+            } finally {
+                finish();
+            }
+        };
+    };
+
+    // Background is deliberately absent: the system defines that class as work
+    // that may take minutes, and on a loaded host a background thread has not
+    // finished booting an isolate within two minutes. It is covered below
+    // without waiting on it.
+    var priorities = [
+        ["userInteractive", NSQualityOfService.UserInteractive],
+        ["userInitiated", NSQualityOfService.UserInitiated],
+        ["default", NSQualityOfService.Default],
+        ["utility", NSQualityOfService.Utility]
+    ];
+
+    priorities.forEach(function (pair) {
+        it("runs the worker thread at " + pair[0] + " quality of service", function (done) {
+            reportQos({ ios: { priority: pair[0] } }, done, function (qos) {
+                expect(qos).toBe(pair[1]);
+            });
+        });
+    });
+
+    it("accepts background priority", function () {
+        var worker;
+        expect(function () {
+            worker = new Worker(entry, { ios: { priority: "background" } });
+        }).not.toThrow();
+        worker.terminate();
+    });
+
+    it("still honors the deprecated iosPriority option", function (done) {
+        reportQos({ iosPriority: "utility" }, done, function (qos) {
+            expect(qos).toBe(NSQualityOfService.Utility);
+        });
+    });
+
+    it("prefers ios.priority over iosPriority when both are given", function (done) {
+        reportQos({ ios: { priority: "userInteractive" }, iosPriority: "background" }, done, function (qos) {
+            expect(qos).toBe(NSQualityOfService.UserInteractive);
+        });
+    });
+
+    it("ignores unknown keys inside ios", function (done) {
+        reportQos({ ios: { priority: "utility", somethingElse: 42 } }, done, function (qos) {
+            expect(qos).toBe(NSQualityOfService.Utility);
+        });
+    });
+
+    it("starts a worker given no options at all", function (done) {
+        reportQos(undefined, done, function (qos) {
+            expect(typeof qos).toBe("number");
+        });
+    });
+
+    it("treats ios: null like an absent ios", function (done) {
+        reportQos({ ios: null, iosPriority: "utility" }, done, function (qos) {
+            expect(qos).toBe(NSQualityOfService.Utility);
+        });
+    });
+
+    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 === "ios") {
+                    throw boom;
+                }
+                return undefined;
+            }
+        });
+        var thrown;
+        try {
+            new Worker(entry, options);
+        } catch (e) {
+            thrown = e;
+        }
+        expect(thrown).toBe(boom);
+    });
+
+    it("throws a TypeError when ios is not an object", function () {
+        expect(function () {
+            new Worker(entry, { ios: 42 });
+        }).toThrowError(TypeError, /"ios"/);
+    });
+
+    it("throws a TypeError for an unknown ios.priority", function () {
+        expect(function () {
+            new Worker(entry, { ios: { priority: "highest" } });
+        }).toThrowError(TypeError, /"ios\.priority"/);
+    });
+
+    it("throws a TypeError for a non-string ios.priority", function () {
+        expect(function () {
+            new Worker(entry, { ios: { priority: 3 } });
+        }).toThrowError(TypeError, /"ios\.priority"/);
+    });
+});

TestRunner/app/tests/index.js

@@ -131,6 +131,7 @@ require("./ApiTests");
 require("./NsRuntimeTests");
 require("./GCFinalizerTests");
 require("./WorkerConcurrentStartupTests");
+require("./WorkerOptionsTests");
 require("./DeclarationConflicts");
 //
 require("./Promises");

TestRunner/app/tests/workerOptions/qosWorker.js

@@ -0,0 +1,4 @@
+// Entry for WorkerOptionsTests: reports the quality of service the runtime
+// gave this worker's thread, which is the only observable effect of the
+// `ios.priority` option.
+postMessage({ qos: NSThread.currentThread.qualityOfService });