cleanup(core): Move JsRealm into a separate module (#18641)

This will help make reviews easier for #15760, which moves a number of
methods related to module loading from `JsRuntime` into `JsRealm`.

core/error.rs

@@ -9,8 +9,8 @@ use std::fmt::Formatter;
 
 use anyhow::Error;
 
+use crate::realm::JsRealm;
 use crate::runtime::GetErrorClassFn;
-use crate::runtime::JsRealm;
 use crate::runtime::JsRuntime;
 use crate::source_map::apply_source_map;
 use crate::source_map::get_source_line;

core/lib.rs

@@ -17,6 +17,7 @@ mod ops;
 mod ops_builtin;
 mod ops_builtin_v8;
 mod ops_metrics;
+mod realm;
 mod resources;
 mod runtime;
 pub mod snapshot_util;
@@ -103,6 +104,7 @@ pub use crate::ops_builtin::op_resources;
 pub use crate::ops_builtin::op_void_async;
 pub use crate::ops_builtin::op_void_sync;
 pub use crate::ops_metrics::OpsTracker;
+pub use crate::realm::JsRealm;
 pub use crate::resources::AsyncResult;
 pub use crate::resources::Resource;
 pub use crate::resources::ResourceId;
@@ -111,7 +113,6 @@ pub use crate::runtime::CompiledWasmModuleStore;
 pub use crate::runtime::CrossIsolateStore;
 pub use crate::runtime::GetErrorClassFn;
 pub use crate::runtime::JsErrorCreateFn;
-pub use crate::runtime::JsRealm;
 pub use crate::runtime::JsRuntime;
 pub use crate::runtime::RuntimeOptions;
 pub use crate::runtime::SharedArrayBufferStore;

core/realm.rs

@@ -0,0 +1,245 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+use crate::bindings;
+use crate::modules::ModuleCode;
+use crate::ops::OpCtx;
+use crate::runtime::exception_to_err_result;
+use anyhow::Error;
+use std::cell::RefCell;
+use std::collections::HashMap;
+use std::collections::HashSet;
+use std::option::Option;
+use std::rc::Rc;
+use v8::HandleScope;
+use v8::Local;
+
+#[derive(Default)]
+pub(crate) struct ContextState {
+  pub(crate) js_recv_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) pending_promise_rejections:
+    HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>,
+  pub(crate) unrefed_ops: HashSet<i32>,
+  // We don't explicitly re-read this prop but need the slice to live alongside
+  // the context
+  pub(crate) op_ctxs: Box<[OpCtx]>,
+}
+
+/// A representation of a JavaScript realm tied to a [`JsRuntime`], that allows
+/// execution in the realm's context.
+///
+/// A [`JsRealm`] instance is a reference to an already existing realm, which
+/// does not hold ownership of it, so instances can be created and dropped as
+/// needed. As such, calling [`JsRealm::new`] doesn't create a new realm, and
+/// cloning a [`JsRealm`] only creates a new reference. See
+/// [`JsRuntime::create_realm`] to create new realms instead.
+///
+/// Despite [`JsRealm`] instances being references, multiple instances that
+/// point to the same realm won't overlap because every operation requires
+/// passing a mutable reference to the [`v8::Isolate`]. Therefore, no operation
+/// on two [`JsRealm`] instances tied to the same isolate can be run at the same
+/// time, regardless of whether they point to the same realm.
+///
+/// # Panics
+///
+/// Every method of [`JsRealm`] will panic if you call it with a reference to a
+/// [`v8::Isolate`] other than the one that corresponds to the current context.
+///
+/// In other words, the [`v8::Isolate`] parameter for all the related [`JsRealm`] methods
+/// must be extracted from the pre-existing [`JsRuntime`].
+///
+/// Example usage with the [`JsRealm::execute_script`] method:
+/// ```
+/// use deno_core::JsRuntime;
+/// use deno_core::RuntimeOptions;
+///
+/// let mut runtime = JsRuntime::new(RuntimeOptions::default());
+/// let new_realm = runtime
+///         .create_realm()
+///         .expect("Handle the error properly");
+/// let source_code = "var a = 0; a + 1";
+/// let result = new_realm
+///         .execute_script_static(runtime.v8_isolate(), "<anon>", source_code)
+///         .expect("Handle the error properly");
+/// # drop(result);
+/// ```
+///
+/// # Lifetime of the realm
+///
+/// As long as the corresponding isolate is alive, a [`JsRealm`] instance will
+/// keep the underlying V8 context alive even if it would have otherwise been
+/// garbage collected.
+#[derive(Clone)]
+pub struct JsRealm(v8::Global<v8::Context>);
+impl JsRealm {
+  pub fn new(context: v8::Global<v8::Context>) -> Self {
+    JsRealm(context)
+  }
+
+  pub fn context(&self) -> &v8::Global<v8::Context> {
+    &self.0
+  }
+
+  pub(crate) fn state(
+    &self,
+    isolate: &mut v8::Isolate,
+  ) -> Rc<RefCell<ContextState>> {
+    self
+      .context()
+      .open(isolate)
+      .get_slot::<Rc<RefCell<ContextState>>>(isolate)
+      .unwrap()
+      .clone()
+  }
+
+  pub(crate) fn state_from_scope(
+    scope: &mut v8::HandleScope,
+  ) -> Rc<RefCell<ContextState>> {
+    let context = scope.get_current_context();
+    context
+      .get_slot::<Rc<RefCell<ContextState>>>(scope)
+      .unwrap()
+      .clone()
+  }
+
+  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
+  pub fn handle_scope<'s>(
+    &self,
+    isolate: &'s mut v8::Isolate,
+  ) -> v8::HandleScope<'s> {
+    v8::HandleScope::with_context(isolate, &self.0)
+  }
+
+  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
+  pub fn global_object<'s>(
+    &self,
+    isolate: &'s mut v8::Isolate,
+  ) -> v8::Local<'s, v8::Object> {
+    let scope = &mut self.handle_scope(isolate);
+    self.0.open(scope).global(scope)
+  }
+
+  fn string_from_code<'a>(
+    scope: &mut HandleScope<'a>,
+    code: &ModuleCode,
+  ) -> Option<Local<'a, v8::String>> {
+    if let Some(code) = code.try_static_ascii() {
+      v8::String::new_external_onebyte_static(scope, code)
+    } else {
+      v8::String::new_from_utf8(
+        scope,
+        code.as_bytes(),
+        v8::NewStringType::Normal,
+      )
+    }
+  }
+
+  /// Executes traditional JavaScript code (traditional = not ES modules) in the
+  /// realm's context.
+  ///
+  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
+  ///
+  /// The `name` parameter can be a filepath or any other string. E.g.:
+  ///
+  ///   - "/some/file/path.js"
+  ///   - "<anon>"
+  ///   - "[native code]"
+  ///
+  /// The same `name` value can be used for multiple executions.
+  ///
+  /// `Error` can usually be downcast to `JsError`.
+  pub fn execute_script_static(
+    &self,
+    isolate: &mut v8::Isolate,
+    name: &'static str,
+    source_code: &'static str,
+  ) -> Result<v8::Global<v8::Value>, Error> {
+    self.execute_script(isolate, name, ModuleCode::from_static(source_code))
+  }
+
+  /// Executes traditional JavaScript code (traditional = not ES modules) in the
+  /// realm's context.
+  ///
+  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
+  ///
+  /// The `name` parameter can be a filepath or any other string. E.g.:
+  ///
+  ///   - "/some/file/path.js"
+  ///   - "<anon>"
+  ///   - "[native code]"
+  ///
+  /// The same `name` value can be used for multiple executions.
+  ///
+  /// `Error` can usually be downcast to `JsError`.
+  pub fn execute_script(
+    &self,
+    isolate: &mut v8::Isolate,
+    name: &'static str,
+    source_code: ModuleCode,
+  ) -> Result<v8::Global<v8::Value>, Error> {
+    let scope = &mut self.handle_scope(isolate);
+
+    let source = Self::string_from_code(scope, &source_code).unwrap();
+    debug_assert!(name.is_ascii());
+    let name =
+      v8::String::new_external_onebyte_static(scope, name.as_bytes()).unwrap();
+    let origin = bindings::script_origin(scope, name);
+
+    let tc_scope = &mut v8::TryCatch::new(scope);
+
+    let script = match v8::Script::compile(tc_scope, source, Some(&origin)) {
+      Some(script) => script,
+      None => {
+        let exception = tc_scope.exception().unwrap();
+        return exception_to_err_result(tc_scope, exception, false);
+      }
+    };
+
+    match script.run(tc_scope) {
+      Some(value) => {
+        let value_handle = v8::Global::new(tc_scope, value);
+        Ok(value_handle)
+      }
+      None => {
+        assert!(tc_scope.has_caught());
+        let exception = tc_scope.exception().unwrap();
+        exception_to_err_result(tc_scope, exception, false)
+      }
+    }
+  }
+
+  // TODO(andreubotella): `mod_evaluate`, `load_main_module`, `load_side_module`
+
+  pub(crate) fn check_promise_rejections(
+    &self,
+    isolate: &mut v8::Isolate,
+  ) -> Result<(), Error> {
+    let context_state_rc = self.state(isolate);
+    let mut context_state = context_state_rc.borrow_mut();
+
+    if context_state.pending_promise_rejections.is_empty() {
+      return Ok(());
+    }
+
+    let key = {
+      context_state
+        .pending_promise_rejections
+        .keys()
+        .next()
+        .unwrap()
+        .clone()
+    };
+    let handle = context_state
+      .pending_promise_rejections
+      .remove(&key)
+      .unwrap();
+    drop(context_state);
+
+    let scope = &mut self.handle_scope(isolate);
+    let exception = v8::Local::new(scope, handle);
+    exception_to_err_result(scope, exception, true)
+  }
+}

core/runtime.rs

@@ -18,6 +18,8 @@ use crate::modules::ModuleMap;
 use crate::op_void_async;
 use crate::op_void_sync;
 use crate::ops::*;
+use crate::realm::ContextState;
+use crate::realm::JsRealm;
 use crate::snapshot_util;
 use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
@@ -41,7 +43,6 @@ use smallvec::SmallVec;
 use std::any::Any;
 use std::cell::RefCell;
 use std::collections::HashMap;
-use std::collections::HashSet;
 use std::collections::VecDeque;
 use std::ffi::c_void;
 use std::option::Option;
@@ -51,8 +52,6 @@ use std::sync::Mutex;
 use std::sync::Once;
 use std::task::Context;
 use std::task::Poll;
-use v8::HandleScope;
-use v8::Local;
 use v8::OwnedIsolate;
 
 type PendingOpFuture = OpCall<(RealmIdx, PromiseId, OpId, OpResult)>;
@@ -154,21 +153,6 @@ pub type SharedArrayBufferStore =
 
 pub type CompiledWasmModuleStore = CrossIsolateStore<v8::CompiledWasmModule>;
 
-#[derive(Default)]
-pub(crate) struct ContextState {
-  js_recv_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) pending_promise_rejections:
-    HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>,
-  pub(crate) unrefed_ops: HashSet<i32>,
-  // We don't explicitly re-read this prop but need the slice to live alongside
-  // the context
-  pub(crate) op_ctxs: Box<[OpCtx]>,
-}
-
 /// Internal state for JsRuntime which is stored in one of v8::Isolate's
 /// embedder slots.
 pub struct JsRuntimeState {
@@ -514,7 +498,7 @@ impl JsRuntime {
 
     {
       let mut state = state_rc.borrow_mut();
-      state.global_realm = Some(JsRealm(global_context.clone()));
+      state.global_realm = Some(JsRealm::new(global_context.clone()));
       state.inspector = inspector;
       state
         .known_realms
@@ -587,7 +571,9 @@ impl JsRuntime {
 
   #[inline]
   pub fn global_context(&mut self) -> v8::Global<v8::Context> {
-    self.global_realm().0
+    let state = self.state.borrow();
+    let global_realm = state.global_realm.as_ref().unwrap();
+    global_realm.context().clone()
   }
 
   #[inline]
@@ -1113,10 +1099,11 @@ impl JsRuntime {
       return;
     }
 
+    let global_context = self.global_context();
     let mut state = self.state.borrow_mut();
     state.inspector = Some(JsRuntimeInspector::new(
       self.v8_isolate.as_mut().unwrap(),
-      state.global_realm.clone().unwrap().0,
+      global_context,
       self.is_main,
     ));
   }
@@ -1441,7 +1428,7 @@ impl EventLoopPendingState {
     let mut num_unrefed_ops = 0;
     for weak_context in &state.known_realms {
       if let Some(context) = weak_context.to_global(isolate) {
-        let realm = JsRealm(context);
+        let realm = JsRealm::new(context);
         num_unrefed_ops += realm.state(isolate).borrow().unrefed_ops.len();
       }
     }
@@ -2198,7 +2185,7 @@ impl JsRuntime {
     let isolate = self.v8_isolate();
     for weak_context in known_realms {
       if let Some(context) = weak_context.to_global(isolate) {
-        JsRealm(context).check_promise_rejections(isolate)?;
+        JsRealm::new(context).check_promise_rejections(isolate)?;
       }
     }
     Ok(())
@@ -2242,7 +2229,7 @@ impl JsRuntime {
         let context = self.state.borrow().known_realms[realm_idx]
           .to_global(isolate)
           .unwrap();
-        JsRealm(context)
+        JsRealm::new(context)
       };
       let context_state_rc = realm.state(isolate);
       let mut context_state = context_state_rc.borrow_mut();
@@ -2433,219 +2420,6 @@ impl JsRuntime {
   }
 }
 
-/// A representation of a JavaScript realm tied to a [`JsRuntime`], that allows
-/// execution in the realm's context.
-///
-/// A [`JsRealm`] instance is a reference to an already existing realm, which
-/// does not hold ownership of it, so instances can be created and dropped as
-/// needed. As such, calling [`JsRealm::new`] doesn't create a new realm, and
-/// cloning a [`JsRealm`] only creates a new reference. See
-/// [`JsRuntime::create_realm`] to create new realms instead.
-///
-/// Despite [`JsRealm`] instances being references, multiple instances that
-/// point to the same realm won't overlap because every operation requires
-/// passing a mutable reference to the [`v8::Isolate`]. Therefore, no operation
-/// on two [`JsRealm`] instances tied to the same isolate can be run at the same
-/// time, regardless of whether they point to the same realm.
-///
-/// # Panics
-///
-/// Every method of [`JsRealm`] will panic if you call it with a reference to a
-/// [`v8::Isolate`] other than the one that corresponds to the current context.
-///
-/// In other words, the [`v8::Isolate`] parameter for all the related [`JsRealm`] methods
-/// must be extracted from the pre-existing [`JsRuntime`].
-///
-/// Example usage with the [`JsRealm::execute_script`] method:
-/// ```
-/// use deno_core::JsRuntime;
-/// use deno_core::RuntimeOptions;
-///
-/// let mut runtime = JsRuntime::new(RuntimeOptions::default());
-/// let new_realm = runtime
-///         .create_realm()
-///         .expect("Handle the error properly");
-/// let source_code = "var a = 0; a + 1";
-/// let result = new_realm
-///         .execute_script_static(runtime.v8_isolate(), "<anon>", source_code)
-///         .expect("Handle the error properly");
-/// # drop(result);
-/// ```
-///
-/// # Lifetime of the realm
-///
-/// As long as the corresponding isolate is alive, a [`JsRealm`] instance will
-/// keep the underlying V8 context alive even if it would have otherwise been
-/// garbage collected.
-#[derive(Clone)]
-pub struct JsRealm(v8::Global<v8::Context>);
-impl JsRealm {
-  pub fn new(context: v8::Global<v8::Context>) -> Self {
-    JsRealm(context)
-  }
-
-  pub fn context(&self) -> &v8::Global<v8::Context> {
-    &self.0
-  }
-
-  fn state(&self, isolate: &mut v8::Isolate) -> Rc<RefCell<ContextState>> {
-    self
-      .context()
-      .open(isolate)
-      .get_slot::<Rc<RefCell<ContextState>>>(isolate)
-      .unwrap()
-      .clone()
-  }
-
-  pub(crate) fn state_from_scope(
-    scope: &mut v8::HandleScope,
-  ) -> Rc<RefCell<ContextState>> {
-    let context = scope.get_current_context();
-    context
-      .get_slot::<Rc<RefCell<ContextState>>>(scope)
-      .unwrap()
-      .clone()
-  }
-
-  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
-  pub fn handle_scope<'s>(
-    &self,
-    isolate: &'s mut v8::Isolate,
-  ) -> v8::HandleScope<'s> {
-    v8::HandleScope::with_context(isolate, &self.0)
-  }
-
-  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
-  pub fn global_object<'s>(
-    &self,
-    isolate: &'s mut v8::Isolate,
-  ) -> v8::Local<'s, v8::Object> {
-    let scope = &mut self.handle_scope(isolate);
-    self.0.open(scope).global(scope)
-  }
-
-  fn string_from_code<'a>(
-    scope: &mut HandleScope<'a>,
-    code: &ModuleCode,
-  ) -> Option<Local<'a, v8::String>> {
-    if let Some(code) = code.try_static_ascii() {
-      v8::String::new_external_onebyte_static(scope, code)
-    } else {
-      v8::String::new_from_utf8(
-        scope,
-        code.as_bytes(),
-        v8::NewStringType::Normal,
-      )
-    }
-  }
-
-  /// Executes traditional JavaScript code (traditional = not ES modules) in the
-  /// realm's context.
-  ///
-  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
-  ///
-  /// The `name` parameter can be a filepath or any other string. E.g.:
-  ///
-  ///   - "/some/file/path.js"
-  ///   - "<anon>"
-  ///   - "[native code]"
-  ///
-  /// The same `name` value can be used for multiple executions.
-  ///
-  /// `Error` can usually be downcast to `JsError`.
-  pub fn execute_script_static(
-    &self,
-    isolate: &mut v8::Isolate,
-    name: &'static str,
-    source_code: &'static str,
-  ) -> Result<v8::Global<v8::Value>, Error> {
-    self.execute_script(isolate, name, ModuleCode::from_static(source_code))
-  }
-
-  /// Executes traditional JavaScript code (traditional = not ES modules) in the
-  /// realm's context.
-  ///
-  /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
-  ///
-  /// The `name` parameter can be a filepath or any other string. E.g.:
-  ///
-  ///   - "/some/file/path.js"
-  ///   - "<anon>"
-  ///   - "[native code]"
-  ///
-  /// The same `name` value can be used for multiple executions.
-  ///
-  /// `Error` can usually be downcast to `JsError`.
-  pub fn execute_script(
-    &self,
-    isolate: &mut v8::Isolate,
-    name: &'static str,
-    source_code: ModuleCode,
-  ) -> Result<v8::Global<v8::Value>, Error> {
-    let scope = &mut self.handle_scope(isolate);
-
-    let source = Self::string_from_code(scope, &source_code).unwrap();
-    debug_assert!(name.is_ascii());
-    let name =
-      v8::String::new_external_onebyte_static(scope, name.as_bytes()).unwrap();
-    let origin = bindings::script_origin(scope, name);
-
-    let tc_scope = &mut v8::TryCatch::new(scope);
-
-    let script = match v8::Script::compile(tc_scope, source, Some(&origin)) {
-      Some(script) => script,
-      None => {
-        let exception = tc_scope.exception().unwrap();
-        return exception_to_err_result(tc_scope, exception, false);
-      }
-    };
-
-    match script.run(tc_scope) {
-      Some(value) => {
-        let value_handle = v8::Global::new(tc_scope, value);
-        Ok(value_handle)
-      }
-      None => {
-        assert!(tc_scope.has_caught());
-        let exception = tc_scope.exception().unwrap();
-        exception_to_err_result(tc_scope, exception, false)
-      }
-    }
-  }
-
-  // TODO(andreubotella): `mod_evaluate`, `load_main_module`, `load_side_module`
-
-  fn check_promise_rejections(
-    &self,
-    isolate: &mut v8::Isolate,
-  ) -> Result<(), Error> {
-    let context_state_rc = self.state(isolate);
-    let mut context_state = context_state_rc.borrow_mut();
-
-    if context_state.pending_promise_rejections.is_empty() {
-      return Ok(());
-    }
-
-    let key = {
-      context_state
-        .pending_promise_rejections
-        .keys()
-        .next()
-        .unwrap()
-        .clone()
-    };
-    let handle = context_state
-      .pending_promise_rejections
-      .remove(&key)
-      .unwrap();
-    drop(context_state);
-
-    let scope = &mut self.handle_scope(isolate);
-    let exception = v8::Local::new(scope, handle);
-    exception_to_err_result(scope, exception, true)
-  }
-}
-
 #[inline]
 pub fn queue_fast_async_op(
   ctx: &OpCtx,