Remove unneeded WebSocket flags

Remove command-line flags --websocket-read-buffer-size and
--websocket-renderer-receive-quota-max which are no longer needed as the
performance investigation has finished.

Also replace the kReceiveQuotaThreshold constant in websocket_channel.cc
with a kReceiveDataPipeCapacity constant in websocket.cc, which better
reflects its current usage.

Change the types of kReadBufferSize and kReceiveDataPipeCapacity to
reflect the types at the points where they are used.

Also mark WebSocketBasicStream as final, as its destructor calls the
Close() virtual method and it would not behave correctly in a subclass.

Also change some constants from "const" to "constexpr".

BUG=865001

Change-Id: I761bd08de534ef27a77a9523c86ea6f1a61b9266
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383396Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803308}

content/browser/browser_child_process_host_impl.cc

@@ -50,8 +50,6 @@
 #include "content/public/common/sandboxed_process_launcher_delegate.h"
 #include "mojo/public/cpp/bindings/scoped_message_error_crash_key.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/tracing/public/cpp/trace_startup.h"
 
@@ -328,8 +326,6 @@ void BrowserChildProcessHostImpl::LaunchWithoutExtraCommandLineSwitches(
   const base::CommandLine& browser_command_line =
       *base::CommandLine::ForCurrentProcess();
   static const char* const kForwardSwitches[] = {
-      net::kWebSocketReadBufferSize,
-      net::kWebSocketReceiveQuotaThreshold,
       service_manager::switches::kDisableInProcessStackTraces,
       switches::kDisableBestEffortTasks,
       switches::kDisableLogging,

net/websockets/websocket_basic_stream.cc

@@ -11,10 +11,8 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
-#include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ -58,7 +56,7 @@ constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
 
 // This uses type uint64_t to match the definition of
 // WebSocketFrameHeader::payload_length in websocket_frame.h.
-const uint64_t kMaxControlFramePayload = 125;
+constexpr uint64_t kMaxControlFramePayload = 125;
 
 // The number of bytes to attempt to read at a time.
 // TODO(ricea): See if there is a better number or algorithm to fulfill our
@@ -71,10 +69,10 @@ const uint64_t kMaxControlFramePayload = 125;
 //  4. We would like to hit any sweet-spots that might exist in terms of network
 //     packet sizes / encryption block sizes / IPC alignment issues, etc.
 #if defined(OS_ANDROID)
-const int kReadBufferSize = 32 * 1024;
+constexpr size_t kReadBufferSize = 32 * 1024;
 #else
 // |2^n - delta| is better than 2^n on Linux. See crrev.com/c/1792208.
-const int kReadBufferSize = 131000;
+constexpr size_t kReadBufferSize = 131000;
 #endif
 
 // Returns the total serialized size of |frames|. This function assumes that
@@ -101,17 +99,13 @@ int CalculateSerializedSizeAndTurnOnMaskBit(
 
 }  // namespace
 
-// Overrides default read buffer size for WebSocket. This flag will be used to
-// investigate the performance issue of crbug.com/865001 and be deleted later
-// on.
-const char kWebSocketReadBufferSize[] = "websocket-read-buffer-size";
-
 WebSocketBasicStream::WebSocketBasicStream(
     std::unique_ptr<Adapter> connection,
     const scoped_refptr<GrowableIOBuffer>& http_read_buffer,
     const std::string& sub_protocol,
     const std::string& extensions)
-    : connection_(std::move(connection)),
+    : read_buffer_(base::MakeRefCounted<IOBufferWithSize>(kReadBufferSize)),
+      connection_(std::move(connection)),
       http_read_buffer_(http_read_buffer),
       sub_protocol_(sub_protocol),
       extensions_(extensions),
@@ -120,21 +114,6 @@ WebSocketBasicStream::WebSocketBasicStream(
   if (http_read_buffer_.get() && http_read_buffer_->offset() == 0)
     http_read_buffer_ = nullptr;
   DCHECK(connection_->is_initialized());
-  base::CommandLine* const command_line =
-      base::CommandLine::ForCurrentProcess();
-  DCHECK(command_line);
-  int websocket_buffer_size = kReadBufferSize;
-  if (command_line->HasSwitch(kWebSocketReadBufferSize)) {
-    std::string size_string =
-        command_line->GetSwitchValueASCII(kWebSocketReadBufferSize);
-    if (!base::StringToInt(size_string, &websocket_buffer_size) ||
-        websocket_buffer_size <= 0) {
-      websocket_buffer_size = kReadBufferSize;
-    }
-  }
-  DVLOG(1) << "WebSocketReadBufferSize is " << websocket_buffer_size;
-  read_buffer_ =
-      (base::MakeRefCounted<IOBufferWithSize>(websocket_buffer_size));
 }
 
 WebSocketBasicStream::~WebSocketBasicStream() { Close(); }

net/websockets/websocket_basic_stream.h

@@ -33,7 +33,7 @@ struct WebSocketFrameChunk;
 // websocket_stream.cc if the class is used for any communication with Google.
 // In such a case, annotation should be passed from the callers to this class
 // and a local annotation can not be used anymore.
-class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream {
+class NET_EXPORT_PRIVATE WebSocketBasicStream final : public WebSocketStream {
  public:
   typedef WebSocketMaskingKey (*WebSocketMaskingKeyGeneratorFunction)();
 
@@ -147,7 +147,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream {
   void AddToIncompleteControlFrameBody(base::span<const char> data);
 
   // Storage for pending reads.
-  scoped_refptr<IOBufferWithSize> read_buffer_;
+  const scoped_refptr<IOBufferWithSize> read_buffer_;
 
   // The connection, wrapped in a ClientSocketHandle so that we can prevent it
   // from being returned to the pool.
@@ -196,8 +196,6 @@ class NET_EXPORT_PRIVATE WebSocketBasicStream : public WebSocketStream {
   CompletionOnceCallback read_callback_;
 };
 
-NET_EXPORT extern const char kWebSocketReadBufferSize[];
-
 }  // namespace net
 
 #endif  // NET_WEBSOCKETS_WEBSOCKET_BASIC_STREAM_H_

net/websockets/websocket_channel.cc

@@ -45,9 +45,9 @@ namespace {
 
 using base::StreamingUtf8Validator;
 
-const size_t kWebSocketCloseCodeLength = 2;
+constexpr size_t kWebSocketCloseCodeLength = 2;
 // Timeout for waiting for the server to acknowledge a closing handshake.
-const int kClosingHandshakeTimeoutSeconds = 60;
+constexpr int kClosingHandshakeTimeoutSeconds = 60;
 // We wait for the server to close the underlying connection as recommended in
 // https://tools.ietf.org/html/rfc6455#section-7.1.1
 // We don't use 2MSL since there're server implementations that don't follow
@@ -55,14 +55,14 @@ const int kClosingHandshakeTimeoutSeconds = 60;
 // connection. It leads to unnecessarily long time before CloseEvent
 // invocation. We want to avoid this rather than strictly following the spec
 // recommendation.
-const int kUnderlyingConnectionCloseTimeoutSeconds = 2;
+constexpr int kUnderlyingConnectionCloseTimeoutSeconds = 2;
 
 using ChannelState = WebSocketChannel::ChannelState;
 
 // Maximum close reason length = max control frame payload -
 //                               status code length
 //                             = 125 - 2
-const size_t kMaximumCloseReasonLength = 125 - kWebSocketCloseCodeLength;
+constexpr size_t kMaximumCloseReasonLength = 125 - kWebSocketCloseCodeLength;
 
 // Check a close status code for strict compliance with RFC6455. This is only
 // used for close codes received from a renderer that we are intending to send
@@ -377,12 +377,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::StartClosingHandshake(
     uint16_t code,
     const std::string& reason) {

net/websockets/websocket_channel.h

@@ -19,7 +19,6 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
-#include "build/build_config.h"
 #include "net/base/net_export.h"
 #include "net/websockets/websocket_event_interface.h"
 #include "net/websockets/websocket_frame.h"
@@ -148,16 +147,6 @@ class NET_EXPORT WebSocketChannel {
   void OnStartOpeningHandshake(
       std::unique_ptr<WebSocketHandshakeRequestInfo> request);
 
-  // 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.
-#if defined(OS_ANDROID)
-  static const uint64_t kReceiveQuotaThreshold = 1 << 15;
-#else
-  // |2^n - delta| is better than 2^n on Linux. See crrev.com/c/1792208.
-  static const uint64_t kReceiveQuotaThreshold = 65500;
-#endif
-
  private:
   // The object passes through a linear progression of states from
   // FRESHLY_CONSTRUCTED to CLOSED, except that the SEND_CLOSED and RECV_CLOSED
@@ -400,8 +389,6 @@ class NET_EXPORT WebSocketChannel {
   DISALLOW_COPY_AND_ASSIGN(WebSocketChannel);
 };
 
-NET_EXPORT extern const char kWebSocketReceiveQuotaThreshold[];
-
 }  // namespace net
 
 #endif  // NET_WEBSOCKETS_WEBSOCKET_CHANNEL_H_

services/network/websocket.cc

@@ -11,7 +11,6 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/location.h"
 #include "base/logging.h"
@@ -19,10 +18,10 @@
 #include "base/numerics/safe_conversions.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"
+#include "build/build_config.h"
 #include "net/base/auth.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
@@ -49,6 +48,15 @@ namespace {
 // reassembly.
 constexpr uint64_t kSmallMessageThreshhold = 1 << 16;
 
+// The capacity of the data pipe to use for received messages, in bytes. Optimal
+// value depends on the platform.
+#if defined(OS_ANDROID)
+constexpr uint32_t kReceiveDataPipeCapacity = 1 << 16;
+#else
+// |2^n - delta| is better than 2^n on Linux. See crrev.com/c/1792208.
+constexpr uint32_t kReceiveDataPipeCapacity = 131000;
+#endif
+
 // Convert a mojom::WebSocketMessageType to a
 // net::WebSocketFrameHeader::OpCode
 net::WebSocketFrameHeader::OpCode MessageTypeToOpCode(
@@ -190,22 +198,9 @@ void WebSocket::WebSocketEventHandler::OnAddChannelResponse(
     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;
-
   const MojoCreateDataPipeOptions data_pipe_options{
       sizeof(MojoCreateDataPipeOptions), MOJO_CREATE_DATA_PIPE_FLAG_NONE, 1,
-      receive_quota_threshold * 2};
+      kReceiveDataPipeCapacity};
   mojo::ScopedDataPipeConsumerHandle readable;
   const MojoResult result =
       mojo::CreateDataPipe(&data_pipe_options, &impl_->writable_, &readable);