From 58a8cfda5278a43feecefb95dfce6b0654cdae7d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 7 Apr 2015 13:11:45 +0200 Subject: [PATCH 1/3] Drop the FromJSValConvertible implementation for interfaces. It doesn't really fit in the design, and native_from_reflector_jsmanaged has gained the usability improvements that it used to lack. --- components/script/dom/bindings/conversions.rs | 11 ----------- components/script/dom/bindings/global.rs | 9 +++------ 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index 1a96dc7c948..35eb5038bc9 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -569,17 +569,6 @@ pub fn native_from_reflector_jsmanaged(mut obj: *mut JSObject) -> Result FromJSValConvertible for Unrooted { - type Config = (); - fn from_jsval(_cx: *mut JSContext, value: JSVal, _option: ()) - -> Result, ()> { - if !value.is_object() { - return Err(()); - } - native_from_reflector_jsmanaged(value.to_object()) - } -} - impl ToJSValConvertible for Root { fn to_jsval(&self, cx: *mut JSContext) -> JSVal { self.r().reflector().to_jsval(cx) diff --git a/components/script/dom/bindings/global.rs b/components/script/dom/bindings/global.rs index 316c51a67e3..233143851e6 100644 --- a/components/script/dom/bindings/global.rs +++ b/components/script/dom/bindings/global.rs @@ -7,7 +7,7 @@ //! This module contains smart pointers to global scopes, to simplify writing //! code that works in workers as well as window scopes. -use dom::bindings::conversions::FromJSValConvertible; +use dom::bindings::conversions::native_from_reflector_jsmanaged; use dom::bindings::js::{JS, JSRef, Root, Unrooted}; use dom::bindings::utils::{Reflectable, Reflector}; use dom::workerglobalscope::{WorkerGlobalScope, WorkerGlobalScopeHelpers}; @@ -22,11 +22,8 @@ use js::{JSCLASS_IS_GLOBAL, JSCLASS_IS_DOMJSCLASS}; use js::glue::{GetGlobalForObjectCrossCompartment}; use js::jsapi::{JSContext, JSObject}; use js::jsapi::{JS_GetClass}; -use js::jsval::ObjectOrNullValue; use url::Url; -use std::ptr; - /// A freely-copyable reference to a rooted global object. #[derive(Copy)] pub enum GlobalRef<'a> { @@ -189,12 +186,12 @@ pub fn global_object_for_js_object(obj: *mut JSObject) -> GlobalUnrooted { let global = GetGlobalForObjectCrossCompartment(obj); let clasp = JS_GetClass(global); assert!(((*clasp).flags & (JSCLASS_IS_DOMJSCLASS | JSCLASS_IS_GLOBAL)) != 0); - match FromJSValConvertible::from_jsval(ptr::null_mut(), ObjectOrNullValue(global), ()) { + match native_from_reflector_jsmanaged(global) { Ok(window) => return GlobalUnrooted::Window(window), Err(_) => (), } - match FromJSValConvertible::from_jsval(ptr::null_mut(), ObjectOrNullValue(global), ()) { + match native_from_reflector_jsmanaged(global) { Ok(worker) => return GlobalUnrooted::Worker(worker), Err(_) => (), } From e3683c85981c231621a624bdb2da62cb9cb66b45 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 7 Apr 2015 13:34:06 +0200 Subject: [PATCH 2/3] Merge the To/FromJSValConvertible implementations for Finite. --- components/script/dom/bindings/conversions.rs | 36 ++++++------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index 35eb5038bc9..f027b8b8504 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -58,6 +58,7 @@ use libc; use std::borrow::ToOwned; use std::default; use std::marker::MarkerTrait; +use std::num::Float; use std::slice; /// A trait to retrieve the constants necessary to check if a `JSObject` @@ -257,24 +258,6 @@ impl FromJSValConvertible for f32 { } } -impl ToJSValConvertible for Finite { - fn to_jsval(&self, cx: *mut JSContext) -> JSVal { - let value = **self; - value.to_jsval(cx) - } -} - -impl FromJSValConvertible for Finite { - type Config = (); - fn from_jsval(cx: *mut JSContext, val: JSVal, option: ()) -> Result, ()> { - let result = FromJSValConvertible::from_jsval(cx, val, option); - let result = result.and_then(|v| { - Finite::::new(v).ok_or(()) - }); - result - } -} - impl ToJSValConvertible for f64 { fn to_jsval(&self, _cx: *mut JSContext) -> JSVal { unsafe { @@ -290,7 +273,7 @@ impl FromJSValConvertible for f64 { } } -impl ToJSValConvertible for Finite { +impl ToJSValConvertible for Finite { #[inline] fn to_jsval(&self, cx: *mut JSContext) -> JSVal { let value = **self; @@ -298,14 +281,15 @@ impl ToJSValConvertible for Finite { } } -impl FromJSValConvertible for Finite { +impl> FromJSValConvertible for Finite { type Config = (); - fn from_jsval(cx: *mut JSContext, val: JSVal, option: ()) -> Result, ()> { - let result = FromJSValConvertible::from_jsval(cx, val, option); - let result = result.and_then(|v| { - Finite::::new(v).ok_or(()) - }); - result + + fn from_jsval(cx: *mut JSContext, value: JSVal, option: ()) -> Result, ()> { + let result = try!(FromJSValConvertible::from_jsval(cx, value, option)); + match Finite::new(result) { + Some(v) => Ok(v), + None => Err(()), + } } } From 6b79d57920a6f91e2fa4f9464e2c46b93ae3486d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 7 Apr 2015 13:35:28 +0200 Subject: [PATCH 3/3] When converting a non-finite float, throw the TypeError from the FromJSValConvertible implementation. This removes some unnecessary custom code in the codegen and makes this implementation follow the convention of having thrown an exception when returning Err() from FromJSValConvertible. --- .../dom/bindings/codegen/CodegenRust.py | 23 +++++-------------- components/script/dom/bindings/conversions.rs | 6 ++++- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index b828ccb74bd..adebf601568 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -887,23 +887,12 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.nullable(): declType = CGWrapper(declType, pre="Option<", post=">") - template = "" - if type.isFloat() and not type.isUnrestricted(): - template = ( - "match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n" - " Ok(v) => v,\n" - " Err(_) => {\n" - " throw_type_error(cx, \"this argument is not a finite floating-point value\");\n" - " %s\n" - " }\n" - "}" % exceptionCode) - else: - #XXXjdm support conversionBehavior here - template = ( - "match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n" - " Ok(v) => v,\n" - " Err(_) => { %s }\n" - "}" % exceptionCode) + #XXXjdm support conversionBehavior here + template = ( + "match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n" + " Ok(v) => v,\n" + " Err(_) => { %s }\n" + "}" % exceptionCode) if defaultValue is not None: if isinstance(defaultValue, IDLNullValue): diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index f027b8b8504..eb6d7d60680 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -33,6 +33,7 @@ //! | union types | `T` | use dom::bindings::codegen::PrototypeList; +use dom::bindings::error::throw_type_error; use dom::bindings::js::{JSRef, Root, Unrooted}; use dom::bindings::num::Finite; use dom::bindings::str::{ByteString, USVString}; @@ -288,7 +289,10 @@ impl> FromJSValConvertible for Finite let result = try!(FromJSValConvertible::from_jsval(cx, value, option)); match Finite::new(result) { Some(v) => Ok(v), - None => Err(()), + None => { + throw_type_error(cx, "this argument is not a finite floating-point value"); + Err(()) + }, } } }