Repository: denoland/deno_core
Pull request #4 (2023-07-02 00:16:21)

chore: Fully adopt the "main realm/context" terminology
It used to be that the realm/context created with the isolate was sometimes called "global context" and sometimes "main context/realm". This goes back to the days before `JsRealm`, when the `JsRuntime::global_context` method returned a `v8::Global` for the only `v8::Context` that existed in the isolate. Nowadays that is no longer the case, and we should call it the main realm.

That change by itself would also rename some properties/variables from `is_global` to `is_main`, which is also used elsewhere to refer to the top-level `JsRuntime`/isolate in a process. Therefore, the names are changed to `is_main_runtime` and `is_main_realm` to disambiguate.


CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/denoland/deno_core?pullRequest=4) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/denoland/deno_core?pullRequest=4) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/denoland/deno_core?pullRequest=4) it.</sub>
andreubotella: Since most of this PR's diff is taken up by updating uses of changed APIs, I thought it would help review if I listed all of the actual changes:

`deno_core` API breaking changes:

- `JsRuntime::global_context` is renamed to `main_context`.
- `JsRuntime::global_realm` is renamed to `main_realm`.

Private API changes:

- `JsRuntime::is_main` is renamed to `is_main_runtime`.
- `JsRuntimeState::global_realm` is renamed to `main_realm`.
- `JsRealmInner::is_global` is renamed to `is_main_realm`.

Variable renames:

- The third argument of `JsRuntimeInspector::new`, which is currently `is_main`, is renamed to `is_main_runtime`. (Also a public documentation change.)
- The `global_context` and `global_realm` variables in `JsRuntime::new_inner` are renamed to `main_context` and `main_realm`, respectively.
- The `global_realm` variables in `JsRuntime::dynamic_import_module_evaluate` and `JsRuntime::mod_evalute` are renamed to `main_realm`.
- The `global_realm` variables in the `test_set_promise_reject_callback_realms` and `js_realm_async_ops` tests (both in `core/runtime/tests/jsrealm.rs`) are renamed to `main_realm`.

Other changes:

- The main realm is shown in the inspector as `main realm`, rather than `global context`. This change is user-facing, but probably not breaking or too important.
- The documentation of `JsRuntime::execute_script` and `JsRuntime::execute_script_static` is updated to replace one reference to "global context" with "main realm", in a context where the difference between context and realm doesn't matter.
- The string used in the `test_set_promise_reject_callback_realms` test to test the main realm is changed from `"global_realm"` to `"main_realm"`.

Patch:
diff --git a/core/inspector.rs b/core/inspector.rs
index bd1080a..eaedd31 100644
--- a/core/inspector.rs
+++ b/core/inspector.rs
@@ -149,7 +149,7 @@ impl JsRuntimeInspector {
   pub fn new(
     scope: &mut v8::HandleScope,
     context: v8::Local<v8::Context>,
-    is_main: bool,
+    is_main_runtime: bool,
   ) -> Rc<RefCell<Self>> {
     let (new_session_tx, new_session_rx) =
       mpsc::unbounded::<InspectorSessionProxy>();
@@ -179,12 +179,12 @@ impl JsRuntimeInspector {
       new_session_rx,
     ));
 
-    // Tell the inspector about the global context.
-    let context_name = v8::inspector::StringView::from(&b"global context"[..]);
+    // Tell the inspector about the main realm.
+    let context_name = v8::inspector::StringView::from(&b"main realm"[..]);
     // NOTE(bartlomieju): this is what Node.js does and it turns out some
     // debuggers (like VSCode) rely on this information to disconnect after
     // program completes
-    let aux_data = if is_main {
+    let aux_data = if is_main_runtime {
       r#"{"isDefault": true}"#
     } else {
       r#"{"isDefault": false}"#
diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs
index 72818ee..496a113 100644
--- a/core/runtime/jsrealm.rs
+++ b/core/runtime/jsrealm.rs
@@ -109,7 +109,7 @@ pub(crate) struct JsRealmInner {
   context_state: Rc<RefCell<ContextState>>,
   context: Rc<v8::Global<v8::Context>>,
   runtime_state: Rc<RefCell<JsRuntimeState>>,
-  is_global: bool,
+  is_main_realm: bool,
 }
 
 impl JsRealmInner {
@@ -117,13 +117,13 @@ impl JsRealmInner {
     context_state: Rc<RefCell<ContextState>>,
     context: v8::Global<v8::Context>,
     runtime_state: Rc<RefCell<JsRuntimeState>>,
-    is_global: bool,
+    is_main_realm: bool,
   ) -> Self {
     Self {
       context_state,
       context: context.into(),
       runtime_state,
-      is_global,
+      is_main_realm,
     }
   }
 
@@ -345,8 +345,8 @@ impl JsRealm {
 
 impl Drop for JsRealm {
   fn drop(&mut self) {
-    // Don't do anything special with the global realm
-    if self.0.is_global {
+    // Don't do anything special with the main realm
+    if self.0.is_main_realm {
       return;
     }
 
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index b9428cb..41c917f 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -228,7 +228,7 @@ pub struct JsRuntime {
   global_object_middlewares: Vec<Box<GlobalObjectMiddlewareFn>>,
   init_mode: InitMode,
   // Marks if this is considered the top-level runtime. Used only by inspector.
-  is_main: bool,
+  is_main_runtime: bool,
 }
 
 /// The runtime type used for snapshot creation.
@@ -307,7 +307,7 @@ pub type CompiledWasmModuleStore = CrossIsolateStore<v8::CompiledWasmModule>;
 /// Internal state for JsRuntime which is stored in one of v8::Isolate's
 /// embedder slots.
 pub struct JsRuntimeState {
-  global_realm: Option<JsRealm>,
+  main_realm: Option<JsRealm>,
   known_realms: Vec<JsRealmInner>,
   pub(crate) has_tick_scheduled: bool,
   pub(crate) pending_dyn_mod_evaluate: Vec<DynImportModEvaluate>,
@@ -331,7 +331,7 @@ pub struct JsRuntimeState {
 
 impl JsRuntimeState {
   pub(crate) fn destroy_all_realms(&mut self) {
-    self.global_realm.take();
+    self.main_realm.take();
     for realm in self.known_realms.drain(..) {
       realm.destroy()
     }
@@ -580,7 +580,7 @@ impl JsRuntime {
       dispatched_exception: None,
       // Some fields are initialized later after isolate is created
       inspector: None,
-      global_realm: None,
+      main_realm: None,
       known_realms: Vec::with_capacity(1),
     }));
 
@@ -646,7 +646,7 @@ impl JsRuntime {
       bindings::wasm_async_resolve_promise_callback,
     );
 
-    let (global_context, snapshotted_data) = {
+    let (main_context, snapshotted_data) = {
       let scope = &mut v8::HandleScope::new(&mut isolate);
 
       let context = create_context(
@@ -673,9 +673,9 @@ impl JsRuntime {
     };
 
     let mut context_scope: v8::HandleScope =
-      v8::HandleScope::with_context(&mut isolate, global_context.clone());
+      v8::HandleScope::with_context(&mut isolate, main_context.clone());
     let scope = &mut context_scope;
-    let context = v8::Local::new(scope, global_context.clone());
+    let context = v8::Local::new(scope, main_context.clone());
 
     bindings::initialize_context(
       scope,
@@ -698,16 +698,12 @@ impl JsRuntime {
       .unwrap_or_else(|| Rc::new(NoopModuleLoader));
 
     {
-      let global_realm = JsRealmInner::new(
-        context_state,
-        global_context,
-        state_rc.clone(),
-        true,
-      );
+      let main_realm =
+        JsRealmInner::new(context_state, main_context, state_rc.clone(), true);
       let mut state = state_rc.borrow_mut();
-      state.global_realm = Some(JsRealm::new(global_realm.clone()));
+      state.main_realm = Some(JsRealm::new(main_realm.clone()));
       state.inspector = inspector;
-      state.known_realms.push(global_realm);
+      state.known_realms.push(main_realm);
     }
     scope.set_data(
       STATE_DATA_OFFSET,
@@ -738,10 +734,10 @@ impl JsRuntime {
       global_object_middlewares,
       extensions: options.extensions,
       module_map: module_map_rc,
-      is_main: options.is_main,
+      is_main_runtime: options.is_main,
     };
 
-    let realm = js_runtime.global_realm();
+    let realm = js_runtime.main_realm();
     // TODO(mmastrac): We should thread errors back out of the runtime
     js_runtime
       .init_extension_js(&realm, maybe_load_callback)
@@ -767,7 +763,7 @@ impl JsRuntime {
   }
 
   #[inline]
-  pub fn global_context(&self) -> v8::Global<v8::Context> {
+  pub fn main_context(&self) -> v8::Global<v8::Context> {
     self
       .inner
       .state
@@ -790,9 +786,9 @@ impl JsRuntime {
   }
 
   #[inline]
-  pub fn global_realm(&mut self) -> JsRealm {
+  pub fn main_realm(&mut self) -> JsRealm {
     let state = self.inner.state.borrow();
-    state.global_realm.clone().unwrap()
+    state.main_realm.clone().unwrap()
   }
 
   /// Returns the extensions that this runtime is using (including internal ones).
@@ -808,7 +804,7 @@ impl JsRuntime {
     let realm = {
       let context_state = Rc::new(RefCell::new(ContextState::default()));
       let op_ctxs: Box<[OpCtx]> = self
-        .global_realm()
+        .main_realm()
         .0
         .state()
         .borrow()
@@ -868,7 +864,7 @@ impl JsRuntime {
 
   #[inline]
   pub fn handle_scope(&mut self) -> v8::HandleScope {
-    self.global_realm().handle_scope(self.v8_isolate())
+    self.main_realm().handle_scope(self.v8_isolate())
   }
 
   /// Initializes JS of provided Extensions in the given realm.
@@ -1129,7 +1125,7 @@ impl JsRuntime {
 
   /// Executes traditional JavaScript code (traditional = not ES modules).
   ///
-  /// The execution takes place on the current global context, so it is possible
+  /// The execution takes place on the current main realm, so it is possible
   /// to maintain local JS state and invoke this method multiple times.
   ///
   /// `name` can be a filepath or any other string, but it is required to be 7-bit ASCII, eg.
@@ -1147,13 +1143,13 @@ impl JsRuntime {
     source_code: ModuleCode,
   ) -> Result<v8::Global<v8::Value>, Error> {
     self
-      .global_realm()
+      .main_realm()
       .execute_script(self.v8_isolate(), name, source_code)
   }
 
   /// Executes traditional JavaScript code (traditional = not ES modules).
   ///
-  /// The execution takes place on the current global context, so it is possible
+  /// The execution takes place on the current main realm, so it is possible
   /// to maintain local JS state and invoke this method multiple times.
   ///
   /// `name` can be a filepath or any other string, but it is required to be 7-bit ASCII, eg.
@@ -1170,7 +1166,7 @@ impl JsRuntime {
     name: &'static str,
     source_code: &'static str,
   ) -> Result<v8::Global<v8::Value>, Error> {
-    self.global_realm().execute_script(
+    self.main_realm().execute_script(
       self.v8_isolate(),
       name,
       ModuleCode::from_static(source_code),
@@ -1272,7 +1268,7 @@ impl JsRuntime {
       return;
     }
 
-    let context = self.global_context();
+    let context = self.main_context();
     let scope = &mut v8::HandleScope::with_context(
       self.inner.v8_isolate.as_mut(),
       context.clone(),
@@ -1280,8 +1276,11 @@ impl JsRuntime {
     let context = v8::Local::new(scope, context);
 
     let mut state = self.inner.state.borrow_mut();
-    state.inspector =
-      Some(JsRuntimeInspector::new(scope, context, self.is_main));
+    state.inspector = Some(JsRuntimeInspector::new(
+      scope,
+      context,
+      self.is_main_runtime,
+    ));
   }
 
   pub fn poll_value(
@@ -1428,7 +1427,7 @@ impl JsRuntime {
           // debugger that the program has finished running and we're ready
           // to exit the process once debugger disconnects.
           if !has_blocking_sessions {
-            let context = self.global_context();
+            let context = self.main_context();
             let scope = &mut self.handle_scope();
             inspector.borrow_mut().context_destroyed(scope, context);
             println!("Program finished. Waiting for inspector to disconnect to exit the process...");
@@ -1573,7 +1572,7 @@ impl JsRuntimeForSnapshot {
 
     // Set the context to be snapshot's default context
     {
-      let context = self.global_context();
+      let context = self.main_context();
       let mut scope = self.handle_scope();
       let local_context = v8::Local::new(&mut scope, context);
       scope.set_default_context(local_context);
@@ -1590,7 +1589,7 @@ impl JsRuntimeForSnapshot {
         module_map.serialize_for_snapshotting(&mut self.handle_scope())
       };
 
-      let context = self.global_context();
+      let context = self.main_context();
       let mut scope = self.handle_scope();
       snapshot_util::set_snapshotted_data(
         &mut scope,
@@ -1725,9 +1724,8 @@ impl JsRuntime {
     // For more details see:
     // https://github.com/denoland/deno/issues/4908
     // https://v8.dev/features/top-level-await#module-execution-order
-    let global_realm =
-      self.inner.state.borrow_mut().global_realm.clone().unwrap();
-    let scope = &mut global_realm.handle_scope(&mut self.inner.v8_isolate);
+    let main_realm = self.inner.state.borrow_mut().main_realm.clone().unwrap();
+    let scope = &mut main_realm.handle_scope(&mut self.inner.v8_isolate);
     let tc_scope = &mut v8::TryCatch::new(scope);
     let module = v8::Local::new(tc_scope, &module_handle);
     let maybe_value = module.evaluate(tc_scope);
@@ -1786,7 +1784,7 @@ impl JsRuntime {
     &mut self,
     id: ModuleId,
   ) -> oneshot::Receiver<Result<(), Error>> {
-    let global_realm = self.global_realm();
+    let main_realm = self.main_realm();
     let state_rc = self.inner.state.clone();
     let module_map_rc = self.module_map.clone();
     let scope = &mut self.handle_scope();
@@ -1886,7 +1884,7 @@ impl JsRuntime {
           .handled_promise_rejections
           .contains(&promise_global);
         if !pending_rejection_was_already_handled {
-          global_realm
+          main_realm
             .0
             .state()
             .borrow_mut()


Test patch:
diff --git a/core/runtime/tests/jsrealm.rs b/core/runtime/tests/jsrealm.rs
index e09373d..8e9d8c5 100644
--- a/core/runtime/tests/jsrealm.rs
+++ b/core/runtime/tests/jsrealm.rs
@@ -16,12 +16,12 @@ use std::task::Poll;
 #[tokio::test]
 async fn test_set_promise_reject_callback_realms() {
   let mut runtime = JsRuntime::new(RuntimeOptions::default());
-  let global_realm = runtime.global_realm();
+  let main_realm = runtime.main_realm();
   let realm1 = runtime.create_realm().unwrap();
   let realm2 = runtime.create_realm().unwrap();
 
   let realm_expectations = &[
-    (&global_realm, "global_realm", 42),
+    (&main_realm, "main_realm", 42),
     (&realm1, "realm1", 140),
     (&realm2, "realm2", 720),
   ];
@@ -63,7 +63,7 @@ async fn test_set_promise_reject_callback_realms() {
 #[test]
 fn js_realm_simple() {
   let mut runtime = JsRuntime::new(Default::default());
-  let main_context = runtime.global_context();
+  let main_context = runtime.main_context();
   let main_global = {
     let scope = &mut runtime.handle_scope();
     let local_global = main_context.open(scope).global(scope);
@@ -158,7 +158,7 @@ fn js_realm_sync_ops() {
   let new_realm = runtime.create_realm().unwrap();
 
   // Test in both realms
-  for realm in [runtime.global_realm(), new_realm].into_iter() {
+  for realm in [runtime.main_realm(), new_realm].into_iter() {
     let ret = realm
       .execute_script_static(
         runtime.v8_isolate(),
@@ -204,13 +204,13 @@ async fn js_realm_async_ops() {
     ..Default::default()
   });
 
-  let global_realm = runtime.global_realm();
+  let main_realm = runtime.main_realm();
   let new_realm = runtime.create_realm().unwrap();
 
   let mut rets = vec![];
 
   // Test in both realms
-  for realm in [global_realm, new_realm].into_iter() {
+  for realm in [main_realm, new_realm].into_iter() {
     let ret = realm
       .execute_script_static(
         runtime.v8_isolate(),
@@ -326,7 +326,7 @@ async fn js_realm_ref_unref_ops() {
   });
 
   poll_fn(move |cx| {
-    let main_realm = runtime.global_realm();
+    let main_realm = runtime.main_realm();
     let other_realm = runtime.create_realm().unwrap();
 
     main_realm
diff --git a/core/runtime/tests/ops.rs b/core/runtime/tests/ops.rs
index 3388f54..9ffe1b4 100644
--- a/core/runtime/tests/ops.rs
+++ b/core/runtime/tests/ops.rs
@@ -382,7 +382,7 @@ async fn test_ref_unref_ops() {
     )
     .unwrap();
   {
-    let realm = runtime.global_realm();
+    let realm = runtime.main_realm();
     assert_eq!(realm.num_pending_ops(), 2);
     assert_eq!(realm.num_unrefed_ops(), 0);
   }
@@ -396,7 +396,7 @@ async fn test_ref_unref_ops() {
     )
     .unwrap();
   {
-    let realm = runtime.global_realm();
+    let realm = runtime.main_realm();
     assert_eq!(realm.num_pending_ops(), 2);
     assert_eq!(realm.num_unrefed_ops(), 2);
   }
@@ -410,7 +410,7 @@ async fn test_ref_unref_ops() {
     )
     .unwrap();
   {
-    let realm = runtime.global_realm();
+    let realm = runtime.main_realm();
     assert_eq!(realm.num_pending_ops(), 2);
     assert_eq!(realm.num_unrefed_ops(), 0);
   }