Improve handling of Xrayables in SpecialPowers.wrap

RESOLVED FIXED in mozilla33

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla33
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

The current code in SpecialPowers.wrap makes various assumptions about XrayWrappers - in particular, that they don't have a meaningful prototype. This was true for XPCWrappedNative Xrays, but isn't really true for WebIDL Xrays (which have Xrayable prototypes), and definitely isn't true for JSXrays (which have HasPrototype=1). We should fix up this detection so that the wrappers work better.
Our Xrays to TypedArrays basically just say "this will be slow - clone instead".
Let's just have SpecialPowers waive Xrays for them to make things work less
surprisingly.
Attachment #8451086 - Flags: review?(gkrizsanits)
Comment on attachment 8451085 [details] [diff] [review]
Part 1 - Check only for WN Xrays when deciding whether the prototype is useless. v1

Gabor is on PTO, and I think Blake is actually a better reviewer here.
Attachment #8451085 - Flags: review?(gkrizsanits) → review?(mrbkap)
Attachment #8451086 - Flags: review?(gkrizsanits) → review?(mrbkap)
Attachment #8451085 - Flags: review?(mrbkap) → review+
Comment on attachment 8451086 [details] [diff] [review]
Part 2 - Include TypedArrays in SpecialPowers Xray waiving list. v1

Review of attachment 8451086 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/specialpowers/content/specialpowersAPI.js
@@ +82,2 @@
>    let className = Cu.getClassName(obj, true);
> +  return arrayClasses.indexOf(className) != -1;

Yuck. Is there really no better way to do this? At the very least, a map might be worth it.
Attachment #8451086 - Flags: review?(mrbkap) → review+
Attachment #8452799 - Flags: review?(mrbkap) → review+
Attachment #8452800 - Flags: review?(mrbkap) → review+
(In reply to Blake Kaplan (:mrbkap) from comment #7)
> Yuck. Is there really no better way to do this? At the very least, a map
> might be worth it.

Given that there are only 9 of them, I doubt that this imposes much overhead relative to all the other stuff that goes on here. Also, this stuff will all be simplified and rewritten with Direct Proxies at some point.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3b5651291f75
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8e182aa529ab
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c1d5d78aec3b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/739be2b870b5
You need to log in before you can comment on or make changes to this bug.