Commit 7ce08952 authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

Reland "[WebSocket] Add --websocket-renderer-receive-quota-max command line flag."

Reason for reland: the culprit was not this patch.

This patch was reverted because of compile failure on android:
https://chromium-review.googlesource.com/c/chromium/src/+/1660391

However, relanding this don't cause compile failure.

Bug: 865001
Change-Id: Ia64a440525791fe67fb9d8a3882b91bc41af262c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1661316Reviewed-by: default avatarTakuto Ikuta <tikuta@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669936}
parent 09140981
......@@ -52,6 +52,7 @@
#include "content/public/common/service_manager_connection.h"
#include "mojo/public/cpp/system/platform_handle.h"
#include "net/websockets/websocket_basic_stream.h"
#include "net/websockets/websocket_channel.h"
#include "services/service_manager/embedder/switches.h"
#include "services/service_manager/public/cpp/constants.h"
......@@ -313,6 +314,7 @@ void BrowserChildProcessHostImpl::LaunchWithoutExtraCommandLineSwitches(
*base::CommandLine::ForCurrentProcess();
static const char* const kForwardSwitches[] = {
net::kWebSocketReadBufferSize,
net::kWebSocketReceiveQuotaThreshold,
service_manager::switches::kDisableInProcessStackTraces,
switches::kDisableBestEffortTasks,
switches::kDisableLogging,
......
......@@ -217,15 +217,16 @@ void WebRequestProxyingWebSocket::ContinueToHeadersReceived() {
void WebRequestProxyingWebSocket::OnAddChannelResponse(
const std::string& selected_protocol,
const std::string& extensions) {
const std::string& extensions,
uint64_t receive_quota_threshold) {
DCHECK(forwarding_handshake_client_);
DCHECK(!is_done_);
is_done_ = true;
ExtensionWebRequestEventRouter::GetInstance()->OnCompleted(
browser_context_, info_map_, &info_.value(), net::ERR_WS_UPGRADE);
forwarding_handshake_client_->OnAddChannelResponse(selected_protocol,
extensions);
forwarding_handshake_client_->OnAddChannelResponse(
selected_protocol, extensions, receive_quota_threshold);
}
void WebRequestProxyingWebSocket::OnAuthRequired(
......
......@@ -75,7 +75,8 @@ class WebRequestProxyingWebSocket
void OnFinishOpeningHandshake(
network::mojom::WebSocketHandshakeResponsePtr response) override;
void OnAddChannelResponse(const std::string& selected_protocol,
const std::string& extensions) override;
const std::string& extensions,
uint64_t receive_quota_threshold) override;
// mojom::AuthenticationHandler method:
void OnAuthRequired(const net::AuthChallengeInfo& auth_info,
......
......@@ -392,6 +392,12 @@ WebSocketChannel::ChannelState WebSocketChannel::SendFrame(
// |this| may have been deleted.
}
// Overrides default quota resend threshold size for WebSocket. This flag will
// be used to investigate the performance issue of crbug.com/865001 and be
// deleted later on.
const char kWebSocketReceiveQuotaThreshold[] =
"websocket-renderer-receive-quota-max";
ChannelState WebSocketChannel::AddReceiveFlowControlQuota(int64_t quota) {
DCHECK(state_ == CONNECTING || state_ == CONNECTED || state_ == SEND_CLOSED ||
state_ == CLOSE_WAIT);
......
......@@ -153,6 +153,11 @@ class NET_EXPORT WebSocketChannel {
void OnFinishOpeningHandshake(
std::unique_ptr<WebSocketHandshakeResponseInfo> response);
// The renderer calls AddReceiveFlowControlQuota() to the browser per
// recerving this amount of data so that the browser can continue sending
// remaining data to the renderer.
static const uint64_t kReceiveQuotaThreshold = 1 << 15;
private:
class PendingReceivedFrame;
......@@ -417,6 +422,8 @@ class NET_EXPORT WebSocketChannel {
DISALLOW_COPY_AND_ASSIGN(WebSocketChannel);
};
NET_EXPORT extern const char kWebSocketReceiveQuotaThreshold[];
} // namespace net
#endif // NET_WEBSOCKETS_WEBSOCKET_CHANNEL_H_
......@@ -62,7 +62,13 @@ interface WebSocketHandshakeClient {
// Response to an AddChannelRequest. |selected_protocol| is the sub-protocol
// the server selected, or empty if no sub-protocol was selected.
// |extensions| is the list of extensions negotiated for the connection.
OnAddChannelResponse(string selected_protocol, string extensions);
// default threshold value
// |receive_quota_threshold| is the value that the renderer calls
// AddReceiveFlowControlQuota() to the browser per receiving this value
// so that the browser can continue sending remaining data to the renderer.
OnAddChannelResponse(string selected_protocol,
string extensions,
uint64 receive_quota_threshold);
};
interface WebSocketClient {
......
......@@ -15,6 +15,7 @@
#include "base/macros.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/threading/thread_task_runner_handle.h"
......@@ -27,6 +28,7 @@
#include "net/http/http_response_headers.h"
#include "net/http/http_util.h"
#include "net/ssl/ssl_info.h"
#include "net/websockets/websocket_basic_stream.h"
#include "net/websockets/websocket_channel.h"
#include "net/websockets/websocket_errors.h"
#include "net/websockets/websocket_frame.h" // for WebSocketFrameHeader::OpCode
......@@ -147,7 +149,20 @@ void WebSocket::WebSocketEventHandler::OnAddChannelResponse(
impl_->handshake_succeeded_ = true;
impl_->pending_connection_tracker_.OnCompleteHandshake();
impl_->handshake_client_->OnAddChannelResponse(selected_protocol, extensions);
base::CommandLine* const command_line =
base::CommandLine::ForCurrentProcess();
DCHECK(command_line);
uint64_t receive_quota_threshold =
net::WebSocketChannel::kReceiveQuotaThreshold;
if (command_line->HasSwitch(net::kWebSocketReceiveQuotaThreshold)) {
std::string flag_string =
command_line->GetSwitchValueASCII(net::kWebSocketReceiveQuotaThreshold);
if (!base::StringToUint64(flag_string, &receive_quota_threshold))
receive_quota_threshold = net::WebSocketChannel::kReceiveQuotaThreshold;
}
DVLOG(3) << "receive_quota_threshold is " << receive_quota_threshold;
impl_->handshake_client_->OnAddChannelResponse(selected_protocol, extensions,
receive_quota_threshold);
}
void WebSocket::WebSocketEventHandler::OnDataFrame(
......
......@@ -521,8 +521,9 @@ void WebSocketChannelImpl::ProcessSendQueue() {
}
void WebSocketChannelImpl::AddReceiveFlowControlIfNecessary() {
if (!handle_ || received_data_size_for_flow_control_ <
kReceivedDataSizeForFlowControlHighWaterMark) {
DCHECK(receive_quota_threshold_.has_value());
if (!handle_ ||
received_data_size_for_flow_control_ < receive_quota_threshold_.value()) {
return;
}
handle_->AddReceiveFlowControlQuota(received_data_size_for_flow_control_);
......@@ -530,10 +531,10 @@ void WebSocketChannelImpl::AddReceiveFlowControlIfNecessary() {
}
void WebSocketChannelImpl::InitialReceiveFlowControl() {
DCHECK(receive_quota_threshold_.has_value());
DCHECK_EQ(received_data_size_for_flow_control_, 0u);
DCHECK(handle_);
handle_->AddReceiveFlowControlQuota(
kReceivedDataSizeForFlowControlHighWaterMark * 2);
handle_->AddReceiveFlowControlQuota(receive_quota_threshold_.value() * 2);
}
void WebSocketChannelImpl::AbortAsyncOperations() {
......@@ -548,6 +549,7 @@ void WebSocketChannelImpl::HandleDidClose(bool was_clean,
const String& reason) {
handshake_throttle_.reset();
handle_.reset();
receive_quota_threshold_.reset();
AbortAsyncOperations();
if (!client_) {
return;
......@@ -562,15 +564,18 @@ void WebSocketChannelImpl::HandleDidClose(bool was_clean,
void WebSocketChannelImpl::DidConnect(WebSocketHandle* handle,
const String& selected_protocol,
const String& extensions) {
const String& extensions,
uint64_t receive_quota_threshold) {
NETWORK_DVLOG(1) << this << " DidConnect(" << handle << ", "
<< String(selected_protocol) << ", " << String(extensions)
<< ")";
<< ", " << receive_quota_threshold << ")";
DCHECK(handle_);
DCHECK_EQ(handle, handle_.get());
DCHECK(client_);
receive_quota_threshold_ = receive_quota_threshold;
if (!throttle_passed_) {
connect_info_ =
std::make_unique<ConnectInfo>(selected_protocol, extensions);
......
......@@ -156,7 +156,8 @@ class MODULES_EXPORT WebSocketChannelImpl final : public WebSocketChannel,
// WebSocketHandleClient functions.
void DidConnect(WebSocketHandle*,
const String& selected_protocol,
const String& extensions) override;
const String& extensions,
uint64_t receive_quota_threshold) override;
void DidStartOpeningHandshake(
WebSocketHandle*,
network::mojom::blink::WebSocketHandshakeRequestPtr) override;
......@@ -219,7 +220,7 @@ class MODULES_EXPORT WebSocketChannelImpl final : public WebSocketChannel,
scoped_refptr<base::SingleThreadTaskRunner> file_reading_task_runner_;
static const uint64_t kReceivedDataSizeForFlowControlHighWaterMark = 1 << 15;
base::Optional<uint64_t> receive_quota_threshold_;
};
std::ostream& operator<<(std::ostream&, const WebSocketChannelImpl*);
......
......@@ -166,7 +166,8 @@ class WebSocketChannelImplTest : public PageTestBase {
EXPECT_CALL(*ChannelClient(), DidConnect(String("a"), String("b")));
}
EXPECT_TRUE(Channel()->Connect(KURL("ws://localhost/"), "x"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
testing::Mock::VerifyAndClearExpectations(this);
}
......@@ -176,6 +177,8 @@ class WebSocketChannelImplTest : public PageTestBase {
MockWebSocketHandshakeThrottle* handshake_throttle_;
Persistent<WebSocketChannelImpl> channel_;
uint64_t sum_of_consumed_buffered_amount_;
static const uint64_t kDefaultReceiveQuotaThreshold = 1 << 15;
};
MATCHER_P2(MemEq,
......@@ -221,7 +224,8 @@ TEST_F(WebSocketChannelImplTest, connectSuccess) {
EXPECT_EQ("x", protocols[0]);
checkpoint.Call(1);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
}
TEST_F(WebSocketChannelImplTest, sendText) {
......@@ -834,7 +838,8 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest, ThrottleSucceedsFirst) {
checkpoint.Call(1);
ChannelImpl()->OnCompletion(base::nullopt);
checkpoint.Call(2);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
}
TEST_F(WebSocketChannelImplHandshakeThrottleTest, HandshakeSucceedsFirst) {
......@@ -850,7 +855,8 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest, HandshakeSucceedsFirst) {
}
Channel()->Connect(url(), "");
checkpoint.Call(1);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
checkpoint.Call(2);
ChannelImpl()->OnCompletion(base::nullopt);
}
......@@ -887,7 +893,8 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(checkpoint, Call(1));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
Channel()->Fail("close during handshake",
mojom::ConsoleMessageLevel::kWarning,
std::make_unique<SourceLocation>(String(), 0, 0, nullptr));
......@@ -920,7 +927,8 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(checkpoint, Call(1));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
Channel()->Close(WebSocketChannelImpl::kCloseEventCodeGoingAway, "");
checkpoint.Call(1);
}
......@@ -948,7 +956,8 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(checkpoint, Call(1));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
Channel()->Disconnect();
checkpoint.Call(1);
}
......@@ -976,7 +985,8 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(*ChannelClient(), DidClose(_, _, _));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
ChannelImpl()->OnCompletion("Connection blocked by throttle");
}
......
......@@ -42,7 +42,8 @@ class WebSocketHandleClient {
// Called when the handle is opened.
virtual void DidConnect(WebSocketHandle*,
const String& selected_protocol,
const String& extensions) = 0;
const String& extensions,
uint64_t receive_quota_threshold) = 0;
// Called when the browser starts the opening handshake.
// This notification can be omitted when the inspector is not active.
......
......@@ -164,15 +164,17 @@ void WebSocketHandleImpl::OnFinishOpeningHandshake(
client_->DidFinishOpeningHandshake(this, std::move(response));
}
void WebSocketHandleImpl::OnAddChannelResponse(const String& protocol,
const String& extensions) {
void WebSocketHandleImpl::OnAddChannelResponse(
const String& protocol,
const String& extensions,
uint64_t receive_quota_threshold) {
NETWORK_DVLOG(1) << this << " OnAddChannelResponse(" << protocol << ", "
<< extensions << ")";
<< extensions << ", " << receive_quota_threshold << ")";
if (!client_)
return;
client_->DidConnect(this, protocol, extensions);
client_->DidConnect(this, protocol, extensions, receive_quota_threshold);
// |this| can be deleted here.
}
......
......@@ -68,7 +68,8 @@ class WebSocketHandleImpl
void OnFinishOpeningHandshake(
network::mojom::blink::WebSocketHandshakeResponsePtr) override;
void OnAddChannelResponse(const String& selected_protocol,
const String& extensions) override;
const String& extensions,
uint64_t receive_quota_threshold) override;
// network::mojom::blink::WebSocketClient methods:
void OnDataFrame(bool fin,
network::mojom::blink::WebSocketMessageType,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment