Closed Bug 910431 Opened 6 years ago Closed 6 years ago

Electrolysis: Permission code followup

Categories

(Firefox :: General, defect)

All
Windows 2000
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 26

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #908692 +++
Attached patch fix-getelement (obsolete) — Splinter Review
Attachment #796866 - Flags: review?(gavin.sharp)
Comment on attachment 796866 [details] [diff] [review]
fix-getelement

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

There are some other implementations of nsIContentPermissionRequest. Most of them are ok, but the one in DesktopNotification.cpp also throws.  And there's http://mxr.mozilla.org/mozilla-central/source/dom/base/nsContentPermissionHelper.cpp#88 which throws under certain circumstances and I'm not sure if that one needs to be changed. Do you know when is that one used?

::: browser/components/nsBrowserGlue.js
@@ +1734,5 @@
>    QueryInterface: XPCOMUtils.generateQI([Ci.nsIContentPermissionPrompt]),
>  
>    _getBrowserForRequest: function (aRequest) {
> +    // "element" is only defined in e10s mode.
> +    var browser = aRequest.element;

s/var/let/
Attachment #796866 - Flags: review?(gavin.sharp) → feedback+
(In reply to :Felipe Gomes from comment #2)
> Comment on attachment 796866 [details] [diff] [review]
> fix-getelement
> 
> Review of attachment 796866 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> There are some other implementations of nsIContentPermissionRequest. Most of
> them are ok, but the one in DesktopNotification.cpp also throws.
Ups don't know why I didn't fix.
> And there's
> http://mxr.mozilla.org/mozilla-central/source/dom/base/
> nsContentPermissionHelper.cpp#88 which throws under certain circumstances
> and I'm not sure if that one needs to be changed. Do you know when is that
> one used?
This is actually the one we use in the e10s case. It proxies some of the stuff, and is the only thing that implements cross process requests. If we have no mParent doing anything with this object will throw, so we might as well do it early.
> 
> ::: browser/components/nsBrowserGlue.js
> @@ +1734,5 @@
> >    QueryInterface: XPCOMUtils.generateQI([Ci.nsIContentPermissionPrompt]),
> >  
> >    _getBrowserForRequest: function (aRequest) {
> > +    // "element" is only defined in e10s mode.
> > +    var browser = aRequest.element;
> 
> s/var/let/
:)
Attachment #796866 - Attachment is obsolete: true
Attachment #798971 - Flags: review?(felipc)
Attachment #798971 - Flags: review?(felipc) → review+
https://hg.mozilla.org/mozilla-central/rev/b67be84c58e2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.