Bug 697343 - Add slice hook to DOM bindings like NodeList. r=peterv,bhackett
☠☠ backed out by fbc0b4c30fd0 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 02 Dec 2013 11:32:22 -0500
changeset 174733 0809370fabdb4261b6025efe3cba71af06470edf
parent 174732 4ef4764c1b559115664fbd4534629e48d45e3cf0
child 174734 22770b30545b26ee7280f94c6126e59c1f49d16c
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, bhackett
bugs697343
milestone28.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 697343 - Add slice hook to DOM bindings like NodeList. r=peterv,bhackett
dom/bindings/Codegen.py
js/src/jsarray.cpp
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -7567,18 +7567,23 @@ class CGClassForwardDeclare(CGThing):
         return ''
     def deps(self):
         return set()
 
 class CGProxySpecialOperation(CGPerSignatureCall):
     """
     Base class for classes for calling an indexed or named special operation
     (don't use this directly, use the derived classes below).
-    """
-    def __init__(self, descriptor, operation):
+
+    If checkFound is False, will just assert that the prop is found instead of
+    checking that it is before wrapping the value.
+    """
+    def __init__(self, descriptor, operation, checkFound=True):
+        self.checkFound = checkFound;
+
         nativeName = MakeNativeName(descriptor.binaryNames.get(operation, operation))
         operation = descriptor.operations[operation]
         assert len(operation.signatures()) == 1
         signature = operation.signatures()[0]
 
         (returnType, arguments) = signature
 
         # We pass len(arguments) as the final argument so that the
@@ -7614,45 +7619,68 @@ class CGProxySpecialOperation(CGPerSigna
                          "found"))
         return args
 
     def wrap_return_value(self):
         if not self.idlNode.isGetter() or self.templateValues is None:
             return ""
 
         wrap = CGGeneric(wrapForType(self.returnType, self.descriptor, self.templateValues))
-        wrap = CGIfWrapper(wrap, "found")
+        if self.checkFound:
+            wrap = CGIfWrapper(wrap, "found")
+        else:
+            wrap = CGList([CGGeneric("MOZ_ASSERT(found);"), wrap], "\n")
         return "\n" + wrap.define()
 
 class CGProxyIndexedOperation(CGProxySpecialOperation):
     """
     Class to generate a call to an indexed operation.
-    """
-    def __init__(self, descriptor, name):
-        CGProxySpecialOperation.__init__(self, descriptor, name)
+
+    If doUnwrap is False, the caller is responsible for making sure a variable
+    named 'self' holds the C++ object somewhere where the code we generate
+    will see it.
+
+    If checkFound is False, will just assert that the prop is found instead of
+    checking that it is before wrapping the value.
+    """
+    def __init__(self, descriptor, name, doUnwrap=True, checkFound=True):
+        self.doUnwrap = doUnwrap
+        CGProxySpecialOperation.__init__(self, descriptor, name, checkFound)
     def define(self):
         # Our first argument is the id we're getting.
         argName = self.arguments[0].identifier.name
         if argName == "index":
             # We already have our index in a variable with that name
             setIndex = ""
         else:
             setIndex = "uint32_t %s = index;\n" % argName
-        return (setIndex +
-                "%s* self = UnwrapProxy(proxy);\n" +
+        if self.doUnwrap:
+            unwrap = "%s* self = UnwrapProxy(proxy);\n"
+        else:
+            unwrap = ""
+        return (setIndex + unwrap +
                 CGProxySpecialOperation.define(self))
 
 class CGProxyIndexedGetter(CGProxyIndexedOperation):
     """
     Class to generate a call to an indexed getter. If templateValues is not None
     the returned value will be wrapped with wrapForType using templateValues.
-    """
-    def __init__(self, descriptor, templateValues=None):
+
+    If doUnwrap is False, the caller is responsible for making sure a variable
+    named 'self' holds the C++ object somewhere where the code we generate
+    will see it.
+
+    If checkFound is False, will just assert that the prop is found instead of
+    checking that it is before wrapping the value.
+    """
+    def __init__(self, descriptor, templateValues=None, doUnwrap=True,
+                 checkFound=True):
         self.templateValues = templateValues
-        CGProxyIndexedOperation.__init__(self, descriptor, 'IndexedGetter')
+        CGProxyIndexedOperation.__init__(self, descriptor, 'IndexedGetter',
+                                         doUnwrap, checkFound)
 
 class CGProxyIndexedPresenceChecker(CGProxyIndexedGetter):
     """
     Class to generate a call that checks whether an indexed property exists.
 
     For now, we just delegate to CGProxyIndexedGetter
     """
     def __init__(self, descriptor):
@@ -8279,16 +8307,66 @@ class CGDOMJSProxyHandler_finalize(Class
     def __init__(self, descriptor):
         args = [Argument('JSFreeOp*', 'fop'), Argument('JSObject*', 'proxy')]
         ClassMethod.__init__(self, "finalize", "void", args)
         self.descriptor = descriptor
     def getBody(self):
         return ("%s self = UnwrapProxy(proxy);\n\n" % (self.descriptor.nativeType + "*") +
                 finalizeHook(self.descriptor, FINALIZE_HOOK_NAME, self.args[0].name).define())
 
+class CGDOMJSProxyHandler_slice(ClassMethod):
+    def __init__(self, descriptor):
+        assert descriptor.supportsIndexedProperties()
+
+        args = [Argument('JSContext*', 'cx'),
+                Argument('JS::Handle<JSObject*>', 'proxy'),
+                Argument('uint32_t', 'begin'),
+                Argument('uint32_t', 'end'),
+                Argument('JS::Handle<JSObject*>', 'array')]
+        ClassMethod.__init__(self, "slice", "bool", args)
+        self.descriptor = descriptor
+
+    def getBody(self):
+        # Just like getOwnPropertyNames we'll assume that we have no holes, so
+        # we have all properties from 0 to length.  If that ever changes
+        # (unlikely), we'll need to do something a bit more clever with how we
+        # forward on to our ancestor.
+        header = CGGeneric(
+            'JS::Rooted<JS::Value> temp(cx);\n'
+            'MOZ_ASSERT(!xpc::WrapperFactory::IsXrayWrapper(proxy),\n'
+            '           "Should not have a XrayWrapper here");\n'
+            '\n'
+            '%s* self = UnwrapProxy(proxy);\n'
+            'uint32_t length = self->Length();\n'
+            "// Compute the end of the indices we'll get ourselves\n"
+            'uint32_t ourEnd = std::max(begin, std::min(end, length));' %
+            self.descriptor.nativeType)
+
+        successCode = ("js::UnsafeDefineElement(cx, array, index - begin, temp);\n"
+                       "continue;")
+        templateValues = {'jsvalRef': 'temp', 'jsvalHandle': '&temp',
+                          'obj': 'proxy', 'successCode': successCode}
+        get = CGProxyIndexedGetter(self.descriptor, templateValues, False, False)
+
+        getOurElements = CGWrapper(
+            CGIndenter(get),
+            pre="for (uint32_t index = begin; index < ourEnd; ++index) {\n",
+            post="\n}")
+
+        getProtoElements = CGIfWrapper(
+            CGGeneric("JS::Rooted<JSObject*> proto(cx);\n"
+                      "if (!js::GetObjectProto(cx, proxy, &proto)) {\n"
+                      "  return false;\n"
+                      "}\n"
+                      "return js::SliceSlowly(cx, proto, proxy, ourEnd, end, array);"),
+            "end > ourEnd")
+
+        return CGList([header, getOurElements, getProtoElements,
+                       CGGeneric("return true;")], "\n\n").define();
+
 class CGDOMJSProxyHandler_getInstance(ClassMethod):
     def __init__(self):
         ClassMethod.__init__(self, "getInstance", "DOMProxyHandler*", [], static=True)
     def getBody(self):
         return """static DOMProxyHandler instance;
 return &instance;"""
 
 class CGDOMJSProxyHandler(CGClass):
@@ -8303,16 +8381,19 @@ class CGDOMJSProxyHandler(CGClass):
                    CGDOMJSProxyHandler_getOwnPropertyNames(descriptor),
                    CGDOMJSProxyHandler_hasOwn(descriptor),
                    CGDOMJSProxyHandler_get(descriptor),
                    CGDOMJSProxyHandler_className(descriptor),
                    CGDOMJSProxyHandler_finalizeInBackground(descriptor),
                    CGDOMJSProxyHandler_finalize(descriptor),
                    CGDOMJSProxyHandler_getInstance(),
                    CGDOMJSProxyHandler_delete(descriptor)]
+        if descriptor.supportsIndexedProperties():
+            methods.append(CGDOMJSProxyHandler_slice(descriptor))
+
         CGClass.__init__(self, 'DOMProxyHandler',
                          bases=[ClassBase('mozilla::dom::DOMProxyHandler')],
                          constructors=constructors,
                          methods=methods)
 
 class CGDOMJSProxyHandlerDeclarer(CGThing):
     """
     A class for declaring a DOMProxyHandler.
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -152,33 +152,34 @@ ToId(JSContext *cx, uint32_t index, Muta
 /*
  * If the property at the given index exists, get its value into location
  * pointed by vp and set *hole to false. Otherwise set *hole to true and *vp
  * to JSVAL_VOID. This function assumes that the location pointed by vp is
  * properly rooted and can be used as GC-protected storage for temporaries.
  */
 template<typename IndexType>
 static inline bool
-DoGetElement(JSContext *cx, HandleObject obj, IndexType index, bool *hole, MutableHandleValue vp)
+DoGetElement(JSContext *cx, HandleObject obj, HandleObject receiver,
+             IndexType index, bool *hole, MutableHandleValue vp)
 {
     RootedId id(cx);
 
     if (!ToId(cx, index, &id))
         return false;
 
     RootedObject obj2(cx);
     RootedShape prop(cx);
     if (!JSObject::lookupGeneric(cx, obj, id, &obj2, &prop))
         return false;
 
     if (!prop) {
         vp.setUndefined();
         *hole = true;
     } else {
-        if (!JSObject::getGeneric(cx, obj, obj, id, vp))
+        if (!JSObject::getGeneric(cx, obj, receiver, id, vp))
             return false;
         *hole = false;
     }
     return true;
 }
 
 template<typename IndexType>
 static void
@@ -191,34 +192,42 @@ AssertGreaterThanZero(IndexType index)
 template<>
 void
 AssertGreaterThanZero(uint32_t index)
 {
 }
 
 template<typename IndexType>
 static bool
-GetElement(JSContext *cx, HandleObject obj, IndexType index, bool *hole, MutableHandleValue vp)
+GetElement(JSContext *cx, HandleObject obj, HandleObject receiver,
+           IndexType index, bool *hole, MutableHandleValue vp)
 {
     AssertGreaterThanZero(index);
     if (obj->isNative() && index < obj->getDenseInitializedLength()) {
         vp.set(obj->getDenseElement(uint32_t(index)));
         if (!vp.isMagic(JS_ELEMENTS_HOLE)) {
             *hole = false;
             return true;
         }
     }
     if (obj->is<ArgumentsObject>()) {
         if (obj->as<ArgumentsObject>().maybeGetElement(uint32_t(index), vp)) {
             *hole = false;
             return true;
         }
     }
 
-    return DoGetElement(cx, obj, index, hole, vp);
+    return DoGetElement(cx, obj, receiver, index, hole, vp);
+}
+
+template<typename IndexType>
+static inline bool
+GetElement(JSContext *cx, HandleObject obj, IndexType index, bool *hole, MutableHandleValue vp)
+{
+    return GetElement(cx, obj, obj, index, hole, vp);
 }
 
 static bool
 GetElementsSlow(JSContext *cx, HandleObject aobj, uint32_t length, Value *vp)
 {
     for (uint32_t i = 0; i < length; i++) {
         if (!JSObject::getElement(cx, aobj, aobj, i, MutableHandleValue::fromMarkedLocation(&vp[i])))
             return false;
@@ -2728,36 +2737,56 @@ array_slice(JSContext *cx, unsigned argc
             narr->setDenseInitializedLength(initLength);
             narr->initDenseElements(0, &obj->getDenseElement(begin), initLength);
         }
         args.rval().setObject(*narr);
         return true;
     }
 
     if (js::SliceOp op = obj->getOps()->slice) {
-        if (!op(cx, obj, begin, end, narr))
-            return false;
-
-        args.rval().setObject(*narr);
-        return true;
+        // Ensure that we have dense elements, so that DOM can use js::UnsafeDefineElement.
+        JSObject::EnsureDenseResult result = narr->ensureDenseElements(cx, 0, end - begin);
+        if (result == JSObject::ED_FAILED)
+             return false;
+
+        if (result == JSObject::ED_OK) {
+            if (!op(cx, obj, begin, end, narr))
+                return false;
+
+            args.rval().setObject(*narr);
+            return true;
+        }
+
+        // Fallthrough
+        JS_ASSERT(result == JSObject::ED_SPARSE);
     }
 
+
+    if (!SliceSlowly(cx, obj, obj, begin, end, narr))
+        return false;
+
+    args.rval().setObject(*narr);
+    return true;
+}
+
+JS_FRIEND_API(bool)
+js::SliceSlowly(JSContext* cx, HandleObject obj, HandleObject receiver,
+                uint32_t begin, uint32_t end, HandleObject result)
+{
     RootedValue value(cx);
     for (uint32_t slot = begin; slot < end; slot++) {
         bool hole;
         if (!JS_CHECK_OPERATION_LIMIT(cx) ||
-            !GetElement(cx, obj, slot, &hole, &value))
+            !GetElement(cx, obj, receiver, slot, &hole, &value))
         {
             return false;
         }
-        if (!hole && !JSObject::defineElement(cx, narr, slot - begin, value))
+        if (!hole && !JSObject::defineElement(cx, result, slot - begin, value))
             return false;
     }
-
-    args.rval().setObject(*narr);
     return true;
 }
 
 /* ES5 15.4.4.20. */
 static bool
 array_filter(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1160,16 +1160,24 @@ js::SetObjectMetadata(JSContext *cx, Han
 }
 
 JS_FRIEND_API(JSObject *)
 js::GetObjectMetadata(JSObject *obj)
 {
     return obj->getMetadata();
 }
 
+JS_FRIEND_API(void)
+js::UnsafeDefineElement(JSContext *cx, JS::HandleObject obj, uint32_t index, JS::HandleValue value)
+{
+    JS_ASSERT(obj->isNative());
+    JS_ASSERT(index < obj->getDenseInitializedLength());
+    obj->setDenseElementWithType(cx, index, value);
+}
+
 JS_FRIEND_API(bool)
 js_DefineOwnProperty(JSContext *cx, JSObject *objArg, jsid idArg,
                      JS::Handle<js::PropertyDescriptor> descriptor, bool *bp)
 {
     RootedObject obj(cx, objArg);
     RootedId id(cx, idArg);
     JS_ASSERT(cx->runtime()->heapState == js::Idle);
     CHECK_REQUEST(cx);
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -1734,16 +1734,23 @@ SetObjectMetadataCallback(JSContext *cx,
 /* Manipulate the metadata associated with an object. */
 
 JS_FRIEND_API(bool)
 SetObjectMetadata(JSContext *cx, JS::HandleObject obj, JS::HandleObject metadata);
 
 JS_FRIEND_API(JSObject *)
 GetObjectMetadata(JSObject *obj);
 
+JS_FRIEND_API(void)
+UnsafeDefineElement(JSContext *cx, JS::HandleObject obj, uint32_t index, JS::HandleValue value);
+
+JS_FRIEND_API(bool)
+SliceSlowly(JSContext* cx, JS::HandleObject obj, JS::HandleObject receiver,
+            uint32_t begin, uint32_t end, JS::HandleObject result);
+
 /* ES5 8.12.8. */
 extern JS_FRIEND_API(bool)
 DefaultValue(JSContext *cx, JS::HandleObject obj, JSType hint, JS::MutableHandleValue vp);
 
 /*
  * Helper function. To approximate a call to the [[DefineOwnProperty]] internal
  * method described in ES5, first call this, then call JS_DefinePropertyById.
  *