Implement nsIContentPolicy.shouldProcess for plugin resource loads

RESOLVED FIXED in Firefox 36

Status

()

Core
Plug-ins
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bsmedberg, Assigned: bsmedberg)

Tracking

unspecified
mozilla37
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox36 fixed, firefox37 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Plugin resource loads currently don't use nsIContentPolicy.shouldProcess, which means that we don't have MIME type data for them. This patch fixes that and is necessary for the experiment in bug 1108668.
(Assignee)

Updated

2 years ago
Assignee: nobody → benjamin
(Assignee)

Comment 1

2 years ago
Created attachment 8546021 [details]
Reviewboard request
Attachment #8546021 - Flags: review?(bzbarsky)
Comment on attachment 8546021 [details]
Reviewboard request

r+, but please see the reviewboard comments.  I wish it actually set the r+ state here instead of putting me through the extra work... filed bug 1119416.
Attachment #8546021 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c40a9e043ce
(Assignee)

Comment 4

2 years ago
Comment on attachment 8546021 [details]
Reviewboard request

Approval Request Comment
[Feature/regressing bug #]: API needed for the experiment in bug 1108668
[User impact if declined]: Can't run the experiment
[Describe test coverage new/current, TBPL]: Manually verified with the experiment code
[Risks and why]: There is a risk that nsIContentPolicy implementations will see new notifications that they didn't expect and will somehow fail. I judge this risk as fairly low, and is mainly a risk with addons.
[String/UUID change made/needed]: None
Attachment #8546021 - Flags: approval-mozilla-aurora?
Backed out for failures in test_streamNotify.html:
https://treeherder.mozilla.org/logviewer.html#?job_id=5277932&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=5278924&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/157873c02e6d
Flags: needinfo?(benjamin)
This also failed with this patch:
https://treeherder.mozilla.org/logviewer.html#?job_id=5280947&repo=mozilla-inbound
status-firefox36: --- → affected
status-firefox37: --- → affected
(Assignee)

Comment 7

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/621fadc8702b I had to move the code later in the method so that error returns triggered notifications properly.
Flags: needinfo?(benjamin)
https://hg.mozilla.org/mozilla-central/rev/621fadc8702b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8546021 [details]
Reviewboard request

[Triage Comment]
After the merge.
Attachment #8546021 - Flags: approval-mozilla-beta+
Attachment #8546021 - Flags: approval-mozilla-aurora?
Attachment #8546021 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 10

2 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/c447c4b139a9 (this made 37 before the merge)
status-firefox36: affected → fixed
status-firefox37: affected → fixed
You need to log in before you can comment on or make changes to this bug.