Bug 865975. Better rooting for the 'this', 'callable', and 'rval' values in WebIDL callbacks. r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 26 Apr 2013 13:41:21 -0400
changeset 130097 d85886f848015fad80f12723a992213d1f153e31
parent 130096 03423d078b99d36ff59905500c3c6034e87e89e8
child 130098 9fb3df70550ae4a5da50cc197098dd9eae80d2a5
push id24596
push userryanvm@gmail.com
push dateSat, 27 Apr 2013 01:20:57 +0000
treeherdermozilla-central@0e45f1b9521f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs865975
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 865975. Better rooting for the 'this', 'callable', and 'rval' values in WebIDL callbacks. r=bholley
dom/bindings/CallbackInterface.cpp
dom/bindings/CallbackInterface.h
dom/bindings/Codegen.py
--- a/dom/bindings/CallbackInterface.cpp
+++ b/dom/bindings/CallbackInterface.cpp
@@ -8,23 +8,23 @@
 #include "jsapi.h"
 #include "mozilla/dom/BindingUtils.h"
 
 namespace mozilla {
 namespace dom {
 
 bool
 CallbackInterface::GetCallableProperty(JSContext* cx, const char* aPropName,
-				       JS::Value* aCallable)
+                                       JS::MutableHandle<JS::Value> aCallable)
 {
-  if (!JS_GetProperty(cx, mCallback, aPropName, aCallable)) {
+  if (!JS_GetProperty(cx, mCallback, aPropName, aCallable.address())) {
     return false;
   }
-  if (!aCallable->isObject() ||
-      !JS_ObjectIsCallable(cx, &aCallable->toObject())) {
+  if (!aCallable.isObject() ||
+      !JS_ObjectIsCallable(cx, &aCallable.toObject())) {
     ThrowErrorMessage(cx, MSG_NOT_CALLABLE);
     return false;
   }
 
   return true;
 }
 
 } // namespace dom
--- a/dom/bindings/CallbackInterface.h
+++ b/dom/bindings/CallbackInterface.h
@@ -26,16 +26,16 @@ class CallbackInterface : public Callbac
 public:
   explicit CallbackInterface(JSObject* aCallback)
     : CallbackObject(aCallback)
   {
   }
 
 protected:
   bool GetCallableProperty(JSContext* cx, const char* aPropName,
-                           JS::Value* aCallable);
+                           JS::MutableHandle<JS::Value> aCallable);
 
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_CallbackFunction_h
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -8677,23 +8677,23 @@ class CGCallback(CGClass):
                 ])]
 
     def getMethodImpls(self, method):
         assert method.needThisHandling
         args = list(method.args)
         # Strip out the JSContext*/JSObject* args
         # that got added.
         assert args[0].name == "cx" and args[0].argType == "JSContext*"
-        assert args[1].name == "aThisObj" and args[1].argType == "JSObject*"
+        assert args[1].name == "aThisObj" and args[1].argType == "JS::Handle<JSObject*>"
         args = args[2:]
         # Record the names of all the arguments, so we can use them when we call
         # the private method.
         argnames = [arg.name for arg in args]
         argnamesWithThis = ["s.GetContext()", "thisObjJS"] + argnames
-        argnamesWithoutThis = ["s.GetContext()", "nullptr"] + argnames
+        argnamesWithoutThis = ["s.GetContext()", "JS::NullPtr()"] + argnames
         # Now that we've recorded the argnames for our call to our private
         # method, insert our optional argument for deciding whether the
         # CallSetup should re-throw exceptions on aRv.
         args.append(Argument("ExceptionHandling", "aExceptionHandling",
                              "eReportExceptions"))
         # And now insert our template argument.
         argsWithoutThis = list(args)
         args.insert(0, Argument("const T&",  "thisObj"))
@@ -8701,18 +8701,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 =\n"
-            "  WrapCallThisObject(s.GetContext(), CallbackPreserveColor(), thisObj);\n"
+            "JS::Rooted<JSObject*> thisObjJS(s.GetContext(),\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,
@@ -8851,17 +8851,17 @@ class CallbackMember(CGNativeMember):
             "${argvDecl}"
             "${convertArgs}"
             "${doCall}"
             "${returnResult}").substitute(replacements)
 
     def getResultConversion(self):
         replacements = {
             "val": "rval",
-            "valPtr": "&rval",
+            "valPtr": "rval.address()",
             "holderName" : "rvalHolder",
             "declName" : "rvalDecl",
             # We actually want to pass in a null scope object here, because
             # wrapping things into our current compartment (that of mCallback)
             # is what we want.
             "obj": "nullptr"
             }
 
@@ -8959,17 +8959,17 @@ class CallbackMember(CGNativeMember):
         if not self.needThisHandling:
             # Since we don't need this handling, we're the actual method that
             # will be called, so we need an aRethrowExceptions argument.
             return args + [Argument("ExceptionHandling", "aExceptionHandling",
                                     "eReportExceptions")]
         # We want to allow the caller to pass in a "this" object, as
         # well as a JSContext.
         return [Argument("JSContext*", "cx"),
-                Argument("JSObject*", "aThisObj")] + args
+                Argument("JS::Handle<JSObject*>", "aThisObj")] + args
 
     def getCallSetup(self):
         if self.needThisHandling:
             # It's been done for us already
             return ""
         return string.Template(
             "CallSetup s(mCallback, aRv, aExceptionHandling);\n"
             "JSContext* cx = s.GetContext();\n"
@@ -8998,47 +8998,47 @@ class CallbackMember(CGNativeMember):
                               (type, idlObject.identifier.name,
                                idlObject.location))
 
 class CallbackMethod(CallbackMember):
     def __init__(self, sig, name, descriptorProvider, needThisHandling):
         CallbackMember.__init__(self, sig, name, descriptorProvider,
                                 needThisHandling)
     def getRvalDecl(self):
-        return "JS::Value rval = JSVAL_VOID;\n"
+        return "JS::Rooted<JS::Value> rval(cx, JS::UndefinedValue());\n"
 
     def getCall(self):
         replacements = {
             "errorReturn" : self.getDefaultRetval(),
             "thisObj": self.getThisObj(),
             "getCallable": self.getCallableDecl()
             }
         if self.argCount > 0:
             replacements["argv"] = "argv.begin()"
             replacements["argc"] = "argc"
         else:
             replacements["argv"] = "nullptr"
             replacements["argc"] = "0"
         return string.Template("${getCallable}"
                 "if (!JS_CallFunctionValue(cx, ${thisObj}, callable,\n"
-                "                          ${argc}, ${argv}, &rval)) {\n"
+                "                          ${argc}, ${argv}, rval.address())) {\n"
                 "  aRv.Throw(NS_ERROR_UNEXPECTED);\n"
                 "  return${errorReturn};\n"
                 "}\n").substitute(replacements)
 
 class CallCallback(CallbackMethod):
     def __init__(self, callback, descriptorProvider):
         CallbackMethod.__init__(self, callback.signatures()[0], "Call",
                                 descriptorProvider, needThisHandling=True)
 
     def getThisObj(self):
         return "aThisObj"
 
     def getCallableDecl(self):
-        return "JS::Value callable = JS::ObjectValue(*mCallback);\n"
+        return "JS::Rooted<JS::Value> callable(cx, JS::ObjectValue(*mCallback));\n"
 
 class CallbackOperation(CallbackMethod):
     def __init__(self, method, signature, descriptor):
         self.singleOperation = descriptor.interface.isSingleOperationInterface()
         self.ensureASCIIName(method)
         self.methodName = method.identifier.name
         CallbackMethod.__init__(self, signature,
                                 MakeNativeName(self.methodName),
@@ -9059,20 +9059,20 @@ class CallbackOperation(CallbackMethod):
             "methodName": self.methodName
             }
         getCallableFromProp = string.Template(
                 'if (!GetCallableProperty(cx, "${methodName}", &callable)) {\n'
                 '  aRv.Throw(NS_ERROR_UNEXPECTED);\n'
                 '  return${errorReturn};\n'
                 '}\n').substitute(replacements)
         if not self.singleOperation:
-            return 'JS::Value callable;\n' + getCallableFromProp
+            return 'JS::Rooted<JS::Value> callable(cx);\n' + getCallableFromProp
         return (
             'bool isCallable = JS_ObjectIsCallable(cx, mCallback);\n'
-            'JS::Value callable;\n'
+            'JS::Rooted<JS::Value> callable(cx);\n'
             'if (isCallable) {\n'
             '  callable = JS::ObjectValue(*mCallback);\n'
             '} else {\n'
             '%s'
             '}\n' % CGIndenter(CGGeneric(getCallableFromProp)).define())
 
 class CallbackGetter(CallbackMember):
     def __init__(self, attr, descriptor):
@@ -9080,25 +9080,25 @@ class CallbackGetter(CallbackMember):
         self.attrName = attr.identifier.name
         CallbackMember.__init__(self,
                                 (attr.type, []),
                                 callbackGetterName(attr),
                                 descriptor,
                                 needThisHandling=False)
 
     def getRvalDecl(self):
-        return "JS::Value rval = JSVAL_VOID;\n"
+        return "JS::Rooted<JS::Value> rval(cx, JS::UndefinedValue());\n"
 
     def getCall(self):
         replacements = {
             "errorReturn" : self.getDefaultRetval(),
             "attrName": self.attrName
             }
         return string.Template(
-            'if (!JS_GetProperty(cx, mCallback, "${attrName}", &rval)) {\n'
+            'if (!JS_GetProperty(cx, mCallback, "${attrName}", rval.address())) {\n'
             '  aRv.Throw(NS_ERROR_UNEXPECTED);\n'
             '  return${errorReturn};\n'
             '}\n').substitute(replacements);
 
 class CallbackSetter(CallbackMember):
     def __init__(self, attr, descriptor):
         self.ensureASCIIName(attr)
         self.attrName = attr.identifier.name