Resolve callback moved from Behavior to mod_instantiate() (#1999)

This simplifies the Behavior trait and makes it more explicit where the
resolve callback is being made.

Also s/StartupScript/Script

cli/cli_behavior.rs

@@ -2,7 +2,6 @@
 use crate::isolate_state::*;
 use crate::ops;
 use deno_core::deno_buf;
-use deno_core::deno_mod;
 use deno_core::Behavior;
 use deno_core::Op;
 use deno_core::StartupData;
@@ -43,10 +42,6 @@ impl Behavior for CliBehavior {
     self.startup_data.take()
   }
 
-  fn resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod {
-    self.state_resolve(specifier, referrer)
-  }
-
   fn dispatch(
     &mut self,
     control: &[u8],

cli/compiler.rs

@@ -9,7 +9,6 @@ use crate::startup_data;
 use crate::workers;
 use crate::workers::WorkerBehavior;
 use deno_core::deno_buf;
-use deno_core::deno_mod;
 use deno_core::Behavior;
 use deno_core::Buf;
 use deno_core::Op;
@@ -51,10 +50,6 @@ impl Behavior for CompilerBehavior {
     Some(startup_data::compiler_isolate_init())
   }
 
-  fn resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod {
-    self.state_resolve(specifier, referrer)
-  }
-
   fn dispatch(
     &mut self,
     control: &[u8],

cli/isolate.rs

@@ -14,6 +14,7 @@ use deno_core::Behavior;
 use deno_core::JSError;
 use futures::Async;
 use futures::Future;
+use std::sync::atomic::Ordering;
 use std::sync::Arc;
 
 pub trait DenoBehavior: Behavior + IsolateStateContainer + Send {}
@@ -122,9 +123,17 @@ impl<B: DenoBehavior> Isolate<B> {
 
     self.mod_load_deps(id)?;
 
+    let state = self.state.clone();
+
+    let mut resolve = move |specifier: &str, referrer: deno_mod| -> deno_mod {
+      state.metrics.resolve_count.fetch_add(1, Ordering::Relaxed);
+      let mut modules = state.modules.lock().unwrap();
+      modules.resolve_cb(&state.dir, specifier, referrer)
+    };
+
     self
       .inner
-      .mod_instantiate(id)
+      .mod_instantiate(id, &mut resolve)
       .map_err(RustOrJsError::from)?;
     if !is_prefetch {
       self.inner.mod_evaluate(id).map_err(RustOrJsError::from)?;

cli/isolate_state.rs

@@ -5,7 +5,6 @@ use crate::flags;
 use crate::global_timer::GlobalTimer;
 use crate::modules::Modules;
 use crate::permissions::DenoPermissions;
-use deno_core::deno_mod;
 use deno_core::Buf;
 use futures::sync::mpsc as async_mpsc;
 use std;
@@ -144,16 +143,3 @@ impl IsolateState {
 pub trait IsolateStateContainer {
   fn state(&self) -> Arc<IsolateState>;
 }
-
-/// Provides state_resolve function for IsolateStateContainer implementors
-pub trait IsolateStateModuleResolution: IsolateStateContainer {
-  fn state_resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod {
-    let state = self.state();
-    state.metrics.resolve_count.fetch_add(1, Ordering::Relaxed);
-    let mut modules = state.modules.lock().unwrap();
-    modules.resolve_cb(&state.dir, specifier, referrer)
-  }
-}
-
-// Auto implementation for all IsolateStateContainer implementors
-impl<T> IsolateStateModuleResolution for T where T: IsolateStateContainer {}

cli/startup_data.rs

@@ -1,6 +1,6 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
 use deno_core::deno_buf;
-use deno_core::{StartupData, StartupScript};
+use deno_core::{Script, StartupData};
 
 pub fn deno_isolate_init() -> StartupData {
   if cfg!(feature = "no-snapshot-init") {
@@ -11,7 +11,7 @@ pub fn deno_isolate_init() -> StartupData {
     #[cfg(feature = "check-only")]
     let source_bytes = vec![];
 
-    StartupData::Script(StartupScript {
+    StartupData::Script(Script {
       filename: "gen/bundle/main.js".to_string(),
       source: std::str::from_utf8(source_bytes).unwrap().to_string(),
     })
@@ -38,7 +38,7 @@ pub fn compiler_isolate_init() -> StartupData {
     #[cfg(feature = "check-only")]
     let source_bytes = vec![];
 
-    StartupData::Script(StartupScript {
+    StartupData::Script(Script {
       filename: "gen/bundle/compiler.js".to_string(),
       source: std::str::from_utf8(source_bytes).unwrap().to_string(),
     })

core/http_bench.rs

@@ -102,17 +102,12 @@ impl Behavior for HttpBench {
   fn startup_data(&mut self) -> Option<StartupData> {
     let js_source = include_str!("http_bench.js");
 
-    Some(StartupData::Script(StartupScript {
+    Some(StartupData::Script(Script {
       source: js_source.to_string(),
       filename: "http_bench.js".to_string(),
     }))
   }
 
-  fn resolve(&mut self, _specifier: &str, _referrer: deno_mod) -> deno_mod {
-    // HttpBench doesn't do ES modules.
-    unimplemented!()
-  }
-
   fn dispatch(
     &mut self,
     control: &[u8],

core/isolate.rs

@@ -43,7 +43,7 @@ impl Future for PendingOp {
 }
 
 /// Stores a script used to initalize a Isolate
-pub struct StartupScript {
+pub struct Script {
   pub source: String,
   pub filename: String,
 }
@@ -52,7 +52,7 @@ pub struct StartupScript {
 /// either a binary snapshot or a javascript source file
 /// in the form of the StartupScript struct.
 pub enum StartupData {
-  Script(StartupScript),
+  Script(Script),
   Snapshot(deno_buf),
 }
 
@@ -63,9 +63,6 @@ pub trait Behavior {
   /// Isolate is created.
   fn startup_data(&mut self) -> Option<StartupData>;
 
-  /// Called during mod_instantiate() to resolve imports.
-  fn resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod;
-
   /// Called whenever libdeno.send() is called in JavaScript. zero_copy_buf
   /// corresponds to the second argument of libdeno.send().
   fn dispatch(
@@ -329,27 +326,46 @@ impl<B: Behavior> Isolate<B> {
     }
     out
   }
+}
 
-  pub fn mod_instantiate(&self, id: deno_mod) -> Result<(), JSError> {
+/// Called during mod_instantiate() to resolve imports.
+type ResolveFn = dyn FnMut(&str, deno_mod) -> deno_mod;
+
+/// Used internally by Isolate::mod_instantiate to wrap ResolveFn and
+/// encapsulate pointer casts.
+struct ResolveContext<'a> {
+  resolve_fn: &'a mut ResolveFn,
+}
+
+impl<'a> ResolveContext<'a> {
+  #[inline]
+  fn as_raw_ptr(&mut self) -> *mut c_void {
+    self as *mut _ as *mut c_void
+  }
+
+  #[inline]
+  unsafe fn from_raw_ptr(ptr: *mut c_void) -> &'a mut Self {
+    &mut *(ptr as *mut _)
+  }
+}
+
+impl<B: Behavior> Isolate<B> {
+  pub fn mod_instantiate(
+    &mut self,
+    id: deno_mod,
+    resolve_fn: &mut ResolveFn,
+  ) -> Result<(), JSError> {
+    let libdeno_isolate = self.libdeno_isolate;
+    let mut ctx = ResolveContext { resolve_fn };
     unsafe {
       libdeno::deno_mod_instantiate(
-        self.libdeno_isolate,
+        libdeno_isolate,
-        self.as_raw_ptr(),
+        ctx.as_raw_ptr(),
         id,
         Self::resolve_cb,
       )
     };
-    if let Some(js_error) = self.last_exception() {
-      return Err(js_error);
-    }
-    Ok(())
-  }
 
-  pub fn mod_evaluate(&mut self, id: deno_mod) -> Result<(), JSError> {
-    self.shared_init();
-    unsafe {
-      libdeno::deno_mod_evaluate(self.libdeno_isolate, self.as_raw_ptr(), id)
-    };
     if let Some(js_error) = self.last_exception() {
       return Err(js_error);
     }
@@ -362,10 +378,23 @@ impl<B: Behavior> Isolate<B> {
     specifier_ptr: *const libc::c_char,
     referrer: deno_mod,
   ) -> deno_mod {
-    let isolate = unsafe { Isolate::<B>::from_raw_ptr(user_data) };
+    let ResolveContext { resolve_fn } =
+      unsafe { ResolveContext::from_raw_ptr(user_data) };
     let specifier_c: &CStr = unsafe { CStr::from_ptr(specifier_ptr) };
     let specifier: &str = specifier_c.to_str().unwrap();
-    isolate.behavior.resolve(specifier, referrer)
+
+    resolve_fn(specifier, referrer)
+  }
+
+  pub fn mod_evaluate(&mut self, id: deno_mod) -> Result<(), JSError> {
+    self.shared_init();
+    unsafe {
+      libdeno::deno_mod_evaluate(self.libdeno_isolate, self.as_raw_ptr(), id)
+    };
+    if let Some(js_error) = self.last_exception() {
+      return Err(js_error);
+    }
+    Ok(())
   }
 }
 
@@ -498,7 +527,7 @@ pub fn js_check(r: Result<(), JSError>) {
 #[cfg(test)]
 mod tests {
   use super::*;
-  use std::collections::HashMap;
+  use std::sync::atomic::{AtomicUsize, Ordering};
 
   pub enum TestBehaviorMode {
     AsyncImmediate,
@@ -510,8 +539,6 @@ mod tests {
 
   pub struct TestBehavior {
     pub dispatch_count: usize,
-    pub resolve_count: usize,
-    pub mod_map: HashMap<String, deno_mod>,
     mode: TestBehaviorMode,
   }
 
@@ -519,9 +546,7 @@ mod tests {
     pub fn setup(mode: TestBehaviorMode) -> Isolate<Self> {
       let mut isolate = Isolate::new(TestBehavior {
         dispatch_count: 0,
-        resolve_count: 0,
         mode,
-        mod_map: HashMap::new(),
       });
       js_check(isolate.execute(
         "setup.js",
@@ -536,10 +561,6 @@ mod tests {
       assert_eq!(isolate.behavior.dispatch_count, 0);
       isolate
     }
-
-    pub fn register(&mut self, name: &str, id: deno_mod) {
-      self.mod_map.insert(name.to_string(), id);
-    }
   }
 
   impl Behavior for TestBehavior {
@@ -547,14 +568,6 @@ mod tests {
       None
     }
 
-    fn resolve(&mut self, specifier: &str, _referrer: deno_mod) -> deno_mod {
-      self.resolve_count += 1;
-      match self.mod_map.get(specifier) {
-        Some(id) => *id,
-        None => 0,
-      }
-    }
-
     fn dispatch(
       &mut self,
       control: &[u8],
@@ -632,7 +645,6 @@ mod tests {
       "#,
       ).unwrap();
     assert_eq!(isolate.behavior.dispatch_count, 0);
-    assert_eq!(isolate.behavior.resolve_count, 0);
 
     let imports = isolate.mod_get_imports(mod_a);
     assert_eq!(imports, vec!["b.js".to_string()]);
@@ -643,18 +655,26 @@ mod tests {
     let imports = isolate.mod_get_imports(mod_b);
     assert_eq!(imports.len(), 0);
 
-    js_check(isolate.mod_instantiate(mod_b));
+    let resolve_count = Arc::new(AtomicUsize::new(0));
-    assert_eq!(isolate.behavior.dispatch_count, 0);
+    let resolve_count_ = resolve_count.clone();
-    assert_eq!(isolate.behavior.resolve_count, 0);
 
-    isolate.behavior.register("b.js", mod_b);
+    let mut resolve = move |specifier: &str, _referrer: deno_mod| -> deno_mod {
-    js_check(isolate.mod_instantiate(mod_a));
+      resolve_count_.fetch_add(1, Ordering::SeqCst);
+      assert_eq!(specifier, "b.js");
+      mod_b
+    };
+
+    js_check(isolate.mod_instantiate(mod_b, &mut resolve));
     assert_eq!(isolate.behavior.dispatch_count, 0);
-    assert_eq!(isolate.behavior.resolve_count, 1);
+    assert_eq!(resolve_count.load(Ordering::SeqCst), 0);
+
+    js_check(isolate.mod_instantiate(mod_a, &mut resolve));
+    assert_eq!(isolate.behavior.dispatch_count, 0);
+    assert_eq!(resolve_count.load(Ordering::SeqCst), 1);
 
     js_check(isolate.mod_evaluate(mod_a));
     assert_eq!(isolate.behavior.dispatch_count, 1);
-    assert_eq!(isolate.behavior.resolve_count, 1);
+    assert_eq!(resolve_count.load(Ordering::SeqCst), 1);
   }
 
   #[test]