Bug 1493627 part 1 - Change JSObject::swap return type from bool to void (it always returned true). r=jonco
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 22 Oct 2018 15:25:08 +0000
changeset 490879 271d76b2997b8010c69e15be8f972ce278d99727
parent 490878 d9c2a804cbc8188d5b15e3c8038ea27300ab79a6
child 490880 9697472e6ab7298445ae8f169fe7b1ca5b247f11
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersjonco
bugs1493627
milestone65.0a1
Bug 1493627 part 1 - Change JSObject::swap return type from bool to void (it always returned true). r=jonco Differential Revision: https://phabricator.services.mozilla.com/D9253
js/src/jsapi.cpp
js/src/proxy/CrossCompartmentWrapper.cpp
js/src/vm/JSObject.cpp
js/src/vm/JSObject.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -862,35 +862,31 @@ JS_TransplantObject(JSContext* cx, Handl
     JS::Compartment* destination = target->compartment();
 
     if (origobj->compartment() == destination) {
         // If the original object is in the same compartment as the
         // destination, then we know that we won't find a wrapper in the
         // destination's cross compartment map and that the same
         // object will continue to work.
         AutoRealmUnchecked ar(cx, origobj->nonCCWRealm());
-        if (!JSObject::swap(cx, origobj, target)) {
-            MOZ_CRASH();
-        }
+        JSObject::swap(cx, origobj, target);
         newIdentity = origobj;
     } else if (WrapperMap::Ptr p = destination->lookupWrapper(origv)) {
         // There might already be a wrapper for the original object in
         // the new compartment. If there is, we use its identity and swap
         // in the contents of |target|.
         newIdentity = &p->value().get().toObject();
 
         // When we remove origv from the wrapper map, its wrapper, newIdentity,
         // must immediately cease to be a cross-compartment wrapper. Nuke it.
         destination->removeWrapper(p);
         NukeCrossCompartmentWrapper(cx, newIdentity);
 
         AutoRealm ar(cx, newIdentity);
-        if (!JSObject::swap(cx, newIdentity, target)) {
-            MOZ_CRASH();
-        }
+        JSObject::swap(cx, newIdentity, target);
     } else {
         // Otherwise, we use |target| for the new identity object.
         newIdentity = target;
     }
 
     // Now, iterate through other scopes looking for references to the old
     // object, and update the relevant cross-compartment wrappers. We do this
     // even if origobj is in the same compartment as target and thus
@@ -903,19 +899,17 @@ JS_TransplantObject(JSContext* cx, Handl
     // Lastly, update the original object to point to the new one.
     if (origobj->compartment() != destination) {
         RootedObject newIdentityWrapper(cx, newIdentity);
         AutoRealmUnchecked ar(cx, origobj->nonCCWRealm());
         if (!JS_WrapObject(cx, &newIdentityWrapper)) {
             MOZ_CRASH();
         }
         MOZ_ASSERT(Wrapper::wrappedObject(newIdentityWrapper) == newIdentity);
-        if (!JSObject::swap(cx, origobj, newIdentityWrapper)) {
-            MOZ_CRASH();
-        }
+        JSObject::swap(cx, origobj, newIdentityWrapper);
         if (!origobj->compartment()->putWrapper(cx, CrossCompartmentKey(newIdentity), origv)) {
             MOZ_CRASH();
         }
     }
 
     // The new identity object might be one of several things. Return it to avoid
     // ambiguity.
     MOZ_ASSERT(JS::CellIsNotGray(newIdentity));
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -654,19 +654,17 @@ js::RemapWrapper(JSContext* cx, JSObject
     // If wrap() reused |wobj|, it will have overwritten it and returned with
     // |tobj == wobj|. Otherwise, |tobj| will point to a new wrapper and |wobj|
     // will still be nuked. In the latter case, we replace |wobj| with the
     // contents of the new wrapper in |tobj|.
     if (tobj != wobj) {
         // Now, because we need to maintain object identity, we do a brain
         // transplant on the old object so that it contains the contents of the
         // new one.
-        if (!JSObject::swap(cx, wobj, tobj)) {
-            MOZ_CRASH();
-        }
+        JSObject::swap(cx, wobj, tobj);
     }
 
     // Before swapping, this wrapper came out of wrap(), which enforces the
     // invariant that the wrapper in the map points directly to the key.
     MOZ_ASSERT(Wrapper::wrappedObject(wobj) == newTarget);
 
     // Update the entry in the compartment's wrapper map to point to the old
     // wrapper, which has now been updated (via reuse or swap).
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -1794,17 +1794,17 @@ ProxyObject::initExternalValueArrayAfter
     // Note: we allocate external slots iff the proxy had an inline
     // ProxyValueArray, so at this point reservedSlots points into the
     // old object and we don't have to free anything.
     data.reservedSlots = &valArray->reservedSlots;
     return true;
 }
 
 /* Use this method with extreme caution. It trades the guts of two objects. */
-bool
+void
 JSObject::swap(JSContext* cx, HandleObject a, HandleObject b)
 {
     // Ensure swap doesn't cause a finalizer to not be run.
     MOZ_ASSERT(IsBackgroundFinalized(a->asTenured().getAllocKind()) ==
                IsBackgroundFinalized(b->asTenured().getAllocKind()));
     MOZ_ASSERT(a->compartment() == b->compartment());
 
     // You must have entered the objects' compartment before calling this.
@@ -1971,17 +1971,16 @@ JSObject::swap(JSContext* cx, HandleObje
      */
     JS::Zone* zone = a->zone();
     if (zone->needsIncrementalBarrier()) {
         a->traceChildren(zone->barrierTracer());
         b->traceChildren(zone->barrierTracer());
     }
 
     NotifyGCPostSwap(a, b, r);
-    return true;
 }
 
 static void
 SetClassObject(JSObject* obj, JSProtoKey key, JSObject* cobj, JSObject* proto)
 {
     if (!obj->is<GlobalObject>()) {
         return;
     }
--- a/js/src/vm/JSObject.h
+++ b/js/src/vm/JSObject.h
@@ -484,17 +484,17 @@ class JSObject : public js::gc::Cell
   public:
     static bool nonNativeSetProperty(JSContext* cx, js::HandleObject obj, js::HandleId id,
                                      js::HandleValue v, js::HandleValue receiver,
                                      JS::ObjectOpResult& result);
     static bool nonNativeSetElement(JSContext* cx, js::HandleObject obj, uint32_t index,
                                     js::HandleValue v, js::HandleValue receiver,
                                     JS::ObjectOpResult& result);
 
-    static bool swap(JSContext* cx, JS::HandleObject a, JS::HandleObject b);
+    static void swap(JSContext* cx, JS::HandleObject a, JS::HandleObject b);
 
   private:
     void fixDictionaryShapeAfterSwap();
 
   public:
     inline void initArrayClass();
 
     /*