Closed
Bug 380475
Opened 18 years ago
Closed 17 years ago
[FIX]Make getting URIs from principals a little faster
Categories
(Core :: Security: CAPS, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha6
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: perf, regression)
Attachments
(1 file)
|
(deleted),
patch
|
dveditz
:
review+
Biesinger
:
superreview+
|
Details | Diff | Splinter Review |
Bug 339822 missed the main spot where we set URIs on principals. And in fact, GetURI _is_ on one of the security check critical paths...
The patch implements some of the things that bug talks about, as well as hitting the remaining points where mCodebase is modified.
Attachment #264573 -
Flags: superreview?(cbiesinger)
Attachment #264573 -
Flags: review?(dveditz)
|
||
Comment 1•18 years ago
|
||
Comment on attachment 264573 [details] [diff] [review]
Proposed patch
mCodebaseImmutable is to avoid the overhead from calling GetMutable every time?
Attachment #264573 -
Flags: superreview?(cbiesinger) → superreview+
|
Assignee | |
Comment 2•18 years ago
|
||
Yep, exactly. And the QI overhead.
|
||
Comment 3•17 years ago
|
||
Comment on attachment 264573 [details] [diff] [review]
Proposed patch
> nsPrincipal::GetURI(nsIURI** aURI)
> {
> if (!mCodebase) {
> *aURI = nsnull;
> return NS_OK;
> }
>
>+ if (mCodebaseImmutable) {
>+ NS_ADDREF(*aURI = mCodebase);
>+ return NS_OK;
>+ }
If you're doing this for perf reasons would it make more sense to check the faster mCodebaseImmutable first, since practically all principals have a codebase? You could combine the checks into a single
if (mCodebaseImmutable || !mCodebase)
Though maybe nsCOMPtr's aren't so bad in optimized builds and the single extra check isn't going to hurt.
r=dveditz
Attachment #264573 -
Flags: review?(dveditz) → review+
|
|
Assignee | |
Comment 4•17 years ago
|
||
Good idea. Switched the order of those checks, and checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
|
||
Updated•17 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•