Bug 1017067 - Merge deleteProperty/deleteElement ops back into a single deleteGeneric op. r=bhackett.
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 05 Jun 2014 13:19:23 -0400
changeset 206442 ad465c247417bfb14250ad59f3a10e7616a190d7
parent 206441 056fd938efc7344a4cb266974505aa1eeae79699
child 206443 c48f50085fecd32758628f3898ff3c02f8674209
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1017067
milestone32.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 1017067 - Merge deleteProperty/deleteElement ops back into a single deleteGeneric op. r=bhackett.
dom/bindings/Codegen.py
js/public/Class.h
js/src/builtin/TypedObject.cpp
js/src/builtin/TypedObject.h
js/src/jsapi.cpp
js/src/jsarray.cpp
js/src/jsfriendapi.h
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/json.cpp
js/src/jsproxy.cpp
js/src/tests/ecma_5/strict/8.12.7-2.js
js/src/vm/Debugger.cpp
js/src/vm/Interpreter.cpp
js/src/vm/ScopeObject.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/src/xpcprivate.h
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -372,18 +372,17 @@ JS_NULL_OBJECT_OPS
   nullptr, /* getGeneric  */
   nullptr, /* getProperty */
   nullptr, /* getElement */
   nullptr, /* setGeneric */
   nullptr, /* setProperty */
   nullptr, /* setElement */
   nullptr, /* getGenericAttributes */
   nullptr, /* setGenericAttributes */
-  nullptr, /* deleteProperty */
-  nullptr, /* deleteElement */
+  nullptr, /* deleteGeneric */
   nullptr, /* watch */
   nullptr, /* unwatch */
   nullptr, /* slice */
   nullptr, /* enumerate */
   JS_ObjectToOuterObject /* thisObject */
 }
 """
         else:
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -227,20 +227,17 @@ typedef bool
 (* StrictElementIdOp)(JSContext *cx, JS::HandleObject obj, uint32_t index,
                       JS::MutableHandleValue vp, bool strict);
 typedef bool
 (* GenericAttributesOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, unsigned *attrsp);
 typedef bool
 (* PropertyAttributesOp)(JSContext *cx, JS::HandleObject obj, JS::Handle<PropertyName*> name,
                          unsigned *attrsp);
 typedef bool
-(* DeletePropertyOp)(JSContext *cx, JS::HandleObject obj, JS::Handle<PropertyName*> name,
-                     bool *succeeded);
-typedef bool
-(* DeleteElementOp)(JSContext *cx, JS::HandleObject obj, uint32_t index, bool *succeeded);
+(* DeleteGenericOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, bool *succeeded);
 
 typedef bool
 (* WatchOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::HandleObject callable);
 
 typedef bool
 (* UnwatchOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id);
 
 typedef bool
@@ -337,41 +334,39 @@ struct ObjectOps
     GenericIdOp         getGeneric;
     PropertyIdOp        getProperty;
     ElementIdOp         getElement;
     StrictGenericIdOp   setGeneric;
     StrictPropertyIdOp  setProperty;
     StrictElementIdOp   setElement;
     GenericAttributesOp getGenericAttributes;
     GenericAttributesOp setGenericAttributes;
-    DeletePropertyOp    deleteProperty;
-    DeleteElementOp     deleteElement;
+    DeleteGenericOp     deleteGeneric;
     WatchOp             watch;
     UnwatchOp           unwatch;
     SliceOp             slice; // Optimized slice, can be null.
-
     JSNewEnumerateOp    enumerate;
     ObjectOp            thisObject;
 };
 
 #define JS_NULL_OBJECT_OPS                                                    \
-    {nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr, \
-     nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr, \
-     nullptr,nullptr}
+    {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,  \
+     nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,  \
+     nullptr, nullptr, nullptr}
 
 } // namespace js
 
 // Classes, objects, and properties.
 
 typedef void (*JSClassInternal)();
 
 struct JSClass {
     JS_CLASS_MEMBERS(JSFinalizeOp);
 
-    void                *reserved[32];
+    void                *reserved[31];
 };
 
 #define JSCLASS_HAS_PRIVATE             (1<<0)  // objects have private slot
 #define JSCLASS_NEW_ENUMERATE           (1<<1)  // has JSNewEnumerateOp hook
 #define JSCLASS_NEW_RESOLVE             (1<<2)  // has JSNewResolveOp hook
 #define JSCLASS_PRIVATE_IS_NSISUPPORTS  (1<<3)  // private is (nsISupports *)
 #define JSCLASS_IS_DOMJSCLASS           (1<<4)  // objects are DOM
 #define JSCLASS_IMPLEMENTS_BARRIERS     (1<<5)  // Correctly implements GC read
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -2088,50 +2088,28 @@ TypedObject::obj_setGenericAttributes(JS
         *attrsp = 0;
         return true;
     }
 
     return JSObject::setGenericAttributes(cx, proto, id, attrsp);
 }
 
 bool
-TypedObject::obj_deleteProperty(JSContext *cx, HandleObject obj,
-                                HandlePropertyName name, bool *succeeded)
+TypedObject::obj_deleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
 {
-    Rooted<jsid> id(cx, NameToId(name));
     if (IsOwnId(cx, obj, id))
         return ReportPropertyError(cx, JSMSG_CANT_DELETE, id);
 
     RootedObject proto(cx, obj->getProto());
     if (!proto) {
         *succeeded = false;
         return true;
     }
 
-    return JSObject::deleteProperty(cx, proto, name, succeeded);
-}
-
-bool
-TypedObject::obj_deleteElement(JSContext *cx, HandleObject obj, uint32_t index,
-                               bool *succeeded)
-{
-    RootedId id(cx);
-    if (!IndexToId(cx, index, &id))
-        return false;
-
-    if (IsOwnId(cx, obj, id))
-        return ReportPropertyError(cx, JSMSG_CANT_DELETE, id);
-
-    RootedObject proto(cx, obj->getProto());
-    if (!proto) {
-        *succeeded = false;
-        return true;
-    }
-
-    return JSObject::deleteElement(cx, proto, index, succeeded);
+    return JSObject::deleteGeneric(cx, proto, id, succeeded);
 }
 
 bool
 TypedObject::obj_enumerate(JSContext *cx, HandleObject obj, JSIterateOp enum_op,
                            MutableHandleValue statep, MutableHandleId idp)
 {
     int32_t index;
 
@@ -2282,18 +2260,17 @@ const Class TransparentTypedObject::clas
         TypedObject::obj_getGeneric,
         TypedObject::obj_getProperty,
         TypedObject::obj_getElement,
         TypedObject::obj_setGeneric,
         TypedObject::obj_setProperty,
         TypedObject::obj_setElement,
         TypedObject::obj_getGenericAttributes,
         TypedObject::obj_setGenericAttributes,
-        TypedObject::obj_deleteProperty,
-        TypedObject::obj_deleteElement,
+        TypedObject::obj_deleteGeneric,
         nullptr, nullptr, // watch/unwatch
         nullptr,   /* slice */
         TypedObject::obj_enumerate,
         nullptr, /* thisObject */
     }
 };
 
 static int32_t
@@ -2628,18 +2605,17 @@ const Class OpaqueTypedObject::class_ = 
         TypedObject::obj_getGeneric,
         TypedObject::obj_getProperty,
         TypedObject::obj_getElement,
         TypedObject::obj_setGeneric,
         TypedObject::obj_setProperty,
         TypedObject::obj_setElement,
         TypedObject::obj_getGenericAttributes,
         TypedObject::obj_setGenericAttributes,
-        TypedObject::obj_deleteProperty,
-        TypedObject::obj_deleteElement,
+        TypedObject::obj_deleteGeneric,
         nullptr, nullptr, // watch/unwatch
         nullptr, // slice
         TypedObject::obj_enumerate,
         nullptr, /* thisObject */
     }
 };
 
 const JSFunctionSpec OpaqueTypedObject::handleStaticMethods[] = {
--- a/js/src/builtin/TypedObject.h
+++ b/js/src/builtin/TypedObject.h
@@ -571,20 +571,17 @@ class TypedObject : public ArrayBufferVi
     static bool obj_setElement(JSContext *cx, HandleObject obj, uint32_t index,
                                MutableHandleValue vp, bool strict);
 
     static bool obj_getGenericAttributes(JSContext *cx, HandleObject obj,
                                          HandleId id, unsigned *attrsp);
     static bool obj_setGenericAttributes(JSContext *cx, HandleObject obj,
                                          HandleId id, unsigned *attrsp);
 
-    static bool obj_deleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name,
-                                   bool *succeeded);
-    static bool obj_deleteElement(JSContext *cx, HandleObject obj, uint32_t index,
-                                  bool *succeeded);
+    static bool obj_deleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded);
 
     static bool obj_enumerate(JSContext *cx, HandleObject obj, JSIterateOp enum_op,
                               MutableHandleValue statep, MutableHandleId idp);
 
   public:
     static size_t offsetOfOwnerSlot();
 
     // Each typed object contains a void* pointer pointing at the
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3626,17 +3626,17 @@ JS_SetUCProperty(JSContext *cx, HandleOb
 
 JS_PUBLIC_API(bool)
 JS_DeletePropertyById2(JSContext *cx, HandleObject obj, HandleId id, bool *result)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj, id);
 
-    return JSObject::deleteByValue(cx, obj, IdToValue(id), result);
+    return JSObject::deleteGeneric(cx, obj, id, result);
 }
 
 JS_PUBLIC_API(bool)
 JS_DeleteElement2(JSContext *cx, HandleObject obj, uint32_t index, bool *result)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
@@ -3648,30 +3648,32 @@ JS_PUBLIC_API(bool)
 JS_DeleteProperty2(JSContext *cx, HandleObject obj, const char *name, bool *result)
 {
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
 
     JSAtom *atom = Atomize(cx, name, strlen(name));
     if (!atom)
         return false;
-    return JSObject::deleteByValue(cx, obj, StringValue(atom), result);
+    RootedId id(cx, AtomToId(atom));
+    return JSObject::deleteGeneric(cx, obj, id, result);
 }
 
 JS_PUBLIC_API(bool)
 JS_DeleteUCProperty2(JSContext *cx, HandleObject obj, const jschar *name, size_t namelen,
                      bool *result)
 {
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
 
     JSAtom *atom = AtomizeChars(cx, name, AUTO_NAMELEN(name, namelen));
     if (!atom)
         return false;
-    return JSObject::deleteByValue(cx, obj, StringValue(atom), result);
+    RootedId id(cx, AtomToId(atom));
+    return JSObject::deleteGeneric(cx, obj, id, result);
 }
 
 JS_PUBLIC_API(bool)
 JS_DeletePropertyById(JSContext *cx, HandleObject obj, HandleId id)
 {
     bool junk;
     return JS_DeletePropertyById2(cx, obj, id, &junk);
 }
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -332,20 +332,20 @@ DeleteArrayElement(JSContext *cx, Handle
                     return false;
             }
         }
 
         *succeeded = true;
         return true;
     }
 
-    if (index <= UINT32_MAX)
-        return JSObject::deleteElement(cx, obj, uint32_t(index), succeeded);
-
-    return JSObject::deleteByValue(cx, obj, DoubleValue(index), succeeded);
+    RootedId id(cx);
+    if (!ToId(cx, index, &id))
+        return false;
+    return JSObject::deleteGeneric(cx, obj, id, succeeded);
 }
 
 /* ES6 20130308 draft 9.3.5 */
 static bool
 DeletePropertyOrThrow(JSContext *cx, HandleObject obj, double index)
 {
     bool succeeded;
     if (!DeleteArrayElement(cx, obj, index, &succeeded))
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -278,18 +278,17 @@ namespace js {
             js::proxy_GetGeneric,                                                       \
             js::proxy_GetProperty,                                                      \
             js::proxy_GetElement,                                                       \
             js::proxy_SetGeneric,                                                       \
             js::proxy_SetProperty,                                                      \
             js::proxy_SetElement,                                                       \
             js::proxy_GetGenericAttributes,                                             \
             js::proxy_SetGenericAttributes,                                             \
-            js::proxy_DeleteProperty,                                                   \
-            js::proxy_DeleteElement,                                                    \
+            js::proxy_DeleteGeneric,                                                    \
             js::proxy_Watch, js::proxy_Unwatch,                                         \
             js::proxy_Slice,                                                            \
             nullptr,             /* enumerate       */                                  \
             nullptr,             /* thisObject      */                                  \
         }                                                                               \
     }
 
 #define PROXY_CLASS_DEF(name, extraSlots, flags, callOp, constructOp)   \
@@ -344,20 +343,17 @@ proxy_SetProperty(JSContext *cx, JS::Han
 extern JS_FRIEND_API(bool)
 proxy_SetElement(JSContext *cx, JS::HandleObject obj, uint32_t index, JS::MutableHandleValue vp,
                  bool strict);
 extern JS_FRIEND_API(bool)
 proxy_GetGenericAttributes(JSContext *cx, JS::HandleObject obj, JS::HandleId id, unsigned *attrsp);
 extern JS_FRIEND_API(bool)
 proxy_SetGenericAttributes(JSContext *cx, JS::HandleObject obj, JS::HandleId id, unsigned *attrsp);
 extern JS_FRIEND_API(bool)
-proxy_DeleteProperty(JSContext *cx, JS::HandleObject obj, JS::Handle<PropertyName*> name,
-                     bool *succeeded);
-extern JS_FRIEND_API(bool)
-proxy_DeleteElement(JSContext *cx, JS::HandleObject obj, uint32_t index, bool *succeeded);
+proxy_DeleteGeneric(JSContext *cx, JS::HandleObject obj, JS::HandleId id, bool *succeeded);
 
 extern JS_FRIEND_API(void)
 proxy_Trace(JSTracer *trc, JSObject *obj);
 extern JS_FRIEND_API(JSObject *)
 proxy_WeakmapKeyDelegate(JSObject *obj);
 extern JS_FRIEND_API(bool)
 proxy_Convert(JSContext *cx, JS::HandleObject proxy, JSType hint, JS::MutableHandleValue vp);
 extern JS_FRIEND_API(void)
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1814,36 +1814,16 @@ JSObject::nonNativeSetElement(JSContext 
 
         WatchpointMap *wpmap = cx->compartment()->watchpointMap;
         if (wpmap && !wpmap->triggerWatchpoint(cx, obj, id, vp))
             return false;
     }
     return obj->getOps()->setElement(cx, obj, index, vp, strict);
 }
 
-/* static */ bool
-JSObject::deleteByValue(JSContext *cx, HandleObject obj, const Value &property, bool *succeeded)
-{
-    uint32_t index;
-    if (IsDefinitelyIndex(property, &index))
-        return deleteElement(cx, obj, index, succeeded);
-
-    RootedValue propval(cx, property);
-
-    JSAtom *name = ToAtom<CanGC>(cx, propval);
-    if (!name)
-        return false;
-
-    if (name->isIndex(&index))
-        return deleteElement(cx, obj, index, succeeded);
-
-    Rooted<PropertyName*> propname(cx, name->asPropertyName());
-    return deleteProperty(cx, obj, propname, succeeded);
-}
-
 JS_FRIEND_API(bool)
 JS_CopyPropertyFrom(JSContext *cx, HandleId id, HandleObject target,
                     HandleObject obj)
 {
     // |obj| and |cx| are generally not same-compartment with |target| here.
     assertSameCompartment(cx, obj, id);
     Rooted<JSPropertyDescriptor> desc(cx);
 
@@ -2711,17 +2691,18 @@ DefineConstructorAndPrototype(JSContext 
 
     if (ctorp)
         *ctorp = ctor;
     return proto;
 
 bad:
     if (named) {
         bool succeeded;
-        JSObject::deleteByValue(cx, obj, StringValue(atom), &succeeded);
+        RootedId id(cx, AtomToId(atom));
+        JSObject::deleteGeneric(cx, obj, id, &succeeded);
     }
     if (cached)
         ClearClassObject(obj, key);
     return nullptr;
 }
 
 JSObject *
 js_InitClass(JSContext *cx, HandleObject obj, JSObject *protoProto_,
@@ -5308,33 +5289,16 @@ baseops::DeleteGeneric(JSContext *cx, Ha
         return false;
     if (!succeeded)
         return true;
 
     return obj->removeProperty(cx, id) && js_SuppressDeletedProperty(cx, obj, id);
 }
 
 bool
-baseops::DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name,
-                        bool *succeeded)
-{
-    Rooted<jsid> id(cx, NameToId(name));
-    return baseops::DeleteGeneric(cx, obj, id, succeeded);
-}
-
-bool
-baseops::DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeeded)
-{
-    RootedId id(cx);
-    if (!IndexToId(cx, index, &id))
-        return false;
-    return baseops::DeleteGeneric(cx, obj, id, succeeded);
-}
-
-bool
 js::WatchGuts(JSContext *cx, JS::HandleObject origObj, JS::HandleId id, JS::HandleObject callable)
 {
     RootedObject obj(cx, GetInnerObject(origObj));
     if (obj->isNative()) {
         // Use sparse indexes for watched objects, as dense elements can be
         // written to without checking the watchpoint map.
         if (!JSObject::sparsifyDenseElements(cx, obj))
             return false;
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -144,22 +144,16 @@ SetElementHelper(JSContext *cx, HandleOb
 
 extern bool
 GetAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp);
 
 extern bool
 SetAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp);
 
 extern bool
-DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name, bool *succeeded);
-
-extern bool
-DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeeded);
-
-extern bool
 DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded);
 
 extern bool
 Watch(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::HandleObject callable);
 
 extern bool
 Unwatch(JSContext *cx, JS::HandleObject obj, JS::HandleId id);
 
@@ -1049,23 +1043,20 @@ class JSObject : public js::ObjectImpl
     {
         js::GenericAttributesOp op = obj->getOps()->getGenericAttributes;
         return (op ? op : js::baseops::GetAttributes)(cx, obj, id, attrsp);
     }
 
     static inline bool setGenericAttributes(JSContext *cx, js::HandleObject obj,
                                             js::HandleId id, unsigned *attrsp);
 
-    static inline bool deleteProperty(JSContext *cx, js::HandleObject obj,
-                                      js::HandlePropertyName name,
-                                      bool *succeeded);
-    static inline bool deleteElement(JSContext *cx, js::HandleObject obj,
-                                     uint32_t index, bool *succeeded);
-    static bool deleteByValue(JSContext *cx, js::HandleObject obj,
-                              const js::Value &property, bool *succeeded);
+    static inline bool deleteGeneric(JSContext *cx, js::HandleObject obj, js::HandleId id,
+                                     bool *succeeded);
+    static inline bool deleteElement(JSContext *cx, js::HandleObject obj, uint32_t index,
+                                     bool *succeeded);
 
     static inline bool watch(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
                              JS::HandleObject callable);
     static inline bool unwatch(JSContext *cx, JS::HandleObject obj, JS::HandleId id);
 
     static bool enumerate(JSContext *cx, JS::HandleObject obj, JSIterateOp iterop,
                           JS::MutableHandleValue statep, JS::MutableHandleId idp)
     {
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -36,34 +36,31 @@ JSObject::setGenericAttributes(JSContext
 JSObject::changePropertyAttributes(JSContext *cx, js::HandleObject obj,
                                    js::HandleShape shape, unsigned attrs)
 {
     return !!changeProperty<js::SequentialExecution>(cx, obj, shape, attrs, 0,
                                                      shape->getter(), shape->setter());
 }
 
 /* static */ inline bool
-JSObject::deleteProperty(JSContext *cx, js::HandleObject obj, js::HandlePropertyName name,
+JSObject::deleteGeneric(JSContext *cx, js::HandleObject obj, js::HandleId id,
                          bool *succeeded)
 {
-    JS::RootedId id(cx, js::NameToId(name));
     js::types::MarkTypePropertyNonData(cx, obj, id);
-    js::DeletePropertyOp op = obj->getOps()->deleteProperty;
-    return (op ? op : js::baseops::DeleteProperty)(cx, obj, name, succeeded);
+    js::DeleteGenericOp op = obj->getOps()->deleteGeneric;
+    return (op ? op : js::baseops::DeleteGeneric)(cx, obj, id, succeeded);
 }
 
 /* static */ inline bool
 JSObject::deleteElement(JSContext *cx, js::HandleObject obj, uint32_t index, bool *succeeded)
 {
     JS::RootedId id(cx);
     if (!js::IndexToId(cx, index, &id))
         return false;
-    js::types::MarkTypePropertyNonData(cx, obj, id);
-    js::DeleteElementOp op = obj->getOps()->deleteElement;
-    return (op ? op : js::baseops::DeleteElement)(cx, obj, index, succeeded);
+    return deleteGeneric(cx, obj, id, succeeded);
 }
 
 /* static */ inline bool
 JSObject::watch(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
                 JS::HandleObject callable)
 {
     js::WatchOp op = obj->getOps()->watch;
     return (op ? op : js::baseops::Watch)(cx, obj, id, callable);
--- a/js/src/json.cpp
+++ b/js/src/json.cpp
@@ -693,17 +693,17 @@ Walk(JSContext *cx, HandleObject holder,
 
                 /* Step 2a(iii)(1). */
                 if (!Walk(cx, obj, id, reviver, &newElement))
                     return false;
 
                 if (newElement.isUndefined()) {
                     /* Step 2a(iii)(2). */
                     bool succeeded;
-                    if (!JSObject::deleteByValue(cx, obj, IdToValue(id), &succeeded))
+                    if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
                         return false;
                 } else {
                     /* Step 2a(iii)(3). */
                     // XXX This definition should ignore success/failure, when
                     //     our property-definition APIs indicate that.
                     if (!JSObject::defineGeneric(cx, obj, id, newElement))
                         return false;
                 }
@@ -721,17 +721,17 @@ Walk(JSContext *cx, HandleObject holder,
                 /* Step 2b(ii)(1). */
                 id = keys[i];
                 if (!Walk(cx, obj, id, reviver, &newElement))
                     return false;
 
                 if (newElement.isUndefined()) {
                     /* Step 2b(ii)(2). */
                     bool succeeded;
-                    if (!JSObject::deleteByValue(cx, obj, IdToValue(id), &succeeded))
+                    if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
                         return false;
                 } else {
                     /* Step 2b(ii)(3). */
                     // XXX This definition should ignore success/failure, when
                     //     our property-definition APIs indicate that.
                     if (!JSObject::defineGeneric(cx, obj, id, newElement))
                         return false;
                 }
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -422,17 +422,17 @@ DirectProxyHandler::getOwnPropertyNames(
     return GetPropertyNames(cx, target, JSITER_OWNONLY | JSITER_HIDDEN, &props);
 }
 
 bool
 DirectProxyHandler::delete_(JSContext *cx, HandleObject proxy, HandleId id, bool *bp)
 {
     assertEnteredPolicy(cx, proxy, id, SET);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
-    return JS_DeletePropertyById2(cx, target, id, bp);
+    return JSObject::deleteGeneric(cx, target, id, bp);
 }
 
 bool
 DirectProxyHandler::enumerate(JSContext *cx, HandleObject proxy,
                               AutoIdVector &props)
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID, ENUMERATE);
     JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype.
@@ -2915,42 +2915,26 @@ js::proxy_SetGenericAttributes(JSContext
     /* Lookup the current property descriptor so we have setter/getter/value. */
     Rooted<PropertyDescriptor> desc(cx);
     if (!Proxy::getOwnPropertyDescriptor(cx, obj, id, &desc))
         return false;
     desc.setAttributes(*attrsp);
     return Proxy::defineProperty(cx, obj, id, &desc);
 }
 
-static bool
-proxy_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
+bool
+js::proxy_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
 {
     bool deleted;
     if (!Proxy::delete_(cx, obj, id, &deleted))
         return false;
     *succeeded = deleted;
     return js_SuppressDeletedProperty(cx, obj, id);
 }
 
-bool
-js::proxy_DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name, bool *succeeded)
-{
-    RootedId id(cx, NameToId(name));
-    return proxy_DeleteGeneric(cx, obj, id, succeeded);
-}
-
-bool
-js::proxy_DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeeded)
-{
-    RootedId id(cx);
-    if (!IndexToId(cx, index, &id))
-        return false;
-    return proxy_DeleteGeneric(cx, obj, id, succeeded);
-}
-
 void
 js::proxy_Trace(JSTracer *trc, JSObject *obj)
 {
     JS_ASSERT(obj->is<ProxyObject>());
     ProxyObject::trace(trc, obj);
 }
 
 /* static */ void
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/strict/8.12.7-2.js
@@ -0,0 +1,21 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+// delete o[p] only performs ToString(p) once, even if there's a strict error.
+var hits = 0;
+var p = {
+    toString: function () {
+        hits++;
+        return "noconfig";
+    }
+};
+assertEq(testLenientAndStrict('var o = Object.freeze({noconfig: "ow"}); delete o[p]',
+                              returns(false), raisesException(TypeError)),
+         true);
+assertEq(hits, 2);
+
+reportCompare(true, true);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -5412,26 +5412,28 @@ DebuggerObject_defineProperties(JSContex
 /*
  * This does a non-strict delete, as a matter of API design. The case where the
  * property is non-configurable isn't necessarily exceptional here.
  */
 static bool
 DebuggerObject_deleteProperty(JSContext *cx, unsigned argc, Value *vp)
 {
     THIS_DEBUGOBJECT_OWNER_REFERENT(cx, argc, vp, "deleteProperty", args, dbg, obj);
-    RootedValue nameArg(cx, args.get(0));
+    RootedId id(cx);
+    if (!ValueToId<CanGC>(cx, args.get(0), &id))
+        return false;
 
     Maybe<AutoCompartment> ac;
     ac.construct(cx, obj);
-    if (!cx->compartment()->wrap(cx, &nameArg))
+    if (!cx->compartment()->wrapId(cx, id.address()))
         return false;
 
     bool succeeded;
     ErrorCopier ec(ac, dbg->toJSObject());
-    if (!JSObject::deleteByValue(cx, obj, nameArg, &succeeded))
+    if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
         return false;
     args.rval().setBoolean(succeeded);
     return true;
 }
 
 enum SealHelperOp { Seal, Freeze, PreventExtensions };
 
 static bool
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -2307,27 +2307,27 @@ CASE(JSOP_DELNAME)
     MutableHandleValue res = REGS.stackHandleAt(-1);
     if (!DeleteNameOperation(cx, name, scopeObj, res))
         goto error;
 }
 END_CASE(JSOP_DELNAME)
 
 CASE(JSOP_DELPROP)
 {
-    RootedPropertyName &name = rootName0;
-    name = script->getName(REGS.pc);
+    RootedId &id = rootId0;
+    id = NameToId(script->getName(REGS.pc));
 
     RootedObject &obj = rootObject0;
     FETCH_OBJECT(cx, -1, obj);
 
     bool succeeded;
-    if (!JSObject::deleteProperty(cx, obj, name, &succeeded))
+    if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
         goto error;
     if (!succeeded && script->strict()) {
-        obj->reportNotConfigurable(cx, NameToId(name));
+        obj->reportNotConfigurable(cx, id);
         goto error;
     }
     MutableHandleValue res = REGS.stackHandleAt(-1);
     res.setBoolean(succeeded);
 }
 END_CASE(JSOP_DELPROP)
 
 CASE(JSOP_DELELEM)
@@ -2335,25 +2335,22 @@ CASE(JSOP_DELELEM)
     /* Fetch the left part and resolve it to a non-null object. */
     RootedObject &obj = rootObject0;
     FETCH_OBJECT(cx, -2, obj);
 
     RootedValue &propval = rootValue0;
     propval = REGS.sp[-1];
 
     bool succeeded;
-    if (!JSObject::deleteByValue(cx, obj, propval, &succeeded))
+    RootedId &id = rootId0;
+    if (!ValueToId<CanGC>(cx, propval, &id))
+        goto error;
+    if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
         goto error;
     if (!succeeded && script->strict()) {
-        // XXX This observably calls ToString(propval).  We should convert to
-        //     PropertyKey and use that to delete, and to report an error if
-        //     necessary!
-        RootedId id(cx);
-        if (!ValueToId<CanGC>(cx, propval, &id))
-            goto error;
         obj->reportNotConfigurable(cx, id);
         goto error;
     }
 
     MutableHandleValue res = REGS.stackHandleAt(-2);
     res.setBoolean(succeeded);
     REGS.sp--;
 }
@@ -3679,17 +3676,18 @@ template bool js::SetProperty<false>(JSC
 template <bool strict>
 bool
 js::DeleteProperty(JSContext *cx, HandleValue v, HandlePropertyName name, bool *bp)
 {
     RootedObject obj(cx, ToObjectFromStack(cx, v));
     if (!obj)
         return false;
 
-    if (!JSObject::deleteProperty(cx, obj, name, bp))
+    RootedId id(cx, NameToId(name));
+    if (!JSObject::deleteGeneric(cx, obj, id, bp))
         return false;
 
     if (strict && !*bp) {
         obj->reportNotConfigurable(cx, NameToId(name));
         return false;
     }
     return true;
 }
@@ -3700,26 +3698,23 @@ template bool js::DeleteProperty<false>(
 template <bool strict>
 bool
 js::DeleteElement(JSContext *cx, HandleValue val, HandleValue index, bool *bp)
 {
     RootedObject obj(cx, ToObjectFromStack(cx, val));
     if (!obj)
         return false;
 
-    if (!JSObject::deleteByValue(cx, obj, index, bp))
+    RootedId id(cx);
+    if (!ValueToId<CanGC>(cx, index, &id))
+        return false;
+    if (!JSObject::deleteGeneric(cx, obj, id, bp))
         return false;
 
     if (strict && !*bp) {
-        // XXX This observably calls ToString(propval).  We should convert to
-        //     PropertyKey and use that to delete, and to report an error if
-        //     necessary!
-        RootedId id(cx);
-        if (!ValueToId<CanGC>(cx, index, &id))
-            return false;
         obj->reportNotConfigurable(cx, id);
         return false;
     }
     return true;
 }
 
 template bool js::DeleteElement<true> (JSContext *, HandleValue, HandleValue, bool *succeeded);
 template bool js::DeleteElement<false>(JSContext *, HandleValue, HandleValue, bool *succeeded);
@@ -3810,17 +3805,18 @@ js::DeleteNameOperation(JSContext *cx, H
 
     if (!scope) {
         // Return true for non-existent names.
         res.setBoolean(true);
         return true;
     }
 
     bool succeeded;
-    if (!JSObject::deleteProperty(cx, scope, name, &succeeded))
+    RootedId id(cx, NameToId(name));
+    if (!JSObject::deleteGeneric(cx, scope, id, &succeeded))
         return false;
     res.setBoolean(succeeded);
     return true;
 }
 
 bool
 js::ImplicitThisOperation(JSContext *cx, HandleObject scopeObj, HandlePropertyName name,
                           MutableHandleValue res)
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -529,29 +529,20 @@ with_GetGenericAttributes(JSContext *cx,
 static bool
 with_SetGenericAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp)
 {
     RootedObject actual(cx, &obj->as<DynamicWithObject>().object());
     return JSObject::setGenericAttributes(cx, actual, id, attrsp);
 }
 
 static bool
-with_DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name,
-                    bool *succeeded)
+with_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
 {
     RootedObject actual(cx, &obj->as<DynamicWithObject>().object());
-    return JSObject::deleteProperty(cx, actual, name, succeeded);
-}
-
-static bool
-with_DeleteElement(JSContext *cx, HandleObject obj, uint32_t index,
-                   bool *succeeded)
-{
-    RootedObject actual(cx, &obj->as<DynamicWithObject>().object());
-    return JSObject::deleteElement(cx, actual, index, succeeded);
+    return JSObject::deleteGeneric(cx, actual, id, succeeded);
 }
 
 static JSObject *
 with_ThisObject(JSContext *cx, HandleObject obj)
 {
     return &obj->as<DynamicWithObject>().withThis();
 }
 
@@ -597,18 +588,17 @@ const Class DynamicWithObject::class_ = 
         with_GetGeneric,
         with_GetProperty,
         with_GetElement,
         with_SetGeneric,
         with_SetProperty,
         with_SetElement,
         with_GetGenericAttributes,
         with_SetGenericAttributes,
-        with_DeleteProperty,
-        with_DeleteElement,
+        with_DeleteGeneric,
         nullptr, nullptr,    /* watch/unwatch */
         nullptr,             /* slice */
         nullptr,             /* enumerate (native enumeration of target doesn't work) */
         with_ThisObject,
     }
 };
 
 /*****************************************************************************/
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -692,18 +692,17 @@ const XPCWrappedNativeJSClass XPC_WN_NoH
         nullptr, // getGeneric
         nullptr, // getProperty
         nullptr, // getElement
         nullptr, // setGeneric
         nullptr, // setProperty
         nullptr, // setElement
         nullptr, // getGenericAttributes
         nullptr, // setGenericAttributes
-        nullptr, // deleteProperty
-        nullptr, // deleteElement
+        nullptr, // deleteGeneric
         nullptr, nullptr, // watch/unwatch
         nullptr, // slice
         XPC_WN_JSOp_Enumerate,
         XPC_WN_JSOp_ThisObject,
     }
   },
   0 // interfacesBitmap
 };
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -907,18 +907,17 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS
         nullptr, /* getGeneric    */                                          \
         nullptr, /* getProperty    */                                         \
         nullptr, /* getElement    */                                          \
         nullptr, /* setGeneric    */                                          \
         nullptr, /* setProperty    */                                         \
         nullptr, /* setElement    */                                          \
         nullptr, /* getGenericAttributes  */                                  \
         nullptr, /* setGenericAttributes  */                                  \
-        nullptr, /* deleteProperty */                                         \
-        nullptr, /* deleteElement */                                          \
+        nullptr, /* deleteGeneric */                                          \
         nullptr, nullptr, /* watch/unwatch */                                 \
         nullptr, /* slice */                                                  \
         XPC_WN_JSOp_Enumerate,                                                \
         XPC_WN_JSOp_ThisObject,                                               \
     }
 
 #define XPC_WN_NoCall_ObjectOps                                               \
     {                                                                         \
@@ -931,18 +930,17 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS
         nullptr, /* getGeneric    */                                          \
         nullptr, /* getProperty    */                                         \
         nullptr, /* getElement    */                                          \
         nullptr, /* setGeneric    */                                          \
         nullptr, /* setProperty    */                                         \
         nullptr, /* setElement    */                                          \
         nullptr, /* getGenericAttributes  */                                  \
         nullptr, /* setGenericAttributes  */                                  \
-        nullptr, /* deleteProperty */                                         \
-        nullptr, /* deleteElement */                                          \
+        nullptr, /* deleteGeneric */                                          \
         nullptr, nullptr, /* watch/unwatch */                                 \
         nullptr, /* slice */                                                  \
         XPC_WN_JSOp_Enumerate,                                                \
         XPC_WN_JSOp_ThisObject,                                               \
     }
 
 // Maybe this macro should check for class->enumerate ==
 // XPC_WN_Shared_Proto_Enumerate or something rather than checking for