Split TYPE_XMLHTTPREQUEST and TYPE_DATAREQUEST for EventSource

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
2 months ago

People

(Reporter: ckerschb, Assigned: ckerschb)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Before we can land Bug 1182544 we need a separation between those content types: 
[1] http://mxr.mozilla.org/mozilla-central/source/dom/fetch/InternalRequest.h#68

Otherwise we can't distinguish the mime type guess within the contentSecuritymanager.
I have already written all the code for switching EventSource from asyncOpen() to using asyncOpen(2). Let's make Bug 1188637 blocking this one.
Blocks: 1182544
Depends on: 1188637
Assignee: nobody → mozilla
Blocks: 1174307
Hey Jonas, since you already reviewed the initial bug (Bug 1174307) when Ehsan introduced internal content policy types I was wondering if you wanna review this one as well. If you prefer Ehsan to review the patch, let me know.
Attachment #8643465 - Flags: review?(jonas)
Oh, I forgot to mention that this patch applies on top of the EventSource patch within Bug 1188637 which is already r+ed and about to land later tonight.
As discussed over IRC it would be great if you could have a final look at the changes within dom/fetch/InternalRequest.cpp which I didn't quite get right in the first iteration of the patch. Thanks!
Attachment #8643465 - Attachment is obsolete: true
Attachment #8643833 - Flags: review?(ehsan)

Comment 5

4 years ago
Comment on attachment 8643833 [details] [diff] [review]
bug_1191107_split_type_xmlhttprequest.patch

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

::: toolkit/components/places/BookmarkJSONUtils.jsm
@@ +209,5 @@
>        let uri = NetUtil.newURI(spec);
>        let channel = NetUtil.newChannel({
>          uri,
>          loadingPrincipal: Services.scriptSecurityManager.getNoAppCodebasePrincipal(uri),
> +        contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_EVENTSOURCE

This should probably be XHR...

::: toolkit/components/places/nsLivemarkService.js
@@ +528,5 @@
>                        createInstance(Ci.nsILoadGroup);
>        let channel = NetUtil.newChannel({
>          uri: this.feedURI.spec,
>          loadingPrincipal: Services.scriptSecurityManager.getNoAppCodebasePrincipal(this.feedURI),
> +        contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_EVENTSOURCE

Same.

::: toolkit/components/search/tests/xpcshell/test_serialize_file.js
@@ +40,5 @@
>    function readAsyncFile(aFile, aCallback) {
>      NetUtil.asyncFetch({
>        uri: NetUtil.newURI(aFile),
>        loadUsingSystemPrincipal: true,
> +      contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_EVENTSOURCE

And here.

::: toolkit/mozapps/update/nsUpdateService.js
@@ +1261,5 @@
>                                        null,      // aLoadingNode
>                                        Services.scriptSecurityManager.getSystemPrincipal(),
>                                        null,      // aTriggeringPrincipal
>                                        Ci.nsILoadInfo.SEC_NORMAL,
> +                                      Ci.nsIContentPolicy.TYPE_INTERNAL_EVENTSOURCE);

Here too.
Attachment #8643833 - Flags: review?(ehsan) → review+
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #5)
> This should probably be XHR...

I changed all of the above - no need for attaching a new patch though.
sorry had to back this out since one of this changes caused errors like xpcshell bustage and https://treeherder.mozilla.org/logviewer.html#?job_id=12554709&repo=mozilla-inbound
Flags: needinfo?(mozilla)
(In reply to Carsten Book [:Tomcat] from comment #9)
> sorry had to back this out since one of this changes caused errors like
> xpcshell bustage and
> https://treeherder.mozilla.org/logviewer.html#?job_id=12554709&repo=mozilla-
> inbound

Thanks Carsten. I thought that there is no need to change the uuid in case one only adds 'constants' to an *.idl. Apparently that's not always the case. So what happenend here is that the idl changes didn't find their way in the xpt on windows which causes the problem here. Anyway, I will re-land that patch with and updated uuid in the *.idl.
Flags: needinfo?(mozilla)
https://hg.mozilla.org/mozilla-central/rev/46f635f60ee0
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.