Fixing bug 408009. Make doGetObjectPrincipal() faster. r+sr=bzbarsky@mit.edu, r+a=brendan@mozilla.org
authorjst@mozilla.org
Wed, 12 Dec 2007 15:02:25 -0800
changeset 8990 53339e02cc14358e28e833f8ff6c53c7d2b3fd38
parent 8989 30a53c37395e40b2f1c34982b6ad9b0c88903fab
child 8991 72c034f96bb81b9e9fb44d113ac9bca86fd80509
push idunknown
push userunknown
push dateunknown
bugs408009
milestone1.9b3pre
Fixing bug 408009. Make doGetObjectPrincipal() faster. r+sr=bzbarsky@mit.edu, r+a=brendan@mozilla.org
caps/include/nsScriptSecurityManager.h
caps/src/nsScriptSecurityManager.cpp
js/src/jsfun.h
js/src/xpconnect/idl/nsIXPConnect.idl
js/src/xpconnect/src/nsXPConnect.cpp
js/src/xpconnect/src/xpcprivate.h
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -404,18 +404,21 @@ private:
     static JSBool JS_DLL_CALLBACK
     CheckObjectAccess(JSContext *cx, JSObject *obj,
                       jsval id, JSAccessMode mode,
                       jsval *vp);
 
     // Returns null if a principal cannot be found; generally callers
     // should error out at that point.
     static nsIPrincipal*
-    doGetObjectPrincipal(JSContext *cx, JSObject *obj,
-                         PRBool aAllowShortCircuit = PR_FALSE);
+    doGetObjectPrincipal(JSContext *cx, JSObject *obj
+#ifdef DEBUG
+                         , PRBool aAllowShortCircuit = PR_TRUE
+#endif
+                         );
 
     // Returns null if a principal cannot be found.  Note that rv can be NS_OK
     // when this happens -- this means that there was no JS running.
     nsIPrincipal*
     doGetSubjectPrincipal(nsresult* rv);
     
     static nsresult 
     ReportError(JSContext* cx, const nsAString& messageTag,
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -52,16 +52,18 @@
 #include "nsNullPrincipal.h"
 #include "nsXPIDLString.h"
 #include "nsCRT.h"
 #include "nsCRTGlue.h"
 #include "nsIJSContextStack.h"
 #include "nsDOMError.h"
 #include "nsDOMCID.h"
 #include "jsdbgapi.h"
+#include "jsarena.h"
+#include "jsfun.h"
 #include "nsIXPConnect.h"
 #include "nsIXPCSecurityManager.h"
 #include "nsTextFormatter.h"
 #include "nsIStringBundle.h"
 #include "nsNetUtil.h"
 #include "nsIProperties.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsIFile.h"
@@ -91,16 +93,24 @@
 
 static NS_DEFINE_CID(kZipReaderCID, NS_ZIPREADER_CID);
 
 nsIIOService    *nsScriptSecurityManager::sIOService = nsnull;
 nsIXPConnect    *nsScriptSecurityManager::sXPConnect = nsnull;
 nsIStringBundle *nsScriptSecurityManager::sStrBundle = nsnull;
 JSRuntime       *nsScriptSecurityManager::sRuntime   = 0;
 
+// Info we need about the JSClasses used by XPConnects wrapped
+// natives, to avoid having to QI to nsIXPConnectWrappedNative all the
+// time when doing security checks.
+static const JSClass *sXPCWrappedNativeJSClass;
+static JSGetObjectOps sXPCWrappedNativeGetObjOps1;
+static JSGetObjectOps sXPCWrappedNativeGetObjOps2;
+
+
 ///////////////////////////
 // Convenience Functions //
 ///////////////////////////
 // Result of this function should not be freed.
 static inline const PRUnichar *
 JSValIDToString(JSContext *cx, const jsval idval)
 {
     JSAutoRequest ar(cx);
@@ -1870,17 +1880,17 @@ nsScriptSecurityManager::GetCertificateP
     NS_ENSURE_ARG(!aCertFingerprint.IsEmpty() &&
                   !aSubjectName.IsEmpty() &&
                   aCertificate);
 
     return DoGetCertificatePrincipal(aCertFingerprint, aSubjectName,
                                      aPrettyName, aCertificate, aURI, PR_TRUE,
                                      result);
 }
-    
+
 nsresult
 nsScriptSecurityManager::DoGetCertificatePrincipal(const nsACString& aCertFingerprint,
                                                    const nsACString& aSubjectName,
                                                    const nsACString& aPrettyName,
                                                    nsISupports* aCertificate,
                                                    nsIURI* aURI,
                                                    PRBool aModifyTable,
                                                    nsIPrincipal **result)
@@ -2281,78 +2291,148 @@ nsScriptSecurityManager::GetObjectPrinci
     if (!*result)
         return NS_ERROR_FAILURE;
     NS_ADDREF(*result);
     return NS_OK;
 }
 
 // static
 nsIPrincipal*
-nsScriptSecurityManager::doGetObjectPrincipal(JSContext *aCx, JSObject *aObj,
-                                              PRBool aAllowShortCircuit)
+nsScriptSecurityManager::doGetObjectPrincipal(JSContext *aCx, JSObject *aObj
+#ifdef DEBUG
+                                              , PRBool aAllowShortCircuit
+#endif
+                                              )
 {
     NS_ASSERTION(aCx && aObj, "Bad call to doGetObjectPrincipal()!");
     nsIPrincipal* result = nsnull;
 
 #ifdef DEBUG
     JSObject* origObj = aObj;
 #endif
     
-    do
-    {
-        const JSClass *jsClass = JS_GetClass(aCx, aObj);
-
-        if (jsClass && !(~jsClass->flags & (JSCLASS_HAS_PRIVATE |
-                                            JSCLASS_PRIVATE_IS_NSISUPPORTS)))
-        {
-            // No need to refcount |priv| here.
-            nsISupports *priv = (nsISupports *)JS_GetPrivate(aCx, aObj);
-
-            /*
-             * If it's a wrapped native (as most
-             * JSCLASS_PRIVATE_IS_NSISUPPORTS objects are in mozilla),
-             * check the underlying native instead.
-             */
-            nsCOMPtr<nsIXPConnectWrappedNative> xpcWrapper =
-                do_QueryInterface(priv);
-
-            if (NS_LIKELY(xpcWrapper != nsnull))
-            {
-                if (NS_UNLIKELY(aAllowShortCircuit))
-                {
-                    result = xpcWrapper->GetObjectPrincipal();
+    const JSClass *jsClass = JS_GET_CLASS(aCx, aObj);
+
+    // A common case seen in this code is that we enter this function
+    // with aObj being a Function object, whose parent is a Call
+    // object. Neither of those have object principals, so we can skip
+    // those objects here before we enter the below loop. That way we
+    // avoid wasting time checking properties of their classes etc in
+    // the loop.
+
+    if (jsClass == &js_FunctionClass) {
+        aObj = JS_GetParent(aCx, aObj);
+
+        if (!aObj)
+            return nsnull;
+
+        jsClass = JS_GET_CLASS(aCx, aObj);
+
+        if (jsClass == &js_CallClass) {
+            aObj = JS_GetParent(aCx, aObj);
+
+            if (!aObj)
+                return nsnull;
+
+            jsClass = JS_GET_CLASS(aCx, aObj);
+        }
+    }
+
+    do {
+        // Note: jsClass is set before this loop, and also at the
+        // *end* of this loop.
+
+        // NOTE: These class and getObjectOps hook checks better match
+        // what IS_WRAPPER_CLASS() does in xpconnect!
+        if (jsClass == sXPCWrappedNativeJSClass ||
+            jsClass->getObjectOps == sXPCWrappedNativeGetObjOps1 ||
+            jsClass->getObjectOps == sXPCWrappedNativeGetObjOps2) {
+            nsIXPConnectWrappedNative *xpcWrapper =
+                (nsIXPConnectWrappedNative *)JS_GetPrivate(aCx, aObj);
+
+            if (xpcWrapper) {
+                nsISupports *native = xpcWrapper->Native();
+                char ch = jsClass->name[0];
+
+                // XXXjst: Ideally this code would simply call into
+                // xpcWrapper->GetObjectPrincipal() and use that if we
+                // find a principal through that. If not, we can fall
+                // back to the below code. See bug 317240.
+
+                // For classes that are unlikely to be window object
+                // classes (Window, ModalContentWindow, and
+                // ChromeWindow), check if the native pointer is an
+                // nsINode. Note that this is merely a performance
+                // optimization, so missing this optimization is
+                // non-critical and must result in us finding the same
+                // principal that we would have gotten by asking the
+                // nsINode here.
+                if (ch != 'W' && ch != 'M' && ch != 'C'
+#ifdef DEBUG
+                    && aAllowShortCircuit
+#endif
+                    ) {
+                    nsCOMPtr<nsINode> node = do_QueryInterface(native);
+
+                    if (node) {
+                        result = node->NodePrincipal();
+
+                        // NodePrincipal() *always* returns a
+                        // principal.
+
+                        break;
+                    }
                 }
-                else
-                {
-                    nsCOMPtr<nsIScriptObjectPrincipal> objPrin;
-                    objPrin = do_QueryWrappedNative(xpcWrapper);
-                    if (objPrin)
-                    {
-                        result = objPrin->GetPrincipal();
-                    }                    
+
+                // If not, check if it points to an
+                // nsIScriptObjectPrincipal
+                nsCOMPtr<nsIScriptObjectPrincipal> objPrin =
+                    do_QueryInterface(native);
+                if (objPrin) {
+                    result = objPrin->GetPrincipal();
+
+                    if (result) {
+                        break;
+                    }
                 }
             }
-            else
-            {
-                nsCOMPtr<nsIScriptObjectPrincipal> objPrin;
-                objPrin = do_QueryInterface(priv);
-                if (objPrin)
-                {
-                    result = objPrin->GetPrincipal();
+        } else if (!(~jsClass->flags & (JSCLASS_HAS_PRIVATE |
+                                        JSCLASS_PRIVATE_IS_NSISUPPORTS))) {
+            nsISupports *priv = (nsISupports *)JS_GetPrivate(aCx, aObj);
+
+#ifdef DEBUG
+            if (aAllowShortCircuit) {
+                nsCOMPtr<nsIXPConnectWrappedNative> xpcWrapper =
+                    do_QueryInterface(priv);
+
+                NS_ASSERTION(!xpcWrapper,
+                             "Uh, an nsIXPConnectWrappedNative with the "
+                             "wrong JSClass or getObjectOps hooks!");
+            }
+#endif
+
+            nsCOMPtr<nsIScriptObjectPrincipal> objPrin =
+                do_QueryInterface(priv);
+
+            if (objPrin) {
+                result = objPrin->GetPrincipal();
+
+                if (result) {
+                    break;
                 }
             }
-
-            if (result)
-            {
-                break;
-            }
         }
 
         aObj = JS_GetParent(aCx, aObj);
-    } while (aObj);
+
+        if (!aObj)
+            break;
+
+        jsClass = JS_GET_CLASS(aCx, aObj);
+    } while (1);
 
     NS_ASSERTION(!aAllowShortCircuit ||
                  result == doGetObjectPrincipal(aCx, origObj, PR_FALSE),
                  "Principal mismatch.  Not good");
     
     return result;
 }
 
@@ -3227,16 +3307,20 @@ nsresult nsScriptSecurityManager::Init()
     rv = runtimeService->GetRuntime(&sRuntime);
     NS_ENSURE_SUCCESS(rv, rv);
 
 #ifdef DEBUG
     JSCheckAccessOp oldCallback =
 #endif
         JS_SetCheckObjectAccessCallback(sRuntime, CheckObjectAccess);
 
+    sXPConnect->GetXPCWrappedNativeJSClassInfo(&sXPCWrappedNativeJSClass,
+                                               &sXPCWrappedNativeGetObjOps1,
+                                               &sXPCWrappedNativeGetObjOps2);
+
     // For now, assert that no callback was set previously
     NS_ASSERTION(!oldCallback, "Someone already set a JS CheckObjectAccess callback");
     return NS_OK;
 }
 
 static nsScriptSecurityManager *gScriptSecMan = nsnull;
 
 jsval nsScriptSecurityManager::sEnabledID   = JSVAL_VOID;
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -97,17 +97,17 @@ struct JSFunction {
 #define FUN_FAST_NATIVE(fun) (((fun)->flags & JSFUN_FAST_NATIVE)              \
                               ? (JSFastNative) (fun)->u.n.native              \
                               : NULL)
 #define FUN_MINARGS(fun)     (((fun)->flags & JSFUN_FAST_NATIVE)              \
                               ? (fun)->u.n.minargs                            \
                               : (fun)->nargs)
 
 extern JSClass js_ArgumentsClass;
-extern JSClass js_CallClass;
+extern JS_FRIEND_DATA(JSClass) js_CallClass;
 
 /* JS_FRIEND_DATA so that VALUE_IS_FUNCTION is callable from the shell. */
 extern JS_FRIEND_DATA(JSClass) js_FunctionClass;
 
 /*
  * NB: jsapi.h and jsobj.h must be included before any call to this macro.
  */
 #define VALUE_IS_FUNCTION(cx, v)                                              \
--- a/js/src/xpconnect/idl/nsIXPConnect.idl
+++ b/js/src/xpconnect/idl/nsIXPConnect.idl
@@ -57,16 +57,18 @@
 %}
 
 /***************************************************************************/
 
 [ptr] native JSContextPtr(JSContext);
 [ptr] native JSObjectPtr(JSObject);
 [ptr] native JSValPtr(jsval);
       native JSVal(jsval);
+[ptr] native JSClassConstPtr(const JSClass);
+      native JSGetObjectOps(JSGetObjectOps);
       native JSID(jsid);
 [ptr] native voidPtrPtr(void*);
 [ptr] native nsScriptObjectTracerPtr(nsScriptObjectTracer);
 [ref] native nsCCTraversalCallbackRef(nsCycleCollectionTraversalCallback);
 
 /***************************************************************************/
 
 %{ C++
@@ -201,17 +203,17 @@ interface nsIXPConnectWrappedNative : ns
 
 #ifndef XPCONNECT_STANDALONE
     /**
      * Get the object principal for this wrapper.  Note that this may well end
      * up being null; in that case one should seek principals elsewhere.  Null
      * here does NOT indicate system principal or no principals at all, just
      * that this wrapper doesn't have an intrinsic one.
      */
-    virtual nsIPrincipal* GetObjectPrincipal() const = 0;   
+    virtual nsIPrincipal* GetObjectPrincipal() const = 0;
 #endif
     
 protected:
     nsISupports *mIdentity;
 public:
 %}
 };
 
@@ -440,17 +442,17 @@ interface nsIXPCFunctionThisTranslator :
 %{ C++
 // For use with the service manager
 // {CB6593E0-F9B2-11d2-BDD6-000064657374}
 #define NS_XPCONNECT_CID \
 { 0xcb6593e0, 0xf9b2, 0x11d2, \
     { 0xbd, 0xd6, 0x0, 0x0, 0x64, 0x65, 0x73, 0x74 } }
 %}
 
-[uuid(9f45711b-bf4f-4af6-a71d-d165f042986d)]
+[uuid(256a41a5-10e6-40a1-8392-8baa4adfabe2)]
 interface nsIXPConnect : nsISupports
 {
 %{ C++
   NS_DEFINE_STATIC_CID_ACCESSOR(NS_XPCONNECT_CID)
 %}
 
     void
     initClasses(in JSContextPtr aJSContext,
@@ -746,9 +748,19 @@ interface nsIXPConnect : nsISupports
 
     /**
      * Note aJSContext as a child to the cycle collector.
      * @param aJSContext The JSContext to note.
      * @param aCb The cycle collection traversal callback.
      */
     [noscript,notxpcom] void noteJSContext(in JSContextPtr aJSContext,
                                            in nsCCTraversalCallbackRef aCb);
+
+    /**
+     * Get the JSClass and JSGetObjectOps pointers to use for
+     * identifying JSObjects that hold nsIXPConnectWrappedNative
+     * pointers in their private date. See IS_WRAPPER_CLASS in
+     * xpcprivate.h for details.
+     */
+    void GetXPCWrappedNativeJSClassInfo(out JSClassConstPtr clazz,
+                                        out JSGetObjectOps ops1,
+                                        out JSGetObjectOps ops2);
 };
--- a/js/src/xpconnect/src/nsXPConnect.cpp
+++ b/js/src/xpconnect/src/nsXPConnect.cpp
@@ -1856,16 +1856,33 @@ nsXPConnect::EvalInSandboxObject(const n
     NS_ENSURE_SUCCESS(rv, rv);
 
     return xpc_EvalInSandbox(cx, obj, source,
                              NS_ConvertUTF16toUTF8(source).get(), 1,
                              returnStringOnly, rval);
 #endif /* XPCONNECT_STANDALONE */
 }
 
+/* void GetXPCWrappedNativeJSClassInfo(out JSClassConstPtr clazz, out JSObjectOpsConstPtr ops1, out JSObjectOpsConstPtr ops2); */
+NS_IMETHODIMP
+nsXPConnect::GetXPCWrappedNativeJSClassInfo(const JSClass **clazz,
+                                            JSGetObjectOps *ops1,
+                                            JSGetObjectOps *ops2)
+{
+    // Expose the JSClass and JSGetObjectOps pointers used by
+    // IS_WRAPPER_CLASS(). If that macro ever changes, this function
+    // needs to stay in sync.
+
+    *clazz = &XPC_WN_NoHelper_JSClass.base;
+    *ops1 = XPC_WN_GetObjectOpsNoCall;
+    *ops2 = XPC_WN_GetObjectOpsWithCall;
+
+    return NS_OK;
+}
+
 /* nsIXPConnectJSObjectHolder getWrappedNativePrototype (in JSContextPtr aJSContext, in JSObjectPtr aScope, in nsIClassInfo aClassInfo); */
 NS_IMETHODIMP 
 nsXPConnect::GetWrappedNativePrototype(JSContext * aJSContext, 
                                        JSObject * aScope, 
                                        nsIClassInfo *aClassInfo, 
                                        nsIXPConnectJSObjectHolder **_retval)
 {
     XPCCallContext ccx(NATIVE_CALLER, aJSContext);
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -246,16 +246,24 @@ extern const char XPC_XPCONNECT_CONTRACT
     if(src) \
         result = (char*) nsMemory::Clone(src, \
                                 sizeof(char)*(strlen(src)+1)); \
     else \
         result = nsnull; \
     *dest = result; \
     return (result || !src) ? NS_OK : NS_ERROR_OUT_OF_MEMORY
 
+
+// NOTE!!!
+//
+// If this ever changes,
+// nsScriptSecurityManager::doGetObjectPrincipal() *must* be updated
+// also!
+//
+// NOTE!!!
 #define IS_WRAPPER_CLASS(clazz)                                               \
           ((clazz) == &XPC_WN_NoHelper_JSClass.base ||                        \
            (clazz)->getObjectOps == XPC_WN_GetObjectOpsNoCall ||              \
            (clazz)->getObjectOps == XPC_WN_GetObjectOpsWithCall)
 
 /***************************************************************************/
 // Auto locking support class...