From 854a3dff681fbc17c7d8cd9706274115efd07169 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 28 Aug 2016 17:08:49 +0200 Subject: [PATCH 1/6] Assert that ImageData::data is not null --- components/script/dom/imagedata.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/script/dom/imagedata.rs b/components/script/dom/imagedata.rs index abcb9b5640b..85376c0d4ad 100644 --- a/components/script/dom/imagedata.rs +++ b/components/script/dom/imagedata.rs @@ -37,6 +37,7 @@ impl ImageData { unsafe { let cx = global.get_cx(); let js_object: *mut JSObject = JS_NewUint8ClampedArray(cx, width * height * 4); + assert!(!js_object.is_null()); if let Some(vec) = data { let mut is_shared = false; @@ -58,6 +59,7 @@ impl ImageData { let mut is_shared = false; let data: *const uint8_t = JS_GetUint8ClampedArrayData(self.Data(cx), &mut is_shared, ptr::null()) as *const uint8_t; + assert!(!data.is_null()); assert!(!is_shared); let len = self.Width() * self.Height() * 4; slice::from_raw_parts(data, len as usize).to_vec() @@ -82,6 +84,7 @@ impl ImageDataMethods for ImageData { // https://html.spec.whatwg.org/multipage/#dom-imagedata-data fn Data(&self, _: *mut JSContext) -> *mut JSObject { + assert!(!self.data.get().is_null()); self.data.get() } } From 2400b91d05fe314879718b6183a5e40c37dbbdd5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 28 Aug 2016 17:10:03 +0200 Subject: [PATCH 2/6] Don't bother with the global in ImageData::get_image_data --- components/script/dom/canvasrenderingcontext2d.rs | 2 +- components/script/dom/htmlcanvaselement.rs | 3 +-- components/script/dom/imagedata.rs | 6 +++--- components/script/dom/webglrenderingcontext.rs | 3 +-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs index f3c74b59250..61cc2520165 100644 --- a/components/script/dom/canvasrenderingcontext2d.rs +++ b/components/script/dom/canvasrenderingcontext2d.rs @@ -1100,7 +1100,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D { dirtyY: Finite, dirtyWidth: Finite, dirtyHeight: Finite) { - 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); diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index 45c1916f928..73b51962fe4 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/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. diff --git a/components/script/dom/imagedata.rs b/components/script/dom/imagedata.rs index 85376c0d4ad..011d155eb26 100644 --- a/components/script/dom/imagedata.rs +++ b/components/script/dom/imagedata.rs @@ -53,12 +53,12 @@ impl ImageData { } #[allow(unsafe_code)] - pub fn get_data_array(&self, global: &GlobalRef) -> Vec { + pub fn get_data_array(&self) -> Vec { unsafe { - let cx = global.get_cx(); let mut is_shared = false; + assert!(!self.data.get().is_null()); let data: *const uint8_t = - JS_GetUint8ClampedArrayData(self.Data(cx), &mut is_shared, ptr::null()) as *const uint8_t; + JS_GetUint8ClampedArrayData(self.data.get(), &mut is_shared, ptr::null()) as *const uint8_t; assert!(!data.is_null()); assert!(!is_shared); let len = self.Width() * self.Height() * 4; diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 5ff793527ec..51f79503e84 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -305,8 +305,7 @@ impl WebGLRenderingContext { // complexity is worth it. let (pixels, size) = match source { ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement::ImageData(image_data) => { - let global = self.global(); - (image_data.get_data_array(&global.r()), image_data.get_size()) + (image_data.get_data_array(), image_data.get_size()) }, ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement::HTMLImageElement(image) => { let img_url = match image.get_url() { From 897a81e95bb0cfd165d1bd2b417dc967dc5199c8 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 28 Aug 2016 20:33:34 +0200 Subject: [PATCH 3/6] Improve some TestBinding methods We make them return sensical things in a sensical way. --- components/script/dom/testbinding.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 9143c8b0188..a11afb19c0d 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -26,6 +26,7 @@ use dom::bindings::weakref::MutableWeakRef; use dom::blob::{Blob, BlobImpl}; use dom::url::URL; use js::jsapi::{HandleObject, HandleValue, JSContext, JSObject}; +use js::jsapi::{JS_NewPlainObject, JS_NewUint8ClampedArray}; use js::jsval::{JSVal, NullValue}; use std::borrow::ToOwned; use std::ptr; @@ -137,10 +138,24 @@ impl TestBindingMethods for TestBinding { ByteStringOrLong::ByteString(ByteString::new(vec!())) } fn SetUnion9Attribute(&self, _: ByteStringOrLong) {} - fn ArrayAttribute(&self, _: *mut JSContext) -> *mut JSObject { NullValue().to_object_or_null() } + #[allow(unsafe_code)] + fn ArrayAttribute(&self, cx: *mut JSContext) -> *mut JSObject { + unsafe { + rooted!(in(cx) let array = JS_NewUint8ClampedArray(cx, 16)); + assert!(!array.is_null()); + array.get() + } + } fn AnyAttribute(&self, _: *mut JSContext) -> JSVal { NullValue() } fn SetAnyAttribute(&self, _: *mut JSContext, _: HandleValue) {} - fn ObjectAttribute(&self, _: *mut JSContext) -> *mut JSObject { panic!() } + #[allow(unsafe_code)] + fn ObjectAttribute(&self, cx: *mut JSContext) -> *mut JSObject { + unsafe { + rooted!(in(cx) let obj = JS_NewPlainObject(cx)); + assert!(!obj.is_null()); + obj.get() + } + } fn SetObjectAttribute(&self, _: *mut JSContext, _: *mut JSObject) {} fn GetBooleanAttributeNullable(&self) -> Option { Some(false) } @@ -242,7 +257,9 @@ impl TestBindingMethods for TestBinding { Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned()) } fn ReceiveAny(&self, _: *mut JSContext) -> JSVal { NullValue() } - fn ReceiveObject(&self, _: *mut JSContext) -> *mut JSObject { panic!() } + fn ReceiveObject(&self, cx: *mut JSContext) -> *mut JSObject { + self.ObjectAttribute(cx) + } fn ReceiveUnion(&self) -> HTMLElementOrLong { HTMLElementOrLong::Long(0) } fn ReceiveUnion2(&self) -> EventOrString { EventOrString::String(DOMString::new()) } fn ReceiveUnion3(&self) -> StringOrLongSequence { StringOrLongSequence::LongSequence(vec![]) } @@ -283,7 +300,9 @@ impl TestBindingMethods for TestBinding { fn ReceiveNullableInterface(&self) -> Option> { Some(Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned())) } - fn ReceiveNullableObject(&self, _: *mut JSContext) -> *mut JSObject { ptr::null_mut() } + fn ReceiveNullableObject(&self, cx: *mut JSContext) -> *mut JSObject { + self.GetObjectAttributeNullable(cx) + } fn ReceiveNullableUnion(&self) -> Option { Some(HTMLElementOrLong::Long(0)) } From 3e32948a39ced28c12f38ce115abcb56e67e72d9 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 28 Aug 2016 21:50:23 +0200 Subject: [PATCH 4/6] Root js_object in TextEncoder::Encode --- components/script/dom/textencoder.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/script/dom/textencoder.rs b/components/script/dom/textencoder.rs index 84100e723ba..69ce6048ae3 100644 --- a/components/script/dom/textencoder.rs +++ b/components/script/dom/textencoder.rs @@ -74,12 +74,13 @@ impl TextEncoderMethods for TextEncoder { unsafe { let encoded = self.encoder.encode(&input.0, EncoderTrap::Strict).unwrap(); let length = encoded.len() as u32; - let js_object: *mut JSObject = JS_NewUint8Array(cx, length); + rooted!(in(cx) let js_object = JS_NewUint8Array(cx, length)); + assert!(!js_object.is_null()); let mut is_shared = false; - let js_object_data: *mut uint8_t = JS_GetUint8ArrayData(js_object, &mut is_shared, ptr::null()); + let js_object_data: *mut uint8_t = JS_GetUint8ArrayData(js_object.get(), &mut is_shared, ptr::null()); assert!(!is_shared); ptr::copy_nonoverlapping(encoded.as_ptr(), js_object_data, length as usize); - js_object + js_object.get() } } } From 6e1523f4ae61c16578a462c2e5335cbc95a6ef04 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 29 Aug 2016 00:35:17 +0200 Subject: [PATCH 5/6] Compile WebIDL return type "object" to NonZero<*mut JSObject> --- .../dom/bindings/codegen/CodegenRust.py | 6 ++- components/script/dom/bindings/iterable.rs | 50 ++++++++++--------- components/script/dom/crypto.rs | 6 ++- components/script/dom/document.rs | 20 +++++--- components/script/dom/imagedata.rs | 6 ++- components/script/dom/testbinding.rs | 15 +++--- components/script/dom/textencoder.rs | 5 +- .../script/dom/webglrenderingcontext.rs | 6 ++- components/script/dom/xmldocument.rs | 4 +- components/servo/Cargo.lock | 2 +- ports/cef/Cargo.lock | 2 +- 11 files changed, 74 insertions(+), 48 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 7842e08bfa7..8a169fe2c31 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1349,7 +1349,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=">") @@ -5323,6 +5326,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', diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs index 20eb84bffde..a3092ed73b6 100644 --- a/components/script/dom/bindings/iterable.rs +++ b/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 IterableIterator { /// 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> { 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()) } + }) } } diff --git a/components/script/dom/crypto.rs b/components/script/dom/crypto.rs index 6e0351d37ee..165a3834227 100644 --- a/components/script/dom/crypto.rs +++ b/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> { + assert!(!input.is_null()); let mut data = match unsafe { array_buffer_view_data::(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) }) } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index cbd4b2ac253..6a06adcafc7 100644 --- a/components/script/dom/document.rs +++ b/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; @@ -90,7 +91,7 @@ use euclid::point::Point2D; use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode}; use ipc_channel::ipc::{self, IpcSender}; use js::jsapi::JS_GetRuntime; -use js::jsapi::{JSContext, JSObject, JSRuntime}; +use js::jsapi::{JSContext, JSObject, JSRuntime, JS_NewPlainObject}; use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER}; use msg::constellation_msg::{Key, KeyModifiers, KeyState}; use msg::constellation_msg::{PipelineId, ReferrerPolicy, SubpageId}; @@ -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,10 @@ 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, found: &mut bool) + -> NonZero<*mut JSObject> { #[derive(JSTraceable, HeapSizeOf)] struct NamedElementFilter { name: Atom, @@ -2759,11 +2761,15 @@ impl DocumentMethods for Document { *found = true; // TODO: Step 2. // Step 3. - return first.reflector().get_jsobject().get(); + return unsafe { + NonZero::new(first.reflector().get_jsobject().get()) + }; } } else { *found = false; - return ptr::null_mut(); + return unsafe { + NonZero::new(JS_NewPlainObject(cx)) + }; } } // Step 4. @@ -2772,7 +2778,9 @@ impl DocumentMethods for Document { name: name, }; let collection = HTMLCollection::create(self.window(), root, box filter); - collection.reflector().get_jsobject().get() + unsafe { + NonZero::new(collection.reflector().get_jsobject().get()) + } } // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names diff --git a/components/script/dom/imagedata.rs b/components/script/dom/imagedata.rs index 011d155eb26..0959a52eb32 100644 --- a/components/script/dom/imagedata.rs +++ b/components/script/dom/imagedata.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::codegen::Bindings::ImageDataBinding; use dom::bindings::codegen::Bindings::ImageDataBinding::ImageDataMethods; use dom::bindings::global::GlobalRef; @@ -82,9 +83,10 @@ impl ImageDataMethods for ImageData { self.height } + #[allow(unsafe_code)] // https://html.spec.whatwg.org/multipage/#dom-imagedata-data - fn Data(&self, _: *mut JSContext) -> *mut JSObject { + fn Data(&self, _: *mut JSContext) -> NonZero<*mut JSObject> { assert!(!self.data.get().is_null()); - self.data.get() + unsafe { NonZero::new(self.data.get()) } } } diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index a11afb19c0d..21439c0b19a 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -4,6 +4,7 @@ // check-tidy: no specs after this line +use core::nonzero::NonZero; use dom::bindings::codegen::Bindings::EventListenerBinding::EventListener; use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::codegen::Bindings::TestBindingBinding; @@ -139,21 +140,21 @@ impl TestBindingMethods for TestBinding { } fn SetUnion9Attribute(&self, _: ByteStringOrLong) {} #[allow(unsafe_code)] - fn ArrayAttribute(&self, cx: *mut JSContext) -> *mut JSObject { + fn ArrayAttribute(&self, cx: *mut JSContext) -> NonZero<*mut JSObject> { unsafe { rooted!(in(cx) let array = JS_NewUint8ClampedArray(cx, 16)); assert!(!array.is_null()); - array.get() + NonZero::new(array.get()) } } fn AnyAttribute(&self, _: *mut JSContext) -> JSVal { NullValue() } fn SetAnyAttribute(&self, _: *mut JSContext, _: HandleValue) {} #[allow(unsafe_code)] - fn ObjectAttribute(&self, cx: *mut JSContext) -> *mut JSObject { + fn ObjectAttribute(&self, cx: *mut JSContext) -> NonZero<*mut JSObject> { unsafe { rooted!(in(cx) let obj = JS_NewPlainObject(cx)); assert!(!obj.is_null()); - obj.get() + NonZero::new(obj.get()) } } fn SetObjectAttribute(&self, _: *mut JSContext, _: *mut JSObject) {} @@ -208,7 +209,7 @@ impl TestBindingMethods for TestBinding { fn SetInterfaceAttributeWeak(&self, url: Option<&URL>) { self.url.set(url); } - fn GetObjectAttributeNullable(&self, _: *mut JSContext) -> *mut JSObject { ptr::null_mut() } + fn GetObjectAttributeNullable(&self, _: *mut JSContext) -> Option> { None } fn SetObjectAttributeNullable(&self, _: *mut JSContext, _: *mut JSObject) {} fn GetUnionAttributeNullable(&self) -> Option { Some(HTMLElementOrLong::Long(0)) @@ -257,7 +258,7 @@ impl TestBindingMethods for TestBinding { Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned()) } fn ReceiveAny(&self, _: *mut JSContext) -> JSVal { NullValue() } - fn ReceiveObject(&self, cx: *mut JSContext) -> *mut JSObject { + fn ReceiveObject(&self, cx: *mut JSContext) -> NonZero<*mut JSObject> { self.ObjectAttribute(cx) } fn ReceiveUnion(&self) -> HTMLElementOrLong { HTMLElementOrLong::Long(0) } @@ -300,7 +301,7 @@ impl TestBindingMethods for TestBinding { fn ReceiveNullableInterface(&self) -> Option> { Some(Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned())) } - fn ReceiveNullableObject(&self, cx: *mut JSContext) -> *mut JSObject { + fn ReceiveNullableObject(&self, cx: *mut JSContext) -> Option> { self.GetObjectAttributeNullable(cx) } fn ReceiveNullableUnion(&self) -> Option { diff --git a/components/script/dom/textencoder.rs b/components/script/dom/textencoder.rs index 69ce6048ae3..674243dded8 100644 --- a/components/script/dom/textencoder.rs +++ b/components/script/dom/textencoder.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::codegen::Bindings::TextEncoderBinding; use dom::bindings::codegen::Bindings::TextEncoderBinding::TextEncoderMethods; use dom::bindings::error::{Error, Fallible}; @@ -70,7 +71,7 @@ impl TextEncoderMethods for TextEncoder { #[allow(unsafe_code)] // https://encoding.spec.whatwg.org/#dom-textencoder-encode - fn Encode(&self, cx: *mut JSContext, input: USVString) -> *mut JSObject { + fn Encode(&self, cx: *mut JSContext, input: USVString) -> NonZero<*mut JSObject> { unsafe { let encoded = self.encoder.encode(&input.0, EncoderTrap::Strict).unwrap(); let length = encoded.len() as u32; @@ -80,7 +81,7 @@ impl TextEncoderMethods for TextEncoder { let js_object_data: *mut uint8_t = JS_GetUint8ArrayData(js_object.get(), &mut is_shared, ptr::null()); assert!(!is_shared); ptr::copy_nonoverlapping(encoded.as_ptr(), js_object_data, length as usize); - js_object.get() + NonZero::new(js_object.get()) } } } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 51f79503e84..aadf03d4211 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use canvas_traits::{CanvasCommonMsg, CanvasMsg, byte_swap}; +use core::nonzero::NonZero; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextMethods; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{self, WebGLContextAttributes}; @@ -631,8 +632,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.14 - fn GetExtension(&self, _cx: *mut JSContext, _name: DOMString) -> *mut JSObject { - 0 as *mut JSObject + fn GetExtension(&self, _cx: *mut JSContext, _name: DOMString) + -> Option> { + None } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3 diff --git a/components/script/dom/xmldocument.rs b/components/script/dom/xmldocument.rs index 5f6b6e1919c..348fb4056cc 100644 --- a/components/script/dom/xmldocument.rs +++ b/components/script/dom/xmldocument.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; use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods; use dom::bindings::codegen::Bindings::XMLDocumentBinding::{self, XMLDocumentMethods}; @@ -87,7 +88,8 @@ impl XMLDocumentMethods for XMLDocument { } // 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, found: &mut bool) + -> NonZero<*mut JSObject> { self.upcast::().NamedGetter(_cx, name, found) } } diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index 3acb51618b1..27511be4596 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -1086,7 +1086,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.3" -source = "git+https://github.com/servo/rust-mozjs#f06428fab33a6ae633584a1a7e1bf4e3ef7914b3" +source = "git+https://github.com/servo/rust-mozjs#f5444dd82b864a88cf874c66c75aed478fd88d22" dependencies = [ "cmake 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index 6546afb20f2..47d3ed01631 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -994,7 +994,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.3" -source = "git+https://github.com/servo/rust-mozjs#f06428fab33a6ae633584a1a7e1bf4e3ef7914b3" +source = "git+https://github.com/servo/rust-mozjs#f5444dd82b864a88cf874c66c75aed478fd88d22" dependencies = [ "cmake 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", From 7dfb336be8dae1e2be9b898c374b6715e2a00ac7 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 29 Aug 2016 00:55:29 +0200 Subject: [PATCH 6/6] Use Option to return from getters This removes the cumbersome &mut bool argument and offers overall a more readable code. --- .../dom/bindings/codegen/CodegenRust.py | 19 ++++++------ components/script/dom/cssstyledeclaration.rs | 30 +++++++++---------- components/script/dom/document.rs | 16 ++++------ components/script/dom/domrectlist.rs | 3 +- components/script/dom/domstringmap.rs | 6 ++-- components/script/dom/domtokenlist.rs | 6 ++-- components/script/dom/filelist.rs | 6 ++-- components/script/dom/htmlcollection.rs | 12 +++----- .../script/dom/htmlformcontrolscollection.rs | 10 +++---- components/script/dom/htmlformelement.rs | 4 +-- components/script/dom/mimetypearray.rs | 4 +-- components/script/dom/namednodemap.rs | 12 +++----- components/script/dom/nodelist.rs | 6 ++-- components/script/dom/plugin.rs | 4 +-- components/script/dom/pluginarray.rs | 4 +-- components/script/dom/radionodelist.rs | 4 +-- components/script/dom/storage.rs | 6 ++-- components/script/dom/stylesheetlist.rs | 6 ++-- components/script/dom/testbindingiterable.rs | 8 ++--- components/script/dom/testbindingproxy.rs | 4 +-- components/script/dom/touchlist.rs | 6 ++-- components/script/dom/xmldocument.rs | 5 ++-- 22 files changed, 72 insertions(+), 109 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 8a169fe2c31..cf918feb8e3 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -18,6 +18,7 @@ from WebIDL import ( BuiltinTypes, IDLBuiltinType, IDLNullValue, + IDLNullableType, IDLType, IDLInterfaceMember, IDLUndefinedValue, @@ -4555,6 +4556,8 @@ class CGProxySpecialOperation(CGPerSignatureCall): 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. @@ -4577,8 +4580,6 @@ class CGProxySpecialOperation(CGPerSignatureCall): 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): @@ -4587,10 +4588,6 @@ class CGProxySpecialOperation(CGPerSignatureCall): 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): @@ -4598,7 +4595,7 @@ class CGProxySpecialOperation(CGPerSignatureCall): return "" wrap = CGGeneric(wrapForType(**self.templateValues)) - wrap = CGIfWrapper("found", wrap) + wrap = CGIfWrapper("let Some(result) = result", wrap) return "\n" + wrap.define() @@ -4974,7 +4971,7 @@ class CGDOMJSProxyHandler_hasOwn(CGAbstractExternMethod): " 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: @@ -4990,7 +4987,7 @@ if RUST_JSID_IS_STRING(id) { } if !has_on_proto { %s - *bp = found; + *bp = result.is_some(); return true; } } @@ -5274,7 +5271,9 @@ class CGInterfaceTrait(CGThing): 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' diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 076a2283b67..0335a7498c0 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/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::(); - 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 { + let index = index as usize; + let elem = self.owner.upcast::(); + 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 diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 6a06adcafc7..55ef9c9f654 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -91,7 +91,7 @@ use euclid::point::Point2D; use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode}; use ipc_channel::ipc::{self, IpcSender}; use js::jsapi::JS_GetRuntime; -use js::jsapi::{JSContext, JSObject, JSRuntime, JS_NewPlainObject}; +use js::jsapi::{JSContext, JSObject, JSRuntime}; use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER}; use msg::constellation_msg::{Key, KeyModifiers, KeyState}; use msg::constellation_msg::{PipelineId, ReferrerPolicy, SubpageId}; @@ -2691,8 +2691,7 @@ impl DocumentMethods for Document { #[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) - -> NonZero<*mut JSObject> { + fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option> { #[derive(JSTraceable, HeapSizeOf)] struct NamedElementFilter { name: Atom, @@ -2758,28 +2757,23 @@ impl DocumentMethods for Document { .peekable(); if let Some(first) = elements.next() { if elements.peek().is_none() { - *found = true; // TODO: Step 2. // Step 3. return unsafe { - NonZero::new(first.reflector().get_jsobject().get()) + Some(NonZero::new(first.reflector().get_jsobject().get())) }; } } else { - *found = false; - return unsafe { - NonZero::new(JS_NewPlainObject(cx)) - }; + return None; } } // Step 4. - *found = true; let filter = NamedElementFilter { name: name, }; let collection = HTMLCollection::create(self.window(), root, box filter); unsafe { - NonZero::new(collection.reflector().get_jsobject().get()) + Some(NonZero::new(collection.reflector().get_jsobject().get())) } } diff --git a/components/script/dom/domrectlist.rs b/components/script/dom/domrectlist.rs index adb246158f4..86774a49ff3 100644 --- a/components/script/dom/domrectlist.rs +++ b/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> { - *found = index < self.rects.len() as u32; + fn IndexedGetter(&self, index: u32) -> Option> { self.Item(index) } } diff --git a/components/script/dom/domstringmap.rs b/components/script/dom/domstringmap.rs index c5e534d242e..11479d6b5df 100644 --- a/components/script/dom/domstringmap.rs +++ b/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 { + self.element.get_custom_attr(name) } // https://html.spec.whatwg.org/multipage/#the-domstringmap-interface:supported-property-names diff --git a/components/script/dom/domtokenlist.rs b/components/script/dom/domtokenlist.rs index 364b6366282..c1c09de94c2 100644 --- a/components/script/dom/domtokenlist.rs +++ b/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 { - let item = self.Item(index); - *found = item.is_some(); - item + fn IndexedGetter(&self, index: u32) -> Option { + self.Item(index) } } diff --git a/components/script/dom/filelist.rs b/components/script/dom/filelist.rs index 4f8e976f5f7..8adbe1ed467 100644 --- a/components/script/dom/filelist.rs +++ b/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> { - let item = self.Item(index); - *found = item.is_some(); - item + fn IndexedGetter(&self, index: u32) -> Option> { + self.Item(index) } } diff --git a/components/script/dom/htmlcollection.rs b/components/script/dom/htmlcollection.rs index 95134556bc0..2f226590719 100644 --- a/components/script/dom/htmlcollection.rs +++ b/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> { - let maybe_elem = self.Item(index); - *found = maybe_elem.is_some(); - maybe_elem + fn IndexedGetter(&self, index: u32) -> Option> { + self.Item(index) } // check-tidy: no specs after this line - fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option> { - let maybe_elem = self.NamedItem(name); - *found = maybe_elem.is_some(); - maybe_elem + fn NamedGetter(&self, name: DOMString) -> Option> { + self.NamedItem(name) } // https://dom.spec.whatwg.org/#interface-htmlcollection diff --git a/components/script/dom/htmlformcontrolscollection.rs b/components/script/dom/htmlformcontrolscollection.rs index 9229b854b26..e52a541225f 100644 --- a/components/script/dom/htmlformcontrolscollection.rs +++ b/components/script/dom/htmlformcontrolscollection.rs @@ -77,10 +77,8 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection { } // https://html.spec.whatwg.org/multipage/#dom-htmlformcontrolscollection-nameditem - fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option { - let maybe_elem = self.NamedItem(name); - *found = maybe_elem.is_some(); - maybe_elem + fn NamedGetter(&self, name: DOMString) -> Option { + self.NamedItem(name) } // https://html.spec.whatwg.org/multipage/#the-htmlformcontrolscollection-interface:supported-property-names @@ -93,7 +91,7 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection { // https://github.com/servo/servo/issues/5875 // // https://dom.spec.whatwg.org/#dom-htmlcollection-item - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option> { - self.collection.IndexedGetter(index, found) + fn IndexedGetter(&self, index: u32) -> Option> { + self.collection.IndexedGetter(index) } } diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs index db9e3cb8d41..fa16618b564 100644 --- a/components/script/dom/htmlformelement.rs +++ b/components/script/dom/htmlformelement.rs @@ -230,9 +230,9 @@ impl HTMLFormElementMethods for HTMLFormElement { } // https://html.spec.whatwg.org/multipage/#dom-form-item - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option> { + fn IndexedGetter(&self, index: u32) -> Option> { let elements = self.Elements(); - elements.IndexedGetter(index, found) + elements.IndexedGetter(index) } } diff --git a/components/script/dom/mimetypearray.rs b/components/script/dom/mimetypearray.rs index 96fc48c86d0..de820f6d06a 100644 --- a/components/script/dom/mimetypearray.rs +++ b/components/script/dom/mimetypearray.rs @@ -46,12 +46,12 @@ impl MimeTypeArrayMethods for MimeTypeArray { } // https://html.spec.whatwg.org/multipage/#dom-mimetypearray-item - fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option> { + fn IndexedGetter(&self, _index: u32) -> Option> { None } // check-tidy: no specs after this line - fn NamedGetter(&self, _name: DOMString, _found: &mut bool) -> Option> { + fn NamedGetter(&self, _name: DOMString) -> Option> { None } diff --git a/components/script/dom/namednodemap.rs b/components/script/dom/namednodemap.rs index 10b6b8982ae..9edc1b1e93b 100644 --- a/components/script/dom/namednodemap.rs +++ b/components/script/dom/namednodemap.rs @@ -85,17 +85,13 @@ impl NamedNodeMapMethods for NamedNodeMap { } // https://dom.spec.whatwg.org/#dom-namednodemap-item - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option> { - let item = self.Item(index); - *found = item.is_some(); - item + fn IndexedGetter(&self, index: u32) -> Option> { + self.Item(index) } // check-tidy: no specs after this line - fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option> { - let item = self.GetNamedItem(name); - *found = item.is_some(); - item + fn NamedGetter(&self, name: DOMString) -> Option> { + self.GetNamedItem(name) } // https://heycam.github.io/webidl/#dfn-supported-property-names diff --git a/components/script/dom/nodelist.rs b/components/script/dom/nodelist.rs index 2503378187e..8f8a5515592 100644 --- a/components/script/dom/nodelist.rs +++ b/components/script/dom/nodelist.rs @@ -75,10 +75,8 @@ impl NodeListMethods for NodeList { } // https://dom.spec.whatwg.org/#dom-nodelist-item - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option> { - let item = self.Item(index); - *found = item.is_some(); - item + fn IndexedGetter(&self, index: u32) -> Option> { + self.Item(index) } } diff --git a/components/script/dom/plugin.rs b/components/script/dom/plugin.rs index dc4ca4fe42d..222e9a2840a 100644 --- a/components/script/dom/plugin.rs +++ b/components/script/dom/plugin.rs @@ -45,12 +45,12 @@ impl PluginMethods for Plugin { } // https://html.spec.whatwg.org/multipage/#dom-plugin-item - fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option> { + fn IndexedGetter(&self, _index: u32) -> Option> { unreachable!() } // check-tidy: no specs after this line - fn NamedGetter(&self, _name: DOMString, _found: &mut bool) -> Option> { + fn NamedGetter(&self, _name: DOMString) -> Option> { unreachable!() } diff --git a/components/script/dom/pluginarray.rs b/components/script/dom/pluginarray.rs index aabba4928a4..aa6b779280d 100644 --- a/components/script/dom/pluginarray.rs +++ b/components/script/dom/pluginarray.rs @@ -50,12 +50,12 @@ impl PluginArrayMethods for PluginArray { } // https://html.spec.whatwg.org/multipage/#dom-pluginarray-item - fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option> { + fn IndexedGetter(&self, _index: u32) -> Option> { None } // check-tidy: no specs after this line - fn NamedGetter(&self, _name: DOMString, _found: &mut bool) -> Option> { + fn NamedGetter(&self, _name: DOMString) -> Option> { None } diff --git a/components/script/dom/radionodelist.rs b/components/script/dom/radionodelist.rs index d88fc69eacd..9bbdae00c85 100644 --- a/components/script/dom/radionodelist.rs +++ b/components/script/dom/radionodelist.rs @@ -105,7 +105,7 @@ impl RadioNodeListMethods for RadioNodeList { // https://github.com/servo/servo/issues/5875 // // https://dom.spec.whatwg.org/#dom-nodelist-item - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option> { - self.node_list.IndexedGetter(index, found) + fn IndexedGetter(&self, index: u32) -> Option> { + self.node_list.IndexedGetter(index) } } diff --git a/components/script/dom/storage.rs b/components/script/dom/storage.rs index d6d1e1968f8..7827cdb6b15 100644 --- a/components/script/dom/storage.rs +++ b/components/script/dom/storage.rs @@ -136,10 +136,8 @@ impl StorageMethods for Storage { } // check-tidy: no specs after this line - fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option { - let item = self.GetItem(name); - *found = item.is_some(); - item + fn NamedGetter(&self, name: DOMString) -> Option { + self.GetItem(name) } fn NamedSetter(&self, name: DOMString, value: DOMString) -> ErrorResult { diff --git a/components/script/dom/stylesheetlist.rs b/components/script/dom/stylesheetlist.rs index 1b7eb643e50..721ac06525c 100644 --- a/components/script/dom/stylesheetlist.rs +++ b/components/script/dom/stylesheetlist.rs @@ -46,9 +46,7 @@ impl StyleSheetListMethods for StyleSheetList { } // check-tidy: no specs after this line - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option>{ - let item = self.Item(index); - *found = item.is_some(); - item + fn IndexedGetter(&self, index: u32) -> Option> { + self.Item(index) } } diff --git a/components/script/dom/testbindingiterable.rs b/components/script/dom/testbindingiterable.rs index 1e462a98531..2ad0df6dbb6 100644 --- a/components/script/dom/testbindingiterable.rs +++ b/components/script/dom/testbindingiterable.rs @@ -34,10 +34,8 @@ impl TestBindingIterable { impl TestBindingIterableMethods for TestBindingIterable { fn Add(&self, v: DOMString) { self.vals.borrow_mut().push(v); } fn Length(&self) -> u32 { self.vals.borrow().len() as u32 } - fn GetItem(&self, n: u32) -> DOMString { self.vals.borrow().get(n as usize).unwrap().clone() } - fn IndexedGetter(&self, n: u32, found: &mut bool) -> DOMString { - let s = self.GetItem(n); - *found = true; - s + fn GetItem(&self, n: u32) -> DOMString { self.IndexedGetter(n).unwrap_or_default() } + fn IndexedGetter(&self, n: u32) -> Option { + self.vals.borrow().get(n as usize).cloned() } } diff --git a/components/script/dom/testbindingproxy.rs b/components/script/dom/testbindingproxy.rs index 3308639305c..45e66bc5919 100644 --- a/components/script/dom/testbindingproxy.rs +++ b/components/script/dom/testbindingproxy.rs @@ -23,10 +23,10 @@ impl TestBindingProxyMethods for TestBindingProxy { fn SetItem(&self, _: u32, _: DOMString) -> () {} fn RemoveItem(&self, _: DOMString) -> () {} fn Stringifier(&self) -> DOMString { DOMString::new() } - fn IndexedGetter(&self, _: u32, _: &mut bool) -> DOMString { DOMString::new() } + fn IndexedGetter(&self, _: u32) -> Option { None } fn NamedDeleter(&self, _: DOMString) -> () {} fn IndexedSetter(&self, _: u32, _: DOMString) -> () {} fn NamedSetter(&self, _: DOMString, _: DOMString) -> () {} - fn NamedGetter(&self, _: DOMString, _: &mut bool) -> DOMString { DOMString::new() } + fn NamedGetter(&self, _: DOMString) -> Option { None } } diff --git a/components/script/dom/touchlist.rs b/components/script/dom/touchlist.rs index ae5313e855e..14bb8a68766 100644 --- a/components/script/dom/touchlist.rs +++ b/components/script/dom/touchlist.rs @@ -42,9 +42,7 @@ impl TouchListMethods for TouchList { } /// https://w3c.github.io/touch-events/#widl-TouchList-item-getter-Touch-unsigned-long-index - fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option> { - let item = self.Item(index); - *found = item.is_some(); - item + fn IndexedGetter(&self, index: u32) -> Option> { + self.Item(index) } } diff --git a/components/script/dom/xmldocument.rs b/components/script/dom/xmldocument.rs index 348fb4056cc..1a00f67b80c 100644 --- a/components/script/dom/xmldocument.rs +++ b/components/script/dom/xmldocument.rs @@ -88,8 +88,7 @@ impl XMLDocumentMethods for XMLDocument { } // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter - fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString, found: &mut bool) - -> NonZero<*mut JSObject> { - self.upcast::().NamedGetter(_cx, name, found) + fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option> { + self.upcast::().NamedGetter(_cx, name) } }