review fixes and other misc fixes

* Rename `allow_native`/`allow-native` to `allow_plugin`/`allow-plugin`. This makes more sense with the primary public api function being `openPlugin`.
* Fully intagrate the newly added permission type `plugin`.
* Add docs to `Deno.openPlugin`.
* Generate op names for native plugin ops that avoid potential collision.

Lastly this commit adds a delayed future to native plugins async test op. This should properly test the function of contexts/tasks/wakers in async native plugin ops. This would not function correctly with futures 0.1, and thus was a major blocker. This test proves this issue no longer exists in this implementation. The most analogous problem I can think of here is loading two copies of the same TS lib: the share the same type thus are interoperable, but don't share the same module local data(closest comparison to TLS used to store current task in futures). This isn't a problem anymore, since futures now pass `Context` values directly during calls to `poll` with the new API.

cli/flags.rs

@@ -48,7 +48,7 @@ pub struct DenoFlags {
   pub net_whitelist: Vec<String>,
   pub allow_env: bool,
   pub allow_run: bool,
-  pub allow_native: bool,
+  pub allow_plugin: bool,
   pub allow_hrtime: bool,
   pub no_prompts: bool,
   pub no_fetch: bool,
@@ -107,8 +107,8 @@ fn add_run_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
         .help("Allow running subprocesses"),
     )
     .arg(
-      Arg::with_name("allow-native")
-        .long("allow-native")
+      Arg::with_name("allow-plugin")
+        .long("allow-plugin")
         .help("Allow opening native plugins"),
     )
     .arg(
@@ -849,8 +849,8 @@ fn parse_run_args(mut flags: DenoFlags, matches: &ArgMatches) -> DenoFlags {
   if matches.is_present("allow-run") {
     flags.allow_run = true;
   }
-  if matches.is_present("allow-native") {
-    flags.allow_native = true;
+  if matches.is_present("allow-plugin") {
+    flags.allow_plugin = true;
   }
   if matches.is_present("allow-hrtime") {
     flags.allow_hrtime = true;
@@ -862,7 +862,7 @@ fn parse_run_args(mut flags: DenoFlags, matches: &ArgMatches) -> DenoFlags {
     flags.allow_run = true;
     flags.allow_read = true;
     flags.allow_write = true;
-    flags.allow_native = true;
+    flags.allow_plugin = true;
     flags.allow_hrtime = true;
   }
   if matches.is_present("no-fetch") {
@@ -980,7 +980,7 @@ pub fn flags_from_vec(
       flags.allow_run = true;
       flags.allow_read = true;
       flags.allow_write = true;
-      flags.allow_native = true;
+      flags.allow_plugin = true;
       flags.allow_hrtime = true;
       let code: &str = eval_match.value_of("code").unwrap();
       argv.extend(vec![code.to_string()]);
@@ -1149,7 +1149,7 @@ pub fn flags_from_vec(
       flags.allow_run = true;
       flags.allow_read = true;
       flags.allow_write = true;
-      flags.allow_native = true;
+      flags.allow_plugin = true;
       flags.allow_hrtime = true;
       argv.push(XEVAL_URL.to_string());
 
@@ -1193,7 +1193,7 @@ pub fn flags_from_vec(
       flags.allow_run = true;
       flags.allow_read = true;
       flags.allow_write = true;
-      flags.allow_native = true;
+      flags.allow_plugin = true;
       flags.allow_hrtime = true;
       DenoSubcommand::Repl
     }
@@ -1353,7 +1353,7 @@ mod tests {
         allow_run: true,
         allow_read: true,
         allow_write: true,
-        allow_native: true,
+        allow_plugin: true,
         allow_hrtime: true,
         ..DenoFlags::default()
       }
@@ -1504,7 +1504,7 @@ mod tests {
         allow_run: true,
         allow_read: true,
         allow_write: true,
-        allow_native: true,
+        allow_plugin: true,
         allow_hrtime: true,
         ..DenoFlags::default()
       }
@@ -1524,7 +1524,7 @@ mod tests {
         allow_run: true,
         allow_read: true,
         allow_write: true,
-        allow_native: true,
+        allow_plugin: true,
         allow_hrtime: true,
         ..DenoFlags::default()
       }
@@ -1552,7 +1552,7 @@ mod tests {
         allow_run: true,
         allow_read: true,
         allow_write: true,
-        allow_native: true,
+        allow_plugin: true,
         allow_hrtime: true,
         ..DenoFlags::default()
       }

cli/js/lib.deno_runtime.d.ts

@@ -892,6 +892,7 @@ declare namespace Deno {
     | "write"
     | "net"
     | "env"
+    | "plugin"
     | "hrtime";
   /** https://w3c.github.io/permissions/#status-of-a-permission */
   export type PermissionState = "granted" | "denied" | "prompt";
@@ -909,6 +910,9 @@ declare namespace Deno {
   interface EnvPermissionDescriptor {
     name: "env";
   }
+  interface PluginPermissionDescriptor {
+    name: "plugin";
+  }
   interface HrtimePermissionDescriptor {
     name: "hrtime";
   }
@@ -918,6 +922,7 @@ declare namespace Deno {
     | ReadWritePermissionDescriptor
     | NetPermissionDescriptor
     | EnvPermissionDescriptor
+    | PluginPermissionDescriptor
     | HrtimePermissionDescriptor;
 
   export class Permissions {
@@ -987,6 +992,14 @@ declare namespace Deno {
     };
   }
 
+  /** Open and initalize a native plugin.
+   * Requires the `--allow-plugin` flag.
+   *
+   *        const plugin = Deno.openPlugin("./path/to/some/plugin.so");
+   *        const some_op = plugin.ops.some_op;
+   *        const response = some_op.dispatch(new Uint8Array([1,2,3,4]));
+   *        console.log(`Response from native plugin ${response}`);
+   */
   export function openPlugin(filename: string): NativePlugin;
 
   // @url js/net.d.ts

cli/js/permissions.ts

@@ -11,6 +11,7 @@ export type PermissionName =
   | "net"
   | "env"
   | "run"
+  | "plugin"
   | "hrtime";
 // NOTE: Keep in sync with cli/permissions.rs
 
@@ -31,6 +32,9 @@ interface NetPermissionDescriptor {
 interface EnvPermissionDescriptor {
   name: "env";
 }
+interface PluginPermissionDescriptor {
+  name: "plugin";
+}
 interface HrtimePermissionDescriptor {
   name: "hrtime";
 }
@@ -40,6 +44,7 @@ type PermissionDescriptor =
   | ReadWritePermissionDescriptor
   | NetPermissionDescriptor
   | EnvPermissionDescriptor
+  | PluginPermissionDescriptor
   | HrtimePermissionDescriptor;
 
 /** https://w3c.github.io/permissions/#permissionstatus */

cli/js/permissions_test.ts

@@ -7,19 +7,27 @@ const knownPermissions: Deno.PermissionName[] = [
   "write",
   "net",
   "env",
+  "plugin",
   "hrtime"
 ];
 
-for (const grant of knownPermissions) {
-  testPerm({ [grant]: true }, async function envGranted(): Promise<void> {
+function genFunc(grant: Deno.PermissionName): () => Promise<void> {
+  const gen: () => Promise<void> = async function Granted(): Promise<void> {
     const status0 = await Deno.permissions.query({ name: grant });
     assert(status0 != null);
     assertEquals(status0.state, "granted");
 
     const status1 = await Deno.permissions.revoke({ name: grant });
     assert(status1 != null);
     assertEquals(status1.state, "prompt");
-  });
+  };
+  // Properly name these generated functions.
+  Object.defineProperty(gen, "name", { value: grant + "Granted" });
+  return gen;
+}
+
+for (const grant of knownPermissions) {
+  testPerm({ [grant]: true }, genFunc(grant));
 }
 
 test(async function permissionInvalidName(): Promise<void> {

cli/js/test_util.ts

@@ -26,6 +26,7 @@ interface TestPermissions {
   net?: boolean;
   env?: boolean;
   run?: boolean;
+  plugin?: boolean;
   hrtime?: boolean;
 }
 
@@ -35,6 +36,7 @@ export interface Permissions {
   net: boolean;
   env: boolean;
   run: boolean;
+  plugin: boolean;
   hrtime: boolean;
 }
 
@@ -48,6 +50,7 @@ async function getProcessPermissions(): Promise<Permissions> {
     write: await isGranted("write"),
     net: await isGranted("net"),
     env: await isGranted("env"),
+    plugin: await isGranted("plugin"),
     hrtime: await isGranted("hrtime")
   };
 }
@@ -75,8 +78,9 @@ function permToString(perms: Permissions): string {
   const n = perms.net ? 1 : 0;
   const e = perms.env ? 1 : 0;
   const u = perms.run ? 1 : 0;
+  const p = perms.plugin ? 1 : 0;
   const h = perms.hrtime ? 1 : 0;
-  return `permR${r}W${w}N${n}E${e}U${u}H${h}`;
+  return `permR${r}W${w}N${n}E${e}U${u}P${p}H${h}`;
 }
 
 function registerPermCombination(perms: Permissions): void {
@@ -93,6 +97,7 @@ function normalizeTestPermissions(perms: TestPermissions): Permissions {
     net: !!perms.net,
     run: !!perms.run,
     env: !!perms.env,
+    plugin: !!perms.plugin,
     hrtime: !!perms.hrtime
   };
 }
@@ -120,6 +125,7 @@ export function test(fn: testing.TestFunction): void {
       net: false,
       env: false,
       run: false,
+      plugin: false,
       hrtime: false
     },
     fn
@@ -176,6 +182,7 @@ test(function permissionsMatches(): void {
         net: false,
         env: false,
         run: false,
+        plugin: false,
         hrtime: false
       },
       normalizeTestPermissions({ read: true })
@@ -190,6 +197,7 @@ test(function permissionsMatches(): void {
         net: false,
         env: false,
         run: false,
+        plugin: false,
         hrtime: false
       },
       normalizeTestPermissions({})
@@ -204,6 +212,7 @@ test(function permissionsMatches(): void {
         net: true,
         env: true,
         run: true,
+        plugin: true,
         hrtime: true
       },
       normalizeTestPermissions({ read: true })
@@ -219,6 +228,7 @@ test(function permissionsMatches(): void {
         net: true,
         env: false,
         run: false,
+        plugin: false,
         hrtime: false
       },
       normalizeTestPermissions({ read: true })
@@ -234,6 +244,7 @@ test(function permissionsMatches(): void {
         net: true,
         env: true,
         run: true,
+        plugin: true,
         hrtime: true
       },
       {
@@ -242,6 +253,7 @@ test(function permissionsMatches(): void {
         net: true,
         env: true,
         run: true,
+        plugin: true,
         hrtime: true
       }
     )

cli/ops/native_plugins.rs

@@ -60,7 +60,7 @@ pub fn op_open_native_plugin(
   let args: OpenNativePluginArgs = serde_json::from_value(args)?;
   let (filename, filename_) = deno_fs::resolve_from_cwd(&args.filename)?;
 
-  state.check_native(&filename_)?;
+  state.check_plugin(&filename_)?;
 
   let lib = open_plugin(filename)?;
   let plugin_resource = NativePluginResource {
@@ -81,7 +81,13 @@ pub fn op_open_native_plugin(
   };
   init_fn(&mut init_context);
   for op in init_context.ops {
-    let op_id = registry.register(&op.0, op.1);
+    // Register each plugin op in the `OpRegistry` with the name
+    // formated like this `native_plugin_{plugin_rid}_{name}`.
+    // The inclusion of prefix and rid is designed to avoid any
+    // op name collision beyond the bound of a single loaded
+    // native plugin instance.
+    let op_id =
+      registry.register(&format!("native_plugin_{}_{}", rid, op.0), op.1);
     plugin_resource.ops.insert(op.0, op_id);
   }
 

cli/ops/permissions.rs

@@ -53,6 +53,7 @@ pub fn op_revoke_permission(
     "write" => state.permissions.allow_write.revoke(),
     "net" => state.permissions.allow_net.revoke(),
     "env" => state.permissions.allow_env.revoke(),
+    "plugin" => state.permissions.allow_plugin.revoke(),
     "hrtime" => state.permissions.allow_hrtime.revoke(),
     _ => {}
   };
@@ -86,6 +87,7 @@ pub fn op_request_permission(
       .permissions
       .request_net(&args.url.as_ref().map(String::as_str)),
     "env" => Ok(state.permissions.request_env()),
+    "plugin" => Ok(state.permissions.request_plugin()),
     "hrtime" => Ok(state.permissions.request_hrtime()),
     n => Err(type_error(format!("No such permission name: {}", n))),
   }?;