From e13b23ce2ad9ce8eaf6b694235bd0f6e5f1a48d2 Mon Sep 17 00:00:00 2001 From: mkarolin Date: Mon, 23 Sep 2019 09:18:22 -0400 Subject: [PATCH] WebSocketHandshakeClient::OnConnectionEstablished signature change. Chromium changes: https://chromium.googlesource.com/chromium/src/+/26e73cfd90affbf1bdf3efbe80bf2903b0e866ae commit 26e73cfd90affbf1bdf3efbe80bf2903b0e866ae Author: Yutaka Hirano Date: Tue Aug 6 02:38:13 2019 +0000 Stop giving WebSocketClient to WebSocket creation function Instead, give it as an argument of mojom.WebSocketHandshakeClient.OnConnectionEstablished. This is a follow up CL for https://chromium-review.googlesource.com/c/chromium/src/+/1728917. Passing the mojo::InterfaceRequest at OnConnectionEstablished is less error prone because 1) It is unable to bind the implementation before that, and 2) It is now clear that we should detect connection errors on |handshake_client| until the connection is established, and |client| after that. Bug: 989406, 967524 https://chromium.googlesource.com/chromium/src/+/1007403a797957b8af46e962cdfc1f6f671954a4 commit 1007403a797957b8af46e962cdfc1f6f671954a4 Author: Yoichi Osato Date: Thu Aug 8 13:11:57 2019 +0000 [WebSocket] Use Datapipe to transfer DataFrame buffer instead of ReadOnlyBuffer This CL introduces mojo DataPipe instead of ReadOnlyBuffer. With DataPipe, we can reduce memory copy and optimize quota control. Unfortunately, this CL makes performance slower: ToT: 147 MB/s Patch: 128 MB/s That's because this CL replaces minimize part and we will do the optimization later on. If the optimization doesn't go over the original score before M78 (will be branch cut on Sep 5th), we will revert this change. Bug: 865001 https://chromium.googlesource.com/chromium/src/+/fcaa2a2c441e28a5cb95cc90946712efb0ce085f commit fcaa2a2c441e28a5cb95cc90946712efb0ce085f Author: Yoichi Osato Date: Wed Aug 28 08:22:36 2019 +0000 [WebSocket] Remove manual quota control for receiving. Because we're using mojo data pipe, which has own quota control, to transfer received data, we don't have manual one, or mojom::WebSocket.AddReceiveFlowControlQuota(). This CL does a sort of work removing the function. mojom::WebSocket.AddReceiveFlowControlQuota(quota) did 5 tasks: browser side(mainly on WebSocketChannel) 1. Trigger the first ReadFrame if renderer gets not throttled. For #1, this patch moves the trigger to mojo WebSocket.StartReceiving. 2. Trigger next ReadFrame if there is enough quota. 3. Send pending dataframes based on added quota. This patch moves task #2 and #3 to websocket.cc and datapipe itself. 4. Dropchannel if all pending frames are received by renderer when closed. Because we can care #4 w/o renderer ping back, this patch move RespondToClosingHandshake()to WebSocketChannel::ReadFrames(). renderer side(mainly on WebSocketChannelImpl) 5. Ping browser that backpressure is turned off. For task #5, this patch changes not to call the mojo but throttle datapipe reading with WebSocketHandleImpl::ConsumePendingDataFrames(). receive-arraybuffer-1MBx100.htmll?iteration=100 measurement on local build: ToT: 144 MB/s (stdev: 4.56 MB/s) Patch: 208 MB/s (stdev: 6.15 MB/s) (+44% to ToT, +41% to ReadOnlyBuffer) ReadOnlyBuffer: 147 MB/s https://chromium.googlesource.com/chromium/src/+/a461132e64f53f45997aaa4222e05cdf5d10ea56 commit a461132e64f53f45997aaa4222e05cdf5d10ea56 Author: Julie Jeongeun Kim Date: Fri Aug 30 04:13:36 2019 +0000 Reland "Convert WebSocket to new Mojo types" This is a reland of 06a7200b370f38f39c97a4810d0931ab5596014e It updates WebSocketChannelImplTest::EstablishConnection with new Mojo types without InterfacePtr and MakeRequest. TBR=jam@chromium.org Original change's description: > Convert WebSocket to new Mojo types > > This CL converts WebSocketPtr, to new Mojo types and > updates OnConnectionEstablished from websocket.mojom. > > Bug: 955171, 978694 > Change-Id: I82416b209e2380241b64ebd3d829dd8bde5247c7 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1773016 > Commit-Queue: Julie Kim > Reviewed-by: John Abd-El-Malek > Reviewed-by: Sam McNally > Reviewed-by: Yutaka Hirano > Reviewed-by: Ken Rockot > Cr-Commit-Position: refs/heads/master@{#691574} Bug: 955171, 978694 --- browser/net/brave_proxying_web_socket.cc | 9 +++++---- browser/net/brave_proxying_web_socket.h | 12 +++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/browser/net/brave_proxying_web_socket.cc b/browser/net/brave_proxying_web_socket.cc index 26f02750aaf5..b793d34630b0 100644 --- a/browser/net/brave_proxying_web_socket.cc +++ b/browser/net/brave_proxying_web_socket.cc @@ -204,15 +204,16 @@ void BraveProxyingWebSocket::ContinueToHeadersReceived() { } void BraveProxyingWebSocket::OnConnectionEstablished( - network::mojom::WebSocketPtr websocket, + mojo::PendingRemote websocket, + mojo::PendingReceiver client_receiver, const std::string& selected_protocol, const std::string& extensions, - uint64_t receive_quota_threshold) { + mojo::ScopedDataPipeConsumerHandle readable) { DCHECK(forwarding_handshake_client_); DCHECK(!is_done_); forwarding_handshake_client_->OnConnectionEstablished( - std::move(websocket), selected_protocol, extensions, - receive_quota_threshold); + std::move(websocket), std::move(client_receiver), selected_protocol, + extensions, std::move(readable)); OnError(net::ERR_FAILED); } diff --git a/browser/net/brave_proxying_web_socket.h b/browser/net/brave_proxying_web_socket.h index d81125a474a5..a0f8bff0d93c 100644 --- a/browser/net/brave_proxying_web_socket.h +++ b/browser/net/brave_proxying_web_socket.h @@ -18,8 +18,8 @@ #include "brave/browser/net/resource_context_data.h" #include "brave/browser/net/url_context.h" #include "content/public/browser/content_browser_client.h" -#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/binding_set.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" #include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/resource_response.h" #include "services/network/public/mojom/network_context.mojom.h" @@ -73,10 +73,12 @@ class BraveProxyingWebSocket : public network::mojom::WebSocketHandshakeClient, network::mojom::WebSocketHandshakeRequestPtr request) override; void OnResponseReceived( network::mojom::WebSocketHandshakeResponsePtr response) override; - void OnConnectionEstablished(network::mojom::WebSocketPtr websocket, - const std::string& selected_protocol, - const std::string& extensions, - uint64_t receive_quota_threshold) override; + void OnConnectionEstablished( + mojo::PendingRemote websocket, + mojo::PendingReceiver client_receiver, + const std::string& selected_protocol, + const std::string& extensions, + mojo::ScopedDataPipeConsumerHandle readable) override; // network::mojom::AuthenticationHandler method: void OnAuthRequired(const net::AuthChallengeInfo& auth_info,