Hold a weak ref to the principal in AutoEntryScript

RESOLVED FIXED in mozilla32

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

(Blocks: 1 bug)

unspecified
mozilla32
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Created attachment 8432708 [details] [diff] [review]
Hold a weak ref to the principal in AutoEntryScript, to reduce call overhead
Attachment #8432708 - Flags: review?(bobbyholley)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
What are your thoughts re bug 1019081 comment 9?
Flags: needinfo?(bzbarsky)
In practice the only caller that sets mWebIDLCallerPrincipal is the CallSetup ctor, and it only does this in the aIsJSImplementedWebIDL case.

Is it possible in this case for the principal we get from SubjectPrincipal() to not be owned by anything on the stack?  I don't think so: we came in via a JSNative, and its callee is an object that owns the relevant compartment and principal.

So I think this is in fact safe.  I'm open to comments to make the restrictions on setting mWebIDLCallerPrincipal clearer!
Flags: needinfo?(bzbarsky)
Comment on attachment 8432708 [details] [diff] [review]
Hold a weak ref to the principal in AutoEntryScript, to reduce call overhead

Review of attachment 8432708 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/ScriptSettings.h
@@ +179,5 @@
>  private:
>    JSAutoCompartment mAc;
>    dom::ScriptSettingsStack& mStack;
> +  // It's safe to make this a weak pointer, since it's the subject principal
> +  // when we go on the stack, so can't go away until after we're gone.

Can you add the aforementioned nuances to this comment?
Attachment #8432708 - Flags: review?(bobbyholley) → review+
Done, and https://hg.mozilla.org/integration/mozilla-inbound/rev/62c85e7fc6ad
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla32
https://hg.mozilla.org/mozilla-central/rev/62c85e7fc6ad
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.