Use Option<T> to return from getters

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

@@ -18,6 +18,7 @@
     BuiltinTypes,
     IDLBuiltinType,
     IDLNullValue,
+    IDLNullableType,
     IDLType,
     IDLInterfaceMember,
     IDLUndefinedValue,
@@ -1349,7 +1350,10 @@ def getRetvalDeclarationForType(returnType, descriptorProvider):
     if returnType.isAny():
         return CGGeneric("JSVal")
     if returnType.isObject() or returnType.isSpiderMonkeyInterface():
-        return CGGeneric("*mut JSObject")
+        result = CGGeneric("NonZero<*mut JSObject>")
+        if returnType.nullable():
+            result = CGWrapper(result, pre="Option<", post=">")
+        return result
     if returnType.isSequence():
         result = getRetvalDeclarationForType(innerSequenceType(returnType), descriptorProvider)
         result = CGWrapper(result, pre="Vec<", post=">")
@@ -4552,6 +4556,8 @@ def __init__(self, descriptor, operation):
         signature = operation.signatures()[0]
 
         (returnType, arguments) = signature
+        if operation.isGetter() and not returnType.nullable():
+            returnType = IDLNullableType(returnType.location, returnType)
 
         # We pass len(arguments) as the final argument so that the
         # CGPerSignatureCall won't do any argument conversion of its own.
@@ -4574,8 +4580,6 @@ def __init__(self, descriptor, operation):
             self.cgRoot.prepend(instantiateJSToNativeConversionTemplate(
                 template, templateValues, declType, argument.identifier.name))
             self.cgRoot.prepend(CGGeneric("rooted!(in(cx) let value = desc.value);"))
-        elif operation.isGetter():
-            self.cgRoot.prepend(CGGeneric("let mut found = false;"))
 
     def getArguments(self):
         def process(arg):
@@ -4584,18 +4588,14 @@ def process(arg):
                 argVal += ".r()"
             return argVal
         args = [(a, process(a)) for a in self.arguments]
-        if self.idlNode.isGetter():
-            args.append((FakeArgument(BuiltinTypes[IDLBuiltinType.Types.boolean],
-                                      self.idlNode),
-                         "&mut found"))
         return args
 
     def wrap_return_value(self):
         if not self.idlNode.isGetter() or self.templateValues is None:
             return ""
 
         wrap = CGGeneric(wrapForType(**self.templateValues))
-        wrap = CGIfWrapper("found", wrap)
+        wrap = CGIfWrapper("let Some(result) = result", wrap)
         return "\n" + wrap.define()
 
 
@@ -4971,7 +4971,7 @@ def getBody(self):
                        "    let this = UnwrapProxy(proxy);\n" +
                        "    let this = &*this;\n" +
                        CGIndenter(CGProxyIndexedGetter(self.descriptor)).define() + "\n" +
-                       "    *bp = found;\n" +
+                       "    *bp = result.is_some();\n" +
                        "    return true;\n" +
                        "}\n\n")
         else:
@@ -4987,7 +4987,7 @@ def getBody(self):
     }
     if !has_on_proto {
         %s
-        *bp = found;
+        *bp = result.is_some();
         return true;
     }
 }
@@ -5271,7 +5271,9 @@ def members():
 
                     infallible = 'infallible' in descriptor.getExtendedAttributes(operation)
                     if operation.isGetter():
-                        arguments = method_arguments(descriptor, rettype, arguments, trailing=("found", "&mut bool"))
+                        if not rettype.nullable():
+                            rettype = IDLNullableType(rettype.location, rettype)
+                        arguments = method_arguments(descriptor, rettype, arguments)
 
                         # If this interface 'supports named properties', then we
                         # should be able to access 'supported property names'
@@ -5323,6 +5325,7 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries
         enums = []
 
     return CGImports(cgthings, descriptors, callbacks, dictionaries, enums, [
+        'core::nonzero::NonZero',
         'js',
         'js::JSCLASS_GLOBAL_SLOT_COUNT',
         'js::JSCLASS_IS_DOMJSCLASS',

components/script/dom/bindings/iterable.rs

@@ -6,6 +6,7 @@
 
 //! Implementation of `iterable<...>` and `iterable<..., ...>` WebIDL declarations.
 
+use core::nonzero::NonZero;
 use dom::bindings::codegen::Bindings::IterableIteratorBinding::IterableKeyAndValueResult;
 use dom::bindings::codegen::Bindings::IterableIteratorBinding::IterableKeyOrValueResult;
 use dom::bindings::error::Fallible;
@@ -95,38 +96,41 @@ impl<T: Reflectable + JSTraceable + Iterable> IterableIterator<T> {
 
     /// Return the next value from the iterable object.
     #[allow(non_snake_case)]
-    pub fn Next(&self, cx: *mut JSContext) -> Fallible<*mut JSObject> {
+    pub fn Next(&self, cx: *mut JSContext) -> Fallible<NonZero<*mut JSObject>> {
         let index = self.index.get();
         rooted!(in(cx) let mut value = UndefinedValue());
         rooted!(in(cx) let mut rval = ptr::null_mut());
-        if index >= self.iterable.get_iterable_length() {
-            return dict_return(cx, rval.handle_mut(), true, value.handle())
-                .map(|_| rval.handle().get());
-        }
-        let result = match self.type_ {
-            IteratorType::Keys => {
-                unsafe {
-                    self.iterable.get_key_at_index(index).to_jsval(cx, value.handle_mut());
+        let result = if index >= self.iterable.get_iterable_length() {
+            dict_return(cx, rval.handle_mut(), true, value.handle())
+        } else {
+            match self.type_ {
+                IteratorType::Keys => {
+                    unsafe {
+                        self.iterable.get_key_at_index(index).to_jsval(cx, value.handle_mut());
+                    }
+                    dict_return(cx, rval.handle_mut(), false, value.handle())
                 }
-                dict_return(cx, rval.handle_mut(), false, value.handle())
-            }
-            IteratorType::Values => {
-                unsafe {
-                    self.iterable.get_value_at_index(index).to_jsval(cx, value.handle_mut());
+                IteratorType::Values => {
+                    unsafe {
+                        self.iterable.get_value_at_index(index).to_jsval(cx, value.handle_mut());
+                    }
+                    dict_return(cx, rval.handle_mut(), false, value.handle())
                 }
-                dict_return(cx, rval.handle_mut(), false, value.handle())
-            }
-            IteratorType::Entries => {
-                rooted!(in(cx) let mut key = UndefinedValue());
-                unsafe {
-                    self.iterable.get_key_at_index(index).to_jsval(cx, key.handle_mut());
-                    self.iterable.get_value_at_index(index).to_jsval(cx, value.handle_mut());
+                IteratorType::Entries => {
+                    rooted!(in(cx) let mut key = UndefinedValue());
+                    unsafe {
+                        self.iterable.get_key_at_index(index).to_jsval(cx, key.handle_mut());
+                        self.iterable.get_value_at_index(index).to_jsval(cx, value.handle_mut());
+                    }
+                    key_and_value_return(cx, rval.handle_mut(), key.handle(), value.handle())
                 }
-                key_and_value_return(cx, rval.handle_mut(), key.handle(), value.handle())
             }
         };
         self.index.set(index + 1);
-        result.map(|_| rval.handle().get())
+        result.map(|_| {
+            assert!(!rval.is_null());
+            unsafe { NonZero::new(rval.get()) }
+        })
     }
 }
 

components/script/dom/canvasrenderingcontext2d.rs

@@ -1100,7 +1100,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
                      dirtyY: Finite<f64>,
                      dirtyWidth: Finite<f64>,
                      dirtyHeight: Finite<f64>) {
-        let data = imagedata.get_data_array(&self.global().r());
+        let data = imagedata.get_data_array();
         let offset = Point2D::new(*dx, *dy);
         let image_data_size = Size2D::new(imagedata.Width() as f64, imagedata.Height() as f64);
 

components/script/dom/crypto.rs

@@ -2,6 +2,7 @@
  * 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 core::nonzero::NonZero;
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::CryptoBinding;
 use dom::bindings::codegen::Bindings::CryptoBinding::CryptoMethods;
@@ -43,7 +44,8 @@ impl CryptoMethods for Crypto {
     fn GetRandomValues(&self,
                        _cx: *mut JSContext,
                        input: *mut JSObject)
-                       -> Fallible<*mut JSObject> {
+                       -> Fallible<NonZero<*mut JSObject>> {
+        assert!(!input.is_null());
         let mut data = match unsafe { array_buffer_view_data::<u8>(input) } {
             Some(data) => data,
             None => {
@@ -62,7 +64,7 @@ impl CryptoMethods for Crypto {
 
         self.rng.borrow_mut().fill_bytes(&mut data);
 
-        Ok(input)
+        Ok(unsafe { NonZero::new(input) })
     }
 }
 

components/script/dom/cssstyledeclaration.rs

@@ -100,18 +100,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-item
     fn Item(&self, index: u32) -> DOMString {
-        let index = index as usize;
-        let elem = self.owner.upcast::<Element>();
-        let style_attribute = elem.style_attribute().borrow();
-        style_attribute.as_ref().and_then(|declarations| {
-            declarations.declarations.get(index)
-        }).map(|&(ref declaration, importance)| {
-            let mut css = declaration.to_css_string();
-            if importance.important() {
-                css += " !important";
-            }
-            DOMString::from(css)
-        }).unwrap_or_else(DOMString::new)
+        self.IndexedGetter(index).unwrap_or_default()
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
@@ -333,10 +322,19 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     }
 
     // https://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> DOMString {
-        let rval = self.Item(index);
-        *found = index < self.Length();
-        rval
+    fn IndexedGetter(&self, index: u32) -> Option<DOMString> {
+        let index = index as usize;
+        let elem = self.owner.upcast::<Element>();
+        let style_attribute = elem.style_attribute().borrow();
+        style_attribute.as_ref().and_then(|declarations| {
+            declarations.declarations.get(index)
+        }).map(|&(ref declaration, importance)| {
+            let mut css = declaration.to_css_string();
+            if importance.important() {
+                css += " !important";
+            }
+            DOMString::from(css)
+        })
     }
 
     // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext

components/script/dom/document.rs

@@ -2,6 +2,7 @@
  * 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 core::nonzero::NonZero;
 use document_loader::{DocumentLoader, LoadType};
 use dom::activation::{ActivationSource, synthetic_click_activation};
 use dom::attr::Attr;
@@ -116,7 +117,6 @@ use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::default::Default;
 use std::iter::once;
 use std::mem;
-use std::ptr;
 use std::rc::Rc;
 use std::sync::Arc;
 use string_cache::{Atom, QualName};
@@ -2689,8 +2689,9 @@ impl DocumentMethods for Document {
         self.set_body_attribute(&atom!("text"), value)
     }
 
+    #[allow(unsafe_code)]
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
-    fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString, found: &mut bool) -> *mut JSObject {
+    fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option<NonZero<*mut JSObject>> {
         #[derive(JSTraceable, HeapSizeOf)]
         struct NamedElementFilter {
             name: Atom,
@@ -2756,23 +2757,24 @@ impl DocumentMethods for Document {
                                    .peekable();
             if let Some(first) = elements.next() {
                 if elements.peek().is_none() {
-                    *found = true;
                     // TODO: Step 2.
                     // Step 3.
-                    return first.reflector().get_jsobject().get();
+                    return unsafe {
+                        Some(NonZero::new(first.reflector().get_jsobject().get()))
+                    };
                 }
             } else {
-                *found = false;
-                return ptr::null_mut();
+                return None;
             }
         }
         // Step 4.
-        *found = true;
         let filter = NamedElementFilter {
             name: name,
         };
         let collection = HTMLCollection::create(self.window(), root, box filter);
-        collection.reflector().get_jsobject().get()
+        unsafe {
+            Some(NonZero::new(collection.reflector().get_jsobject().get()))
+        }
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names

components/script/dom/domrectlist.rs

@@ -52,8 +52,7 @@ impl DOMRectListMethods for DOMRectList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<DOMRect>> {
-        *found = index < self.rects.len() as u32;
+    fn IndexedGetter(&self, index: u32) -> Option<Root<DOMRect>> {
         self.Item(index)
     }
 }

components/script/dom/domstringmap.rs

@@ -47,10 +47,8 @@ impl DOMStringMapMethods for DOMStringMap {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-domstringmap-nameditem
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> DOMString {
-        let attr = self.element.get_custom_attr(name);
-        *found = attr.is_some();
-        attr.unwrap_or_default()
+    fn NamedGetter(&self, name: DOMString) -> Option<DOMString> {
+        self.element.get_custom_attr(name)
     }
 
     // https://html.spec.whatwg.org/multipage/#the-domstringmap-interface:supported-property-names

components/script/dom/domtokenlist.rs

@@ -171,9 +171,7 @@ impl DOMTokenListMethods for DOMTokenList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<DOMString> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<DOMString> {
+        self.Item(index)
     }
 }

components/script/dom/filelist.rs

@@ -55,9 +55,7 @@ impl FileListMethods for FileList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<File>> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<Root<File>> {
+        self.Item(index)
     }
 }

components/script/dom/htmlcanvaselement.rs

@@ -273,11 +273,10 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement {
         // Step 3.
         let raw_data = match *self.context.borrow() {
             Some(CanvasContext::Context2d(ref context)) => {
-                let window = window_from_node(self);
                 let image_data = try!(context.GetImageData(Finite::wrap(0f64), Finite::wrap(0f64),
                                                            Finite::wrap(self.Width() as f64),
                                                            Finite::wrap(self.Height() as f64)));
-                image_data.get_data_array(&GlobalRef::Window(window.r()))
+                image_data.get_data_array()
             }
             None => {
                 // Each pixel is fully-transparent black.

components/script/dom/htmlcollection.rs

@@ -317,17 +317,13 @@ impl HTMLCollectionMethods for HTMLCollection {
     }
 
     // https://dom.spec.whatwg.org/#dom-htmlcollection-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Element>> {
-        let maybe_elem = self.Item(index);
-        *found = maybe_elem.is_some();
-        maybe_elem
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Element>> {
+        self.Item(index)
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option<Root<Element>> {
-        let maybe_elem = self.NamedItem(name);
-        *found = maybe_elem.is_some();
-        maybe_elem
+    fn NamedGetter(&self, name: DOMString) -> Option<Root<Element>> {
+        self.NamedItem(name)
     }
 
     // https://dom.spec.whatwg.org/#interface-htmlcollection