Bug 867844 part 1. Fix rooting hazards in LegacyCall. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 06 May 2013 08:30:40 -0400
changeset 141911 aed0d9db6a9ce16b86f1a23cb7f391fd4c8d077f
parent 141910 4995d566bd6dd50bca5cf1a1a0b9cf047cc0d0dc
child 141912 f3881fe6a5b8de245b06c57d1977065e0f31aba2
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)
reviewerssmaug
bugs867844
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 867844 part 1. Fix rooting hazards in LegacyCall. r=smaug
content/base/src/nsObjectLoadingContent.cpp
content/base/src/nsObjectLoadingContent.h
dom/bindings/Codegen.py
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -2843,17 +2843,17 @@ nsObjectLoadingContent::GetContentDocume
     return nullptr;
   }
 
   return sub_doc;
 }
 
 JS::Value
 nsObjectLoadingContent::LegacyCall(JSContext* aCx,
-                                   JS::Value aThisVal,
+                                   JS::Handle<JS::Value> aThisVal,
                                    const Sequence<JS::Value>& aArguments,
                                    ErrorResult& aRv)
 {
   nsCOMPtr<nsIContent> thisContent =
     do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
   JS::Rooted<JSObject*> obj(aCx, thisContent->GetWrapper());
   MOZ_ASSERT(obj, "How did we get called?");
 
@@ -2880,17 +2880,18 @@ nsObjectLoadingContent::LegacyCall(JSCon
        arg != arg_end;
        ++arg) {
     if (!JS_WrapValue(aCx, arg)) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return JS::UndefinedValue();
     }
   }
 
-  if (!JS_WrapValue(aCx, &aThisVal)) {
+  JS::Rooted<JS::Value> thisVal(aCx, aThisVal);
+  if (!JS_WrapValue(aCx, thisVal.address())) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return JS::UndefinedValue();
   }
 
   nsRefPtr<nsNPAPIPluginInstance> pi;
   nsresult rv = ScriptRequestPluginInstance(aCx, getter_AddRefs(pi));
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
@@ -2898,33 +2899,33 @@ nsObjectLoadingContent::LegacyCall(JSCon
   }
 
   // if there's no plugin around for this object, throw.
   if (!pi) {
     aRv.Throw(NS_ERROR_NOT_AVAILABLE);
     return JS::UndefinedValue();
   }
 
-  JSObject *pi_obj;
-  JSObject *pi_proto;
-
-  rv = GetPluginJSObject(aCx, obj, pi, &pi_obj, &pi_proto);
+  JS::Rooted<JSObject*> pi_obj(aCx);
+  JS::Rooted<JSObject*> pi_proto(aCx);
+
+  rv = GetPluginJSObject(aCx, obj, pi, pi_obj.address(), pi_proto.address());
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
     return JS::UndefinedValue();
   }
 
   if (!pi_obj) {
     aRv.Throw(NS_ERROR_NOT_AVAILABLE);
     return JS::UndefinedValue();
   }
 
-  JS::Value retval;
-  bool ok = ::JS::Call(aCx, aThisVal, pi_obj, args.Length(),
-                       args.Elements(), &retval);
+  JS::Rooted<JS::Value> retval(aCx);
+  bool ok = ::JS::Call(aCx, thisVal, pi_obj, args.Length(),
+                       args.Elements(), retval.address());
   if (!ok) {
     aRv.Throw(NS_ERROR_FAILURE);
     return JS::UndefinedValue();
   }
 
   Telemetry::Accumulate(Telemetry::PLUGIN_CALLED_DIRECTLY, true);
   return retval;
 }
@@ -2965,20 +2966,20 @@ nsObjectLoadingContent::SetupProtoChain(
     return;
   }
 
   if (!pi) {
     // No plugin around for this object.
     return;
   }
 
-  JSObject *pi_obj; // XPConnect-wrapped peer object, when we get it.
-  JSObject *pi_proto; // 'pi.__proto__'
-
-  rv = GetPluginJSObject(aCx, aObject, pi, &pi_obj, &pi_proto);
+  JS::Rooted<JSObject*> pi_obj(aCx); // XPConnect-wrapped peer object, when we get it.
+  JS::Rooted<JSObject*> pi_proto(aCx); // 'pi.__proto__'
+
+  rv = GetPluginJSObject(aCx, aObject, pi, pi_obj.address(), pi_proto.address());
   if (NS_FAILED(rv)) {
     return;
   }
 
   if (!pi_obj) {
     // Didn't get a plugin instance JSObject, nothing we can do then.
     return;
   }
--- a/content/base/src/nsObjectLoadingContent.h
+++ b/content/base/src/nsObjectLoadingContent.h
@@ -190,17 +190,17 @@ class nsObjectLoadingContent : public ns
     void CancelPlayPreview(mozilla::ErrorResult& aRv)
     {
       aRv = CancelPlayPreview();
     }
     void SwapFrameLoaders(nsXULElement& aOtherOwner, mozilla::ErrorResult& aRv)
     {
       aRv.Throw(NS_ERROR_NOT_IMPLEMENTED);
     }
-    JS::Value LegacyCall(JSContext* aCx, JS::Value aThisVal,
+    JS::Value LegacyCall(JSContext* aCx, JS::Handle<JS::Value> aThisVal,
                          const mozilla::dom::Sequence<JS::Value>& aArguments,
                          mozilla::ErrorResult& aRv);
 
   protected:
     /**
      * Begins loading the object when called
      *
      * Attributes of |this| QI'd to nsIContent will be inspected, depending on
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4307,17 +4307,17 @@ if (global.Failed()) {
             argsPre.append("cx")
 
         if idlNode.isMethod() and idlNode.isLegacycaller():
             # If we can have legacycaller with identifier, we can't
             # just use the idlNode to determine whether we're
             # generating code for the legacycaller or not.
             assert idlNode.isIdentifierLess()
             # Pass in our thisVal
-            argsPre.append("JS_THIS_VALUE(cx, vp)")
+            argsPre.append("args.thisv()")
 
         cgThings.extend([CGArgumentConverter(arguments[i], i, self.getArgv(),
                                              self.getArgc(), self.descriptor,
                                              invalidEnumValueFatal=not setter,
                                              allowTreatNonCallableAsNull=setter,
                                              lenientFloatCode=lenientFloatCode) for
                          i in range(argConversionStartsAt, self.argCount)])
 
@@ -4802,36 +4802,39 @@ class CGAbstractBindingMethod(CGAbstract
     """
     Common class to generate the JSNatives for all our methods, getters, and
     setters.  This will generate the function declaration and unwrap the
     |this| object.  Subclasses are expected to override the generate_code
     function to do the rest of the work.  This function should return a
     CGThing which is already properly indented.
     """
     def __init__(self, descriptor, name, args, unwrapFailureCode=None,
-                 getThisObj="JS_THIS_OBJECT(cx, vp)"):
+                 getThisObj="&args.computeThis(cx).toObject()",
+                 callArgs="JS::CallArgs args = JS::CallArgsFromVp(argc, vp);"):
         CGAbstractStaticMethod.__init__(self, descriptor, name, "JSBool", args)
 
         if unwrapFailureCode is None:
             self.unwrapFailureCode = 'return ThrowErrorMessage(cx, MSG_DOES_NOT_IMPLEMENT_INTERFACE, "%s");' % self.descriptor.name
         else:
             self.unwrapFailureCode = unwrapFailureCode
         self.getThisObj = getThisObj
+        self.callArgs = callArgs
 
     def definition_body(self):
         # Our descriptor might claim that we're not castable, simply because
         # we're someone's consequential interface.  But for this-unwrapping, we
         # know that we're the real deal.  So fake a descriptor here for
         # consumption by CastableObjectUnwrapper.
-        getThis = CGGeneric("""JS::RootedObject obj(cx, %s);
+        getThis = CGGeneric("""%s
+JS::RootedObject obj(cx, %s);
 if (!obj) {
   return false;
 }
 
-%s* self;""" % (self.getThisObj, self.descriptor.nativeType))
+%s* self;""" % (self.callArgs, self.getThisObj, self.descriptor.nativeType))
         unwrapThis = CGGeneric(
             str(CastableObjectUnwrapper(
                         self.descriptor,
                         "obj", "self", self.unwrapFailureCode)))
         return CGList([ CGIndenter(getThis), CGIndenter(unwrapThis),
                         self.generate_code() ], "\n").define()
 
     def generate_code(self):
@@ -4934,17 +4937,17 @@ class CGNewResolveHook(CGAbstractBinding
     def __init__(self, descriptor):
         self._needNewResolve = descriptor.interface.getExtendedAttribute("NeedNewResolve")
         args = [Argument('JSContext*', 'cx'), Argument('JSHandleObject', 'obj_'),
                 Argument('JSHandleId', 'id'), Argument('unsigned', 'flags'),
                 Argument('JS::MutableHandle<JSObject*>', 'objp')]
         # Our "self" is actually the callee in this case, not the thisval.
         CGAbstractBindingMethod.__init__(
             self, descriptor, NEWRESOLVE_HOOK_NAME,
-            args, getThisObj="obj_")
+            args, getThisObj="obj_", callArgs="")
 
     def define(self):
         if not self._needNewResolve:
             return ""
         return CGAbstractBindingMethod.define(self)
 
     def generate_code(self):
         return CGIndenter(CGGeneric("return self->DoNewResolve(cx, obj, id, flags, objp);"))