STR:

1. Load http://janodvarko.cz/tests/network/cors/example1/
2. Open the Network panel + split console
3. Click the button on the page
4. The Console says:
   Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource ...
   But the Request in the Network panel isn't marked as blocked => BUG

Honza

Patch that fixes the issue:

diff --git a/devtools/server/actors/network-monitor/network-response-listener.js b/devtool--- a/devtools/server/actors/network-monitor/network-response-listener.js
+++ b/devtools/server/actors/network-monitor/network-response-listener.js
@@ -509,16 +509,25 @@ NetworkResponseListener.prototype = {
     try {
       const properties = this.request.QueryInterface(Ci.nsIPropertyBag);
       reason = this.request.loadInfo.requestBlockingReason;
       id = properties.getProperty("cancelledByExtension");
     } catch (err) {
       // "cancelledByExtension" doesn't have to be available.
     }

+    console.log("_onComplete " + this.request.URI.spec +
+      ", " + reason +
+      ", " + id +
+      ", " + this.request.canceled, this.request);
+
+    if (this.request.canceled && !reason) {
+      reason = "unknown";
+    }
+
     this.httpActivity.owner.addResponseContent(response, {
       discardResponseBody: this.httpActivity.discardResponseBody,
       truncated: this.truncated,
       blockedReason: reason,
       blockingExtension: id,
     });

     this._wrappedNotificationCallbacks = null;

But , why the this.request.loadInfo.requestBlockingReason is empty?

Honza

I am increasing the priority for this.

We landed bug 1555057 this week (78) that fixes several issues (and other bug reports) related to blocked resources. It would be great if this bug could also land in 78, so we can outreach DevEdition 78 users (next cycle) with this great feature.

Honza

Junior, Thomas, something you can help with perhaps?

What I see is we successfully cancelled the request in child process in OnStartRequest here
https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/netwerk/protocol/http/nsCORSListenerProxy.cpp#420

Hence the content is not available to the consumers, which is good.

I believe DevTool is listening to some event in the parent process, right?
If yes, RecvCancel is inevitable too late for parent, which is after OnStopRequest in this case.
However, we need to check the header, so OnStartRequest is the earliest point.

Patch like comment 1 is needed IMO, or any suggestion for the http-on* events given this scenario?

(In reply to Junior from comment #4)

What I see is we successfully cancelled the request in child process in OnStartRequest here
https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/netwerk/protocol/http/nsCORSListenerProxy.cpp#420

I am seeing that this call is setting an internal mStatus variable to NS_ERROR_DOM_BAD_URI.

So, it looks like we could use request.status, it's set to 0x805303F4, which corresponds to NS_ERROR_DOM_BAD_URI

Any chance to get a reason that says something like: CORS header ‘Access-Control-Allow-Origin’ missing ?

It would more correspond to what the Console warning says:
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://odvarko.com/tests/network/cors/example1/data.json. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing).

See the attached screenshot (coming from the test case in comment #0)

Hence the content is not available to the consumers, which is good.

I believe DevTool is listening to some event in the parent process, right?
Correct, the Network panel is listening in the parent process.

Honza

Any chance to get a reason that says something like: CORS header ‘Access-Control-Allow-Origin’ missing ?

It would more correspond to what the Console warning says:
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://odvarko.com/tests/network/cors/example1/data.json. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing).

Something like what but 1493599, putting reason in nsILoadInfo::requestBlockingReason?

That is, we'll bring the requestBlockingReason in PHttpChannel::SendCancel

Does it work for you?

On the other hand, I see open failure is filed on child process
https://searchfox.org/mozilla-central/rev/a40ef31fc9af34a99ceda6d65cdc4573d52b83d2/netwerk/protocol/http/HttpChannelChild.cpp#2395

That could be another solution? But I like the solution in Comment 6. Provide a patch soon

Comment 8 should fix the issue of empty requestBlockingReason
Could you confirm it works, :Honza?

(In reply to Junior from comment #9)

Comment 8 should fix the issue of empty requestBlockingReason
Could you confirm it works, :Honza?

This is great, thanks for working on this!

Works for me, I can see 'CORS Missing Allow Origin' now.
See the attached screenshot.

(btw. my patch mentioned in comment #1 is not needed)

Honza

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=578ecaa1f86310fe37259ab4b659bc79b13ce900&selectedTaskRun=K3KJ3y7aSqK2mS0ewfqwhg-0&searchStr=Windows%2C10%2Cx64%2Cdebug%2CMochitests%2Ctest-windows10-64%2Fdebug-mochitest-devtools-chrome-e10s-2%2CM%28dt2%29

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=303293521&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/75cccddb6e1448409a1f1ee6cdb6b9dc587b244b

[task 2020-05-21T23:56:15.995Z] 23:56:15     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_filter-01.js | The tooltip latency is correct. - 
[task 2020-05-21T23:56:15.995Z] 23:56:15     INFO - Displayed status: null
[task 2020-05-21T23:56:15.996Z] 23:56:15     INFO - Displayed code: 
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - Tooltip status: Blocked
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - Buffered messages finished
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_filter-01.js | The displayed status is correct. - Got null, expected 200
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - Stack trace:
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochikit/content/browser-test.js:test_is:1327
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:verifyRequestItemTarget:673
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_filter-01.js:testContents:533
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_filter-01.js:null:256
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1064
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1104
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:927
[task 2020-05-21T23:56:15.998Z] 23:56:15     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:918
[task 2020-05-21T23:56:16.000Z] 23:56:16     INFO - Not taking screenshot here: see the one that was previously logged

Depends on D76255

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2089580c2bfd4e079f6826f1af2edf8bdb47a4c4

(btw. my patch mentioned in comment #1 is not needed)

I assume devtool listen to some event in OnStartRequest, which should be wrong :)

(In reply to Junior from comment #15)

(btw. my patch mentioned in comment #1 is not needed)

I assume devtool listen to some event in OnStartRequest, which should be wrong :)

All the patch is actually doing is setting the reason to unknown if request.loadInfo.requestBlockingReason; is empty.
I could land it if it feels safer. Perhaps there can still be cases where request.loadInfo.requestBlockingReason; is not set?

Honza

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #16)

(In reply to Junior from comment #15)

(btw. my patch mentioned in comment #1 is not needed)

I assume devtool listen to some event in OnStartRequest, which should be wrong :)

All the patch is actually doing is setting the reason to unknown if request.loadInfo.requestBlockingReason; is empty.
I could land it if it feels safer. Perhaps there can still be cases where request.loadInfo.requestBlockingReason; is not set?

Honza

Only three places set the blockingReason: redirect, asyncOpen, and OnStartReqeust.
We already have the former two, and we handle the third now.
That is, cancelling due to CORS should be fine.

What I worried about is the error might arrive too late, but Comment 10 shows we're good.
Sorry I'm not familiar with devtool code, so I judged the patch by my bad intuition :p

To conclude, let's move the patch forward.

(In reply to Junior from comment #17)

To conclude, let's move the patch forward.

Agreed, it's safer. I filled a follow up bug 1640099.
Patch attached and I asked @bomsy for review.

Thanks for helping to improve the support for CORS in DevTools Junior, great work!

Honza

https://hg.mozilla.org/mozilla-central/rev/266fbe02da32
https://hg.mozilla.org/mozilla-central/rev/1934710853b1