Bug 1055755 - Remove |existing| arg from JSCompartment::wrap. (r=billm)
☠☠ backed out by 33781a3a5201 ☠ ☠
authorEric Faust <efaustbmo@gmail.com>
Tue, 06 Jan 2015 13:12:06 -0800
changeset 222276 205f8fa00772c35ad26f9320639988e27dfae247
parent 222275 c3ce3636860899d280d218b15867d056e6e1bafe
child 222277 a64998089d386b7a67d01711c017fd2b36ad062e
push id53570
push userefaustbmo@gmail.com
push dateTue, 06 Jan 2015 21:12:55 +0000
treeherdermozilla-inbound@205f8fa00772 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1055755
milestone37.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 1055755 - Remove |existing| arg from JSCompartment::wrap. (r=billm)
js/src/jsapi-tests/testBug604087.cpp
js/src/jsapi.h
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jscompartmentinlines.h
js/src/jswrapper.h
js/src/proxy/CrossCompartmentWrapper.cpp
js/src/proxy/Proxy.cpp
js/src/proxy/Wrapper.cpp
js/src/vm/ProxyObject.h
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
--- a/js/src/jsapi-tests/testBug604087.cpp
+++ b/js/src/jsapi-tests/testBug604087.cpp
@@ -39,18 +39,17 @@ static JSObject *
 PreWrap(JSContext *cx, JS::HandleObject scope, JS::HandleObject obj,
         JS::HandleObject objectPassedToWrap)
 {
     JS_GC(JS_GetRuntime(cx));
     return obj;
 }
 
 static JSObject *
-Wrap(JSContext *cx, JS::HandleObject existing, JS::HandleObject obj,
-     JS::HandleObject parent)
+Wrap(JSContext *cx, JS::HandleObject obj, JS::HandleObject parent)
 {
     return js::Wrapper::New(cx, obj, parent, &js::CrossCompartmentWrapper::singleton);
 }
 
 static const JSWrapObjectCallbacks WrapObjectCallbacks = {
     Wrap,
     PreWrap
 };
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -796,25 +796,20 @@ typedef bool
                     JS::MutableHandleValue rval);
 
 typedef bool
 (* JSLocaleToUnicode)(JSContext *cx, const char *src, JS::MutableHandleValue rval);
 
 /*
  * Callback used to ask the embedding for the cross compartment wrapper handler
  * that implements the desired prolicy for this kind of object in the
- * destination compartment. |obj| is the object to be wrapped. If |existing| is
- * non-nullptr, it will point to an existing wrapper object that should be
- * re-used if possible. |existing| is guaranteed to be a cross-compartment
- * wrapper with a lazily-defined prototype and the correct global. It is
- * guaranteed not to wrap a function.
+ * destination compartment. |obj| is the object to be wrapped.
  */
 typedef JSObject *
-(* JSWrapObjectCallback)(JSContext *cx, JS::HandleObject existing, JS::HandleObject obj,
-                         JS::HandleObject parent);
+(* JSWrapObjectCallback)(JSContext *cx, JS::HandleObject obj, JS::HandleObject parent);
 
 /*
  * Callback used by the wrap hook to ask the embedding to prepare an object
  * for wrapping in a context. This might include unwrapping other wrappers
  * or even finding a more suitable object for the new compartment.
  */
 typedef JSObject *
 (* JSPreWrapCallback)(JSContext *cx, JS::HandleObject scope, JS::HandleObject obj,
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -338,22 +338,20 @@ JSCompartment::wrap(JSContext *cx, Mutab
     if (!putWrapper(cx, CrossCompartmentKey(key), StringValue(copy)))
         return false;
 
     strp.set(copy);
     return true;
 }
 
 bool
-JSCompartment::wrap(JSContext *cx, MutableHandleObject obj, HandleObject existingArg)
+JSCompartment::wrap(JSContext *cx, MutableHandleObject obj)
 {
     MOZ_ASSERT(!cx->runtime()->isAtomsCompartment(this));
     MOZ_ASSERT(cx->compartment() == this);
-    MOZ_ASSERT_IF(existingArg, existingArg->compartment() == cx->compartment());
-    MOZ_ASSERT_IF(existingArg, IsDeadProxyObject(existingArg));
 
     if (!obj)
         return true;
     AutoDisableProxyCheck adpc(cx->runtime());
 
     // Wrappers should really be parented to the wrapped parent of the wrapped
     // object, but in that case a wrapped global object would have a nullptr
     // parent without being a proper global object (JSCLASS_IS_GLOBAL). Instead,
@@ -416,30 +414,17 @@ JSCompartment::wrap(JSContext *cx, Mutab
     RootedValue key(cx, ObjectValue(*obj));
     if (WrapperMap::Ptr p = crossCompartmentWrappers.lookup(CrossCompartmentKey(key))) {
         obj.set(&p->value().get().toObject());
         MOZ_ASSERT(obj->is<CrossCompartmentWrapperObject>());
         MOZ_ASSERT(obj->getParent() == global);
         return true;
     }
 
-    RootedObject existing(cx, existingArg);
-    if (existing) {
-        // Is it possible to reuse |existing|?
-        if (!existing->getTaggedProto().isLazy() ||
-            // Note: Class asserted above, so all that's left to check is callability
-            existing->isCallable() ||
-            existing->getParent() != global ||
-            obj->isCallable())
-        {
-            existing = nullptr;
-        }
-    }
-
-    obj.set(cb->wrap(cx, existing, obj, global));
+    obj.set(cb->wrap(cx, obj, global));
     if (!obj)
         return false;
 
     // We maintain the invariant that the key in the cross-compartment wrapper
     // map is always directly wrapped by the value.
     MOZ_ASSERT(Wrapper::wrappedObject(obj) == &key.get().toObject());
 
     return putWrapper(cx, CrossCompartmentKey(key), ObjectValue(*obj));
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -350,22 +350,20 @@ struct JSCompartment
     JSCompartment(JS::Zone *zone, const JS::CompartmentOptions &options);
     ~JSCompartment();
 
     bool init(JSContext *cx);
 
     /* Mark cross-compartment wrappers. */
     void markCrossCompartmentWrappers(JSTracer *trc);
 
-    inline bool wrap(JSContext *cx, JS::MutableHandleValue vp,
-                     JS::HandleObject existing = js::NullPtr());
+    inline bool wrap(JSContext *cx, JS::MutableHandleValue vp);
 
     bool wrap(JSContext *cx, js::MutableHandleString strp);
-    bool wrap(JSContext *cx, JS::MutableHandleObject obj,
-              JS::HandleObject existingArg = js::NullPtr());
+    bool wrap(JSContext *cx, JS::MutableHandleObject obj);
     bool wrap(JSContext *cx, JS::MutableHandle<js::PropertyDescriptor> desc);
     bool wrap(JSContext *cx, JS::MutableHandle<js::PropDesc> desc);
 
     template<typename T> bool wrap(JSContext *cx, JS::AutoVectorRooter<T> &vec) {
         for (size_t i = 0; i < vec.length(); ++i) {
             if (!wrap(cx, vec[i]))
                 return false;
         }
--- a/js/src/jscompartmentinlines.h
+++ b/js/src/jscompartmentinlines.h
@@ -49,20 +49,18 @@ js::AutoCompartment::AutoCompartment(Exc
 }
 
 js::AutoCompartment::~AutoCompartment()
 {
     cx_->leaveCompartment(origin_);
 }
 
 inline bool
-JSCompartment::wrap(JSContext *cx, JS::MutableHandleValue vp, JS::HandleObject existing)
+JSCompartment::wrap(JSContext *cx, JS::MutableHandleValue vp)
 {
-    MOZ_ASSERT_IF(existing, vp.isObject());
-
     /* Only GC things have to be wrapped or copied. */
     if (!vp.isMarkable())
         return true;
 
     /*
      * Symbols are GC things, but never need to be wrapped or copied because
      * they are always allocated in the atoms compartment.
      */
@@ -109,16 +107,16 @@ JSCompartment::wrap(JSContext *cx, JS::M
         cacheResult = &p->value().get().toObject();
 #else
         vp.set(p->value());
         return true;
 #endif
     }
 
     JS::RootedObject obj(cx, &vp.toObject());
-    if (!wrap(cx, &obj, existing))
+    if (!wrap(cx, &obj))
         return false;
     vp.setObject(*obj);
     MOZ_ASSERT_IF(cacheResult, obj == cacheResult);
     return true;
 }
 
 #endif /* jscompartmentinlines_h */
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -67,17 +67,17 @@ class JS_FRIEND_API(Wrapper) : public Di
     };
 
     virtual bool defaultValue(JSContext *cx, HandleObject obj, JSType hint,
                               MutableHandleValue vp) const MOZ_OVERRIDE;
 
     static JSObject *New(JSContext *cx, JSObject *obj, JSObject *parent, const Wrapper *handler,
                          const WrapperOptions &options = WrapperOptions());
 
-    static JSObject *Renew(JSContext *cx, JSObject *existing, JSObject *obj, const Wrapper *handler);
+    static JSObject *Renew(JSContext *cx, JSObject *obj, const Wrapper *handler);
 
     static const Wrapper *wrapperHandler(JSObject *wrapper);
 
     static JSObject *wrappedObject(JSObject *wrapper);
 
     unsigned flags() const {
         return mFlags;
     }
@@ -212,18 +212,17 @@ class JS_FRIEND_API(SecurityWrapper) : p
     typedef Base Permissive;
     typedef SecurityWrapper<Base> Restrictive;
 };
 
 typedef SecurityWrapper<Wrapper> SameCompartmentSecurityWrapper;
 typedef SecurityWrapper<CrossCompartmentWrapper> CrossCompartmentSecurityWrapper;
 
 extern JSObject *
-TransparentObjectWrapper(JSContext *cx, HandleObject existing, HandleObject obj,
-                         HandleObject parent);
+TransparentObjectWrapper(JSContext *cx, HandleObject obj, HandleObject parent);
 
 inline bool
 IsWrapper(JSObject *obj)
 {
     return IsProxy(obj) && GetProxyHandler(obj)->family() == &Wrapper::family;
 }
 
 // Given a JSObject, returns that object stripped of wrappers. If
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -521,35 +521,27 @@ js::RemapWrapper(JSContext *cx, JSObject
     WrapperMap::Ptr p = wcompartment->lookupWrapper(origv);
     MOZ_ASSERT(&p->value().unsafeGet()->toObject() == wobj);
     wcompartment->removeWrapper(p);
 
     // When we remove origv from the wrapper map, its wrapper, wobj, must
     // immediately cease to be a cross-compartment wrapper. Neuter it.
     NukeCrossCompartmentWrapper(cx, wobj);
 
-    // First, we wrap it in the new compartment. We try to use the existing
-    // wrapper, |wobj|, since it's been nuked anyway. The wrap() function has
-    // the choice to reuse |wobj| or not.
+    // First, we wrap it in the new compartment.
     RootedObject tobj(cx, newTarget);
     AutoCompartment ac(cx, wobj);
-    if (!wcompartment->wrap(cx, &tobj, wobj))
+    if (!wcompartment->wrap(cx, &tobj))
         MOZ_CRASH();
 
-    // 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();
-    }
+    // 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();
 
     // 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).
     MOZ_ASSERT(wobj->is<WrapperObject>());
--- a/js/src/proxy/Proxy.cpp
+++ b/js/src/proxy/Proxy.cpp
@@ -828,31 +828,16 @@ const Class* const js::ProxyClassPtr = &
 JS_FRIEND_API(JSObject *)
 js::NewProxyObject(JSContext *cx, const BaseProxyHandler *handler, HandleValue priv, JSObject *proto_,
                    JSObject *parent_, const ProxyOptions &options)
 {
     return ProxyObject::New(cx, handler, priv, TaggedProto(proto_), parent_,
                             options);
 }
 
-void
-ProxyObject::renew(JSContext *cx, const BaseProxyHandler *handler, Value priv)
-{
-    MOZ_ASSERT_IF(IsCrossCompartmentWrapper(this), IsDeadProxyObject(this));
-    MOZ_ASSERT(getParent() == cx->global());
-    MOZ_ASSERT(getClass() == &ProxyObject::class_);
-    MOZ_ASSERT(!getClass()->ext.innerObject);
-    MOZ_ASSERT(hasLazyPrototype());
-
-    setHandler(handler);
-    setCrossCompartmentPrivate(priv);
-    setExtra(0, UndefinedValue());
-    setExtra(1, UndefinedValue());
-}
-
 JS_FRIEND_API(JSObject *)
 js_InitProxyClass(JSContext *cx, HandleObject obj)
 {
     static const JSFunctionSpec static_methods[] = {
         JS_FN("create",         proxy_create,          2, 0),
         JS_FN("createFunction", proxy_createFunction,  3, 0),
         JS_FN("revocable",      proxy_revocable,       2, 0),
         JS_FS_END
--- a/js/src/proxy/Wrapper.cpp
+++ b/js/src/proxy/Wrapper.cpp
@@ -37,23 +37,16 @@ Wrapper::New(JSContext *cx, JSObject *ob
              const WrapperOptions &options)
 {
     MOZ_ASSERT(parent);
 
     RootedValue priv(cx, ObjectValue(*obj));
     return NewProxyObject(cx, handler, priv, options.proto(), parent, options);
 }
 
-JSObject *
-Wrapper::Renew(JSContext *cx, JSObject *existing, JSObject *obj, const Wrapper *handler)
-{
-    existing->as<ProxyObject>().renew(cx, handler, ObjectValue(*obj));
-    return existing;
-}
-
 const Wrapper *
 Wrapper::wrapperHandler(JSObject *wrapper)
 {
     MOZ_ASSERT(wrapper->is<WrapperObject>());
     return static_cast<const Wrapper*>(wrapper->as<ProxyObject>().handler());
 }
 
 JSObject *
@@ -122,18 +115,17 @@ js::UnwrapOneChecked(JSObject *obj, bool
 const char Wrapper::family = 0;
 const Wrapper Wrapper::singleton((unsigned)0);
 const Wrapper Wrapper::singletonWithPrototype((unsigned)0, true);
 JSObject *Wrapper::defaultProto = TaggedProto::LazyProto;
 
 /* Compartments. */
 
 extern JSObject *
-js::TransparentObjectWrapper(JSContext *cx, HandleObject existing, HandleObject obj,
-                             HandleObject parent)
+js::TransparentObjectWrapper(JSContext *cx, HandleObject obj, HandleObject parent)
 {
     // Allow wrapping outer window proxies.
     MOZ_ASSERT(!obj->is<WrapperObject>() || obj->getClass()->ext.innerObject);
     return Wrapper::New(cx, obj, parent, &CrossCompartmentWrapper::singleton);
 }
 
 ErrorCopier::~ErrorCopier()
 {
--- a/js/src/vm/ProxyObject.h
+++ b/js/src/vm/ProxyObject.h
@@ -94,18 +94,16 @@ class ProxyObject : public JSObject
                (clasp->flags & JSCLASS_IMPLEMENTS_BARRIERS) &&
                clasp->trace == proxy_Trace &&
                !clasp->call && !clasp->construct;
     }
 
   public:
     static unsigned grayLinkExtraSlot(JSObject *obj);
 
-    void renew(JSContext *cx, const BaseProxyHandler *handler, Value priv);
-
     static void trace(JSTracer *trc, JSObject *obj);
 
     void nuke(const BaseProxyHandler *handler);
 
     static const Class class_;
 };
 
 } // namespace js
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -377,18 +377,17 @@ SelectAddonWrapper(JSContext *cx, Handle
     else if (wrapper == &PermissiveXrayDOM::singleton)
         return &AddonWrapper<PermissiveXrayDOM>::singleton;
 
     // |wrapper| is not supported for interposition, so we don't do it.
     return wrapper;
 }
 
 JSObject *
-WrapperFactory::Rewrap(JSContext *cx, HandleObject existing, HandleObject obj,
-                       HandleObject parent)
+WrapperFactory::Rewrap(JSContext *cx, HandleObject obj, HandleObject parent)
 {
     MOZ_ASSERT(!IsWrapper(obj) ||
                GetProxyHandler(obj) == &XrayWaiver ||
                js::GetObjectClass(obj)->ext.innerObject,
                "wrapped object passed to rewrap");
     MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(JS_GetClass(obj)), "trying to wrap a holder");
     MOZ_ASSERT(!js::IsInnerObject(obj));
     // We sometimes end up here after nsContentUtils has been shut down but before
@@ -491,19 +490,16 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
                 NS_WARNING("Trying to expose eval or Function to non-subsuming content!");
                 wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>::singleton;
             }
         }
     }
 
     DEBUG_CheckUnwrapSafety(obj, wrapper, origin, target);
 
-    if (existing)
-        return Wrapper::Renew(cx, existing, obj, wrapper);
-
     return Wrapper::New(cx, obj, parent, wrapper);
 }
 
 // Call WaiveXrayAndWrap when you have a JS object that you don't want to be
 // wrapped in an Xray wrapper. cx->compartment is the compartment that will be
 // using the returned object. If the object to be wrapped is already in the
 // correct compartment, then this returns the unwrapped object.
 bool
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -40,17 +40,16 @@ class WrapperFactory {
     // Prepare a given object for wrapping in a new compartment.
     static JSObject *PrepareForWrapping(JSContext *cx,
                                         JS::HandleObject scope,
                                         JS::HandleObject obj,
                                         JS::HandleObject objectPassedToWrap);
 
     // Rewrap an object that is about to cross compartment boundaries.
     static JSObject *Rewrap(JSContext *cx,
-                            JS::HandleObject existing,
                             JS::HandleObject obj,
                             JS::HandleObject parent);
 
     // Wrap wrapped object into a waiver wrapper and then re-wrap it.
     static bool WaiveXrayAndWrap(JSContext *cx, JS::MutableHandleValue vp);
     static bool WaiveXrayAndWrap(JSContext *cx, JS::MutableHandleObject object);
 };