Commit fb2cba11 authored by Takuto Ikuta's avatar Takuto Ikuta Committed by Commit Bot

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

This reverts commit 04d43cf5.

Reason for revert: This is culprit of https://ci.chromium.org/p/chromium/builders/try/android-kitkat-arm-rel?limit=200

Original change's description:
> [WebSocket] Add --websocket-renderer-receive-quota-max command line flag.
> 
> This patch adds the flag that overrides default threshold value
> that the renderer calls AddReceiveFlowControlQuota() to the browser per
> receiving this amount of data so that the browser can continue sending
> remaining data to the renderer.
> Default value is 32k Bytes.
> 
> The more the value is, browser tends the less stop sending packets
> and then consumes more memory.
> 
> This flag will be used to investigate the performance issue.
> You can use this flag both on chrome and content_shell:
> $ ./chrome.exe --websocket-renderer-receive-quota-max=3000000
> This flag sets the threshold 3M Bytes.
> 
> Implementation detail:
> This patch moves the default value constant from blink to
> net/websockets/websocket_channel.h so that we can pass the command line
> flag to the renderer.
> This change also enables us to modify the value while considering sent buffer
> from a server later on for further optimization.
> 
> Bug: 865001
> Change-Id: Ib0f9638ece24fbc0523b94345d09a4382e9cf6a1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642850
> Commit-Queue: Yoichi Osato <yoichio@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#669123}

TBR=kinuko@chromium.org,ricea@chromium.org,yhirano@chromium.org,yoichio@chromium.org,karandeepb@chromium.org

Change-Id: Ie05433698213a42e86d27c94d1d995f93ff8c549
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 865001
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660391Reviewed-by: default avatarTakuto Ikuta <tikuta@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669164}
parent b6725092
......@@ -52,7 +52,6 @@
#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"
......@@ -314,7 +313,6 @@ void BrowserChildProcessHostImpl::LaunchWithoutExtraCommandLineSwitches(
*base::CommandLine::ForCurrentProcess();
static const char* const kForwardSwitches[] = {
net::kWebSocketReadBufferSize,
net::kWebSocketReceiveQuotaThreshold,
service_manager::switches::kDisableInProcessStackTraces,
switches::kDisableBestEffortTasks,
switches::kDisableLogging,
......
......@@ -217,16 +217,15 @@ void WebRequestProxyingWebSocket::ContinueToHeadersReceived() {
void WebRequestProxyingWebSocket::OnAddChannelResponse(
const std::string& selected_protocol,
const std::string& extensions,
uint64_t receive_quota_threshold) {
const std::string& extensions) {
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, receive_quota_threshold);
forwarding_handshake_client_->OnAddChannelResponse(selected_protocol,
extensions);
}
void WebRequestProxyingWebSocket::OnAuthRequired(
......
......@@ -75,8 +75,7 @@ class WebRequestProxyingWebSocket
void OnFinishOpeningHandshake(
network::mojom::WebSocketHandshakeResponsePtr response) override;
void OnAddChannelResponse(const std::string& selected_protocol,
const std::string& extensions,
uint64_t receive_quota_threshold) override;
const std::string& extensions) override;
// mojom::AuthenticationHandler method:
void OnAuthRequired(const net::AuthChallengeInfo& auth_info,
......
......@@ -392,12 +392,6 @@ 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,11 +153,6 @@ 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;
......@@ -422,8 +417,6 @@ class NET_EXPORT WebSocketChannel {
DISALLOW_COPY_AND_ASSIGN(WebSocketChannel);
};
NET_EXPORT extern const char kWebSocketReceiveQuotaThreshold[];
} // namespace net
#endif // NET_WEBSOCKETS_WEBSOCKET_CHANNEL_H_
......@@ -62,13 +62,7 @@ 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.
// 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);
OnAddChannelResponse(string selected_protocol, string extensions);
};
interface WebSocketClient {
......
......@@ -15,7 +15,6 @@
#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"
......@@ -28,7 +27,6 @@
#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
......@@ -149,20 +147,7 @@ void WebSocket::WebSocketEventHandler::OnAddChannelResponse(
impl_->handshake_succeeded_ = true;
impl_->pending_connection_tracker_.OnCompleteHandshake();
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);
impl_->handshake_client_->OnAddChannelResponse(selected_protocol, extensions);
}
void WebSocket::WebSocketEventHandler::OnDataFrame(
......
......@@ -521,9 +521,8 @@ void WebSocketChannelImpl::ProcessSendQueue() {
}
void WebSocketChannelImpl::AddReceiveFlowControlIfNecessary() {
DCHECK(receive_quota_threshold_.has_value());
if (!handle_ ||
received_data_size_for_flow_control_ < receive_quota_threshold_.value()) {
if (!handle_ || received_data_size_for_flow_control_ <
kReceivedDataSizeForFlowControlHighWaterMark) {
return;
}
handle_->AddReceiveFlowControlQuota(received_data_size_for_flow_control_);
......@@ -531,10 +530,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(receive_quota_threshold_.value() * 2);
handle_->AddReceiveFlowControlQuota(
kReceivedDataSizeForFlowControlHighWaterMark * 2);
}
void WebSocketChannelImpl::AbortAsyncOperations() {
......@@ -549,7 +548,6 @@ void WebSocketChannelImpl::HandleDidClose(bool was_clean,
const String& reason) {
handshake_throttle_.reset();
handle_.reset();
receive_quota_threshold_.reset();
AbortAsyncOperations();
if (!client_) {
return;
......@@ -564,18 +562,15 @@ void WebSocketChannelImpl::HandleDidClose(bool was_clean,
void WebSocketChannelImpl::DidConnect(WebSocketHandle* handle,
const String& selected_protocol,
const String& extensions,
uint64_t receive_quota_threshold) {
const String& extensions) {
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,8 +156,7 @@ class MODULES_EXPORT WebSocketChannelImpl final : public WebSocketChannel,
// WebSocketHandleClient functions.
void DidConnect(WebSocketHandle*,
const String& selected_protocol,
const String& extensions,
uint64_t receive_quota_threshold) override;
const String& extensions) override;
void DidStartOpeningHandshake(
WebSocketHandle*,
network::mojom::blink::WebSocketHandshakeRequestPtr) override;
......@@ -220,7 +219,7 @@ class MODULES_EXPORT WebSocketChannelImpl final : public WebSocketChannel,
scoped_refptr<base::SingleThreadTaskRunner> file_reading_task_runner_;
base::Optional<uint64_t> receive_quota_threshold_;
static const uint64_t kReceivedDataSizeForFlowControlHighWaterMark = 1 << 15;
};
std::ostream& operator<<(std::ostream&, const WebSocketChannelImpl*);
......
......@@ -166,8 +166,7 @@ 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"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
testing::Mock::VerifyAndClearExpectations(this);
}
......@@ -177,8 +176,6 @@ 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,
......@@ -224,8 +221,7 @@ TEST_F(WebSocketChannelImplTest, connectSuccess) {
EXPECT_EQ("x", protocols[0]);
checkpoint.Call(1);
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
}
TEST_F(WebSocketChannelImplTest, sendText) {
......@@ -838,8 +834,7 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest, ThrottleSucceedsFirst) {
checkpoint.Call(1);
ChannelImpl()->OnCompletion(base::nullopt);
checkpoint.Call(2);
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
}
TEST_F(WebSocketChannelImplHandshakeThrottleTest, HandshakeSucceedsFirst) {
......@@ -855,8 +850,7 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest, HandshakeSucceedsFirst) {
}
Channel()->Connect(url(), "");
checkpoint.Call(1);
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
checkpoint.Call(2);
ChannelImpl()->OnCompletion(base::nullopt);
}
......@@ -893,8 +887,7 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(checkpoint, Call(1));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
Channel()->Fail("close during handshake",
mojom::ConsoleMessageLevel::kWarning,
std::make_unique<SourceLocation>(String(), 0, 0, nullptr));
......@@ -927,8 +920,7 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(checkpoint, Call(1));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
Channel()->Close(WebSocketChannelImpl::kCloseEventCodeGoingAway, "");
checkpoint.Call(1);
}
......@@ -956,8 +948,7 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(checkpoint, Call(1));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
Channel()->Disconnect();
checkpoint.Call(1);
}
......@@ -985,8 +976,7 @@ TEST_F(WebSocketChannelImplHandshakeThrottleTest,
EXPECT_CALL(*ChannelClient(), DidClose(_, _, _));
}
Channel()->Connect(url(), "");
HandleClient()->DidConnect(Handle(), String("a"), String("b"),
kDefaultReceiveQuotaThreshold);
HandleClient()->DidConnect(Handle(), String("a"), String("b"));
ChannelImpl()->OnCompletion("Connection blocked by throttle");
}
......
......@@ -42,8 +42,7 @@ class WebSocketHandleClient {
// Called when the handle is opened.
virtual void DidConnect(WebSocketHandle*,
const String& selected_protocol,
const String& extensions,
uint64_t receive_quota_threshold) = 0;
const String& extensions) = 0;
// Called when the browser starts the opening handshake.
// This notification can be omitted when the inspector is not active.
......
......@@ -164,17 +164,15 @@ void WebSocketHandleImpl::OnFinishOpeningHandshake(
client_->DidFinishOpeningHandshake(this, std::move(response));
}
void WebSocketHandleImpl::OnAddChannelResponse(
const String& protocol,
const String& extensions,
uint64_t receive_quota_threshold) {
void WebSocketHandleImpl::OnAddChannelResponse(const String& protocol,
const String& extensions) {
NETWORK_DVLOG(1) << this << " OnAddChannelResponse(" << protocol << ", "
<< extensions << ", " << receive_quota_threshold << ")";
<< extensions << ")";
if (!client_)
return;
client_->DidConnect(this, protocol, extensions, receive_quota_threshold);
client_->DidConnect(this, protocol, extensions);
// |this| can be deleted here.
}
......
......@@ -68,8 +68,7 @@ class WebSocketHandleImpl
void OnFinishOpeningHandshake(
network::mojom::blink::WebSocketHandshakeResponsePtr) override;
void OnAddChannelResponse(const String& selected_protocol,
const String& extensions,
uint64_t receive_quota_threshold) override;
const String& extensions) 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