Whitelist for slice args that do not need to be mutable

crates/web-sys/tests/wasm/element.js

@@ -151,6 +151,11 @@ export function new_title() {
   return document.createElement("title");
 }
 
+export function new_webgl_rendering_context() {
+  const canvas = document.createElement('canvas');
+  return canvas.getContext('webgl');
+}
+
 export function new_xpath_result() {
     let xmlDoc = new DOMParser().parseFromString("<root><value>tomato</value></root>", "application/xml");
     let xpathResult = xmlDoc.evaluate("/root//value", xmlDoc, null, XPathResult.ANY_TYPE, null);

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

@@ -56,6 +56,7 @@ pub mod style_element;
 pub mod table_element;
 pub mod title_element;
 pub mod xpath_result;
+pub mod whitelisted_immutable_slices;
 
 #[wasm_bindgen_test]
 fn deref_works() {

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

@@ -0,0 +1,54 @@
+//! When generating our web_sys APIs we default to setting slice references that
+//! get passed to JS as mutable in case they get mutated in JS.
+//!
+//! In certain cases we know for sure that the slice will not get mutated - for
+//! example when working with the WebGlRenderingContext APIs.
+//!
+//! These tests ensure that whitelisted methods do indeed accept mutable slices.
+//! Especially important since this whitelist is stringly typed and currently
+//! maintained by hand.
+//!
+//! @see https://github.com/rustwasm/wasm-bindgen/issues/1005
+
+use wasm_bindgen::prelude::*;
+use wasm_bindgen_test::*;
+use web_sys::WebGlRenderingContext;
+
+#[wasm_bindgen(module = "./tests/wasm/element.js")]
+extern "C" {
+    fn new_webgl_rendering_context() -> WebGlRenderingContext;
+    // TODO: Add a function to create another type to test here.
+    // These functions come from element.js
+}
+
+// TODO: Uncomment WebGlRenderingContext test...
+// Currently commented out because WebGl isn't working in geckodriver.
+// It currently works in chromedriver so if you need to run this in the meantime you can
+// uncomment this block and run it in using chromedriver.
+//
+// CHROMEDRIVER=chromedriver cargo test --manifest-path crates/web-sys/Cargo.toml --target wasm32-unknown-unknown --all-features test_webgl_rendering_context_immutable_slices
+
+// Ensure that our whitelisted WebGlRenderingContext methods work
+//#[wasm_bindgen_test]
+//fn test_webgl_rendering_context_immutable_slices() {
+//    let gl = new_webgl_rendering_context();
+//
+//    gl.vertex_attrib1fv_with_f32_array(0, &[1.]);
+//    gl.vertex_attrib2fv_with_f32_array(0, &[1.]);
+//    gl.vertex_attrib3fv_with_f32_array(0, &[1.]);
+//    gl.vertex_attrib4fv_with_f32_array(0, &[1.]);
+//
+//    gl.uniform1fv_with_f32_array(None, &[1.]);
+//    gl.uniform2fv_with_f32_array(None, &[1.]);
+//    gl.uniform3fv_with_f32_array(None, &[1.]);
+//    gl.uniform4fv_with_f32_array(None, &[1.]);
+//
+//    gl.uniform_matrix2fv_with_f32_array(None, false, &[1.]);
+//    gl.uniform_matrix3fv_with_f32_array(None, false, &[1.]);
+//    gl.uniform_matrix4fv_with_f32_array(None, false, &[1.]);
+//}
+
+// TODO:
+//#[wasm_bindgen_test]
+//fn test_another_types_immutable_slices_here() {
+//}

crates/webidl-tests/lib.rs

@@ -1 +1 @@
-// intentionally left blank
+// Intentionally left blank in order for Cargo to work

crates/webidl/Cargo.toml

@@ -14,6 +14,7 @@ Support for parsing WebIDL specific to wasm-bindgen
 [dependencies]
 failure = "0.1.2"
 heck = "0.3"
+lazy_static = "1.2.0"
 log = "0.4.1"
 proc-macro2 = "0.4.8"
 quote = '0.6'

crates/webidl/src/first_pass.rs

@@ -83,6 +83,9 @@ pub(crate) struct CallbackInterfaceData<'src> {
 #[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Copy)]
 pub(crate) enum OperationId<'src> {
     Constructor(IgnoreTraits<&'src str>),
+    /// The name of a function in crates/web-sys/webidls/enabled/*.webidl
+    ///
+    /// ex: Operation(Some("vertexAttrib1fv"))
     Operation(Option<&'src str>),
     IndexingGetter,
     IndexingSetter,

crates/webidl/src/idl_type.rs

@@ -41,7 +41,10 @@ pub(crate) enum IdlType<'a> {
     Uint16Array,
     Int32Array,
     Uint32Array,
-    Float32Array,
+    Float32Array {
+        /// Whether or not the generated web-sys function should use an immutable slice
+        immutable: bool
+    },
     Float64Array,
     ArrayBufferView,
     BufferSource,
@@ -324,14 +327,23 @@ impl<'a> ToIdlType<'a> for Identifier<'a> {
     }
 }
 
+// We default to Float32Array's being mutable, but in certain cases where we're certain that
+// slices won't get mutated on the JS side (such as the WebGL APIs) we might, later in the flow,
+// instead use the immutable version.
+impl<'a> ToIdlType<'a> for term::Float32Array {
+    fn to_idl_type(&self, _record: &FirstPassRecord<'a>) -> IdlType<'a> {
+        IdlType::Float32Array {immutable: false}
+    }
+}
+
 macro_rules! terms_to_idl_type {
     ($($t:tt => $r:tt)*) => ($(
         impl<'a> ToIdlType<'a> for term::$t {
             fn to_idl_type(&self, _record: &FirstPassRecord<'a>) -> IdlType<'a> {
                 IdlType::$r
             }
         }
-    )*)
+    )*);
 }
 
 terms_to_idl_type! {
@@ -358,7 +370,6 @@ terms_to_idl_type! {
     Uint16Array => Uint16Array
     Uint32Array => Uint32Array
     Uint8ClampedArray => Uint8ClampedArray
-    Float32Array => Float32Array
     Float64Array => Float64Array
     ArrayBufferView => ArrayBufferView
     BufferSource => BufferSource
@@ -396,7 +407,7 @@ impl<'a> IdlType<'a> {
             IdlType::Uint16Array => dst.push_str("u16_array"),
             IdlType::Int32Array => dst.push_str("i32_array"),
             IdlType::Uint32Array => dst.push_str("u32_array"),
-            IdlType::Float32Array => dst.push_str("f32_array"),
+            IdlType::Float32Array { .. } => dst.push_str("f32_array"),
             IdlType::Float64Array => dst.push_str("f64_array"),
             IdlType::ArrayBufferView => dst.push_str("array_buffer_view"),
             IdlType::BufferSource => dst.push_str("buffer_source"),
@@ -501,16 +512,16 @@ impl<'a> IdlType<'a> {
 
             IdlType::ArrayBuffer => js_sys("ArrayBuffer"),
             IdlType::DataView => None,
-            IdlType::Int8Array => Some(array("i8", pos)),
-            IdlType::Uint8Array => Some(array("u8", pos)),
-            IdlType::Uint8ArrayMut => Some(array("u8", pos)),
-            IdlType::Uint8ClampedArray => Some(clamped(array("u8", pos))),
-            IdlType::Int16Array => Some(array("i16", pos)),
-            IdlType::Uint16Array => Some(array("u16", pos)),
-            IdlType::Int32Array => Some(array("i32", pos)),
-            IdlType::Uint32Array => Some(array("u32", pos)),
-            IdlType::Float32Array => Some(array("f32", pos)),
-            IdlType::Float64Array => Some(array("f64", pos)),
+            IdlType::Int8Array => Some(array("i8", pos, false)),
+            IdlType::Uint8Array => Some(array("u8", pos, false)),
+            IdlType::Uint8ArrayMut => Some(array("u8", pos, false)),
+            IdlType::Uint8ClampedArray => Some(clamped(array("u8", pos, false))),
+            IdlType::Int16Array => Some(array("i16", pos, false)),
+            IdlType::Uint16Array => Some(array("u16", pos, false)),
+            IdlType::Int32Array => Some(array("i32", pos, false)),
+            IdlType::Uint32Array => Some(array("u32", pos, false)),
+            IdlType::Float32Array {immutable} => Some(array("f32", pos, *immutable)),
+            IdlType::Float64Array => Some(array("f64", pos, false)),
 
             IdlType::ArrayBufferView | IdlType::BufferSource => js_sys("Object"),
             IdlType::Interface(name)

crates/webidl/src/lib.rs

@@ -21,6 +21,8 @@ extern crate quote;
 extern crate syn;
 extern crate wasm_bindgen_backend as backend;
 extern crate weedle;
+#[macro_use]
+extern crate lazy_static;
 
 mod error;
 mod first_pass;

crates/webidl/src/util.rs

@@ -13,6 +13,8 @@ use weedle::literal::{ConstValue, FloatLit, IntegerLit};
 use first_pass::{FirstPassRecord, OperationData, OperationId, Signature};
 use idl_type::{IdlType, ToIdlType};
 
+use std::collections::HashSet;
+
 /// For variadic operations an overload with a `js_sys::Array` argument is generated alongside with
 /// `operation_name_0`, `operation_name_1`, `operation_name_2`, ..., `operation_name_n` overloads
 /// which have the count of arguments for passing values to the variadic argument
@@ -67,13 +69,13 @@ pub fn mdn_doc(class: &str, method: Option<&str>) -> String {
     format!("[MDN Documentation]({})", link).into()
 }
 
-// Array type is borrowed for arguments (`&[T]`) and owned for return value (`Vec<T>`).
-pub(crate) fn array(base_ty: &str, pos: TypePosition) -> syn::Type {
+// Array type is borrowed for arguments (`&mut [T]` or `&[T]`) and owned for return value (`Vec<T>`).
+pub(crate) fn array(base_ty: &str, pos: TypePosition, immutable: bool) -> syn::Type {
     match pos {
         TypePosition::Argument => {
             shared_ref(
                 slice_ty(ident_ty(raw_ident(base_ty))),
-                /*mutable =*/ true,
+                /*mutable =*/ !immutable,
             )
         }
         TypePosition::Return => vec_ty(ident_ty(raw_ident(base_ty))),
@@ -433,7 +435,10 @@ impl<'src> FirstPassRecord<'src> {
                     );
                     signatures.push((signature, idl_args.clone()));
                 }
-                idl_args.push(arg.ty.to_idl_type(self));
+
+                let mut idl_type = arg.ty.to_idl_type(self);
+                let idl_type = maybe_adjust(idl_type, id);
+                idl_args.push(idl_type);
             }
             signatures.push((signature, idl_args));
         }
@@ -707,3 +712,73 @@ pub fn public() -> syn::Visibility {
         pub_token: Default::default(),
     })
 }
+
+
+/// When generating our web_sys APIs we default to setting slice references that
+/// get passed to JS as mutable in case they get mutated in JS.
+///
+/// In certain cases we know for sure that the slice will not get mutated - for
+/// example when working with the WebGlRenderingContext APIs.
+///
+/// Here we implement a whitelist for those cases. This whitelist is currently
+/// maintained by hand.
+///
+/// When adding to this whitelist add tests to crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs
+fn maybe_adjust<'a>(mut idl_type: IdlType<'a>, id: &'a OperationId) -> IdlType<'a> {
+    let op = match id {
+        OperationId::Operation(Some(op)) => op,
+        _ => return idl_type
+    };
+
+    if IMMUTABLE_F32_WHITELIST.contains(op) {
+        flag_slices_immutable(&mut idl_type)
+    }
+
+    // TODO: Add other whitelisted slices here, such as F64 or u8..
+
+    idl_type
+
+}
+
+fn flag_slices_immutable(ty: &mut IdlType) {
+    match ty {
+        IdlType::Float32Array { immutable } => *immutable = true,
+        IdlType::Union(list) => {
+            for item in list { flag_slices_immutable(item); }
+        }
+
+        // TODO: ... other recursive cases like Nullable handled here
+
+        // catch-all for everything else like Object
+        _ => {}
+    }
+}
+
+lazy_static! {
+    /// These functions will have their f32 slice arguments be immutable.
+    static ref IMMUTABLE_F32_WHITELIST: HashSet<&'static str> = {
+        let mut set = HashSet::new();
+
+        let fn_names = vec![
+            // WebGlRenderingContext
+            "uniform1fv",
+            "uniform2fv",
+            "uniform3fv",
+            "uniform4fv",
+            "uniformMatrix2fv",
+            "uniformMatrix3fv",
+            "uniformMatrix4fv",
+            "vertexAttrib1fv",
+            "vertexAttrib2fv",
+            "vertexAttrib3fv",
+            "vertexAttrib4fv",
+            // TODO: Add another type's functions here. Leave a comment header with the type name
+        ];
+
+        for fn_name in fn_names {
+            set.insert(fn_name);
+        }
+
+        set
+    };
+}