bug 660430 - do not call the security manager during GC-marking of GC wrappers. r=gal
authorIgor Bukanov <igor@mir2.org>
Mon, 30 May 2011 21:36:16 +0200
changeset 70637 131a5a94e29acf77e38ef2d91476532e8f6db9b3
parent 70636 4e334541e5216a5347ef03e5791d5042f142b529
child 70638 a4a11ea5646c047585d2baf092fa447d53e5a0cb
push idunknown
push userunknown
push dateunknown
reviewersgal
bugs660430
milestone6.0a1
bug 660430 - do not call the security manager during GC-marking of GC wrappers. r=gal
js/src/xpconnect/src/XPCWrapper.cpp
js/src/xpconnect/src/XPCWrapper.h
js/src/xpconnect/src/xpccomponents.cpp
js/src/xpconnect/src/xpcprivate.h
js/src/xpconnect/src/xpcwrappednative.cpp
js/src/xpconnect/src/xpcwrappednativejsops.cpp
--- a/js/src/xpconnect/src/XPCWrapper.cpp
+++ b/js/src/xpconnect/src/XPCWrapper.cpp
@@ -130,17 +130,17 @@ Unwrap(JSContext *cx, JSObject *wrapper)
       return nsnull;
     return wrapper->unwrap();
   }
 
   return nsnull;
 }
 
 JSObject *
-UnsafeUnwrapSecurityWrapper(JSContext *cx, JSObject *obj)
+UnsafeUnwrapSecurityWrapper(JSObject *obj)
 {
   if (obj->isProxy()) {
     return obj->unwrap();
   }
 
   return obj;
 }
 
--- a/js/src/xpconnect/src/XPCWrapper.h
+++ b/js/src/xpconnect/src/XPCWrapper.h
@@ -91,14 +91,14 @@ IsSecurityWrapper(JSObject *wrapper)
  * Since this is meant to be called from functions like
  * XPCWrappedNative::GetWrappedNativeOfJSObject, it does not set an
  * exception on |cx|.
  */
 JSObject *
 Unwrap(JSContext *cx, JSObject *wrapper);
 
 JSObject *
-UnsafeUnwrapSecurityWrapper(JSContext *cx, JSObject *obj);
+UnsafeUnwrapSecurityWrapper(JSObject *obj);
 
 } // namespace XPCWrapper
 
 
 #endif
--- a/js/src/xpconnect/src/xpccomponents.cpp
+++ b/js/src/xpconnect/src/xpccomponents.cpp
@@ -3012,17 +3012,17 @@ SandboxImport(JSContext *cx, uintN argc,
         funname = JS_ValueToString(cx, argv[1]);
         if (!funname)
             return JS_FALSE;
         argv[1] = STRING_TO_JSVAL(funname);
     } else {
         // NB: funobj must only be used to get the JSFunction out.
         JSObject *funobj = JSVAL_TO_OBJECT(argv[0]);
         if (funobj->isProxy()) {
-            funobj = XPCWrapper::UnsafeUnwrapSecurityWrapper(cx, funobj);
+            funobj = XPCWrapper::UnsafeUnwrapSecurityWrapper(funobj);
         }
 
         JSAutoEnterCompartment ac;
         if (!ac.enter(cx, funobj)) {
             return JS_FALSE;
         }
 
         JSFunction *fun = JS_ValueToFunction(cx, OBJECT_TO_JSVAL(funobj));
@@ -3555,17 +3555,17 @@ xpc_EvalInSandbox(JSContext *cx, JSObjec
             if (fp && !system) {
                 ssm->IsCapabilityEnabled("UniversalXPConnect", &system);
                 NS_ASSERTION(system, "Bad caller!");
             }
         }
     }
 #endif
 
-    sandbox = XPCWrapper::UnsafeUnwrapSecurityWrapper(cx, sandbox);
+    sandbox = XPCWrapper::UnsafeUnwrapSecurityWrapper(sandbox);
     if (!sandbox || sandbox->getJSClass() != &SandboxClass) {
         return NS_ERROR_INVALID_ARG;
     }
 
     nsIScriptObjectPrincipal *sop =
         (nsIScriptObjectPrincipal*)xpc_GetJSPrivate(sandbox);
     NS_ASSERTION(sop, "Invalid sandbox passed");
     nsCOMPtr<nsIPrincipal> prin = sop->GetPrincipal();
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -1437,20 +1437,16 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS
 // XPC_WN_Shared_Proto_Enumerate or something rather than checking for
 // 4 classes?
 #define IS_PROTO_CLASS(clazz)                                                 \
     ((clazz) == &XPC_WN_NoMods_WithCall_Proto_JSClass ||                      \
      (clazz) == &XPC_WN_NoMods_NoCall_Proto_JSClass ||                        \
      (clazz) == &XPC_WN_ModsAllowed_WithCall_Proto_JSClass ||                 \
      (clazz) == &XPC_WN_ModsAllowed_NoCall_Proto_JSClass)
 
-// Comes from xpcwrappednativeops.cpp
-extern void
-xpc_TraceForValidWrapper(JSTracer *trc, XPCWrappedNative* wrapper);
-
 /***************************************************************************/
 
 namespace XPCWrapper {
 
 enum WrapperType {
     UNKNOWN         = 0,
     NONE            = 0,
     XPCNW_IMPLICIT  = 1 << 0,
@@ -2574,16 +2570,18 @@ public:
     GetUsedOnly(XPCCallContext& ccx,
                 nsISupports* Object,
                 XPCWrappedNativeScope* Scope,
                 XPCNativeInterface* Interface,
                 XPCWrappedNative** wrapper);
 
     // If pobj2 is not null and *pobj2 is not null after the call then *pobj2
     // points to an object for which IS_SLIM_WRAPPER_OBJECT is true.
+    // cx is null when invoked from the marking phase of the GC. In this case
+    // fubobj must be null as well.
     static XPCWrappedNative*
     GetWrappedNativeOfJSObject(JSContext* cx, JSObject* obj,
                                JSObject* funobj = nsnull,
                                JSObject** pobj2 = nsnull,
                                XPCWrappedNativeTearOff** pTearOff = nsnull);
     static XPCWrappedNative*
     GetAndMorphWrappedNativeOfJSObject(JSContext* cx, JSObject* obj)
     {
--- a/js/src/xpconnect/src/xpcwrappednative.cpp
+++ b/js/src/xpconnect/src/xpcwrappednative.cpp
@@ -1746,16 +1746,22 @@ XPCWrappedNative*
 XPCWrappedNative::GetWrappedNativeOfJSObject(JSContext* cx,
                                              JSObject* obj,
                                              JSObject* funobj,
                                              JSObject** pobj2,
                                              XPCWrappedNativeTearOff** pTearOff)
 {
     NS_PRECONDITION(obj, "bad param");
 
+    // fubobj must be null if called without cx.
+    NS_PRECONDITION(cx || !funobj, "bad param");
+
+    // *pTeaorOff must be null if pTearOff is given
+    NS_PRECONDITION(!pTearOff || !*pTearOff, "bad param");
+
     JSObject* cur;
 
     XPCWrappedNativeProto* proto = nsnull;
     nsIClassInfo* protoClassInfo = nsnull;
 
     // If we were passed a function object then we need to find the correct
     // wrapper out of those that might be in the callee obj's proto chain.
 
@@ -1765,17 +1771,17 @@ XPCWrappedNative::GetWrappedNativeOfJSOb
         OBJ_TO_INNER_OBJECT(cx, funObjParent);
         NS_ASSERTION(funObjParent, "funobj has no parent");
 
         js::Class* funObjParentClass = funObjParent->getClass();
 
         if(IS_PROTO_CLASS(funObjParentClass))
         {
             NS_ASSERTION(funObjParent->getParent(), "funobj's parent (proto) is global");
-            proto = (XPCWrappedNativeProto*) xpc_GetJSPrivate(funObjParent);
+            proto = (XPCWrappedNativeProto*) funObjParent->getPrivate();
             if(proto)
                 protoClassInfo = proto->GetClassInfo();
         }
         else if(IS_WRAPPER_CLASS(funObjParentClass))
         {
             cur = funObjParent;
             goto return_wrapper;
         }
@@ -1787,28 +1793,29 @@ XPCWrappedNative::GetWrappedNativeOfJSOb
         }
         else
         {
             NS_ERROR("function object has parent of unknown class!");
             return nsnull;
         }
     }
 
+  restart:
     for(cur = obj; cur; cur = cur->getProto())
     {
         // this is on two lines to make the compiler happy given the goto.
         js::Class* clazz;
         clazz = cur->getClass();
 
         if(IS_WRAPPER_CLASS(clazz))
         {
 return_wrapper:
             JSBool isWN = IS_WN_WRAPPER_OBJECT(cur);
             XPCWrappedNative* wrapper =
-                isWN ? (XPCWrappedNative*) xpc_GetJSPrivate(cur) : nsnull;
+                isWN ? (XPCWrappedNative*) cur->getPrivate() : nsnull;
             if(proto)
             {
                 XPCWrappedNativeProto* wrapper_proto =
                     isWN ? wrapper->GetProto() : GetSlimWrapperProto(cur);
                 if(proto != wrapper_proto &&
                    (!protoClassInfo || !wrapper_proto ||
                     protoClassInfo != wrapper_proto->GetClassInfo()))
                     continue;
@@ -1817,38 +1824,41 @@ return_wrapper:
                 *pobj2 = isWN ? nsnull : cur;
             return wrapper;
         }
 
         if(IS_TEAROFF_CLASS(clazz))
         {
 return_tearoff:
             XPCWrappedNative* wrapper =
-                (XPCWrappedNative*) xpc_GetJSPrivate(cur->getParent());
+                (XPCWrappedNative*) cur->getParent()->getPrivate();
             if(proto && proto != wrapper->GetProto() &&
                (proto->GetScope() != wrapper->GetScope() ||
                 !protoClassInfo || !wrapper->GetProto() ||
                 protoClassInfo != wrapper->GetProto()->GetClassInfo()))
                 continue;
             if(pobj2)
                 *pobj2 = nsnull;
-            XPCWrappedNativeTearOff* to =
-                (XPCWrappedNativeTearOff*) xpc_GetJSPrivate(cur);
+            XPCWrappedNativeTearOff* to = (XPCWrappedNativeTearOff*) cur->getPrivate();
             if(!to)
                 return nsnull;
             if(pTearOff)
                 *pTearOff = to;
             return wrapper;
         }
 
         // Unwrap any wrapper wrappers.
-        JSObject *unsafeObj;
-        if((unsafeObj = XPCWrapper::Unwrap(cx, cur)))
-            return GetWrappedNativeOfJSObject(cx, unsafeObj, funobj, pobj2,
-                                              pTearOff);
+        JSObject *unsafeObj = cx
+                              ? XPCWrapper::Unwrap(cx, cur)
+                              : XPCWrapper::UnsafeUnwrapSecurityWrapper(cur);
+        if(unsafeObj)
+        {
+            obj = unsafeObj;
+            goto restart;
+        }
     }
 
     if(pobj2)
         *pobj2 = nsnull;
     return nsnull;
 }
 
 JSBool
--- a/js/src/xpconnect/src/xpcwrappednativejsops.cpp
+++ b/js/src/xpconnect/src/xpcwrappednativejsops.cpp
@@ -708,18 +708,18 @@ TraceScopeJSObjects(JSTracer *trc, XPCWr
     obj = scope->GetPrototypeJSFunction();
     if(obj)
     {
         JS_CALL_OBJECT_TRACER(trc, obj,
                               "XPCWrappedNativeScope::mPrototypeJSFunction");
     }
 }
 
-void
-xpc_TraceForValidWrapper(JSTracer *trc, XPCWrappedNative* wrapper)
+static void
+TraceForValidWrapper(JSTracer *trc, XPCWrappedNative* wrapper)
 {
     // NOTE: It might be nice to also do the wrapper->Mark() call here too
     // when we are called during the marking phase of JS GC to mark the
     // wrapper's and wrapper's proto's interface sets.
     //
     // We currently do that in the GC callback code. The reason we don't do that
     // here is because the bits used in that marking do unpleasant things to the
     // member counts in the interface and interface set objects. Those counts
@@ -738,34 +738,46 @@ xpc_TraceForValidWrapper(JSTracer *trc, 
     // need to use the JSClass. So, some marking is required for protection.
 
     wrapper->TraceJS(trc);
      
     TraceScopeJSObjects(trc, wrapper->GetScope());
 }
 
 static void
-XPC_WN_Shared_Trace(JSTracer *trc, JSObject *obj)
+MarkWrappedNative(JSTracer *trc, JSObject *obj, bool helper)
 {
     JSObject *obj2;
+
+    // Pass null for the first JSContext* parameter  to skip any security
+    // checks and to avoid potential state change there.
     XPCWrappedNative* wrapper =
-        XPCWrappedNative::GetWrappedNativeOfJSObject(trc->context, obj, nsnull,
-                                                     &obj2);
+        XPCWrappedNative::GetWrappedNativeOfJSObject(nsnull, obj, nsnull, &obj2);
 
     if(wrapper)
     {
         if(wrapper->IsValid())
-             xpc_TraceForValidWrapper(trc, wrapper);
+        {
+            if(helper)
+                wrapper->GetScriptableCallback()->Trace(wrapper, trc, obj);
+             TraceForValidWrapper(trc, wrapper);
+        }
     }
     else if(obj2)
     {
         GetSlimWrapperProto(obj2)->TraceJS(trc);
     }
 }
 
+static void
+XPC_WN_Shared_Trace(JSTracer *trc, JSObject *obj)
+{
+    MarkWrappedNative(trc, obj, false);
+}
+
 static JSBool
 XPC_WN_NoHelper_Resolve(JSContext *cx, JSObject *obj, jsid id)
 {
     MORPH_SLIM_WRAPPER(cx, obj);
     XPCCallContext ccx(JS_CALLER, cx, obj, nsnull, id);
     XPCWrappedNative* wrapper = ccx.GetWrapper();
     THROW_AND_RETURN_IF_BAD_WRAPPER(cx, wrapper);
 
@@ -1093,32 +1105,17 @@ XPC_WN_Helper_Finalize(JSContext *cx, JS
         return;
     wrapper->GetScriptableCallback()->Finalize(wrapper, cx, obj);
     wrapper->FlatJSObjectFinalized(cx);
 }
 
 static void
 XPC_WN_Helper_Trace(JSTracer *trc, JSObject *obj)
 {
-    JSObject *obj2;
-    XPCWrappedNative* wrapper =
-        XPCWrappedNative::GetWrappedNativeOfJSObject(trc->context, obj, nsnull,
-                                                     &obj2);
-    if(wrapper)
-    {
-        if(wrapper->IsValid())
-        {
-            wrapper->GetScriptableCallback()->Trace(wrapper, trc, obj);
-            xpc_TraceForValidWrapper(trc, wrapper);
-        }
-    }
-    else if(obj2)
-    {
-        GetSlimWrapperProto(obj2)->TraceJS(trc);
-    }
+    MarkWrappedNative(trc, obj, true);
 }
 
 static JSBool
 XPC_WN_Helper_NewResolve(JSContext *cx, JSObject *obj, jsid id, uintN flags,
                          JSObject **objp)
 {
     nsresult rv = NS_OK;
     JSBool retval = JS_TRUE;