When the network service is in use (--enable-network-service, see also parent bug), WebSockets requests should be made through the service.
We had an implementation of this in the old mandoline network service, see https://codereview.chromium.org/1873463003/ for inspiration.
IIUC what websockets does is a subset of what other parts in net do. So I would like to wait the other parts to find their solutions, and then use them for websockets. Does it make sense?
Implementing this basically just means moving //content/browser/websockets into //net.
A possibly incomplete list of non-net hooks we need:
* SSL handshake failure handling
* Extensions
* Safe browsing
As yhirano said, once solutions are in place for doing this stuff for HTTP, adapting it for WebSockets should be trivial.
@ricea I'm not sure what you mean by moving content/browser/websockets into /net? afaik that code translates IPC from the renderer into net/ calls where the websocket code lives. We still need a layer that handles IPC with mojo service.
We already have a network service that can handle http/https. Perhaps a VC to talk through this would clarify things?
OK, dougt@. My understanding is that this work is not yet blocking any urgent s13n work. So, we're giving less priority to it. That's why there has been no activity. If it's wrong, let us know. ricea@ would work on it.
A bit of context I heard from side-channel: this is not blocking other work, while //content tests with s13n Network Service are starting to pass about 99%, that means that this might start to block the finishing date of the entire work at some point.
I am investigating the failure of http/tests/devtools/websocket/websocket-handshake.js. My findings so far:
The test fails because brotli is not enabled (so the Accept-Encoding header doesn't match).
With the network service, brotli is enabled in the NetworkContextParams.
It's not currently clear to me why this isn't being picked up by the WebSocket connections.
I had it backwards. content_shell normally doesn't have brotli enabled. The enable-brotli feature is set in chrome/browser/net/default_network_context_params.cc. The fact that enabling the network service also enables brotli appears to be unintended.
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)
For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge for CL: https://crrev.com/86c6f785ad1f00343ce9a0d4c46973d4d510a623 to M67, please answer followings:
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M67?
* Any other important details to justify the merge.
Please note M67 is already in Beta, so merge bar is very high. Thank you.
@govind: this is a very low risk change that'll give us data we need to figure out if we can change some extensions API. We'd really prefer not to wait an extra 6 weeks to get this data.
Comment 1 by jam@chromium.org
, May 11 2017