Bug 1136925 part 2. Stop passing a parent to Wrapper::New. r=waldo,bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 26 Feb 2015 15:58:59 -0500
changeset 231123 84c7cadc7dd22d536019a6b8b5eec86b1f1c4c84
parent 231122 2dac197193b72d9c43bd6c50553c8f683ec195a2
child 231124 2e9b1150861bab37b5120e712fea7e872f4d8bd8
push id56186
push userbzbarsky@mozilla.com
push dateFri, 27 Feb 2015 14:45:07 +0000
treeherdermozilla-inbound@f80f5e04611c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswaldo, bholley
bugs1136925
milestone39.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 1136925 part 2. Stop passing a parent to Wrapper::New. r=waldo,bholley
dom/base/nsGlobalWindow.cpp
js/src/jsapi-tests/testBug604087.cpp
js/src/jsapi.h
js/src/jscompartment.cpp
js/src/jswrapper.h
js/src/proxy/Wrapper.cpp
js/src/shell/js.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -1052,23 +1052,25 @@ nsChromeOuterWindowProxy::className(JSCo
 
     return "ChromeWindow";
 }
 
 const nsChromeOuterWindowProxy
 nsChromeOuterWindowProxy::singleton;
 
 static JSObject*
-NewOuterWindowProxy(JSContext *cx, JS::Handle<JSObject*> parent, bool isChrome)
-{
-  JSAutoCompartment ac(cx, parent);
+NewOuterWindowProxy(JSContext *cx, JS::Handle<JSObject*> global, bool isChrome)
+{
+  JSAutoCompartment ac(cx, global);
+  MOZ_ASSERT(js::GetGlobalForObjectCrossCompartment(global) == global);
+
   js::WrapperOptions options;
   options.setClass(&OuterWindowProxyClass);
   options.setSingleton(true);
-  JSObject *obj = js::Wrapper::New(cx, parent, parent,
+  JSObject *obj = js::Wrapper::New(cx, global,
                                    isChrome ? &nsChromeOuterWindowProxy::singleton
                                             : &nsOuterWindowProxy::singleton,
                                    options);
 
   NS_ASSERTION(js::GetObjectClass(obj)->ext.innerObject, "bad class");
   return obj;
 }
 
--- a/js/src/jsapi-tests/testBug604087.cpp
+++ b/js/src/jsapi-tests/testBug604087.cpp
@@ -39,33 +39,32 @@ 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 existing, JS::HandleObject obj)
 {
-    return js::Wrapper::New(cx, obj, parent, &js::CrossCompartmentWrapper::singleton);
+    return js::Wrapper::New(cx, obj, &js::CrossCompartmentWrapper::singleton);
 }
 
 static const JSWrapObjectCallbacks WrapObjectCallbacks = {
     Wrap,
     PreWrap
 };
 
 BEGIN_TEST(testBug604087)
 {
     js::WrapperOptions options;
     options.setClass(&OuterWrapperClass);
     options.setSingleton(true);
-    JS::RootedObject outerObj(cx, js::Wrapper::New(cx, global, global, &js::Wrapper::singleton, options));
+    JS::RootedObject outerObj(cx, js::Wrapper::New(cx, global, &js::Wrapper::singleton, options));
     JS::RootedObject compartment2(cx, JS_NewGlobalObject(cx, getGlobalClass(), nullptr, JS::FireOnNewGlobalHook));
     JS::RootedObject compartment3(cx, JS_NewGlobalObject(cx, getGlobalClass(), nullptr, JS::FireOnNewGlobalHook));
     JS::RootedObject compartment4(cx, JS_NewGlobalObject(cx, getGlobalClass(), nullptr, JS::FireOnNewGlobalHook));
 
     JS::RootedObject c2wrapper(cx, wrap(cx, outerObj, compartment2));
     CHECK(c2wrapper);
     c2wrapper->as<js::ProxyObject>().setExtra(0, js::Int32Value(2));
 
@@ -76,17 +75,17 @@ BEGIN_TEST(testBug604087)
     JS::RootedObject c4wrapper(cx, wrap(cx, outerObj, compartment4));
     CHECK(c4wrapper);
     c4wrapper->as<js::ProxyObject>().setExtra(0, js::Int32Value(4));
     compartment4 = c4wrapper = nullptr;
 
     JS::RootedObject next(cx);
     {
         JSAutoCompartment ac(cx, compartment2);
-        next = js::Wrapper::New(cx, compartment2, compartment2, &js::Wrapper::singleton, options);
+        next = js::Wrapper::New(cx, compartment2, &js::Wrapper::singleton, options);
         CHECK(next);
     }
 
     JS_SetWrapObjectCallbacks(JS_GetRuntime(cx), &WrapObjectCallbacks);
     CHECK(JS_TransplantObject(cx, outerObj, next));
     return true;
 }
 END_TEST(testBug604087)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -733,18 +733,17 @@ typedef bool
  * 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.
  */
 typedef JSObject *
-(* JSWrapObjectCallback)(JSContext *cx, JS::HandleObject existing, JS::HandleObject obj,
-                         JS::HandleObject parent);
+(* JSWrapObjectCallback)(JSContext *cx, JS::HandleObject existing, JS::HandleObject obj);
 
 /*
  * 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
@@ -438,17 +438,17 @@ JSCompartment::wrap(JSContext *cx, Mutab
             existing->isCallable() ||
             existing->getParent() != global ||
             obj->isCallable())
         {
             existing = nullptr;
         }
     }
 
-    obj.set(cb->wrap(cx, existing, obj, global));
+    obj.set(cb->wrap(cx, existing, obj));
     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/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -64,17 +64,17 @@ class JS_FRIEND_API(Wrapper) : public Di
     enum Flags {
         CROSS_COMPARTMENT = 1 << 0,
         LAST_USED_FLAG = CROSS_COMPARTMENT
     };
 
     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,
+    static JSObject *New(JSContext *cx, JSObject *obj, const Wrapper *handler,
                          const WrapperOptions &options = WrapperOptions());
 
     static JSObject *Renew(JSContext *cx, JSObject *existing, JSObject *obj, const Wrapper *handler);
 
     static const Wrapper *wrapperHandler(JSObject *wrapper);
 
     static JSObject *wrappedObject(JSObject *wrapper);
 
@@ -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 existing, HandleObject obj);
 
 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/Wrapper.cpp
+++ b/js/src/proxy/Wrapper.cpp
@@ -28,23 +28,21 @@ Wrapper::defaultValue(JSContext *cx, Han
 {
     vp.set(ObjectValue(*proxy->as<ProxyObject>().target()));
     if (hint == JSTYPE_VOID)
         return ToPrimitive(cx, vp);
     return ToPrimitive(cx, hint, vp);
 }
 
 JSObject *
-Wrapper::New(JSContext *cx, JSObject *obj, JSObject *parent, const Wrapper *handler,
+Wrapper::New(JSContext *cx, JSObject *obj, const Wrapper *handler,
              const WrapperOptions &options)
 {
-    MOZ_ASSERT(parent);
-
     RootedValue priv(cx, ObjectValue(*obj));
-    return NewProxyObject(cx, handler, priv, options.proto(), parent, options);
+    return NewProxyObject(cx, handler, priv, options.proto(), nullptr, options);
 }
 
 JSObject *
 Wrapper::Renew(JSContext *cx, JSObject *existing, JSObject *obj, const Wrapper *handler)
 {
     existing->as<ProxyObject>().renew(cx, handler, ObjectValue(*obj));
     return existing;
 }
@@ -122,22 +120,21 @@ 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 existing, HandleObject obj)
 {
     // Allow wrapping outer window proxies.
     MOZ_ASSERT(!obj->is<WrapperObject>() || obj->getClass()->ext.innerObject);
-    return Wrapper::New(cx, obj, parent, &CrossCompartmentWrapper::singleton);
+    return Wrapper::New(cx, obj, &CrossCompartmentWrapper::singleton);
 }
 
 ErrorCopier::~ErrorCopier()
 {
     JSContext *cx = ac->context()->asJSContext();
     if (ac->origin() != cx->compartment() && cx->isExceptionPending()) {
         RootedValue exc(cx);
         if (cx->getPendingException(&exc) && exc.isObject() && exc.toObject().is<ErrorObject>()) {
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -3963,17 +3963,17 @@ WrapWithProto(JSContext *cx, unsigned ar
     if (!obj.isObject() || !proto.isObjectOrNull()) {
         JS_ReportErrorNumber(cx, my_GetErrorMessage, nullptr, JSSMSG_INVALID_ARGS,
                              "wrapWithProto");
         return false;
     }
 
     WrapperOptions options(cx);
     options.setProto(proto.toObjectOrNull());
-    JSObject *wrapped = Wrapper::New(cx, &obj.toObject(), &obj.toObject().global(),
+    JSObject *wrapped = Wrapper::New(cx, &obj.toObject(),
                                      &Wrapper::singletonWithPrototype, options);
     if (!wrapped)
         return false;
 
     args.rval().setObject(*wrapped);
     return true;
 }
 
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -73,19 +73,17 @@ JSObject *
 WrapperFactory::CreateXrayWaiver(JSContext *cx, HandleObject obj)
 {
     // The caller is required to have already done a lookup.
     // NB: This implictly performs the assertions of GetXrayWaiver.
     MOZ_ASSERT(!GetXrayWaiver(obj));
     XPCWrappedNativeScope *scope = ObjectScope(obj);
 
     JSAutoCompartment ac(cx, obj);
-    JSObject *waiver = Wrapper::New(cx, obj,
-                                    JS_GetGlobalForObject(cx, obj),
-                                    &XrayWaiver);
+    JSObject *waiver = Wrapper::New(cx, obj, &XrayWaiver);
     if (!waiver)
         return nullptr;
 
     // Add the new waiver to the map. It's important that we only ever have
     // one waiver for the lifetime of the target object.
     if (!scope->mWaiverWrapperMap) {
         scope->mWaiverWrapperMap =
           JSObject2JSObjectMap::newMap(XPC_WRAPPER_MAP_LENGTH);
@@ -377,18 +375,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 existing, HandleObject obj)
 {
     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
@@ -499,17 +496,17 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
         }
     }
 
     DEBUG_CheckUnwrapSafety(obj, wrapper, origin, target);
 
     if (existing)
         return Wrapper::Renew(cx, existing, obj, wrapper);
 
-    return Wrapper::New(cx, obj, parent, wrapper);
+    return Wrapper::New(cx, obj, 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
 WrapperFactory::WaiveXrayAndWrap(JSContext *cx, MutableHandleValue vp)
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -41,18 +41,17 @@ class WrapperFactory {
     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);
+                            JS::HandleObject obj);
 
     // 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);
 };
 
 extern const js::Wrapper XrayWaiver;