Bug 864727 part 4. Pass a handle for the scope object to all the various Wrap*Object stuff in BindingUtils. r=ms2ger
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 25 Apr 2013 12:29:53 -0400
changeset 140837 3011f288bfe7da7bafa1f52853341cfd5dd7fdc1
parent 140836 d13e71853e55348494a954403ef3f81c519d3daf
child 140838 1b1381894c4402d105cf59e45078bd9573facd01
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersms2ger
bugs864727
milestone23.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 864727 part 4. Pass a handle for the scope object to all the various Wrap*Object stuff in BindingUtils. r=ms2ger Note: The JS::Rooted in CGWrapWithCacheMethod is just there until we start passing a handle to Wrap().
content/canvas/src/WebGLContextUtils.h
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/DOMJSClass.h
--- a/content/canvas/src/WebGLContextUtils.h
+++ b/content/canvas/src/WebGLContextUtils.h
@@ -15,20 +15,20 @@ namespace mozilla {
 template <typename WebGLObjectType>
 JS::Value
 WebGLContext::WebGLObjectAsJSValue(JSContext *cx, const WebGLObjectType *object, ErrorResult& rv) const
 {
     if (!object) {
         return JS::NullValue();
     }
     MOZ_ASSERT(this == object->Context());
-    JS::Value v;
-    JSObject* wrapper = GetWrapper();
+    JS::Rooted<JS::Value> v(cx);
+    JS::Rooted<JSObject*> wrapper(cx, GetWrapper());
     JSAutoCompartment ac(cx, wrapper);
-    if (!dom::WrapNewBindingObject(cx, wrapper, const_cast<WebGLObjectType*>(object), &v)) {
+    if (!dom::WrapNewBindingObject(cx, wrapper, const_cast<WebGLObjectType*>(object), v.address())) {
         rv.Throw(NS_ERROR_FAILURE);
         return JS::NullValue();
     }
     return v;
 }
 
 template <typename WebGLObjectType>
 JSObject*
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -545,17 +545,18 @@ WrapNewBindingForSameCompartment(JSConte
 
 // Create a JSObject wrapping "value", if there isn't one already, and store it
 // in *vp.  "value" must be a concrete class that implements a
 // GetWrapperPreserveColor() which can return its existing wrapper, if any, and
 // a WrapObject() which will try to create a wrapper. Typically, this is done by
 // having "value" inherit from nsWrapperCache.
 template <class T>
 MOZ_ALWAYS_INLINE bool
-WrapNewBindingObject(JSContext* cx, JSObject* scope, T* value, JS::Value* vp)
+WrapNewBindingObject(JSContext* cx, JS::Handle<JSObject*> scope, T* value,
+                     JS::Value* vp)
 {
   MOZ_ASSERT(value);
   JSObject* obj = value->GetWrapperPreserveColor();
   bool couldBeDOMBinding = CouldBeDOMBinding(value);
   if (obj) {
     xpc_UnmarkNonNullGrayObject(obj);
   } else {
     // Inline this here while we have non-dom objects in wrapper caches.
@@ -607,26 +608,31 @@ WrapNewBindingObject(JSContext* cx, JSOb
   return (sameCompartment && IS_SLIM_WRAPPER(obj)) || JS_WrapValue(cx, vp);
 }
 
 // Create a JSObject wrapping "value", for cases when "value" is a
 // non-wrapper-cached object using WebIDL bindings.  "value" must implement a
 // WrapObject() method taking a JSContext and a scope.
 template <class T>
 inline bool
-WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope, T* value,
-                                     JS::Value* vp)
+WrapNewBindingNonWrapperCachedObject(JSContext* cx,
+                                     JS::Handle<JSObject*> scopeArg,
+                                     T* value, JS::Value* vp)
 {
   MOZ_ASSERT(value);
   // We try to wrap in the compartment of the underlying object of "scope"
   JSObject* obj;
   {
     // scope for the JSAutoCompartment so that we restore the compartment
     // before we call JS_WrapValue.
     Maybe<JSAutoCompartment> ac;
+    // Maybe<Handle> doesn't so much work, and in any case, adding
+    // more Maybe (one for a Rooted and one for a Handle) adds more
+    // code (and branches!) than just adding a single rooted.
+    JS::Rooted<JSObject*> scope(cx, scopeArg);
     if (js::IsWrapper(scope)) {
       scope = js::CheckedUnwrap(scope, /* stopAtOuter = */ false);
       if (!scope)
         return false;
       ac.construct(cx, scope);
     }
 
     obj = value->WrapObject(cx, scope);
@@ -643,30 +649,35 @@ WrapNewBindingNonWrapperCachedObject(JSC
 }
 
 // Create a JSObject wrapping "value", for cases when "value" is a
 // non-wrapper-cached owned object using WebIDL bindings.  "value" must implement a
 // WrapObject() method taking a JSContext, a scope, and a boolean outparam that
 // is true if the JSObject took ownership
 template <class T>
 inline bool
-WrapNewBindingNonWrapperCachedOwnedObject(JSContext* cx, JSObject* scope,
+WrapNewBindingNonWrapperCachedOwnedObject(JSContext* cx,
+                                          JS::Handle<JSObject*> scopeArg,
                                           nsAutoPtr<T>& value, JS::Value* vp)
 {
   // We do a runtime check on value, because otherwise we might in
   // fact end up wrapping a null and invoking methods on it later.
   if (!value) {
     NS_RUNTIMEABORT("Don't try to wrap null objects");
   }
   // We try to wrap in the compartment of the underlying object of "scope"
   JSObject* obj;
   {
     // scope for the JSAutoCompartment so that we restore the compartment
     // before we call JS_WrapValue.
     Maybe<JSAutoCompartment> ac;
+    // Maybe<Handle> doesn't so much work, and in any case, adding
+    // more Maybe (one for a Rooted and one for a Handle) adds more
+    // code (and branches!) than just adding a single rooted.
+    JS::Rooted<JSObject*> scope(cx, scopeArg);
     if (js::IsWrapper(scope)) {
       scope = js::CheckedUnwrap(scope, /* stopAtOuter = */ false);
       if (!scope)
         return false;
       ac.construct(cx, scope);
     }
 
     bool tookOwnership = false;
@@ -687,17 +698,17 @@ WrapNewBindingNonWrapperCachedOwnedObjec
   // sure to JS_WrapValue!
   *vp = JS::ObjectValue(*obj);
   return JS_WrapValue(cx, vp);
 }
 
 // Helper for smart pointers (nsAutoPtr/nsRefPtr/nsCOMPtr).
 template <template <typename> class SmartPtr, typename T>
 inline bool
-WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope,
+WrapNewBindingNonWrapperCachedObject(JSContext* cx, JS::Handle<JSObject*> scope,
                                      const SmartPtr<T>& value, JS::Value* vp)
 {
   return WrapNewBindingNonWrapperCachedObject(cx, scope, value.get(), vp);
 }
 
 // Only set allowNativeWrapper to false if you really know you need it, if in
 // doubt use true. Setting it to false disables security wrappers.
 bool
@@ -1050,18 +1061,18 @@ struct WrapNativeParentFallback<T, true 
   }
 };
 
 // Wrapping of our native parent, for cases when it's a WebIDL object (though
 // possibly preffed off).
 template<typename T, bool hasWrapObject=HasWrapObject<T>::Value >
 struct WrapNativeParentHelper
 {
-  static inline JSObject* Wrap(JSContext* cx, JSObject* scope, T* parent,
-                               nsWrapperCache* cache)
+  static inline JSObject* Wrap(JSContext* cx, JS::Handle<JSObject*> scope,
+                               T* parent, nsWrapperCache* cache)
   {
     MOZ_ASSERT(cache);
 
     JSObject* obj;
     if ((obj = cache->GetWrapper())) {
       return obj;
     }
 
@@ -1076,18 +1087,18 @@ struct WrapNativeParentHelper
   }
 };
 
 // Wrapping of our native parent, for cases when it's not a WebIDL object.  In
 // this case it must be nsISupports.
 template<typename T>
 struct WrapNativeParentHelper<T, false >
 {
-  static inline JSObject* Wrap(JSContext* cx, JSObject* scope, T* parent,
-                               nsWrapperCache* cache)
+  static inline JSObject* Wrap(JSContext* cx, JS::Handle<JSObject*> scope,
+                               T* parent, nsWrapperCache* cache)
   {
     JSObject* obj;
     if (cache && (obj = cache->GetWrapper())) {
 #ifdef DEBUG
       NS_ASSERTION(WrapNativeISupportsParent(cx, scope, parent, cache) == obj,
                    "Unexpected object in nsWrapperCache");
 #endif
       return obj;
@@ -1095,50 +1106,51 @@ struct WrapNativeParentHelper<T, false >
 
     return WrapNativeISupportsParent(cx, scope, parent, cache);
   }
 };
 
 // Wrapping of our native parent.
 template<typename T>
 static inline JSObject*
-WrapNativeParent(JSContext* cx, JSObject* scope, T* p, nsWrapperCache* cache)
+WrapNativeParent(JSContext* cx, JS::Handle<JSObject*> scope, T* p,
+                 nsWrapperCache* cache)
 {
   if (!p) {
     return scope;
   }
 
   return WrapNativeParentHelper<T>::Wrap(cx, scope, p, cache);
 }
 
 // Wrapping of our native parent, when we don't want to explicitly pass in
 // things like the nsWrapperCache for it.
 template<typename T>
 static inline JSObject*
-WrapNativeParent(JSContext* cx, JSObject* scope, const T& p)
+WrapNativeParent(JSContext* cx, JS::Handle<JSObject*> scope, const T& p)
 {
   return WrapNativeParent(cx, scope, GetParentPointer(p), GetWrapperCache(p));
 }
 
 HAS_MEMBER(GetParentObject)
 
 template<typename T, bool WrapperCached=HasGetParentObjectMember<T>::Value>
 struct GetParentObject
 {
-  static JSObject* Get(JSContext* cx, JSObject* obj)
+  static JSObject* Get(JSContext* cx, JS::Handle<JSObject*> obj)
   {
     T* native = UnwrapDOMObject<T>(obj);
     return WrapNativeParent(cx, obj, native->GetParentObject());
   }
 };
 
 template<typename T>
 struct GetParentObject<T, false>
 {
-  static JSObject* Get(JSContext* cx, JSObject* obj)
+  static JSObject* Get(JSContext* cx, JS::Handle<JSObject*> obj)
   {
     MOZ_CRASH();
     return nullptr;
   }
 };
 
 MOZ_ALWAYS_INLINE
 JSObject* GetJSObjectFromCallback(CallbackObject* callback)
@@ -1149,17 +1161,17 @@ JSObject* GetJSObjectFromCallback(Callba
 MOZ_ALWAYS_INLINE
 JSObject* GetJSObjectFromCallback(void* noncallback)
 {
   return nullptr;
 }
 
 template<typename T>
 static inline JSObject*
-WrapCallThisObject(JSContext* cx, JSObject* scope, const T& p)
+WrapCallThisObject(JSContext* cx, JS::Handle<JSObject*> scope, const T& p)
 {
   // Callbacks are nsISupports, so WrapNativeParent will just happily wrap them
   // up as an nsISupports XPCWrappedNative... which is not at all what we want.
   // So we need to special-case them.
   JSObject* obj = GetJSObjectFromCallback(p);
   if (!obj) {
     // WrapNativeParent is a bit of a Swiss army knife that will
     // wrap anything for us.
@@ -1177,36 +1189,36 @@ WrapCallThisObject(JSContext* cx, JSObje
   return obj;
 }
 
 // Helper for calling WrapNewBindingObject with smart pointers
 // (nsAutoPtr/nsRefPtr/nsCOMPtr) or references.
 template <class T, bool isSmartPtr=HasgetMember<T>::Value>
 struct WrapNewBindingObjectHelper
 {
-  static inline bool Wrap(JSContext* cx, JSObject* scope, const T& value,
-                          JS::Value* vp)
+  static inline bool Wrap(JSContext* cx, JS::Handle<JSObject*> scope,
+                          const T& value, JS::Value* vp)
   {
     return WrapNewBindingObject(cx, scope, value.get(), vp);
   }
 };
 
 template <class T>
 struct WrapNewBindingObjectHelper<T, false>
 {
-  static inline bool Wrap(JSContext* cx, JSObject* scope, T& value,
+  static inline bool Wrap(JSContext* cx, JS::Handle<JSObject*> scope, T& value,
                           JS::Value* vp)
   {
     return WrapNewBindingObject(cx, scope, &value, vp);
   }
 };
 
 template<class T>
 inline bool
-WrapNewBindingObject(JSContext* cx, JSObject* scope, T& value,
+WrapNewBindingObject(JSContext* cx, JS::Handle<JSObject*> scope, T& value,
                      JS::Value* vp)
 {
   return WrapNewBindingObjectHelper<T>::Wrap(cx, scope, value, vp);
 }
 
 template <class T>
 inline JSObject*
 GetCallbackFromCallbackObject(T* aObj)
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2018,17 +2018,17 @@ def AssertInheritanceChain(descriptor):
 class CGWrapWithCacheMethod(CGAbstractMethod):
     """
     Create a wrapper JSObject for a given native that implements nsWrapperCache.
 
     properties should be a PropertyArrays instance.
     """
     def __init__(self, descriptor, properties):
         assert descriptor.interface.hasInterfacePrototypeObject()
-        args = [Argument('JSContext*', 'aCx'), Argument('JSObject*', 'aScope'),
+        args = [Argument('JSContext*', 'aCx'), Argument('JSObject*', 'aScopeArg'),
                 Argument(descriptor.nativeType + '*', 'aObject'),
                 Argument('nsWrapperCache*', 'aCache')]
         CGAbstractMethod.__init__(self, descriptor, 'Wrap', 'JSObject*', args)
         self.properties = properties
 
     def definition_body(self):
         if self.descriptor.workers:
             return """  return aObject->GetJSObject();"""
@@ -2036,16 +2036,17 @@ class CGWrapWithCacheMethod(CGAbstractMe
         if self.descriptor.nativeOwnership == 'nsisupports':
             assertISupportsInheritance = (
                 '  MOZ_ASSERT(reinterpret_cast<void*>(aObject) != aCache,\n'
                 '             "nsISupports must be on our primary inheritance chain");\n')
         else:
             assertISupportsInheritance = ""
         return """%s
 %s
+  JS::Rooted<JSObject*> aScope(aCx, aScopeArg); // Temporary!
   JSObject* parent = WrapNativeParent(aCx, aScope, aObject->GetParentObject());
   if (!parent) {
     return NULL;
   }
 
   // That might have ended up wrapping us already, due to the wonders
   // of XBL.  Check for that, and bail out as needed.  Scope so we don't
   // collide with the "obj" we declare in CreateBindingJSObject.
@@ -8583,17 +8584,18 @@ class CGCallback(CGClass):
         setupCall = ("CallSetup s(mCallback, aRv, aExceptionHandling);\n"
                      "if (!s.GetContext()) {\n"
                      "  aRv.Throw(NS_ERROR_UNEXPECTED);\n"
                      "  return${errorReturn};\n"
                      "}\n")
 
         bodyWithThis = string.Template(
             setupCall+
-            "JSObject* thisObjJS = WrapCallThisObject(s.GetContext(), mCallback, thisObj);\n"
+            "JSObject* thisObjJS =\n"
+            "  WrapCallThisObject(s.GetContext(), CallbackPreserveColor(), thisObj);\n"
             "if (!thisObjJS) {\n"
             "  aRv.Throw(NS_ERROR_FAILURE);\n"
             "  return${errorReturn};\n"
             "}\n"
             "return ${methodName}(${callArgs});").substitute({
                 "errorReturn" : method.getDefaultRetval(),
                 "callArgs" : ", ".join(argnamesWithThis),
                 "methodName": method.name,
--- a/dom/bindings/DOMJSClass.h
+++ b/dom/bindings/DOMJSClass.h
@@ -151,17 +151,17 @@ struct NativePropertyHooks
 };
 
 enum DOMObjectType {
   eInstance,
   eInterface,
   eInterfacePrototype
 };
 
-typedef JSObject* (*ParentGetter)(JSContext* aCx, JSObject* aObj);
+typedef JSObject* (*ParentGetter)(JSContext* aCx, JS::Handle<JSObject*> aObj);
 typedef JSObject* (*ProtoGetter)(JSContext* aCx, JSObject* aGlobal);
 
 struct DOMClass
 {
   // A list of interfaces that this object implements, in order of decreasing
   // derivedness.
   const prototypes::ID mInterfaceChain[MAX_PROTOTYPE_CHAIN_LENGTH];