Handle ORB network errors as DevTools issues [chromium/src : main]
Daniel Vogelheim (Gerrit)
Attention is currently required from: Yifan Luo.
Daniel Vogelheim would like Yifan Luo to review this change.
Handle ORB network errors as DevTools issues
ORB "v0.2" changes error handling from silently injecting an
empty response to raising a network error. Since the default
is to log errors on the console this can lead to a very "spam-y"
console. To resolve this, we will always report ORB network
errors (ERR_BLOCKED_BY_ORB) to DevTools as an issue, and will
suppress console messages for the same errors.
Bug: 1492995
Change-Id: Ie8e3139331bc5135f1f3346b33f2c6cb35252475
---
M services/network/cors/cors_url_loader.cc
M third_party/blink/renderer/core/frame/frame_console.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.h
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load-expected.txt
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load.js
6 files changed, 21 insertions(+), 16 deletions(-)
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Vogelheim (Gerrit)
Attention is currently required from: Yifan Luo.
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Yifan Luo (Gerrit)
Attention is currently required from: Daniel Vogelheim.
Patch set 4:Code-Review +1Commit-Queue +2
1 comment:
Patchset:
LGTM
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Vogelheim (Gerrit)
Attention is currently required from: Nate Chapin, Takashi Toyoshima, Yifan Luo.
Daniel Vogelheim would like Nate Chapin and Takashi Toyoshima to review this change.
Handle ORB network errors as DevTools issues
ORB "v0.2" changes error handling from silently injecting an
empty response to raising a network error. Since the default
is to log errors on the console this can lead to a very "spam-y"
console. To resolve this, we will always report ORB network
errors (ERR_BLOCKED_BY_ORB) to DevTools as an issue, and will
suppress console messages for the same errors.
Bug: 1492995
Change-Id: Ie8e3139331bc5135f1f3346b33f2c6cb35252475
---
M services/network/cors/cors_url_loader.cc
M third_party/blink/renderer/core/frame/frame_console.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.h
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load-expected.txt
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load.js
6 files changed, 21 insertions(+), 16 deletions(-)
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Vogelheim (Gerrit)
Attention is currently required from: Nate Chapin, Takashi Toyoshima, Yifan Luo.
1 comment:
Patchset:
Takashi: Owners' review for cors_url_loader.cc and resource_error.*, please.
Nate: Owners' review for frame_console.cc, please.
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Nate Chapin (Gerrit)
Attention is currently required from: Daniel Vogelheim, Takashi Toyoshima, Yifan Luo.
Patch set 4:Code-Review +1
Takashi Toyoshima (Gerrit)
Attention is currently required from: Daniel Vogelheim, Yifan Luo.
3 comments:
Patchset:
lgtm overall, but some minor comments and a question.
File services/network/cors/cors_url_loader.cc:
maybe `and` is more common in verbal comments. Also `&` reminds me boolean algebra, and it makes me to fail to understand the context.
but the way, this is just a question to understand the context, but what happens in the renderer side if v0.1 reports an CORB/ORB-related error with a flag? I meant how the caller in Blink knows the failure?
File third_party/blink/renderer/core/frame/frame_console.cc:
Patch Set #4, Line 134:
issuesCan we say more details here?, i.e "directly from the network service rather than showing a console message here."
Maybe people who read this comment would want to know where we report.
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Vogelheim (Gerrit)
Attention is currently required from: Daniel Vogelheim, Yifan Luo.
Daniel Vogelheim uploaded patch set #5 to this change.
Handle ORB network errors as DevTools issues
ORB "v0.2" changes error handling from silently injecting an
empty response to raising a network error. Since the default
is to log errors on the console this can lead to a very "spam-y"
console. To resolve this, we will always report ORB network
errors (ERR_BLOCKED_BY_ORB) to DevTools as an issue, and will
suppress console messages for the same errors.
Bug: 1492995
Change-Id: Ie8e3139331bc5135f1f3346b33f2c6cb35252475
---
M services/network/cors/cors_url_loader.cc
M third_party/blink/renderer/core/frame/frame_console.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.h
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load-expected.txt
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load.js
6 files changed, 21 insertions(+), 16 deletions(-)
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Vogelheim (Gerrit)
Attention is currently required from: Takashi Toyoshima, Yifan Luo.
2 comments:
File services/network/cors/cors_url_loader.cc:
maybe `and` is more common in verbal comments. Also `&` reminds me boolean algebra, and it makes me to fail to understand the context.
Done. (Sorry about that. English isn't my first language, and sometimes mannerisms from my native language creep into my Enligsh writing.)
but the way, this is just a question to understand the context, but what happens in the renderer side if v0.1 reports an CORB/ORB-related error with a flag? I meant how the caller in Blink knows the failure?
The caller in Blink doesn't know at all. (And doesn't have to.)
The full story is unfortunately a bit longer:
The original design (then still called CORB) is that for CORB-blocked resources, the network stack injects an empty response. So the webapp simply receives a response with an empty body and some headers stripped, as if the server had served up an empty resource. The network service used to pass a "reporting" flag to Blink alongside the response. The "reporting" flag wasn't actually passed along for *all* CORB-blocked responses, but only for responses that a simple heuristic decided were likely to be programmer errors. (https://source.chromium.org/chromium/chromium/src/+/main:services/network/public/cpp/corb/corb_impl.cc;l=986-1005) But the only thing Blink did with that was to pass on the flag to DevTools.
In other words: Blink and the webapp don't have to know; but the website developer should.
Later, other browsers decided to settle on a similar mechanism, ORB, which (from our perspective) is essentially "CORB v2". I'm in the process of migrating Chromium from CORB to ORB. Because of web compat fears, we're doing that step-by-step.
ORB specifies that instead of injecting an empty response, the network should raise a network error. (This was for security reasons, because with a network error you can't distinguish whether a resource is actually there or not. The empty response allows you to probe whether a resource exists and thus leaves a side-channel open.)
As part of the ORB work, I also refactored the network->Blink->DevTools communication, so that the network now directly informs DevTools. Blink is no longer involved in that particular bit. (CorsURLLoader::ReportCorbErrorToDevTools)
So the end-goal is that Blink receives the network error (and nothing else). The network service directly talks to DevTools. The should_report_corb_blocking flag goes away.
This refactoring is presently incomplete, because the Blink API OWNERS asked me to report additional metrics, which require me to correlate ORB-errors with DOM event handlers. (https://groups.google.com/a/chromium.org/g/blink-dev/c/RcuAzHEI2CU/m/Ky16yUsJAQAJ) I'll remove this flag entirely, once ORB v0.2 is fully launched. Then we'll no longer need it.
File third_party/blink/renderer/core/frame/frame_console.cc:
Patch Set #4, Line 134:
issues
Can we say more details here?, i. […]
Done. (I put in a reference to the method, so people can easily look it up.)
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Takashi Toyoshima (Gerrit)
Attention is currently required from: Daniel Vogelheim, Yifan Luo.
Patch set 5:Code-Review +1
2 comments:
Patchset:
thanks!
File services/network/cors/cors_url_loader.cc:
> maybe `and` is more common in verbal comments. […]
Thank you for the detailed background.
That makes my understanding clear!
To view, visit change 4956637. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Vogelheim (Gerrit)
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
Handle ORB network errors as DevTools issues
ORB "v0.2" changes error handling from silently injecting an
empty response to raising a network error. Since the default
is to log errors on the console this can lead to a very "spam-y"
console. To resolve this, we will always report ORB network
errors (ERR_BLOCKED_BY_ORB) to DevTools as an issue, and will
suppress console messages for the same errors.
Bug: 1492995
Change-Id: Ie8e3139331bc5135f1f3346b33f2c6cb35252475
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956637
Reviewed-by: Yifan Luo <l...@chromium.org>
Reviewed-by: Nate Chapin <jap...@chromium.org>
Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>
Commit-Queue: Daniel Vogelheim <voge...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215396}
---
M services/network/cors/cors_url_loader.cc
M third_party/blink/renderer/core/frame/frame_console.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.cc
M third_party/blink/renderer/platform/loader/fetch/resource_error.h
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load-expected.txt
M third_party/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load.js
6 files changed, 21 insertions(+), 16 deletions(-)
Ian Estrada (Gerrit)
Ian Estrada added 1 comment![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |