Bug 1019191 part 22. Eliminate the effectively unused vp argument of xpc_qsUnwrapArgImpl. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 22 Oct 2014 11:40:51 -0400
changeset 211778 cc4056729207d990f1ae3b7ce598bd42f38d0841
parent 211777 660bf8b6a9ad1810c5eddd34810860de8a82d0ef
child 211779 02e63032ce53fd28ba97b648602b0926749df1b3
push id2
push usergszorc@mozilla.com
push dateWed, 29 Oct 2014 00:55:10 +0000
reviewerspeterv
bugs1019191
milestone36.0a1
Bug 1019191 part 22. Eliminate the effectively unused vp argument of xpc_qsUnwrapArgImpl. r=peterv
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
js/xpconnect/src/XPCQuickStubs.cpp
js/xpconnect/src/XPCQuickStubs.h
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -822,38 +822,36 @@ QueryInterface(JSContext* cx, unsigned a
                                                   /* stopAtOuter = */ false));
   if (!obj) {
       JS_ReportError(cx, "Permission denied to access object");
       return false;
   }
 
   // Switch this to UnwrapDOMObjectToISupports once our global objects are
   // using new bindings.
-  JS::Rooted<JS::Value> unusedVal(cx);
   nsISupports* native = nullptr;
   nsCOMPtr<nsISupports> nativeRef;
   UnwrapArg<nsISupports>(cx, obj, &native,
-                         static_cast<nsISupports**>(getter_AddRefs(nativeRef)),
-                         &unusedVal);
+                         static_cast<nsISupports**>(getter_AddRefs(nativeRef)));
   if (!native) {
     return Throw(cx, NS_ERROR_FAILURE);
   }
 
   if (argc < 1) {
     return Throw(cx, NS_ERROR_XPC_NOT_ENOUGH_ARGS);
   }
 
   if (!args[0].isObject()) {
     return Throw(cx, NS_ERROR_XPC_BAD_CONVERT_JS);
   }
 
   nsIJSID* iid;
   SelfRef iidRef;
   obj = &args[0].toObject();
-  if (NS_FAILED(UnwrapArg<nsIJSID>(cx, obj, &iid, &iidRef.ptr, &unusedVal))) {
+  if (NS_FAILED(UnwrapArg<nsIJSID>(cx, obj, &iid, &iidRef.ptr))) {
     return Throw(cx, NS_ERROR_XPC_BAD_CONVERT_JS);
   }
   MOZ_ASSERT(iid);
 
   if (iid->GetID()->Equals(NS_GET_IID(nsIClassInfo))) {
     nsresult rv;
     nsCOMPtr<nsIClassInfo> ci = do_QueryInterface(native, &rv);
     if (NS_FAILED(rv)) {
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -36,17 +36,17 @@
 
 #include "nsWrapperCacheInlines.h"
 
 class nsIJSID;
 class nsPIDOMWindow;
 
 extern nsresult
 xpc_qsUnwrapArgImpl(JSContext* cx, JS::Handle<JSObject*> src, const nsIID& iid, void** ppArg,
-                    nsISupports** ppArgRef, JS::MutableHandle<JS::Value> vp);
+                    nsISupports** ppArgRef);
 
 namespace mozilla {
 namespace dom {
 template<typename DataType> class MozMap;
 
 struct SelfRef
 {
   SelfRef() : ptr(nullptr) {}
@@ -55,22 +55,21 @@ struct SelfRef
 
   nsISupports* ptr;
 };
 
 /** Convert a jsval to an XPCOM pointer. */
 template <class Interface, class StrongRefType>
 inline nsresult
 UnwrapArg(JSContext* cx, JS::Handle<JSObject*> src, Interface** ppArg,
-          StrongRefType** ppArgRef, JS::MutableHandle<JS::Value> vp)
+          StrongRefType** ppArgRef)
 {
   nsISupports* argRef = *ppArgRef;
   nsresult rv = xpc_qsUnwrapArgImpl(cx, src, NS_GET_TEMPLATE_IID(Interface),
-                                    reinterpret_cast<void**>(ppArg), &argRef,
-                                    vp);
+                                    reinterpret_cast<void**>(ppArg), &argRef);
   *ppArgRef = static_cast<StrongRefType*>(argRef);
   return rv;
 }
 
 inline const ErrNum
 GetInvalidThisErrorForMethod(bool aSecurityError)
 {
   return aSecurityError ? MSG_METHOD_THIS_UNWRAPPING_DENIED :
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3672,27 +3672,25 @@ class CastableObjectUnwrapper():
                 "JS::Rooted<JSObject*> uncheckedObj(cx, js::UncheckedUnwrap(%s));\n" % source)
             self.substitution["source"] = "uncheckedObj"
             xpconnectUnwrap = dedent("""
                 nsresult rv;
                 { // Scope for the JSAutoCompartment, because we only
                   // want to be in that compartment for the UnwrapArg call.
                   JS::Rooted<JSObject*> source(cx, ${source});
                   JSAutoCompartment ac(cx, ${source});
-                  JS::Rooted<JS::Value> unused(cx);
-                  rv = UnwrapArg<${type}>(cx, source, &objPtr, &objRef.ptr, &unused);
+                  rv = UnwrapArg<${type}>(cx, source, &objPtr, &objRef.ptr);
                 }
                 """)
         else:
             self.substitution["uncheckedObjDecl"] = ""
             self.substitution["source"] = source
             xpconnectUnwrap = (
                 "JS::Rooted<JSObject*> source(cx, ${source});\n"
-                "JS::Rooted<JS::Value> unused(cx);\n"
-                "nsresult rv = UnwrapArg<${type}>(cx, source, &objPtr, &objRef.ptr, &unused);\n")
+                "nsresult rv = UnwrapArg<${type}>(cx, source, &objPtr, &objRef.ptr);\n")
 
         if descriptor.hasXPConnectImpls:
             self.substitution["codeOnFailure"] = string.Template(
                 "${type} *objPtr;\n"
                 "SelfRef objRef;\n" +
                 xpconnectUnwrap +
                 "if (NS_FAILED(rv)) {\n"
                 "${indentedCodeOnFailure}"
@@ -4762,34 +4760,28 @@ def getJSToNativeConversionInfo(type, de
             # it'll put a non-null pointer in there.
             if forceOwningType:
                 # Don't return a holderType in this case; our declName
                 # will just own stuff.
                 templateBody += "nsRefPtr<" + typeName + "> ${holderName};\n"
             else:
                 holderType = "nsRefPtr<" + typeName + ">"
             templateBody += (
-                "JS::Rooted<JSObject*> source(cx, &${val}.toObject());\n"
-                "JS::Rooted<JS::Value> tmpVal(cx);\n" +
+                "JS::Rooted<JSObject*> source(cx, &${val}.toObject());\n" +
                 typePtr + " tmp;\n"
-                "if (NS_FAILED(UnwrapArg<" + typeName + ">(cx, source, &tmp, static_cast<" + typeName + "**>(getter_AddRefs(${holderName})), &tmpVal))) {\n")
+                "if (NS_FAILED(UnwrapArg<" + typeName + ">(cx, source, &tmp, static_cast<" + typeName + "**>(getter_AddRefs(${holderName}))))) {\n")
             templateBody += CGIndenter(onFailureBadType(failureCode,
                                                         descriptor.interface.identifier.name)).define()
             templateBody += ("}\n"
                              "MOZ_ASSERT(tmp);\n")
 
             if not isDefinitelyObject and not forceOwningType:
-                # Our tmpVal will go out of scope, so we can't rely on it
-                # for rooting
                 templateBody += dedent("""
-                    if (tmpVal != ${val} && !${holderName}) {
-                      // We have to have a strong ref, because we got this off
-                      // some random object that might get GCed
-                      ${holderName} = tmp;
-                    }
+                    // UnwrapArg never sets **ppArg without also setting *ppArgRef
+                    MOZ_ASSERT(${holderName});
                     """)
 
             # And store our tmp, before it goes out of scope.
             templateBody += "${declName} = tmp;\n"
 
         # Just pass failureCode, not onFailureBadType, here, so we'll report the
         # thing as not an object as opposed to not implementing whatever our
         # interface is.
--- a/js/xpconnect/src/XPCQuickStubs.cpp
+++ b/js/xpconnect/src/XPCQuickStubs.cpp
@@ -19,18 +19,17 @@
 using namespace mozilla;
 using namespace JS;
 
 nsresult
 xpc_qsUnwrapArgImpl(JSContext *cx,
                     HandleObject src,
                     const nsIID &iid,
                     void **ppArg,
-                    nsISupports **ppArgRef,
-                    MutableHandleValue vp)
+                    nsISupports **ppArgRef)
 {
     nsISupports *iface = xpc::UnwrapReflectorToISupports(src);
     if (iface) {
         if (NS_FAILED(iface->QueryInterface(iid, ppArg))) {
             *ppArgRef = nullptr;
             return NS_ERROR_XPC_BAD_CONVERT_JS;
         }
 
@@ -54,17 +53,16 @@ xpc_qsUnwrapArgImpl(JSContext *cx,
 
     // We need to go through the QueryInterface logic to make this return
     // the right thing for the various 'special' interfaces; e.g.
     // nsIPropertyBag. We must use AggregatedQueryInterface in cases where
     // there is an outer to avoid nasty recursion.
     rv = wrappedJS->QueryInterface(iid, ppArg);
     if (NS_SUCCEEDED(rv)) {
         *ppArgRef = static_cast<nsISupports*>(*ppArg);
-        vp.setObjectOrNull(wrappedJS->GetJSObject());
     }
     return rv;
 }
 
 namespace xpc {
 
 bool
 NonVoidStringToJsval(JSContext *cx, nsAString &str, MutableHandleValue rval)
--- a/js/xpconnect/src/XPCQuickStubs.h
+++ b/js/xpconnect/src/XPCQuickStubs.h
@@ -8,11 +8,11 @@
 #define xpcquickstubs_h___
 
 #include "XPCForwards.h"
 
 /* XPCQuickStubs.h - Support functions used only by Web IDL bindings, for now. */
 
 nsresult
 xpc_qsUnwrapArgImpl(JSContext *cx, JS::HandleObject src, const nsIID &iid, void **ppArg,
-                    nsISupports **ppArgRef, JS::MutableHandleValue vp);
+                    nsISupports **ppArgRef);
 
 #endif /* xpcquickstubs_h___ */