Bug 698495 part 1. Create a getElementIfPresent method on JSObject with a generic implementation and use it from JSArray code. r=waldo

js/src/jsarray.cpp

@@ -362,22 +362,9 @@ JSObject::arrayGetOwnDataElement(JSContext *cx, size_t i, Value *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.
  */
-static JSBool
-GetElement(JSContext *cx, JSObject *obj, jsdouble index, JSBool *hole, Value *vp)
+static inline JSBool
+DoGetElement(JSContext *cx, JSObject *obj, jsdouble index, JSBool *hole, Value *vp)
 {
-    JS_ASSERT(index >= 0);
-    if (obj->isDenseArray() && index < obj->getDenseArrayInitializedLength() &&
-        !(*vp = obj->getDenseArrayElement(uint32(index))).isMagic(JS_ARRAY_HOLE)) {
-        *hole = JS_FALSE;
-        return JS_TRUE;
-    }
-    if (obj->isArguments()) {
-        if (obj->asArguments()->getElement(uint32(index), vp)) {
-            *hole = JS_FALSE;
-            return true;
-        }
-    }
-
     AutoIdRooter idr(cx);
 
     *hole = JS_FALSE;
@@ -393,8 +380,8 @@ GetElement(JSContext *cx, JSObject *obj, jsdouble index, JSBool *hole, Value *vp
     if (!obj->lookupGeneric(cx, idr.id(), &obj2, &prop))
         return JS_FALSE;
     if (!prop) {
-        *hole = JS_TRUE;
         vp->setUndefined();
+        *hole = JS_TRUE;
     } else {
         if (!obj->getGeneric(cx, idr.id(), vp))
             return JS_FALSE;
@@ -403,6 +390,40 @@ GetElement(JSContext *cx, JSObject *obj, jsdouble index, JSBool *hole, Value *vp
     return JS_TRUE;
 }
 
+static inline JSBool
+DoGetElement(JSContext *cx, JSObject *obj, uint32 index, JSBool *hole, Value *vp)
+{
+    bool present;
+    if (!obj->getElementIfPresent(cx, obj, index, vp, &present))
+        return false;
+
+    *hole = !present;
+    if (*hole)
+        vp->setUndefined();
+
+    return true;
+}
+
+template<typename IndexType>
+static JSBool
+GetElement(JSContext *cx, JSObject *obj, IndexType index, JSBool *hole, Value *vp)
+{
+    JS_ASSERT(index >= 0);
+    if (obj->isDenseArray() && index < obj->getDenseArrayInitializedLength() &&
+        !(*vp = obj->getDenseArrayElement(uint32(index))).isMagic(JS_ARRAY_HOLE)) {
+        *hole = JS_FALSE;
+        return JS_TRUE;
+    }
+    if (obj->isArguments()) {
+        if (obj->asArguments()->getElement(uint32(index), vp)) {
+            *hole = JS_FALSE;
+            return true;
+        }
+    }
+
+    return DoGetElement(cx, obj, index, hole, vp);
+}
+
 namespace js {
 
 static bool
@@ -2663,7 +2684,7 @@ js::array_shift(JSContext *cx, uintN argc, Value *vp)
         }
 
         JSBool hole;
-        if (!GetElement(cx, obj, 0, &hole, &args.rval()))
+        if (!GetElement(cx, obj, 0u, &hole, &args.rval()))
             return JS_FALSE;
 
         /* Slide down the array above the first element. */

js/src/jsobj.h

@@ -1374,6 +1374,10 @@ struct JSObject : js::gc::Cell {
     inline JSBool getProperty(JSContext *cx, JSObject *receiver, js::PropertyName *name,
                               js::Value *vp);
     inline JSBool getElement(JSContext *cx, JSObject *receiver, uint32 index, js::Value *vp);
+    /* If element is not present (e.g. array hole) *present is set to
+       false and the contents of *vp are unusable garbage. */
+    inline JSBool getElementIfPresent(JSContext *cx, JSObject *receiver, uint32 index,
+                                      js::Value *vp, bool *present);
     inline JSBool getSpecial(JSContext *cx, JSObject *receiver, js::SpecialId sid, js::Value *vp);
 
     inline JSBool getGeneric(JSContext *cx, jsid id, js::Value *vp);

js/src/jsobjinlines.h

@@ -1254,6 +1254,33 @@ JSObject::getElement(JSContext *cx, uint32 index, js::Value *vp)
     return getElement(cx, this, index, vp);
 }
 
+inline JSBool
+JSObject::getElementIfPresent(JSContext *cx, JSObject *receiver, uint32 index, js::Value *vp,
+                              bool *present)
+{
+    /* For now, do the index-to-id conversion just once, then use
+     * lookupGeneric/getGeneric.  Once lookupElement and getElement stop both
+     * doing index-to-id conversions, we can use those here.
+     */
+    jsid id;
+    if (!js::IndexToId(cx, index, &id))
+        return false;
+
+    JSObject *obj2;
+    JSProperty *prop;
+    if (!lookupGeneric(cx, id, &obj2, &prop))
+        return false;
+
+    if (!prop) {
+        *present = false;
+        js::Debug_SetValueRangeToCrashOnTouch(vp, 1);
+        return true;
+    }
+
+    *present = true;
+    return getGeneric(cx, receiver, id, vp);
+}
+
 inline JSBool
 JSObject::getSpecial(JSContext *cx, js::SpecialId sid, js::Value *vp)
 {