408009 - Make doGetObjectPrincipal() faster.

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Johnny Stenback (:jst)]

Attachment: Make doGetObjectPrincipal() faster.

nsScriptSecurityManager::doGetObjectPrincipal() spends a good bit of time calling QI on a JSObject's private data to find out if it's a XPConnect wrapped native or not. That QI isn't needed, XPConnect has the information to do that check w/o an expensive QI, so we should expose that to caps and speed things up. I'm attaching a patch that does that, and also adds some code to exclude JSFunction objects (followed by a Call object) at at the start of the parent chain from the loop, and some other tweaks to speed things up as well.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 292705 [details] [diff] [review]
Make doGetObjectPrincipal() faster.

>+++ b/caps/src/nsScriptSecurityManager.cpp
>+                // 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.

Can we please actually assert that (using aAllowShortCircuit; you'll need to change its default value back to true)?  

>+                    if (node) {
>+                        result = node->NodePrincipal();
>+
>+                        if (result) {

NodePrincipal() never returns null.  So no need for this null-check.

>+++ b/js/src/jsfun.h

This probably needs review from Brendan or someone else who knows that code.

r+sr=bzbarsky on the xpconnect/caps changes, with the above issues fixed.

It would be nice to leave in a comment about using GetObjectPrincipal on the wrapper or something; if we could get that to work it would eliminate a QI (to nsIScriptObjectPrincipal on things whose classes start with 'W', 'M', 'C', and to nsINode on everything else).

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 292705 [details] [diff] [review]
Make doGetObjectPrincipal() faster.

Brendan, you cool with the jsfun.h changes in this patch?

Comment 3

[Johnny Stenback (:jst)]

Attachment: Updated fix for checkin.

This fixes what bz pointed out, and also bumps the IID for nsIXPConnect.

Comment 4

[Johnny Stenback (:jst)]

Fix checked in.