Auto merge of #17250 - cbrewster:unwrap_constructor, r=jdm

Unwrap function before calling IsConstructor

`IsConstructor` returns true for all wrappers that are callable. Wrapped
arrow functions are callable and thus `IsConstructor` will return true
if given one; however, if we unwrap the arrow function, `IsConstructor`
will return false. The latter is the correct behavior here.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17250)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-08-16 17:09:37 -05:00 committed by GitHub
commit cdd4de9c06
2 changed files with 10 additions and 6 deletions

View file

@ -27,6 +27,7 @@ use dom::window::Window;
use dom_struct::dom_struct;
use html5ever::{LocalName, Namespace, Prefix};
use js::conversions::ToJSValConvertible;
use js::glue::UnwrapObject;
use js::jsapi::{Construct1, IsCallable, IsConstructor, HandleValueArray, HandleObject, MutableHandleValue};
use js::jsapi::{Heap, JS_GetProperty, JS_SameValue, JSAutoCompartment, JSContext};
use js::jsval::{JSVal, NullValue, ObjectValue, UndefinedValue};
@ -194,7 +195,15 @@ impl CustomElementRegistryMethods for CustomElementRegistry {
let name = LocalName::from(&*name);
// Step 1
if unsafe { !IsConstructor(constructor.get()) } {
// We must unwrap the constructor as all wrappers are constructable if they are callable.
rooted!(in(cx) let unwrapped_constructor = unsafe { UnwrapObject(constructor.get(), 1) });
if unwrapped_constructor.is_null() {
// We do not have permission to access the unwrapped constructor.
return Err(Error::Security);
}
if unsafe { !IsConstructor(unwrapped_constructor.get()) } {
return Err(Error::Type("Second argument of CustomElementRegistry.define is not a constructor".to_owned()));
}

View file

@ -1,5 +0,0 @@
[define.html]
type: testharness
[If constructor is arrow function, should throw a TypeError]
expected: FAIL