auto merge of #1876 : Ms2ger/servo/JSValConvertible-strings, r=jdm

Hat tip to @eddyb and @huonw for most of the clever bits of this PR.

src/components/script/dom/bindings/codegen/CodegenRust.py

@@ -1020,30 +1020,29 @@ for (uint32_t i = 0; i < length; ++i) {
         }
         if treatNullAs not in treatAs:
             raise TypeError("We don't support [TreatNullAs=%s]" % treatNullAs)
-        nullBehavior = treatAs[treatNullAs]
+        if type.nullable():
+            nullBehavior = "()"
+        else:
+            nullBehavior = treatAs[treatNullAs]
 
-        def getConversionCode(varName, isOptional=False):
+        def getConversionCode(isOptional=False):
             strval = "strval"
             if isOptional:
                 strval = "Some(%s)" % strval
-            if type.nullable():
-                call = "jsval_to_domstring(cx, ${val})"
-            else:
-                call = "jsval_to_str(cx, ${val}, %s)" % nullBehavior
+
             conversionCode = (
-                "let strval = %s;\n"
-                "if strval.is_err() {\n"
-                "  return 0;\n"
-                "}\n"
-                "let strval = strval.unwrap();\n"
-                "%s = %s;" % (call, varName, strval))
+                "match FromJSValConvertible::from_jsval(cx, ${val}, %s) {\n"
+                "  Ok(strval) => ${declName} = %s,\n"
+                "  Err(_) => return 0,\n"
+                "}" % (nullBehavior, strval))
+
             if defaultValue is None:
                 return conversionCode
 
             if isinstance(defaultValue, IDLNullValue):
                 assert(type.nullable())
                 return handleDefault(conversionCode,
-                                     "%s.SetNull()" % varName)
+                                     "${declName}.SetNull()")
 
             value = "str::from_utf8(data).to_owned()"
             if type.nullable():
@@ -1051,10 +1050,10 @@ for (uint32_t i = 0; i < length; ++i) {
 
             default = (
                 "static data: [u8, ..%s] = [ %s ];\n"
-                "%s = %s" %
+                "${declName} = %s" %
                 (len(defaultValue.value) + 1,
                  ", ".join(["'" + char + "' as u8" for char in defaultValue.value] + ["0"]),
-                 varName, value))
+                 value))
 
             return handleDefault(conversionCode, default)
 
@@ -1062,7 +1061,7 @@ for (uint32_t i = 0; i < length; ++i) {
             # We have to make a copy, because our jsval may well not
             # live as long as our string needs to.
             declType = CGGeneric("DOMString")
-            return ("%s\n" % getConversionCode("${declName}"),
+            return ("%s\n" % getConversionCode(),
                     declType, None, isOptional, None)
 
         declType = "DOMString"
@@ -1076,8 +1075,7 @@ for (uint32_t i = 0; i < length; ++i) {
 
         return (
             "%s\n" %
-            #"const_cast<%s&>(${declName}) = &${holderName};" %
-            (getConversionCode("${declName}", isOptional)),
+            (getConversionCode(isOptional)),
             CGGeneric(declType), None, #CGGeneric("FakeDependentString"),
             False,
             initialValue)
@@ -1232,7 +1230,7 @@ for (uint32_t i = 0; i < length; ++i) {
         successVal = preSuccess + successVal + postSuccess
     #XXXjdm support conversionBehavior here
     template = (
-        "match FromJSValConvertible::from_jsval(cx, ${val}) {\n"
+        "match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n"
         "  Ok(v) => ${declName} = %s,\n"
         "  Err(_) => %s\n"
         "}" % (successVal, failureCode))
@@ -1535,10 +1533,7 @@ for (uint32_t i = 0; i < length; ++i) {
         return (wrappingCode, False)
 
     if type.isString():
-        if type.nullable():
-            return (wrapAndSetPtr("*${jsvalPtr} = domstring_to_jsval(cx, %s)" % result), False)
-        else:
-            return (wrapAndSetPtr("*${jsvalPtr} = str_to_jsval(cx, %s)" % result), False)
+        return (setValue("(%s).to_jsval(cx)" % result), True)
 
     if type.isEnum():
         if type.nullable():
@@ -1582,7 +1577,7 @@ if %(resultStr)s.is_null() {
     if not type.isPrimitive():
         raise TypeError("Need to learn to wrap %s" % type)
 
-    return (setValue("(%s).to_jsval()" % result), True)
+    return (setValue("(%s).to_jsval(cx)" % result), True)
 
 
 def wrapForType(type, descriptorProvider, templateValues):
@@ -5258,15 +5253,15 @@ class CGBindingRoot(CGThing):
                           'dom::bindings::utils::{CreateDOMGlobal, CreateInterfaceObjects2}',
                           'dom::bindings::utils::{ConstantSpec, cx_for_dom_object, Default}',
                           'dom::bindings::utils::{dom_object_slot, DOM_OBJECT_SLOT, DOMClass}',
-                          'dom::bindings::utils::{DOMJSClass, domstring_to_jsval, Empty}',
+                          'dom::bindings::utils::{DOMJSClass}',
                           'dom::bindings::utils::{FindEnumStringIndex, GetArrayIndexFromId}',
                           'dom::bindings::utils::{GetPropertyOnPrototype, GetProtoOrIfaceArray}',
                           'dom::bindings::utils::{GetReflector, HasPropertyOnPrototype, IntVal}',
-                          'dom::bindings::utils::{jsid_to_str, jsval_to_domstring, jsval_to_str}',
+                          'dom::bindings::utils::{jsid_to_str}',
                           'dom::bindings::utils::{NativePropertyHooks}',
                           'dom::bindings::utils::global_object_for_js_object',
                           'dom::bindings::utils::{Reflectable}',
-                          'dom::bindings::utils::{squirrel_away_unique, str_to_jsval}',
+                          'dom::bindings::utils::{squirrel_away_unique}',
                           'dom::bindings::utils::{ThrowingConstructor,  unwrap, unwrap_jsmanaged}',
                           'dom::bindings::utils::{unwrap_object, VoidVal, with_gc_disabled}',
                           'dom::bindings::utils::{with_gc_enabled, XrayResolveProperty}',
@@ -5275,6 +5270,7 @@ class CGBindingRoot(CGThing):
                           'dom::bindings::callback::{CallSetup,ExceptionHandling}',
                           'dom::bindings::callback::{WrapCallThisObject}',
                           'dom::bindings::conversions::{FromJSValConvertible, ToJSValConvertible}',
+                          'dom::bindings::conversions::{Default, Empty}',
                           'dom::bindings::codegen::*',
                           'dom::bindings::codegen::UnionTypes::*',
                           'dom::bindings::codegen::UnionConversions::*',

src/components/script/dom/bindings/conversions.rs

@@ -2,20 +2,25 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use dom::bindings::utils::jsstring_to_str;
+use servo_util::str::DOMString;
+
 use js::jsapi::{JSBool, JSContext};
 use js::jsapi::{JS_ValueToUint64, JS_ValueToInt64};
 use js::jsapi::{JS_ValueToECMAUint32, JS_ValueToECMAInt32};
 use js::jsapi::{JS_ValueToUint16, JS_ValueToNumber, JS_ValueToBoolean};
+use js::jsapi::{JS_NewUCStringCopyN, JS_ValueToString};
 use js::jsval::JSVal;
-use js::jsval::{NullValue, BooleanValue, Int32Value, UInt32Value};
+use js::jsval::{NullValue, BooleanValue, Int32Value, UInt32Value, StringValue};
 use js::glue::RUST_JS_NumberValue;
+use std::libc;
 
 pub trait ToJSValConvertible {
-    fn to_jsval(&self) -> JSVal;
+    fn to_jsval(&self, cx: *JSContext) -> JSVal;
 }
 
-pub trait FromJSValConvertible {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<Self, ()>;
+pub trait FromJSValConvertible<T> {
+    fn from_jsval(cx: *JSContext, val: JSVal, option: T) -> Result<Self, ()>;
 }
 
 
@@ -32,166 +37,208 @@ unsafe fn convert_from_jsval<T: Default>(
 
 
 impl ToJSValConvertible for bool {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         BooleanValue(*self)
     }
 }
 
-impl FromJSValConvertible for bool {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<bool, ()> {
+impl FromJSValConvertible<()> for bool {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<bool, ()> {
         let result = unsafe { convert_from_jsval(cx, val, JS_ValueToBoolean) };
         result.map(|b| b != 0)
     }
 }
 
 impl ToJSValConvertible for i8 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         Int32Value(*self as i32)
     }
 }
 
-impl FromJSValConvertible for i8 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<i8, ()> {
+impl FromJSValConvertible<()> for i8 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<i8, ()> {
         let result = unsafe { convert_from_jsval(cx, val, JS_ValueToECMAInt32) };
         result.map(|v| v as i8)
     }
 }
 
 impl ToJSValConvertible for u8 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         Int32Value(*self as i32)
     }
 }
 
-impl FromJSValConvertible for u8 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<u8, ()> {
+impl FromJSValConvertible<()> for u8 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<u8, ()> {
         let result = unsafe { convert_from_jsval(cx, val, JS_ValueToECMAInt32) };
         result.map(|v| v as u8)
     }
 }
 
 impl ToJSValConvertible for i16 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         Int32Value(*self as i32)
     }
 }
 
-impl FromJSValConvertible for i16 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<i16, ()> {
+impl FromJSValConvertible<()> for i16 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<i16, ()> {
         let result = unsafe { convert_from_jsval(cx, val, JS_ValueToECMAInt32) };
         result.map(|v| v as i16)
     }
 }
 
 impl ToJSValConvertible for u16 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         Int32Value(*self as i32)
     }
 }
 
-impl FromJSValConvertible for u16 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<u16, ()> {
+impl FromJSValConvertible<()> for u16 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<u16, ()> {
         unsafe { convert_from_jsval(cx, val, JS_ValueToUint16) }
     }
 }
 
 impl ToJSValConvertible for i32 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         Int32Value(*self)
     }
 }
 
-impl FromJSValConvertible for i32 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<i32, ()> {
+impl FromJSValConvertible<()> for i32 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<i32, ()> {
         unsafe { convert_from_jsval(cx, val, JS_ValueToECMAInt32) }
     }
 }
 
 impl ToJSValConvertible for u32 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         UInt32Value(*self)
     }
 }
 
-impl FromJSValConvertible for u32 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<u32, ()> {
+impl FromJSValConvertible<()> for u32 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<u32, ()> {
         unsafe { convert_from_jsval(cx, val, JS_ValueToECMAUint32) }
     }
 }
 
 impl ToJSValConvertible for i64 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         unsafe {
             RUST_JS_NumberValue(*self as f64)
         }
     }
 }
 
-impl FromJSValConvertible for i64 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<i64, ()> {
+impl FromJSValConvertible<()> for i64 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<i64, ()> {
         unsafe { convert_from_jsval(cx, val, JS_ValueToInt64) }
     }
 }
 
 impl ToJSValConvertible for u64 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         unsafe {
             RUST_JS_NumberValue(*self as f64)
         }
     }
 }
 
-impl FromJSValConvertible for u64 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<u64, ()> {
+impl FromJSValConvertible<()> for u64 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<u64, ()> {
         unsafe { convert_from_jsval(cx, val, JS_ValueToUint64) }
     }
 }
 
 impl ToJSValConvertible for f32 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         unsafe {
             RUST_JS_NumberValue(*self as f64)
         }
     }
 }
 
-impl FromJSValConvertible for f32 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<f32, ()> {
+impl FromJSValConvertible<()> for f32 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<f32, ()> {
         let result = unsafe { convert_from_jsval(cx, val, JS_ValueToNumber) };
         result.map(|f| f as f32)
     }
 }
 
 impl ToJSValConvertible for f64 {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, _cx: *JSContext) -> JSVal {
         unsafe {
             RUST_JS_NumberValue(*self)
         }
     }
 }
 
-impl FromJSValConvertible for f64 {
-    fn from_jsval(cx: *JSContext, val: JSVal) -> Result<f64, ()> {
+impl FromJSValConvertible<()> for f64 {
+    fn from_jsval(cx: *JSContext, val: JSVal, _option: ()) -> Result<f64, ()> {
         unsafe { convert_from_jsval(cx, val, JS_ValueToNumber) }
     }
 }
 
+impl ToJSValConvertible for DOMString {
+    fn to_jsval(&self, cx: *JSContext) -> JSVal {
+        unsafe {
+            let string_utf16 = self.to_utf16();
+            let jsstr = JS_NewUCStringCopyN(cx, string_utf16.as_ptr(), string_utf16.len() as libc::size_t);
+            if jsstr.is_null() {
+                fail!("JS_NewUCStringCopyN failed");
+            }
+            StringValue(&*jsstr)
+        }
+    }
+}
+
+#[deriving(Eq)]
+pub enum StringificationBehavior {
+    Default,
+    Empty,
+}
+
+impl Default for StringificationBehavior {
+    fn default() -> StringificationBehavior {
+        Default
+    }
+}
+
+impl FromJSValConvertible<StringificationBehavior> for DOMString {
+    fn from_jsval(cx: *JSContext, value: JSVal, nullBehavior: StringificationBehavior) -> Result<DOMString, ()> {
+        if nullBehavior == Empty && value.is_null() {
+            Ok(~"")
+        } else {
+            let jsstr = unsafe { JS_ValueToString(cx, value) };
+            if jsstr.is_null() {
+                debug!("JS_ValueToString failed");
+                Err(())
+            } else {
+                Ok(jsstring_to_str(cx, jsstr))
+            }
+        }
+    }
+}
+
 impl<T: ToJSValConvertible> ToJSValConvertible for Option<T> {
-    fn to_jsval(&self) -> JSVal {
+    fn to_jsval(&self, cx: *JSContext) -> JSVal {
         match self {
-            &Some(ref value) => value.to_jsval(),
+            &Some(ref value) => value.to_jsval(cx),
             &None => NullValue(),
         }
     }
 }
 
-impl<T: FromJSValConvertible> FromJSValConvertible for Option<T> {
-    fn from_jsval(cx: *JSContext, value: JSVal) -> Result<Option<T>, ()> {
+impl<X: Default, T: FromJSValConvertible<X>> FromJSValConvertible<()> for Option<T> {
+    fn from_jsval(cx: *JSContext, value: JSVal, _: ()) -> Result<Option<T>, ()> {
         if value.is_null_or_undefined() {
             Ok(None)
         } else {
-            let result: Result<T, ()> = FromJSValConvertible::from_jsval(cx, value);
-            result.map(|v| Some(v))
+            let option: X = Default::default();
+            let result: Result<T, ()> = FromJSValConvertible::from_jsval(cx, value, option);
+            result.map(Some)
         }
     }
 }

src/components/script/dom/bindings/utils.rs

@@ -26,7 +26,7 @@ use js::jsapi::{JS_GetClass, JS_LinkConstructorAndPrototype, JS_GetStringCharsAn
 use js::jsapi::{JS_ObjectIsRegExp, JS_ObjectIsDate};
 use js::jsapi::{JS_InternString, JS_GetFunctionObject};
 use js::jsapi::{JS_HasPropertyById, JS_GetPrototype, JS_GetGlobalForObject};
-use js::jsapi::{JS_NewUCStringCopyN, JS_DefineFunctions, JS_DefineProperty};
+use js::jsapi::{JS_DefineFunctions, JS_DefineProperty};
 use js::jsapi::{JS_ValueToString, JS_GetReservedSlot, JS_SetReservedSlot};
 use js::jsapi::{JSContext, JSObject, JSBool, jsid, JSClass, JSNative};
 use js::jsapi::{JSFunctionSpec, JSPropertySpec, JSPropertyDescriptor};
@@ -35,7 +35,7 @@ use js::jsapi::{JSString};
 use js::jsapi::{JS_AllowGC, JS_InhibitGC};
 use js::jsfriendapi::bindgen::JS_NewObjectWithUniqueType;
 use js::jsval::JSVal;
-use js::jsval::{StringValue, PrivateValue, ObjectValue, NullValue, Int32Value};
+use js::jsval::{PrivateValue, ObjectValue, NullValue, Int32Value};
 use js::jsval::{UInt32Value, DoubleValue, BooleanValue, UndefinedValue};
 use js::{JSPROP_ENUMERATE, JSCLASS_IS_GLOBAL, JSCLASS_IS_DOMJSCLASS};
 use js::{JSPROP_PERMANENT, JSID_VOID, JSPROP_NATIVE_ACCESSORS, JSPROP_GETTER};
@@ -161,57 +161,6 @@ pub fn jsid_to_str(cx: *JSContext, id: jsid) -> DOMString {
     }
 }
 
-#[deriving(Eq)]
-pub enum StringificationBehavior {
-    Default,
-    Empty,
-}
-
-pub fn jsval_to_str(cx: *JSContext, v: JSVal,
-                    nullBehavior: StringificationBehavior) -> Result<DOMString, ()> {
-    if v.is_null() && nullBehavior == Empty {
-        Ok(~"")
-    } else {
-        let jsstr = unsafe { JS_ValueToString(cx, v) };
-        if jsstr.is_null() {
-            debug!("JS_ValueToString failed");
-            Err(())
-        } else {
-            Ok(jsstring_to_str(cx, jsstr))
-        }
-    }
-}
-
-pub fn jsval_to_domstring(cx: *JSContext, v: JSVal) -> Result<Option<DOMString>, ()> {
-    if v.is_null_or_undefined() {
-        Ok(None)
-    } else {
-        let jsstr = unsafe { JS_ValueToString(cx, v) };
-        if jsstr.is_null() {
-            debug!("JS_ValueToString failed");
-            Err(())
-        } else {
-            Ok(Some(jsstring_to_str(cx, jsstr)))
-        }
-    }
-}
-
-pub unsafe fn str_to_jsval(cx: *JSContext, string: DOMString) -> JSVal {
-    let string_utf16 = string.to_utf16();
-    let jsstr = JS_NewUCStringCopyN(cx, string_utf16.as_ptr(), string_utf16.len() as libc::size_t);
-    if jsstr.is_null() {
-        fail!("JS_NewUCStringCopyN failed");
-    }
-    StringValue(&*jsstr)
-}
-
-pub unsafe fn domstring_to_jsval(cx: *JSContext, string: Option<DOMString>) -> JSVal {
-    match string {
-        None => NullValue(),
-        Some(s) => str_to_jsval(cx, s),
-    }
-}
-
 // We use slot 0 for holding the raw object.  This is safe for both
 // globals and non-globals.
 pub static DOM_OBJECT_SLOT: uint = 0;