Bug 1329046 - Simplify creation of DataViews backed by cross-compartment ArrayBuffers. r=sfink
authorJeff Walden <jwalden@mit.edu>
Fri, 16 Dec 2016 15:35:03 -0500
changeset 328972 69237a2238259d2ba385bb3cd9540cb21d13ecc2
parent 328971 27dda9c4c3dcbe83f21a3a020beb5c81bef815c7
child 328973 4e9c1a72a8ec163c98fc8e5633bd23cd8ef32290
push id85598
push userjwalden@mit.edu
push dateWed, 11 Jan 2017 21:15:42 +0000
treeherdermozilla-inbound@69237a223825 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1329046
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1329046 - Simplify creation of DataViews backed by cross-compartment ArrayBuffers. r=sfink
js/public/Class.h
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
js/src/vm/GlobalObject.h
js/src/vm/TypedArrayObject.cpp
js/src/vm/TypedArrayObject.h
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -736,17 +736,17 @@ struct JSClass {
 // with the following flags. Failure to use JSCLASS_GLOBAL_FLAGS was
 // previously allowed, but is now an ES5 violation and thus unsupported.
 //
 // JSCLASS_GLOBAL_APPLICATION_SLOTS is the number of slots reserved at
 // the beginning of every global object's slots for use by the
 // application.
 #define JSCLASS_GLOBAL_APPLICATION_SLOTS 5
 #define JSCLASS_GLOBAL_SLOT_COUNT                                             \
-    (JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 2 + 40)
+    (JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 2 + 39)
 #define JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(n)                                    \
     (JSCLASS_IS_GLOBAL | JSCLASS_HAS_RESERVED_SLOTS(JSCLASS_GLOBAL_SLOT_COUNT + (n)))
 #define JSCLASS_GLOBAL_FLAGS                                                  \
     JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(0)
 #define JSCLASS_HAS_GLOBAL_FLAG_AND_SLOTS(clasp)                              \
   (((clasp)->flags & JSCLASS_IS_GLOBAL)                                       \
    && JSCLASS_RESERVED_SLOTS(clasp) >= JSCLASS_GLOBAL_SLOT_COUNT)
 
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -1110,52 +1110,16 @@ ArrayBufferObject::createEmpty(JSContext
     ArrayBufferObject* obj = NewObjectWithClassProto<ArrayBufferObject>(cx, nullptr);
     if (!obj)
         return nullptr;
 
     obj->initEmpty();
     return obj;
 }
 
-bool
-ArrayBufferObject::createDataViewForThisImpl(JSContext* cx, const CallArgs& args)
-{
-    MOZ_ASSERT(IsArrayBufferMaybeShared(args.thisv()));
-
-    /*
-     * This method is only called for |DataView(alienBuf, ...)| which calls
-     * this as |createDataViewForThis.call(alienBuf, byteOffset, byteLength,
-     *                                     DataView.prototype)|,
-     * ergo there must be exactly 3 arguments.
-     */
-    MOZ_ASSERT(args.length() == 3);
-
-    uint32_t byteOffset = args[0].toPrivateUint32();
-    uint32_t byteLength = args[1].toPrivateUint32();
-    Rooted<ArrayBufferObjectMaybeShared*> buffer(cx);
-    buffer = &args.thisv().toObject().as<ArrayBufferObjectMaybeShared>();
-
-    /*
-     * Pop off the passed-along prototype and delegate to normal DataViewObject
-     * construction.
-     */
-    JSObject* obj = DataViewObject::create(cx, byteOffset, byteLength, buffer, &args[2].toObject());
-    if (!obj)
-        return false;
-    args.rval().setObject(*obj);
-    return true;
-}
-
-bool
-ArrayBufferObject::createDataViewForThis(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    return CallNonGenericMethod<IsArrayBufferMaybeShared, createDataViewForThisImpl>(cx, args);
-}
-
 /* static */ ArrayBufferObject::BufferContents
 ArrayBufferObject::externalizeContents(JSContext* cx, Handle<ArrayBufferObject*> buffer,
                                        bool hasStealableContents)
 {
     MOZ_ASSERT(buffer->isPlain(), "Only support doing this on plain ABOs");
     MOZ_ASSERT(!buffer->isDetached(), "must have contents to externalize");
     MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents());
 
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -249,20 +249,16 @@ class ArrayBufferObject : public ArrayBu
     static ArrayBufferObject* create(JSContext* cx, uint32_t nbytes,
                                      HandleObject proto = nullptr,
                                      NewObjectKind newKind = GenericObject);
 
     // Create an ArrayBufferObject that is safely finalizable and can later be
     // initialize()d to become a real, content-visible ArrayBufferObject.
     static ArrayBufferObject* createEmpty(JSContext* cx);
 
-    // Also valid when the buffer is a SharedArrayBuffer.
-    static bool createDataViewForThisImpl(JSContext* cx, const CallArgs& args);
-    static bool createDataViewForThis(JSContext* cx, unsigned argc, Value* vp);
-
     template<typename T>
     static bool createTypedArrayFromBufferImpl(JSContext* cx, const CallArgs& args);
     template<typename T>
     static bool createTypedArrayFromBuffer(JSContext* cx, unsigned argc, Value* vp);
 
     static void copyData(Handle<ArrayBufferObject*> toBuffer,
                          Handle<ArrayBufferObject*> fromBuffer,
                          uint32_t fromIndex, uint32_t count);
--- a/js/src/vm/GlobalObject.h
+++ b/js/src/vm/GlobalObject.h
@@ -65,17 +65,16 @@ class GlobalObject : public NativeObject
      * Count of slots to store built-in prototypes and initial visible
      * properties for the constructors.
      */
     static const unsigned STANDARD_CLASS_SLOTS = JSProto_LIMIT * 2;
 
     enum : unsigned {
         /* Various function values needed by the engine. */
         EVAL = APPLICATION_SLOTS + STANDARD_CLASS_SLOTS,
-        CREATE_DATAVIEW_FOR_THIS,
         THROWTYPEERROR,
 
         /*
          * Instances of the internal createArrayFromBuffer function used by the
          * typed array code, one per typed array element type.
          */
         FROM_BUFFER_UINT8,
         FROM_BUFFER_INT8,
@@ -260,22 +259,16 @@ class GlobalObject : public NativeObject
     }
 
     void setCreateArrayFromBufferHelper(uint32_t slot, Handle<JSFunction*> fun) {
         MOZ_ASSERT(getSlotRef(slot).isUndefined());
         setSlot(slot, ObjectValue(*fun));
     }
 
   public:
-    /* XXX Privatize me! */
-    void setCreateDataViewForThis(Handle<JSFunction*> fun) {
-        MOZ_ASSERT(getSlotRef(CREATE_DATAVIEW_FOR_THIS).isUndefined());
-        setSlot(CREATE_DATAVIEW_FOR_THIS, ObjectValue(*fun));
-    }
-
     template<typename T>
     inline void setCreateArrayFromBuffer(Handle<JSFunction*> fun);
 
   private:
     // Disallow use of unqualified JSObject::create in GlobalObject.
     static GlobalObject* create(...) = delete;
 
     friend struct ::JSRuntime;
@@ -718,21 +711,16 @@ class GlobalObject : public NativeObject
 
     JSObject* getThrowTypeError() const {
         const Value v = getReservedSlot(THROWTYPEERROR);
         MOZ_ASSERT(v.isObject(),
                    "attempting to access [[ThrowTypeError]] too early");
         return &v.toObject();
     }
 
-    Value createDataViewForThis() const {
-        MOZ_ASSERT(dataViewClassInitialized());
-        return getSlot(CREATE_DATAVIEW_FOR_THIS);
-    }
-
     template<typename T>
     inline Value createArrayFromBuffer() const;
 
     static bool isRuntimeCodeGenEnabled(JSContext* cx, Handle<GlobalObject*> global);
 
     // Warn about use of the deprecated watch/unwatch functions in the global
     // in which |obj| was created, if no prior warning was given.
     static bool warnOnceAboutWatch(JSContext* cx, HandleObject obj) {
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -1770,17 +1770,17 @@ DataViewObject::create(JSContext* cx, ui
         if (!arrayBuffer->as<ArrayBufferObject>().addView(cx, &dvobj))
             return nullptr;
     }
 
     return &dvobj;
 }
 
 bool
-DataViewObject::getAndCheckConstructorArgs(JSContext* cx, JSObject* bufobj, const CallArgs& args,
+DataViewObject::getAndCheckConstructorArgs(JSContext* cx, HandleObject bufobj, const CallArgs& args,
                                            uint32_t* byteOffsetPtr, uint32_t* byteLengthPtr)
 {
     if (!IsArrayBufferMaybeShared(bufobj)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NOT_EXPECTED_TYPE,
                                   "DataView", "ArrayBuffer", bufobj->getClass()->name);
         return false;
     }
 
@@ -1885,17 +1885,17 @@ DataViewObject::constructSameCompartment
 // which uses CallNonGenericMethod to switch to compartment B so that
 // the new DataView is created there.
 bool
 DataViewObject::constructWrapped(JSContext* cx, HandleObject bufobj, const CallArgs& args)
 {
     MOZ_ASSERT(args.isConstructing());
     MOZ_ASSERT(bufobj->is<WrapperObject>());
 
-    JSObject* unwrapped = CheckedUnwrap(bufobj);
+    RootedObject unwrapped(cx, CheckedUnwrap(bufobj));
     if (!unwrapped) {
         ReportAccessDenied(cx);
         return false;
     }
 
     // NB: This entails the IsArrayBuffer check
     uint32_t byteOffset, byteLength;
     if (!getAndCheckConstructorArgs(cx, unwrapped, args, &byteOffset, &byteLength))
@@ -1910,25 +1910,37 @@ DataViewObject::constructWrapped(JSConte
 
     Rooted<GlobalObject*> global(cx, cx->compartment()->maybeGlobal());
     if (!proto) {
         proto = global->getOrCreateDataViewPrototype(cx);
         if (!proto)
             return false;
     }
 
-    FixedInvokeArgs<3> args2(cx);
-
-    args2[0].set(PrivateUint32Value(byteOffset));
-    args2[1].set(PrivateUint32Value(byteLength));
-    args2[2].setObject(*proto);
-
-    RootedValue fval(cx, global->createDataViewForThis());
-    RootedValue thisv(cx, ObjectValue(*bufobj));
-    return js::Call(cx, fval, thisv, args2, args.rval());
+    RootedObject dv(cx);
+    {
+        JSAutoCompartment ac(cx, unwrapped);
+
+        Rooted<ArrayBufferObjectMaybeShared*> buffer(cx);
+        buffer = &unwrapped->as<ArrayBufferObjectMaybeShared>();
+
+        RootedObject wrappedProto(cx, proto);
+        if (!cx->compartment()->wrap(cx, &wrappedProto))
+            return false;
+
+        dv = DataViewObject::create(cx, byteOffset, byteLength, buffer, wrappedProto);
+        if (!dv)
+            return false;
+    }
+
+    if (!cx->compartment()->wrap(cx, &dv))
+        return false;
+
+    args.rval().setObject(*dv);
+    return true;
 }
 
 bool
 DataViewObject::class_constructor(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     if (!ThrowIfNotConstructing(cx, args, "DataView"))
@@ -2917,32 +2929,17 @@ DataViewObject::initClass(JSContext* cx)
         return false;
 
     if (!JS_DefineFunctions(cx, proto, DataViewObject::jsfuncs))
         return false;
 
     if (!DefineToStringTag(cx, proto, cx->names().DataView))
         return false;
 
-    /*
-     * Create a helper function to implement the craziness of
-     * |new DataView(new otherWindow.ArrayBuffer())|, and install it in the
-     * global for use by the DataViewObject constructor.
-     */
-    RootedFunction fun(cx, NewNativeFunction(cx, ArrayBufferObject::createDataViewForThis,
-                                             0, nullptr));
-    if (!fun)
-        return false;
-
-    if (!GlobalObject::initBuiltinConstructor(cx, global, JSProto_DataView, ctor, proto))
-        return false;
-
-    global->setCreateDataViewForThis(fun);
-
-    return true;
+    return GlobalObject::initBuiltinConstructor(cx, global, JSProto_DataView, ctor, proto);
 }
 
 void
 DataViewObject::notifyBufferDetached(void* newData)
 {
     setFixedSlot(TypedArrayObject::LENGTH_SLOT, Int32Value(0));
     setFixedSlot(TypedArrayObject::BYTEOFFSET_SLOT, Int32Value(0));
     setPrivate(newData);
--- a/js/src/vm/TypedArrayObject.h
+++ b/js/src/vm/TypedArrayObject.h
@@ -457,22 +457,22 @@ class DataViewObject : public NativeObje
     template<Value ValueGetter(DataViewObject* view)>
     static bool
     getter(JSContext* cx, unsigned argc, Value* vp);
 
     template<Value ValueGetter(DataViewObject* view)>
     static bool
     defineGetter(JSContext* cx, PropertyName* name, HandleNativeObject proto);
 
-    static bool getAndCheckConstructorArgs(JSContext* cx, JSObject* bufobj, const CallArgs& args,
-                                           uint32_t *byteOffset, uint32_t* byteLength);
+    static bool
+    getAndCheckConstructorArgs(JSContext* cx, HandleObject bufobj, const CallArgs& args,
+                               uint32_t* byteOffset, uint32_t* byteLength);
     static bool constructSameCompartment(JSContext* cx, HandleObject bufobj, const CallArgs& args);
     static bool constructWrapped(JSContext* cx, HandleObject bufobj, const CallArgs& args);
 
-    friend bool ArrayBufferObject::createDataViewForThisImpl(JSContext* cx, const CallArgs& args);
     static DataViewObject*
     create(JSContext* cx, uint32_t byteOffset, uint32_t byteLength,
            Handle<ArrayBufferObjectMaybeShared*> arrayBuffer, JSObject* proto);
 
   public:
     static const Class class_;
 
     static Value byteOffsetValue(DataViewObject* view) {