Closed Bug 987163 Opened 6 years ago Closed 5 years ago

Implement Xrays to TypedArray objects

Categories

(Core :: XPConnect, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: bholley, Assigned: bholley)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

Part of XrayToJS.

Given that the array contents can be be arbitrary (at least for Arrays), we should consider enforcing some of the same restrictions mentioned in bug 987111 comment 0 (in particular, not returning callables, and enforcing same-origin).
Depends on: 1015791
Depends on: 972987
Depends on: 1020460
Depends on: 1020609
No longer depends on: 1020460, 972987
Summary: Implement Xrays to Arrays and TypedArrays → Implement Xrays to TypedArrays
Summary: Implement Xrays to TypedArrays → Implement Xrays to TypedArray objects
This should now be ready to land. Doing a final all-platform try push to be sure: https://tbpl.mozilla.org/?tree=Try&rev=47dc2ed0e926

I'm sorry to dump this on you bz, but gabor is out until sometime next week and we're getting near the end of Q2, so I don't want this goal to slip. The patches should be very straightforward (feel free to just skim the test changes).
The Xray code expects to be able to find the ClassSpec via the JSClass of either
instances or standard prototypes. So in the case where these two objects have
different JSClasses, we need to put the information on both.
Attachment #8438871 - Flags: review?(luke)
I realized that the switches are going to be cumbersome for Array stuff, because
we'll have to enumerate each kind of TypedArray as a separate case: statement.
Let's just use |if| so that we can call a helper.
Attachment #8438872 - Flags: review?(bzbarsky)
Attachment #8438873 - Flags: review?(bzbarsky)
From now on, if someone wants to expose a TypedArray to content, they should
use Cu.cloneInto.
Attachment #8438874 - Flags: review?(bzbarsky)
Attachment #8438875 - Flags: review?(bzbarsky)
Attachment #8438871 - Flags: review?(luke) → review+
Comment on attachment 8438872 [details] [diff] [review]
Part 2 - Convert from |switch| statements to |if| statements. v1

Looks like gabor is back, so sending this back to him.

Gabor, this should be pretty quick to review. It's also part of a Q2 goal, so if you have time to review it today I'd appreciate it. :-)
Attachment #8438872 - Flags: review?(bzbarsky) → review?(gkrizsanits)
Attachment #8438873 - Flags: review?(bzbarsky) → review?(gkrizsanits)
Attachment #8438874 - Flags: review?(bzbarsky) → review?(gkrizsanits)
Attachment #8438875 - Flags: review?(bzbarsky) → review?(gkrizsanits)
Attachment #8438876 - Flags: review?(bzbarsky) → review?(gkrizsanits)
Attachment #8438872 - Flags: review?(gkrizsanits) → review+
Attachment #8438873 - Flags: review?(gkrizsanits) → review+
Attachment #8438874 - Flags: review?(gkrizsanits) → review+
Attachment #8438875 - Flags: review?(gkrizsanits) → review+
Comment on attachment 8438876 [details] [diff] [review]
Part 6 - Implement Xrays for TypedArrays. v1

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

::: js/xpconnect/wrappers/XrayWrapper.cpp
@@ +54,5 @@
> +                            key == JSProto_Uint8ClampedArray;
> +#endif
> +    bool isTypedArray = key >= JSProto_Int8Array &&
> +                        key <= JSProto_Uint8ClampedArray;
> +    MOZ_ASSERT(isTypedArray == isTypedArraySlow, "Somebody reordered jsprototypes.h!");

Could you also assert that the difference of JSProto_Uint8ClampedArray and JSProto_Int8Array has not changed? Since if someone adds a new type before JSProto_Uint8ClampedArray nothing will alarm without that.
Attachment #8438876 - Flags: review?(gkrizsanits) → review+
Blocks: 856067
Blocks: 929609
You need to log in before you can comment on or make changes to this bug.