Auto merge of #6378 - Ms2ger:callable, r=nox

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6378)
<!-- Reviewable:end -->

components/script/dom/bindings/callback.rs

@@ -4,6 +4,7 @@
 
 //! Base classes to work with IDL callbacks.
 
+use dom::bindings::error::{Fallible, Error};
 use dom::bindings::global::global_object_for_js_object;
 use dom::bindings::js::JSRef;
 use dom::bindings::utils::Reflectable;
@@ -93,22 +94,20 @@ impl CallbackInterface {
     }
 
     /// Returns the property with the given `name`, if it is a callable object,
-    /// or `Err(())` otherwise. If it returns `Err(())`, a JSAPI exception is
+    /// or an error otherwise.
-    /// pending.
     pub fn get_callable_property(&self, cx: *mut JSContext, name: &str)
-                                 -> Result<JSVal, ()> {
+                                 -> Fallible<JSVal> {
         let mut callable = UndefinedValue();
         unsafe {
-            let name = CString::new(name).unwrap();
+            let c_name = CString::new(name).unwrap();
-            if JS_GetProperty(cx, self.callback(), name.as_ptr(), &mut callable) == 0 {
+            if JS_GetProperty(cx, self.callback(), c_name.as_ptr(), &mut callable) == 0 {
-                return Err(());
+                return Err(Error::JSFailed);
             }
 
             if !callable.is_object() ||
                JS_ObjectIsCallable(cx, callable.to_object()) == 0 {
-                // FIXME(#347)
+                return Err(Error::Type(
-                //ThrowErrorMessage(cx, MSG_NOT_CALLABLE, description.get());
+                    format!("The value of the {} property is not callable", name)));
-                return Err(());
             }
         }
         Ok(callable)

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

@@ -5304,10 +5304,8 @@ class CallbackOperationBase(CallbackMethod):
             "methodName": self.methodName
         }
         getCallableFromProp = string.Template(
-                'match self.parent.get_callable_property(cx, "${methodName}") {\n'
+            'try!(self.parent.get_callable_property(cx, "${methodName}"))'
-                '    Err(_) => return Err(JSFailed),\n'
+        ).substitute(replacements)
-                '    Ok(callable) => callable,\n'
-                '}').substitute(replacements)
         if not self.singleOperation:
             return 'JS::Rooted<JS::Value> callable(cx);\n' + getCallableFromProp
         return (

tests/wpt/metadata/dom/traversal/TreeWalker-acceptNode-filter.html.ini

@@ -1,3 +1,8 @@
 [TreeWalker-acceptNode-filter.html]
   type: testharness
-  expected: CRASH
+  [Testing with filter function that throws]
+    expected: FAIL
+
+  [Testing with filter object that throws]
+    expected: FAIL
+

tests/wpt/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html

@@ -85,23 +85,21 @@ test(function()
     assert_node(walker.currentNode, { type: Element, id: 'B1' });
 }, 'Testing with undefined filter');
 
-// XXX Servo breaks the test when a callback isn't callable
 test(function()
 {
     var walker = document.createTreeWalker(testElement, /*NodeFilter.*/SHOW_ELEMENT, {});
-    assert_throws(null, function () { walker.firstChild(); });
+    assert_throws(new TypeError(), function () { walker.firstChild(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
-    assert_throws(null, function () { walker.nextNode(); });
+    assert_throws(new TypeError(), function () { walker.nextNode(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
 }, 'Testing with object lacking acceptNode property');
 
-// XXX Servo breaks the test when a callback isn't callable
 test(function()
 {
     var walker = document.createTreeWalker(testElement, /*NodeFilter.*/SHOW_ELEMENT, { acceptNode: "foo" });
-    assert_throws(null, function () { walker.firstChild(); });
+    assert_throws(new TypeError(), function () { walker.firstChild(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
-    assert_throws(null, function () { walker.nextNode(); });
+    assert_throws(new TypeError(), function () { walker.nextNode(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
 }, 'Testing with object with non-function acceptNode property');
 
@@ -125,33 +123,31 @@ test(function()
     assert_node(walker.firstChild(), { type: Element, id: 'A1' });
 }, 'Testing acceptNode callee');
 
-// XXX Looks like Servo is doing something wrong when a callback function throws
 test(function()
 {
+    var test_error = { name: "test" };
     var walker = document.createTreeWalker(testElement, /*NodeFilter.*/SHOW_ELEMENT,
                                            function(node) {
-                                               throw('filter exception');
+                                               throw test_error;
-                                               return /*NodeFilter.*/FILTER_ACCEPT;
                                            });
-    assert_throws(null, function () { walker.firstChild(); });
+    assert_throws(test_error, function () { walker.firstChild(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
-    assert_throws(null, function () { walker.nextNode(); });
+    assert_throws(test_error, function () { walker.nextNode(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
 }, 'Testing with filter function that throws');
 
-// XXX Looks like Servo is doing something wrong when a callback function throws
 test(function()
 {
+    var test_error = { name: "test" };
     var walker = document.createTreeWalker(testElement, /*NodeFilter.*/SHOW_ELEMENT,
                                            {
                                                acceptNode : function(node) {
-                                                   throw('filter exception');
+                                                   throw test_error;
-                                                   return /*NodeFilter.*/FILTER_ACCEPT;
                                                }
                                            });
-    assert_throws(null, function () { walker.firstChild(); });
+    assert_throws(test_error, function () { walker.firstChild(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
-    assert_throws(null, function () { walker.nextNode(); });
+    assert_throws(test_error, function () { walker.nextNode(); });
     assert_node(walker.currentNode, { type: Element, id: 'root' });
 }, 'Testing with filter object that throws');