Skip to content

Commit

Permalink
[WebSocket] Remove Quota System from data transfer
Browse files Browse the repository at this point in the history
This commit removes the quota system from renderer to network service
data transfer, and moves completely to mojo data pipe.

This is a follow-up CL from the following CLs:
https://chromium-review.googlesource.com/c/chromium/src/+/2071189
https://chromium-review.googlesource.com/c/chromium/src/+/2206556

Design Doc:
https://docs.google.com/document/d/1YWj1z9r8wxemGdod6S2tkchudhp6PvNaH3qSO0oucfY/

This is a rebase of
https://chromium-review.googlesource.com/c/chromium/src/+/2120193,
originally by Keita Suzuki.

Bug: 1056030
Change-Id: Ibc84448e56cad0cc2d8d4ed4a2ef4411b28be104
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210333
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Auto-Submit: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771824}
  • Loading branch information
ricea authored and Commit Bot committed May 26, 2020
1 parent 8d03a5d commit 250bb01
Show file tree
Hide file tree
Showing 12 changed files with 56 additions and 192 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/net/websocket_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,6 @@ class ExpectInvalidUtf8Client : public network::mojom::WebSocketClient {
NOTREACHED();
}

void AddSendFlowControlQuota(int64_t quota) override {}

void OnDropChannel(bool was_clean,
uint16_t code,
const std::string& reason) override {
Expand Down
17 changes: 3 additions & 14 deletions net/websockets/websocket_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ namespace {

using base::StreamingUtf8Validator;

const int kDefaultSendQuotaHighWaterMark = 1 << 17;
const size_t kWebSocketCloseCodeLength = 2;
// Timeout for waiting for the server to acknowledge a closing handshake.
const int kClosingHandshakeTimeoutSeconds = 60;
Expand Down Expand Up @@ -285,8 +284,6 @@ WebSocketChannel::WebSocketChannel(
URLRequestContext* url_request_context)
: event_interface_(std::move(event_interface)),
url_request_context_(url_request_context),
send_quota_high_water_mark_(kDefaultSendQuotaHighWaterMark),
current_send_quota_(0),
closing_handshake_timeout_(
base::TimeDelta::FromSeconds(kClosingHandshakeTimeoutSeconds)),
underlying_connection_close_timeout_(base::TimeDelta::FromSeconds(
Expand Down Expand Up @@ -373,10 +370,7 @@ WebSocketChannel::ChannelState WebSocketChannel::SendFrame(
sending_text_message_ = !fin;
DCHECK(!fin || state == StreamingUtf8Validator::VALID_ENDPOINT);
}
// TODO(ricea): If current_send_quota_ has dropped below
// send_quota_low_water_mark_, it might be good to increase the "low
// water mark" and "high water mark", but only if the link to the WebSocket
// server is not saturated.

return SendFrameInternal(fin, op_code, std::move(buffer), buffer_size);
// |this| may have been deleted.
}
Expand Down Expand Up @@ -510,12 +504,8 @@ void WebSocketChannel::OnConnectSuccess(
// |stream_request_| is not used once the connection has succeeded.
stream_request_.reset();

// TODO(ricea): Get flow control information from the WebSocketStream once we
// have a multiplexing WebSocketStream.
current_send_quota_ = send_quota_high_water_mark_;
event_interface_->OnAddChannelResponse(
std::move(response), stream_->GetSubProtocol(), stream_->GetExtensions(),
send_quota_high_water_mark_);
std::move(response), stream_->GetSubProtocol(), stream_->GetExtensions());
// |this| may have been deleted after OnAddChannelResponse.
}

Expand Down Expand Up @@ -938,8 +928,6 @@ ChannelState WebSocketChannel::SendFrameInternal(
if (data_being_sent_) {
// Either the link to the WebSocket server is saturated, or several messages
// are being sent in a batch.
// TODO(ricea): Keep some statistics to work out the situation and adjust
// quota appropriately.
if (!data_to_send_next_)
data_to_send_next_ = std::make_unique<SendBuffer>();
data_to_send_next_->AddFrame(std::move(frame), std::move(buffer));
Expand Down Expand Up @@ -993,6 +981,7 @@ ChannelState WebSocketChannel::SendClose(uint16_t code,
std::copy(
reason.begin(), reason.end(), body->data() + kWebSocketCloseCodeLength);
}

return SendFrameInternal(true, WebSocketFrameHeader::kOpCodeClose,
std::move(body), size);
}
Expand Down
13 changes: 0 additions & 13 deletions net/websockets/websocket_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,6 @@ class NET_EXPORT WebSocketChannel {
ChannelState StartClosingHandshake(uint16_t code, const std::string& reason)
WARN_UNUSED_RESULT;

// Returns the current send quota. This value is unsafe to use outside of the
// browser IO thread because it changes asynchronously. The value is only
// valid for the execution of the current Task or until SendFrame() is called,
// whichever happens sooner.
int current_send_quota() const { return current_send_quota_; }

// Starts the connection process, using a specified creator callback rather
// than the default. This is exposed for testing.
void SendAddChannelRequestForTesting(
Expand Down Expand Up @@ -359,13 +353,6 @@ class NET_EXPORT WebSocketChannel {
// during the connection process.
std::unique_ptr<WebSocketStreamRequest> stream_request_;

// The level the quota is refreshed to when it reaches the low_water_mark
// (quota units).
int send_quota_high_water_mark_;
// The current amount of quota that the renderer has available for sending
// on this logical channel (quota units).
int current_send_quota_;

// Timer for the closing handshake.
base::OneShotTimer close_timer_;

Expand Down
Loading

0 comments on commit 250bb01

Please sign in to comment.