1034244 - Improve handling of Xrayables in SpecialPowers.wrap

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Bobby Holley (:bholley)]

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.

Comment 1

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=12f428daa375

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 1 - Check only for WN Xrays when deciding whether the prototype is useless. v1

Comment 3

[Bobby Holley (:bholley)]

Attachment: Part 2 - Include TypedArrays in SpecialPowers Xray waiving list. v1

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.

Comment 4

[Bobby Holley (:bholley)]

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.

Comment 5

[Bobby Holley (:bholley)]

Attachment: Part 3 - Follow the prototype chain of the underlying object in SpecialPowers wrapping. v1

Comment 6

[Bobby Holley (:bholley)]

Attachment: Part 4 - Hoist SpecialPowers Xray waiving into a separate function so that we can apply it at each stage when climbing the prototype chain. v1

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

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.

Comment 8

[Bobby Holley (:bholley)]

(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

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3b5651291f75
https://hg.mozilla.org/mozilla-central/rev/8e182aa529ab
https://hg.mozilla.org/mozilla-central/rev/c1d5d78aec3b
https://hg.mozilla.org/mozilla-central/rev/739be2b870b5