Add support for retrieving the Engine from a JSContext

... and use that wherever we previously stored a pointer to the `Engine` in a static global.

Signed-off-by: Till Schneidereit <till@tillschneidereit.net>

Author: tschneidereit

builtins/web/fetch/fetch-api.cpp

@@ -107,7 +107,8 @@ bool fetch_https(JSContext *cx, HandleObject request_obj, HostString method, Hos
   // If the request body is streamed, we need to wait for streaming to complete
   // before marking the request as pending.
   if (!streaming) {
-    ENGINE->queue_async_task(new ResponseFutureTask(request_obj, pending_handle));
+    api::Engine::from_context(cx)
+      .queue_async_task(new ResponseFutureTask(request_obj, pending_handle));
   }
 
   SetReservedSlot(request_obj, static_cast<uint32_t>(Request::Slots::ResponsePromise),
@@ -345,8 +346,6 @@ bool fetch(JSContext *cx, unsigned argc, Value *vp) {
 const JSFunctionSpec methods[] = {JS_FN("fetch", fetch, 2, JSPROP_ENUMERATE), JS_FS_END};
 
 bool install(api::Engine *engine) {
-  ENGINE = engine;
-
   if (!JS_DefineFunctions(engine->cx(), engine->global(), methods))
     return false;
   if (!request_response::install(engine)) {

builtins/web/fetch/request-response.cpp

@@ -52,8 +52,6 @@ using form_data::MultipartFormData;
 
 using namespace std::literals;
 
-static api::Engine *ENGINE;
-
 bool error_stream_controller_with_pending_exception(JSContext *cx, HandleObject stream) {
   RootedValue exn(cx);
   if (!JS_GetPendingException(cx, &exn))
@@ -528,7 +526,8 @@ bool finish_outgoing_body_streaming(JSContext *cx, HandleObject body_owner) {
             .toPrivate());
     SetReservedSlot(body_owner, static_cast<uint32_t>(Request::Slots::PendingResponseHandle),
                     PrivateValue(nullptr));
-    ENGINE->queue_async_task(new ResponseFutureTask(body_owner, pending_handle));
+    api::Engine::from_context(cx)
+      .queue_async_task(new ResponseFutureTask(body_owner, pending_handle));
   }
 
   return true;
@@ -538,9 +537,10 @@ bool RequestOrResponse::append_body(JSContext *cx, JS::HandleObject self, JS::Ha
   api::TaskCompletionCallback callback, HandleObject callback_receiver) {
   MOZ_ASSERT(!body_used(source));
   MOZ_ASSERT(self != source);
+  auto engine = api::Engine::from_context(cx);
   host_api::HttpIncomingBody *source_body = incoming_body_handle(source);
   host_api::HttpOutgoingBody *dest_body = outgoing_body_handle(self);
-  auto res = dest_body->append(ENGINE, source_body, callback, callback_receiver);
+  auto res = dest_body->append(&engine, source_body, callback, callback_receiver);
   if (auto *err = res.to_err()) {
     HANDLE_ERROR(cx, *err);
     return false;
@@ -1008,7 +1008,7 @@ bool do_body_source_pull(JSContext *cx, HandleObject source, HandleObject body_o
     return true;
   }
 
-  ENGINE->queue_async_task(new BodyFutureTask(source));
+  api::Engine::from_context(cx).queue_async_task(new BodyFutureTask(source));
   return true;
 }
 
@@ -1118,7 +1118,7 @@ bool reader_for_outgoing_body_then_handler(JSContext *cx, JS::HandleObject body_
     // Uint8Array?
     fprintf(stderr, "Error: read operation on body ReadableStream didn't respond with a "
                     "Uint8Array. Received value: ");
-    ENGINE->dump_value(val, stderr);
+    api::Engine::from_context(cx).dump_value(val, stderr);
     return false;
   }
 
@@ -1179,7 +1179,7 @@ bool reader_for_outgoing_body_catch_handler(JSContext *cx, JS::HandleObject body
   // stream errored during the streaming send. Not much we can do, but at least
   // close the stream, and warn.
   fprintf(stderr, "Warning: body ReadableStream closed during body streaming. Exception: ");
-  ENGINE->dump_value(args.get(0), stderr);
+  api::Engine::from_context(cx).dump_value(args.get(0), stderr);
 
   return finish_outgoing_body_streaming(cx, body_owner);
 }
@@ -1198,7 +1198,8 @@ bool RequestOrResponse::maybe_stream_body(JSContext *cx, JS::HandleObject body_o
   if (is_incoming(body_owner)) {
     auto *source_body = incoming_body_handle(body_owner);
     auto *dest_body = destination->body().unwrap();
-    auto res = dest_body->append(ENGINE, source_body, finish_outgoing_body_streaming, nullptr);
+    auto engine = api::Engine::from_context(cx);
+    auto res = dest_body->append(&engine, source_body, finish_outgoing_body_streaming, nullptr);
     if (auto *err = res.to_err()) {
       HANDLE_ERROR(cx, *err);
       return false;
@@ -2687,8 +2688,6 @@ JSObject *Response::create_incoming(JSContext *cx, host_api::HttpIncomingRespons
 namespace request_response {
 
 bool install(api::Engine *engine) {
-  ENGINE = engine;
-
   if (!Request::init_class(engine->cx(), engine->global()))
     return false;
   if (!Response::init_class(engine->cx(), engine->global()))

builtins/web/timers.cpp

@@ -28,7 +28,6 @@ class TimersMap {
 }
 
 static PersistentRooted<js::UniquePtr<TimersMap>> TIMERS_MAP;
-static api::Engine *ENGINE;
 
 class TimerTask final : public api::AsyncTask {
   using TimerArgumentsVector = std::vector<JS::Heap<JS::Value>>;
@@ -119,12 +118,12 @@ class TimerTask final : public api::AsyncTask {
     }
   }
 
-  static bool clear(int32_t timer_id) {
+  static bool clear(api::Engine &engine, int32_t timer_id) {
     if (!TIMERS_MAP->timers_.contains(timer_id)) {
       return false;
     }
 
-    ENGINE->cancel_async_task(TIMERS_MAP->timers_[timer_id]);
+    engine.cancel_async_task(TIMERS_MAP->timers_[timer_id]);
     TIMERS_MAP->timers_.erase(timer_id);
     return true;
   }
@@ -171,7 +170,7 @@ template <bool repeat> bool setTimeout_or_interval(JSContext *cx, const unsigned
   }
 
   const auto timer = new TimerTask(delay, repeat, handler, handler_args);
-  ENGINE->queue_async_task(timer);
+  api::Engine::from_context(cx).queue_async_task(timer);
   args.rval().setInt32(timer->timer_id());
 
   return true;
@@ -193,7 +192,7 @@ template <bool interval> bool clearTimeout_or_interval(JSContext *cx, unsigned a
     return false;
   }
 
-  TimerTask::clear(id);
+  TimerTask::clear(api::Engine::from_context(cx), id);
 
   args.rval().setUndefined();
   return true;
@@ -206,7 +205,6 @@ constexpr JSFunctionSpec methods[] = {
     JS_FN("clearTimeout", clearTimeout_or_interval<false>, 1, JSPROP_ENUMERATE), JS_FS_END};
 
 bool install(api::Engine *engine) {
-  ENGINE = engine;
   TIMERS_MAP.init(engine->cx(), js::MakeUnique<TimersMap>());
   return JS_DefineFunctions(engine->cx(), engine->global(), methods);
 }

include/extension-api.h

@@ -83,8 +83,26 @@ class Engine {
 
 public:
   explicit Engine(std::unique_ptr<EngineConfig> config);
+
+  /**
+   * Returns the `Engine` associated with the `JSContext` instance.
+   *
+   * Currently, StarlingMonkey uses exactly one `Engine` and one `JSContext`. Since that might
+   * change in the future, it's heavily recommended to not store either in static globals.
+   *
+   * @return The `Engine` associated with the `JSContext` instance
+   */
+  // TODO: change to static Engine& from_context(JSContext *cx);
   static Engine *get(JSContext *cx);
 
+  /**
+   * Returns the `JSContext` associated with the `Engine` instance.
+   *
+   * Currently, StarlingMonkey uses exactly one `Engine` and one `JSContext`. Since that might
+   * change in the future, it's heavily recommended to not store either in static globals.
+   *
+   * @return The JSContext associated with the `Engine` instance
+   */
   JSContext *cx();
   JS::HandleObject global();
   EngineState state();