Merge pull request #847 from alexcrichton/fix-window

Move all methods on `Window` back to methods

Author: alexcrichton

crates/backend/src/ast.rs

@@ -98,10 +98,6 @@ pub enum ImportFunctionKind {
         ty: syn::Type,
         kind: MethodKind,
     },
-    ScopedMethod {
-        ty: syn::Type,
-        operation: Operation,
-    },
     Normal,
 }
 
@@ -432,16 +428,10 @@ impl ImportFunction {
                     }
                 };
                 Some(shared::MethodData {
-                    class: Some(class.clone()),
+                    class: class.clone(),
                     kind,
                 })
             }
-            ImportFunctionKind::ScopedMethod { ref operation, .. } => {
-                Some(shared::MethodData {
-                    class: None,
-                    kind: shared::MethodKind::Operation(shared_operation(operation)),
-                })
-            }
             ImportFunctionKind::Normal => None,
         };
 

crates/backend/src/codegen.rs

@@ -772,9 +772,6 @@ impl TryToTokens for ast::ImportFunction {
                 }
                 class_ty = Some(ty);
             }
-            ast::ImportFunctionKind::ScopedMethod { ref ty, .. } => {
-                class_ty = Some(ty);
-            }
             ast::ImportFunctionKind::Normal => {}
         }
         let vis = &self.function.rust_vis;

crates/backend/src/defined.rs

@@ -256,7 +256,6 @@ impl ImportedTypes for ast::ImportFunctionKind {
     {
         match self {
             ast::ImportFunctionKind::Method { ty, .. } => ty.imported_types(f),
-            ast::ImportFunctionKind::ScopedMethod { ty, .. } => ty.imported_types(f),
             ast::ImportFunctionKind::Normal => {}
         }
     }

crates/cli-support/src/js/mod.rs

@@ -2008,30 +2008,7 @@ impl<'a, 'b> SubContext<'a, 'b> {
             }
         };
 
-        let class = match &method_data.class {
-            Some(class) => self.import_name(info, class)?,
-            None => {
-                let op = match &method_data.kind {
-                    shared::MethodKind::Operation(op) => op,
-                    shared::MethodKind::Constructor => {
-                        bail!("\"no class\" methods cannot be constructors")
-                    }
-                };
-                match &op.kind {
-                    shared::OperationKind::Regular => {
-                        return Ok(import.function.name.to_string())
-                    }
-                    shared::OperationKind::Getter(g) => {
-                        return Ok(format!("(() => {})", g));
-                    }
-                    shared::OperationKind::Setter(g) => {
-                        return Ok(format!("(v => {} = v)", g));
-                    }
-                    _ => bail!("\"no class\" methods must be regular/getter/setter"),
-                }
-
-            }
-        };
+        let class = self.import_name(info, &method_data.class)?;
         let op = match &method_data.kind {
             shared::MethodKind::Constructor => return Ok(format!("new {}", class)),
             shared::MethodKind::Operation(op) => op,

crates/js-sys/src/lib.rs

@@ -23,6 +23,8 @@ extern crate wasm_bindgen;
 use std::mem;
 use std::fmt;
 
+use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering::SeqCst};
+use wasm_bindgen::JsCast;
 use wasm_bindgen::prelude::*;
 
 // When adding new imports:
@@ -4100,3 +4102,54 @@ extern {
     #[wasm_bindgen(method)]
     pub fn finally(this: &Promise, cb: &Closure<FnMut()>) -> Promise;
 }
+
+/// Returns a handle to the global scope object.
+///
+/// This allows access to the global properties and global names by accessing
+/// the `Object` returned.
+pub fn global() -> Object {
+    // Cached `Box<JsValue>`, if we've already executed this.
+    //
+    // 0 = not calculated
+    // n = Some(n) == Some(Box<JsValue>)
+    static GLOBAL: AtomicUsize = ATOMIC_USIZE_INIT;
+
+    match GLOBAL.load(SeqCst) {
+        0 => {}
+        n => return unsafe { (*(n as *const JsValue)).clone().unchecked_into() },
+    }
+
+    // Ok we don't have a cached value, let's load one!
+    //
+    // According to StackOverflow you can access the global object via:
+    //
+    //      const global = Function('return this')();
+    //
+    // I think that's because the manufactured function isn't in "strict" mode.
+    // It also turns out that non-strict functions will ignore `undefined`
+    // values for `this` when using the `apply` function.
+    //
+    // As a result we use the equivalent of this snippet to get a handle to the
+    // global object in a sort of roundabout way that should hopefully work in
+    // all contexts like ESM, node, browsers, etc.
+    let this = Function::new_no_args("return this")
+        .call0(&JsValue::undefined())
+        .ok();
+
+    // Note that we avoid `unwrap()` on `call0` to avoid code size bloat, we
+    // just handle the `Err` case as returning a different object.
+    debug_assert!(this.is_some());
+    let this = match this {
+        Some(this) => this,
+        None => return JsValue::undefined().unchecked_into(),
+    };
+
+    let ptr: *mut JsValue = Box::into_raw(Box::new(this.clone()));
+    match GLOBAL.compare_exchange(0, ptr as usize, SeqCst, SeqCst) {
+        // We stored out value, relinquishing ownership of `ptr`
+        Ok(_) => {}
+        // Another thread one, drop our value
+        Err(_) => unsafe { drop(Box::from_raw(ptr)) },
+    }
+    this.unchecked_into()
+}

crates/macro-support/src/parser.rs

@@ -510,7 +510,6 @@ impl<'a> ConvertToAst<(BindgenAttrs, &'a Option<String>)> for syn::ForeignItemFn
 
         let shim = {
             let ns = match kind {
-                ast::ImportFunctionKind::ScopedMethod { .. } |
                 ast::ImportFunctionKind::Normal => (0, "n"),
                 ast::ImportFunctionKind::Method { ref class, .. } => (1, &class[..]),
             };

crates/shared/src/lib.rs

@@ -51,7 +51,7 @@ pub struct ImportFunction {
 
 #[derive(Deserialize, Serialize)]
 pub struct MethodData {
-    pub class: Option<String>,
+    pub class: String,
     pub kind: MethodKind,
 }
 

crates/test/src/rt/detect.rs

@@ -1,7 +1,8 @@
 //! Runtime detection of whether we're in node.js or a browser.
 
 use wasm_bindgen::prelude::*;
-use js_sys::Function;
+use wasm_bindgen::JsCast;
+use js_sys;
 
 #[wasm_bindgen]
 extern {
@@ -13,49 +14,8 @@ extern {
 /// Returns whether it's likely we're executing in a browser environment, as
 /// opposed to node.js.
 pub fn is_browser() -> bool {
-    // This is a bit tricky to define. The basic crux of this is that we want to
-    // test if the `self` identifier is defined. That is defined in browsers
-    // (and web workers!) but not in Node. To that end you might expect:
-    //
-    //      #[wasm_bindgen]
-    //      extern {
-    //          #[wasm_bindgen(js_name = self)]
-    //          static SELF: JsValue;
-    //      }
-    //
-    //      *SELF != JsValue::undefined()
-    //
-    // this currently, however, throws a "not defined" error in JS because the
-    // generated function looks like `function() { return self; }` which throws
-    // an error in Node because `self` isn't defined.
-    //
-    // To work around this limitation we instead lookup the value of `self`
-    // through the `this` object, basically generating `this.self`.
-    //
-    // Unfortunately that's also hard to do! In ESM modes the top-level `this`
-    // object is undefined, meaning that we can't just generate a function that
-    // returns `this.self` as it'll throw "can't access field `self` of
-    // `undefined`" whenever ESMs are being used.
-    //
-    // So finally we reach the current implementation. According to
-    // StackOverflow you can access the global object via:
-    //
-    //      const global = Function('return this')();
-    //
-    // I think that's because the manufactured function isn't in "strict" mode.
-    // It also turns out that non-strict functions will ignore `undefined`
-    // values for `this` when using the `apply` function. Add it all up, and you
-    // get the below code:
-    //
-    // * Manufacture a function
-    // * Call `apply` where we specify `this` but the function ignores it
-    // * Once we have `this`, use a structural getter to get the value of `self`
-    // * Last but not least, test whether `self` is defined or not.
-    //
-    // Whew!
-    let this = Function::new_no_args("return this")
-        .call0(&JsValue::undefined())
-        .unwrap();
-    assert!(this != JsValue::undefined());
-    This::from(this).self_() != JsValue::undefined()
+    // Test whether we're in a browser by seeing if the `self` property is
+    // defined on the global object, which should in turn only be true in
+    // browsers.
+    js_sys::global().unchecked_into::<This>().self_() != JsValue::undefined()
 }

crates/web-sys/src/lib.rs

@@ -15,6 +15,14 @@
 
 extern crate wasm_bindgen;
 extern crate js_sys;
+
 use js_sys::Object;
 
+#[cfg(feature = "Window")]
+pub fn window() -> Option<Window> {
+    use wasm_bindgen::JsCast;
+
+    js_sys::global().dyn_into::<Window>().ok()
+}
+
 include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

crates/web-sys/tests/wasm/location.rs

@@ -1,56 +1,60 @@
 use wasm_bindgen_test::*;
-use web_sys::Window;
+use web_sys::{self, Location};
+
+fn location() -> Location {
+    web_sys::window().unwrap().location()
+}
 
 #[wasm_bindgen_test]
 fn href() {
-    let loc = Window::location();
+    let loc = location();
     loc.href().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn origin() {
-    let loc = Window::location();
+    let loc = location();
     loc.origin().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn protocol() {
-    let loc = Window::location();
+    let loc = location();
     loc.protocol().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn host() {
-    let loc = Window::location();
+    let loc = location();
     loc.host().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn hostname() {
-    let loc = Window::location();
+    let loc = location();
     loc.hostname().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn port() {
-    let loc = Window::location();
+    let loc = location();
     loc.port().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn pathname() {
-    let loc = Window::location();
+    let loc = location();
     loc.pathname().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn search() {
-    let loc = Window::location();
+    let loc = location();
     loc.search().unwrap();
 }
 
 #[wasm_bindgen_test]
 fn hash() {
-    let loc = Window::location();
+    let loc = location();
     loc.hash().unwrap();
 }

crates/webidl-tests/global.js

@@ -1,4 +1,8 @@
-global.global_no_args = () => 3;
-global.global_with_args = (a, b) => a + b;
-global.global_attribute = 'x';
+const map = {
+  global_no_args: () => 3,
+  global_with_args: (a, b) => a + b,
+  global_attribute: 'x',
+};
+
+global.get_global = () => map;
 

crates/webidl-tests/global.rs

@@ -1,13 +1,20 @@
 use js_sys::Object;
+use wasm_bindgen::prelude::*;
 use wasm_bindgen_test::*;
 
 include!(concat!(env!("OUT_DIR"), "/global.rs"));
 
+#[wasm_bindgen]
+extern {
+    fn get_global() -> Global;
+}
+
 #[wasm_bindgen_test]
 fn works() {
-    assert_eq!(Global::global_no_args(), 3);
-    assert_eq!(Global::global_with_args("a", "b"), "ab");
-    assert_eq!(Global::global_attribute(), "x");
-    Global::set_global_attribute("y");
-    assert_eq!(Global::global_attribute(), "y");
+    let x = get_global();
+    assert_eq!(x.global_no_args(), 3);
+    assert_eq!(x.global_with_args("a", "b"), "ab");
+    assert_eq!(x.global_attribute(), "x");
+    x.set_global_attribute("y");
+    assert_eq!(x.global_attribute(), "y");
 }

crates/webidl-tests/simple.js

@@ -87,7 +87,11 @@ global.Unforgeable = class Unforgeable {
   }
 };
 
-global.m = () => 123;
+global.GlobalMethod = class {
+  m() { return 123; }
+};
+
+global.get_global_method = () => new GlobalMethod();
 
 global.Indexing = function () {
   return new Proxy({}, {

crates/webidl-tests/simple.rs

@@ -72,9 +72,16 @@ fn nullable_method() {
     assert!(f.opt(None) == None);
 }
 
+#[wasm_bindgen]
+extern {
+    fn get_global_method() -> GlobalMethod;
+}
+
+
 #[wasm_bindgen_test]
 fn global_method() {
-    assert_eq!(GlobalMethod::m(), 123);
+    let x = get_global_method();
+    assert_eq!(x.m(), 123);
 }
 
 #[wasm_bindgen_test]

crates/webidl/src/first_pass.rs

@@ -44,7 +44,6 @@ pub(crate) struct FirstPassRecord<'src> {
 pub(crate) struct InterfaceData<'src> {
     /// Whether only partial interfaces were encountered
     pub(crate) partial: bool,
-    pub(crate) global: bool,
     pub(crate) attributes: Vec<&'src AttributeInterfaceMember<'src>>,
     pub(crate) consts: Vec<&'src ConstMember<'src>>,
     pub(crate) operations: BTreeMap<OperationId<'src>, OperationData<'src>>,
@@ -373,12 +372,6 @@ fn process_interface_attribute<'src>(
                 false,
             );
         }
-        ExtendedAttribute::Ident(id) if id.lhs_identifier.0 == "Global" => {
-            record.interfaces.get_mut(self_name).unwrap().global = true;
-        }
-        ExtendedAttribute::IdentList(id) if id.identifier.0 == "Global" => {
-            record.interfaces.get_mut(self_name).unwrap().global = true;
-        }
         _ => {}
     }
 }