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

@@ -20,10 +20,10 @@ WebGLContext::WebGLObjectAsJSValue(JSContext *cx, const WebGLObjectType *object,
         return JS::NullValue();
     }
     MOZ_ASSERT(this == object->Context());
-    JS::Value v;
+    JS::Rooted<JS::Value> v(cx);
-    JSObject* wrapper = GetWrapper();
+    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();
     }

dom/bindings/BindingUtils.h

@@ -550,7 +550,8 @@ WrapNewBindingForSameCompartment(JSContext* cx, JSObject* obj,
 // 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();
@@ -612,8 +613,9 @@ WrapNewBindingObject(JSContext* cx, JSObject* scope, T* value, JS::Value* vp)
 // WrapObject() method taking a JSContext and a scope.
 template <class T>
 inline bool
-WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope, T* value,
+WrapNewBindingNonWrapperCachedObject(JSContext* cx,
-                                     JS::Value* vp)
+                                     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"
@@ -622,6 +624,10 @@ WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope, T* value,
     // 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)
@@ -648,7 +654,8 @@ WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope, T* value,
 // 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
@@ -662,6 +669,10 @@ WrapNewBindingNonWrapperCachedOwnedObject(JSContext* cx, JSObject* scope,
     // 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)
@@ -692,7 +703,7 @@ WrapNewBindingNonWrapperCachedOwnedObject(JSContext* cx, JSObject* scope,
 // 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);
@@ -1055,8 +1066,8 @@ struct WrapNativeParentFallback<T, true >
 template<typename T, bool hasWrapObject=HasWrapObject<T>::Value >
 struct WrapNativeParentHelper
 {
-  static inline JSObject* Wrap(JSContext* cx, JSObject* scope, T* parent,
+  static inline JSObject* Wrap(JSContext* cx, JS::Handle<JSObject*> scope,
-                               nsWrapperCache* cache)
+                               T* parent, nsWrapperCache* cache)
   {
     MOZ_ASSERT(cache);
 
@@ -1081,8 +1092,8 @@ struct WrapNativeParentHelper
 template<typename T>
 struct WrapNativeParentHelper<T, false >
 {
-  static inline JSObject* Wrap(JSContext* cx, JSObject* scope, T* parent,
+  static inline JSObject* Wrap(JSContext* cx, JS::Handle<JSObject*> scope,
-                               nsWrapperCache* cache)
+                               T* parent, nsWrapperCache* cache)
   {
     JSObject* obj;
     if (cache && (obj = cache->GetWrapper())) {
@@ -1100,7 +1111,8 @@ struct WrapNativeParentHelper<T, false >
 // 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;
@@ -1113,7 +1125,7 @@ WrapNativeParent(JSContext* cx, JSObject* scope, T* p, nsWrapperCache* cache)
 // 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));
 }
@@ -1123,7 +1135,7 @@ 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());
@@ -1133,7 +1145,7 @@ struct 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;
@@ -1154,7 +1166,7 @@ JSObject* GetJSObjectFromCallback(void* noncallback)
 
 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.
@@ -1182,8 +1194,8 @@ WrapCallThisObject(JSContext* cx, JSObject* scope, const T& p)
 template <class T, bool isSmartPtr=HasgetMember<T>::Value>
 struct WrapNewBindingObjectHelper
 {
-  static inline bool Wrap(JSContext* cx, JSObject* scope, const T& value,
+  static inline bool Wrap(JSContext* cx, JS::Handle<JSObject*> scope,
-                          JS::Value* vp)
+                          const T& value, JS::Value* vp)
   {
     return WrapNewBindingObject(cx, scope, value.get(), vp);
   }
@@ -1192,7 +1204,7 @@ struct WrapNewBindingObjectHelper
 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);
@@ -1201,7 +1213,7 @@ struct WrapNewBindingObjectHelper<T, false>
 
 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);

dom/bindings/Codegen.py

@@ -2023,7 +2023,7 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
     """
     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)
@@ -2041,6 +2041,7 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
             assertISupportsInheritance = ""
         return """%s
 %s
+  JS::Rooted<JSObject*> aScope(aCx, aScopeArg); // Temporary!
   JSObject* parent = WrapNativeParent(aCx, aScope, aObject->GetParentObject());
   if (!parent) {
     return NULL;
@@ -8588,7 +8589,8 @@ class CGCallback(CGClass):
 
         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"

dom/bindings/DOMJSClass.h

@@ -156,7 +156,7 @@ enum DOMObjectType {
   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