Skip to content

Commit

Permalink
Remove many naked new statements from net/websockets.
Browse files Browse the repository at this point in the history
This change is fairly mechanical and purely cosmetic.

Use make_unique instead of creating an object with new and passing that
into a unique_ptr constructor.  (This often changes local variable type,
but when passed as an argument an implicitly cast takes care of this).
Use MakeRefCounted instead of creating an object with new and passing
that into a scoped_refptr constructor.  (This never changes variable
type in this CL).  Do these in constructor initializer lists as well.
Remove unnecessary namespace qualifiers in a few places (where
unique_ptr template parameter has it but new statement does not in the
same line).

Use assignment operator instead of reset() method of unique_ptr.

Use std::move instead of two-step WrapUnique(unique_ptr.release()).

Where object needs to be constructed with new statement and raw pointer
is already available, use WrapUnique instead of passing the raw pointer
to a unique_ptr constructor.

Use auto where appropriate.

Replace NULL by nullptr in one line which is touched anyway.

Make sure to include memory for new, unique_ptr.

Include scoped_refptr.h for scoped_refptr and MakeRefCounted.

Delete unnecessary ref_counted.h include (only needed for RefCounted*
base classes).

Delete unnecessary memory include where corresponding header file
already included it.

Change-Id: I6fa58780e5ae89e2684aabdd21452b76be52a4f8
Reviewed-on: https://chromium-review.googlesource.com/946053
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540826}
  • Loading branch information
Bence Béky authored and Commit Bot committed Mar 5, 2018
1 parent ebe5bdf commit 6562397
Show file tree
Hide file tree
Showing 32 changed files with 178 additions and 255 deletions.
11 changes: 5 additions & 6 deletions net/websockets/websocket_basic_handshake_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ int WebSocketBasicHandshakeStream::SendRequest(
ComputeSecWebSocketAccept(handshake_challenge);

DCHECK(connect_delegate_);
std::unique_ptr<WebSocketHandshakeRequestInfo> request(
new WebSocketHandshakeRequestInfo(url_, base::Time::Now()));
auto request =
std::make_unique<WebSocketHandshakeRequestInfo>(url_, base::Time::Now());
request->headers.CopyFrom(enriched_headers);
connect_delegate_->OnStartOpeningHandshake(std::move(request));

Expand Down Expand Up @@ -497,10 +497,9 @@ std::unique_ptr<WebSocketStream> WebSocketBasicHandshakeStream::Upgrade() {
extension_params_->deflate_parameters.client_context_take_over_mode(),
WebSocketDeflater::NUM_CONTEXT_TAKEOVER_MODE_TYPES);

return std::unique_ptr<WebSocketStream>(new WebSocketDeflateStream(
return std::make_unique<WebSocketDeflateStream>(
std::move(basic_stream), extension_params_->deflate_parameters,
std::unique_ptr<WebSocketDeflatePredictor>(
new WebSocketDeflatePredictorImpl)));
std::make_unique<WebSocketDeflatePredictorImpl>());
} else {
return basic_stream;
}
Expand Down Expand Up @@ -585,7 +584,7 @@ int WebSocketBasicHandshakeStream::ValidateResponse(int rv) {

int WebSocketBasicHandshakeStream::ValidateUpgradeResponse(
const HttpResponseHeaders* headers) {
extension_params_.reset(new WebSocketExtensionParams);
extension_params_ = std::make_unique<WebSocketExtensionParams>();
std::string failure_message;
if (ValidateUpgrade(headers, &failure_message) &&
ValidateSecWebSocketAccept(
Expand Down
1 change: 0 additions & 1 deletion net/websockets/websocket_basic_handshake_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include <vector>

#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "net/base/completion_once_callback.h"
#include "net/base/net_export.h"
Expand Down
16 changes: 8 additions & 8 deletions net/websockets/websocket_basic_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ WebSocketBasicStream::WebSocketBasicStream(
const scoped_refptr<GrowableIOBuffer>& http_read_buffer,
const std::string& sub_protocol,
const std::string& extensions)
: read_buffer_(new IOBufferWithSize(kReadBufferSize)),
: read_buffer_(base::MakeRefCounted<IOBufferWithSize>(kReadBufferSize)),
connection_(std::move(connection)),
http_read_buffer_(http_read_buffer),
sub_protocol_(sub_protocol),
Expand Down Expand Up @@ -166,8 +166,7 @@ int WebSocketBasicStream::WriteFrames(
//
// First calculate the size of the buffer we need to allocate.
int total_size = CalculateSerializedSizeAndTurnOnMaskBit(frames);
scoped_refptr<IOBufferWithSize> combined_buffer(
new IOBufferWithSize(total_size));
auto combined_buffer = base::MakeRefCounted<IOBufferWithSize>(total_size);

char* dest = combined_buffer->data();
int remaining_size = total_size;
Expand Down Expand Up @@ -195,8 +194,8 @@ int WebSocketBasicStream::WriteFrames(
}
DCHECK_EQ(0, remaining_size) << "Buffer size calculation was wrong; "
<< remaining_size << " bytes left over.";
scoped_refptr<DrainableIOBuffer> drainable_buffer(
new DrainableIOBuffer(combined_buffer.get(), total_size));
auto drainable_buffer = base::MakeRefCounted<DrainableIOBuffer>(
combined_buffer.get(), total_size);
return WriteEverything(drainable_buffer, callback);
}

Expand Down Expand Up @@ -348,7 +347,8 @@ int WebSocketBasicStream::ConvertChunkToFrame(
AddToIncompleteControlFrameBody(data_buffer);
} else {
DVLOG(3) << "Creating new storage for an incomplete control frame.";
incomplete_control_frame_body_ = new GrowableIOBuffer();
incomplete_control_frame_body_ =
base::MakeRefCounted<GrowableIOBuffer>();
// This method checks for oversize control frames above, so as long as
// the frame parser is working correctly, this won't overflow. If a bug
// does cause it to overflow, it will CHECK() in
Expand All @@ -364,7 +364,7 @@ int WebSocketBasicStream::ConvertChunkToFrame(
const int body_size = incomplete_control_frame_body_->offset();
DCHECK_EQ(body_size,
static_cast<int>(current_frame_header_->payload_length));
scoped_refptr<IOBufferWithSize> body = new IOBufferWithSize(body_size);
auto body = base::MakeRefCounted<IOBufferWithSize>(body_size);
memcpy(body->data(),
incomplete_control_frame_body_->StartOfBuffer(),
body_size);
Expand Down Expand Up @@ -402,7 +402,7 @@ std::unique_ptr<WebSocketFrame> WebSocketBasicStream::CreateFrame(
if (is_final_chunk_in_message || data_size > 0 ||
current_frame_header_->opcode !=
WebSocketFrameHeader::kOpCodeContinuation) {
result_frame.reset(new WebSocketFrame(opcode));
result_frame = std::make_unique<WebSocketFrame>(opcode);
result_frame->header.CopyFrom(*current_frame_header_);
result_frame->header.final = is_final_chunk_in_message;
result_frame->header.payload_length = data_size;
Expand Down
2 changes: 1 addition & 1 deletion net/websockets/websocket_basic_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <string>
#include <vector>

#include "base/memory/ref_counted.h"
#include "base/memory/scoped_refptr.h"
#include "net/base/completion_callback.h"
#include "net/base/net_export.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
Expand Down
25 changes: 12 additions & 13 deletions net/websockets/websocket_basic_stream_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,12 @@ class WebSocketBasicStreamSocketTest : public WebSocketBasicStreamTest {
size_t reads_count,
MockWrite writes[],
size_t writes_count) {
socket_data_.reset(new StrictStaticSocketDataProvider(
reads, reads_count, writes, writes_count, expect_all_io_to_complete_));
socket_data_ = std::make_unique<StrictStaticSocketDataProvider>(
reads, reads_count, writes, writes_count, expect_all_io_to_complete_);
socket_data_->set_connect_data(MockConnect(SYNCHRONOUS, OK));
factory_.AddSocketDataProvider(socket_data_.get());

std::unique_ptr<ClientSocketHandle> transport_socket(
new ClientSocketHandle);
auto transport_socket = std::make_unique<ClientSocketHandle>();
scoped_refptr<MockTransportSocketParams> params;
transport_socket->Init("a", params, MEDIUM, SocketTag(),
ClientSocketPool::RespectLimits::ENABLED,
Expand All @@ -140,7 +139,7 @@ class WebSocketBasicStreamSocketTest : public WebSocketBasicStreamTest {
}

void SetHttpReadBuffer(const char* data, size_t size) {
http_read_buffer_ = new GrowableIOBuffer;
http_read_buffer_ = base::MakeRefCounted<GrowableIOBuffer>();
http_read_buffer_->SetCapacity(size);
memcpy(http_read_buffer_->data(), data, size);
http_read_buffer_->set_offset(size);
Expand Down Expand Up @@ -244,12 +243,12 @@ class WebSocketBasicStreamSocketWriteTest
// Creates a WebSocketFrame with a wire format matching kWriteFrame and adds
// it to |frames_|.
void PrepareWriteFrame() {
std::unique_ptr<WebSocketFrame> frame(
new WebSocketFrame(WebSocketFrameHeader::kOpCodeText));
auto frame =
std::make_unique<WebSocketFrame>(WebSocketFrameHeader::kOpCodeText);
const size_t payload_size =
kWriteFrameSize - (WebSocketFrameHeader::kBaseHeaderSize +
WebSocketFrameHeader::kMaskingKeyLength);
frame->data = new IOBuffer(payload_size);
frame->data = base::MakeRefCounted<IOBuffer>(payload_size);
memcpy(frame->data->data(),
kWriteFrame + kWriteFrameSize - payload_size,
payload_size);
Expand Down Expand Up @@ -918,8 +917,8 @@ TEST_F(WebSocketBasicStreamSocketWriteTest, WriteNullPong) {
MockWrite(SYNCHRONOUS, kMaskedEmptyPong, kMaskedEmptyPongSize)};
CreateWriteOnly(writes);

std::unique_ptr<WebSocketFrame> frame(
new WebSocketFrame(WebSocketFrameHeader::kOpCodePong));
auto frame =
std::make_unique<WebSocketFrame>(WebSocketFrameHeader::kOpCodePong);
WebSocketFrameHeader& header = frame->header;
header.final = true;
header.masked = true;
Expand All @@ -939,11 +938,11 @@ TEST_F(WebSocketBasicStreamSocketTest, WriteNonNulMask) {
generator_ = &GenerateNonNulMaskingKey;
CreateStream(NULL, 0, writes, arraysize(writes));

std::unique_ptr<WebSocketFrame> frame(
new WebSocketFrame(WebSocketFrameHeader::kOpCodeText));
auto frame =
std::make_unique<WebSocketFrame>(WebSocketFrameHeader::kOpCodeText);
const std::string unmasked_payload = "graphics";
const size_t payload_size = unmasked_payload.size();
frame->data = new IOBuffer(payload_size);
frame->data = base::MakeRefCounted<IOBuffer>(payload_size);
memcpy(frame->data->data(), unmasked_payload.data(), payload_size);
WebSocketFrameHeader& header = frame->header;
header.final = true;
Expand Down
24 changes: 11 additions & 13 deletions net/websockets/websocket_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "base/containers/circular_deque.h"
#include "base/location.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/metrics/histogram_macros.h"
#include "base/numerics/safe_conversions.h"
Expand Down Expand Up @@ -355,7 +354,7 @@ WebSocketChannel::WebSocketChannel(
has_received_close_frame_(false),
received_close_code_(0),
state_(FRESHLY_CONSTRUCTED),
notification_sender_(new HandshakeNotificationSender(this)),
notification_sender_(std::make_unique<HandshakeNotificationSender>(this)),
sending_text_message_(false),
receiving_text_message_(false),
expecting_to_handle_continuation_(false),
Expand Down Expand Up @@ -479,7 +478,8 @@ ChannelState WebSocketChannel::SendFlowControl(int64_t quota) {
const bool final = front.final() && data_size == bytes_to_send;
scoped_refptr<IOBuffer> buffer_to_pass;
if (front.data()) {
buffer_to_pass = new DependentIOBuffer(front.data(), front.offset());
buffer_to_pass =
base::MakeRefCounted<DependentIOBuffer>(front.data(), front.offset());
} else {
DCHECK(!bytes_to_send) << "Non empty data should not be null.";
}
Expand Down Expand Up @@ -617,11 +617,9 @@ void WebSocketChannel::SendAddChannelRequestWithSuppliedCallback(
return;
}
socket_url_ = socket_url;
std::unique_ptr<WebSocketStream::ConnectDelegate> connect_delegate(
new ConnectDelegate(this));
std::unique_ptr<WebSocketHandshakeStreamCreateHelper> create_helper(
new WebSocketHandshakeStreamCreateHelper(connect_delegate.get(),
requested_subprotocols));
auto connect_delegate = std::make_unique<ConnectDelegate>(this);
auto create_helper = std::make_unique<WebSocketHandshakeStreamCreateHelper>(
connect_delegate.get(), requested_subprotocols);
stream_request_ =
callback.Run(socket_url_, std::move(create_helper), origin,
site_for_cookies, additional_headers, url_request_context_,
Expand Down Expand Up @@ -1080,7 +1078,7 @@ ChannelState WebSocketChannel::SendFrameInternal(
DCHECK(state_ == CONNECTED || state_ == RECV_CLOSED);
DCHECK(stream_);

std::unique_ptr<WebSocketFrame> frame(new WebSocketFrame(op_code));
auto frame = std::make_unique<WebSocketFrame>(op_code);
WebSocketFrameHeader& header = frame->header;
header.final = fin;
header.masked = true;
Expand All @@ -1093,12 +1091,12 @@ ChannelState WebSocketChannel::SendFrameInternal(
// TODO(ricea): Keep some statistics to work out the situation and adjust
// quota appropriately.
if (!data_to_send_next_)
data_to_send_next_.reset(new SendBuffer);
data_to_send_next_ = std::make_unique<SendBuffer>();
data_to_send_next_->AddFrame(std::move(frame));
return CHANNEL_ALIVE;
}

data_being_sent_.reset(new SendBuffer);
data_being_sent_ = std::make_unique<SendBuffer>();
data_being_sent_->AddFrame(std::move(frame));
return WriteFrames();
}
Expand Down Expand Up @@ -1136,10 +1134,10 @@ ChannelState WebSocketChannel::SendClose(uint16_t code,
// Special case: translate kWebSocketErrorNoStatusReceived into a Close
// frame with no payload.
DCHECK(reason.empty());
body = new IOBuffer(0);
body = base::MakeRefCounted<IOBuffer>(0);
} else {
const size_t payload_length = kWebSocketCloseCodeLength + reason.length();
body = new IOBuffer(payload_length);
body = base::MakeRefCounted<IOBuffer>(payload_length);
size = payload_length;
base::WriteBigEndian(body->data(), code);
static_assert(sizeof(code) == kWebSocketCloseCodeLength,
Expand Down
2 changes: 1 addition & 1 deletion net/websockets/websocket_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include "base/containers/queue.h"
#include "base/i18n/streaming_utf8_validator.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_refptr.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "net/base/net_export.h"
Expand Down
Loading

0 comments on commit 6562397

Please sign in to comment.