The plan is to inline the principal on nsIScriptObjectPrincipal.
OK, I tried to do this with inlining, and it's just not worth it, imo. Maintaining the member on nsIScriptObjectPrincipal is a huge pain, and would involve decent-sized changes to nsDocument, nsXULDocument, nsXULPrototypeDocument, addition of various notifications to XUL document and proto documents and proto script globals, etc to propagate the changed principal around, etc. In fact, given the XUL implementation of GetPrincipal, it's really just not worth even trying to cache the principal on the window... We'd have to somehow update things when the "master proto" changes, which is somewhat scattered about the code. Just not worth the effort for the smalle gain we'd get. So I'm falling back on plan B -- just cache an nsIDocument pointer in nsGlobalWindow. That still leaves some virtual calls, but it's a lot less QIing, at least.
Created attachment 200234 [details] [diff] [review] Like so
13 years ago
Priority: -- → P1
Summary: Make nsGlobalWindow::GetPrincipal faster → [FIX]Make nsGlobalWindow::GetPrincipal faster
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 200234 [details] [diff] [review] Like so r=mrbkap
Attachment #200234 - Flags: review?(mrbkap) → review+
Comment on attachment 200234 [details] [diff] [review] Like so sr=jst
Attachment #200234 - Flags: superreview?(jst) → superreview+
We could also in addition to this inline nsIDocument::GetPrincipal(). To do that we either need the inline getter to call a virtual method that gets the principal if not already set (since we now create principals lazily), or we'd need to change our code to eagerly create principals as soon as we've got a URI to create it from etc. I'd vote for the latter... maybe do that in a separate bug if you don't want it done here...
> We could also in addition to this inline nsIDocument::GetPrincipal(). Not without dealing with the whole XUL mess. nsXULDocument overrides GetPrincipal and gets it from the proto, but the proto it's getting it from can change under conditions that I haven't figured out yet, etc... :(
Fixed. This will really start to matter once we fix bug 289655, since this code will be hit more then.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.