Closed Bug 408009 Opened 15 years ago Closed 15 years ago

Make doGetObjectPrincipal() faster.


(Core :: Security: CAPS, defect)

Not set





(Reporter: jst, Assigned: jst)



(Keywords: perf)


(2 files)

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.
Flags: blocking1.9+
Attachment #292705 - Flags: superreview?(bzbarsky)
Attachment #292705 - Flags: review?(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).
Attachment #292705 - Flags: superreview?(bzbarsky)
Attachment #292705 - Flags: superreview+
Attachment #292705 - Flags: review?(bzbarsky)
Attachment #292705 - Flags: review+
Comment on attachment 292705 [details] [diff] [review]
Make doGetObjectPrincipal() faster.

Brendan, you cool with the jsfun.h changes in this patch?
Attachment #292705 - Flags: review?(brendan)
Attachment #292705 - Flags: review?(brendan) → review+
Attachment #292705 - Flags: approval1.9+
This fixes what bz pointed out, and also bumps the IID for nsIXPConnect.
Fix checked in.
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.