Skip to content

Commit

Permalink
Make SpdyHeaderBlock non-copyable.
Browse files Browse the repository at this point in the history 
Also change all remaining invocations of the copy constructor or copy assignment
operator to move or explicit copy.

This CL lands server change 126070406 by bnc.

BUG=488484, 621905

Review-Url: https://codereview.chromium.org/2102253003
Cr-Commit-Position: refs/heads/master@{#403165}
  • Loading branch information
bnc authored and Commit bot committed Jun 30, 2016
1 parent b9debcd commit 94893a7
Show file tree
Hide file tree
Showing 39 changed files with 435 additions and 345 deletions.
5 changes: 4 additions & 1 deletion components/cronet/ios/test/quic_test_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "components/cronet/ios/test/quic_test_server.h"

#include <utility>

#include "base/bind.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
Expand Down Expand Up @@ -53,7 +55,8 @@ void SetupQuicInMemoryCache() {
net::SpdyHeaderBlock trailers;
trailers.ReplaceOrAppendHeader(kHelloTrailerName, kHelloTrailerValue);
net::QuicInMemoryCache::GetInstance()->AddResponse(
kTestServerHost, kHelloPath, headers, kHelloBodyValue, trailers);
kTestServerHost, kHelloPath, std::move(headers), kHelloBodyValue,
std::move(trailers));
}

void StartQuicServerOnServerThread(const base::FilePath& test_files_root,
Expand Down
18 changes: 9 additions & 9 deletions net/http/bidirectional_stream_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class TestDelegateBase : public BidirectionalStream::Delegate {
void OnHeadersReceived(const SpdyHeaderBlock& response_headers) override {
CHECK(!not_expect_callback_);

response_headers_ = response_headers;
response_headers_ = response_headers.Clone();
if (!do_not_start_read_)
StartOrContinueReading();
}
Expand All @@ -100,7 +100,7 @@ class TestDelegateBase : public BidirectionalStream::Delegate {
void OnTrailersReceived(const SpdyHeaderBlock& trailers) override {
CHECK(!not_expect_callback_);

trailers_ = trailers;
trailers_ = trailers.Clone();
if (run_until_completion_)
loop_->Quit();
}
Expand Down Expand Up @@ -490,7 +490,7 @@ TEST_F(BidirectionalStreamTest, TestReadDataAfterClose) {
rv = delegate->ReadData();
EXPECT_EQ(OK, rv); // EOF.

const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ("header-value", response_headers.find("header-name")->second);
EXPECT_EQ(1, delegate->on_data_read_count());
Expand Down Expand Up @@ -981,7 +981,7 @@ TEST_F(BidirectionalStreamTest, TestBuffering) {
EXPECT_EQ(kUploadDataSize * 3,
static_cast<int>(delegate->data_received().size()));

const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ("header-value", response_headers.find("header-name")->second);
EXPECT_EQ(0, delegate->on_data_sent_count());
Expand Down Expand Up @@ -1063,7 +1063,7 @@ TEST_F(BidirectionalStreamTest, TestBufferingWithTrailers) {
EXPECT_EQ(1, delegate->on_data_read_count());
EXPECT_EQ(kUploadDataSize * 3,
static_cast<int>(delegate->data_received().size()));
const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ("header-value", response_headers.find("header-name")->second);
EXPECT_EQ("bar", delegate->trailers().find("foo")->second);
Expand Down Expand Up @@ -1324,7 +1324,7 @@ TEST_P(BidirectionalStreamTest, CancelOrDeleteStreamDuringOnHeadersReceived) {
delegate->Start(std::move(request_info), http_session_.get());
// Makes sure delegate does not get called.
base::RunLoop().RunUntilIdle();
const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ("header-value", response_headers.find("header-name")->second);
EXPECT_EQ(0u, delegate->data_received().size());
Expand Down Expand Up @@ -1383,7 +1383,7 @@ TEST_P(BidirectionalStreamTest, CancelOrDeleteStreamDuringOnDataRead) {
delegate->Start(std::move(request_info), http_session_.get());
// Makes sure delegate does not get called.
base::RunLoop().RunUntilIdle();
const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ("header-value", response_headers.find("header-name")->second);
EXPECT_EQ(kUploadDataSize * 1,
Expand Down Expand Up @@ -1448,7 +1448,7 @@ TEST_P(BidirectionalStreamTest, CancelOrDeleteStreamDuringOnTrailersReceived) {
delegate->Start(std::move(request_info), http_session_.get());
// Makes sure delegate does not get called.
base::RunLoop().RunUntilIdle();
const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ("header-value", response_headers.find("header-name")->second);
EXPECT_EQ("bar", delegate->trailers().find("foo")->second);
Expand Down Expand Up @@ -1565,7 +1565,7 @@ TEST_F(BidirectionalStreamTest, TestHonorAlternativeServiceHeader) {
delegate->SetRunUntilCompletion(true);
delegate->Start(std::move(request_info), http_session_.get());

const SpdyHeaderBlock response_headers = delegate->response_headers();
const SpdyHeaderBlock& response_headers = delegate->response_headers();
EXPECT_EQ("200", response_headers.find(":status")->second);
EXPECT_EQ(alt_svc_header_value, response_headers.find("alt-svc")->second);
EXPECT_EQ(0, delegate->on_data_sent_count());
Expand Down
4 changes: 2 additions & 2 deletions net/http/http_network_transaction_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5017,7 +5017,7 @@ TEST_P(HttpNetworkTransactionTest,
connect2_block[spdy_util_.GetPathKey()] = "mail.example.org:443";
}
std::unique_ptr<SpdySerializedFrame> connect2(
spdy_util_.ConstructSpdySyn(3, connect2_block, LOWEST, false));
spdy_util_.ConstructSpdySyn(3, std::move(connect2_block), LOWEST, false));

std::unique_ptr<SpdySerializedFrame> conn_resp2(
spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 3));
Expand Down Expand Up @@ -13697,7 +13697,7 @@ TEST_P(HttpNetworkTransactionTest, DoNotUseSpdySessionForHttpOverTunnel) {
req2_block[spdy_util_.GetSchemeKey()] = "http";
req2_block[spdy_util_.GetPathKey()] = "/";
std::unique_ptr<SpdySerializedFrame> req2(
spdy_util_.ConstructSpdySyn(3, req2_block, MEDIUM, true));
spdy_util_.ConstructSpdySyn(3, std::move(req2_block), MEDIUM, true));

MockWrite writes1[] = {
CreateMockWrite(*connect, 0), CreateMockWrite(*wrapped_req1, 2),
Expand Down
2 changes: 1 addition & 1 deletion net/http/http_stream_factory_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ class TestBidirectionalDelegate : public BidirectionalStreamImpl::Delegate {
private:
void OnStreamReady(bool request_headers_sent) override {}
void OnHeadersReceived(const SpdyHeaderBlock& response_headers) override {
response_headers_ = response_headers;
response_headers_ = response_headers.Clone();
loop_.Quit();
}
void OnDataRead(int bytes_read) override { NOTREACHED(); }
Expand Down
22 changes: 11 additions & 11 deletions net/quic/bidirectional_stream_quic_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate {
CHECK(!on_failed_called_);
CHECK(!not_expect_callback_);

response_headers_ = response_headers;
response_headers_ = response_headers.Clone();
loop_->Quit();
}

Expand All @@ -117,7 +117,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate {
CHECK(!on_failed_called_);
CHECK(!not_expect_callback_);

trailers_ = trailers;
trailers_ = trailers.Clone();
loop_->Quit();
}

Expand Down Expand Up @@ -493,11 +493,11 @@ class BidirectionalStreamQuicImplTest
std::unique_ptr<QuicReceivedPacket> ConstructResponseTrailersPacket(
QuicPacketNumber packet_number,
bool fin,
const SpdyHeaderBlock& trailers,
SpdyHeaderBlock trailers,
size_t* spdy_headers_frame_length,
QuicStreamOffset* offset) {
return server_maker_.MakeResponseHeadersPacket(
packet_number, stream_id_, !kIncludeVersion, fin, trailers,
packet_number, stream_id_, !kIncludeVersion, fin, std::move(trailers),
spdy_headers_frame_length, offset);
}

Expand Down Expand Up @@ -659,7 +659,7 @@ TEST_P(BidirectionalStreamQuicImplTest, GetRequest) {
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
EXPECT_EQ(OK, cb2.WaitForResult());
Expand Down Expand Up @@ -783,7 +783,7 @@ TEST_P(BidirectionalStreamQuicImplTest, CoalesceDataBuffersNotHeadersFrame) {
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
trailers.erase(kFinalOffsetHeaderKey);
Expand Down Expand Up @@ -879,7 +879,7 @@ TEST_P(BidirectionalStreamQuicImplTest,
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
trailers.erase(kFinalOffsetHeaderKey);
Expand Down Expand Up @@ -982,7 +982,7 @@ TEST_P(BidirectionalStreamQuicImplTest,
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
trailers.erase(kFinalOffsetHeaderKey);
Expand Down Expand Up @@ -1062,7 +1062,7 @@ TEST_P(BidirectionalStreamQuicImplTest, PostRequest) {
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
trailers.erase(kFinalOffsetHeaderKey);
Expand Down Expand Up @@ -1139,7 +1139,7 @@ TEST_P(BidirectionalStreamQuicImplTest, PutRequest) {
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
trailers.erase(kFinalOffsetHeaderKey);
Expand Down Expand Up @@ -1658,7 +1658,7 @@ TEST_P(BidirectionalStreamQuicImplTest, DeleteStreamDuringOnTrailersReceived) {
trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(kResponseBody));
// Server sends trailers.
ProcessPacket(ConstructResponseTrailersPacket(
4, kFin, trailers, &spdy_trailers_frame_length, &offset));
4, kFin, trailers.Clone(), &spdy_trailers_frame_length, &offset));

delegate->WaitUntilNextCallback(); // OnTrailersReceived
trailers.erase(kFinalOffsetHeaderKey);
Expand Down
18 changes: 10 additions & 8 deletions net/quic/quic_chromium_client_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <utility>

#include "base/bind_helpers.h"
#include "base/callback_helpers.h"
#include "base/location.h"
#include "base/threading/thread_task_runner_handle.h"
Expand Down Expand Up @@ -42,7 +43,8 @@ void QuicChromiumClientStream::OnStreamHeadersComplete(bool fin,
if (decompressed_headers().empty() && !decompressed_trailers().empty()) {
DCHECK(trailers_decompressed());
// The delegate will read the trailers via a posted task.
NotifyDelegateOfHeadersCompleteLater(received_trailers(), frame_len);
NotifyDelegateOfHeadersCompleteLater(received_trailers().Clone(),
frame_len);
} else {
DCHECK(!headers_delivered_);
SpdyHeaderBlock headers;
Expand All @@ -58,7 +60,7 @@ void QuicChromiumClientStream::OnStreamHeadersComplete(bool fin,
session_->OnInitialHeadersComplete(id(), headers);

// The delegate will read the headers via a posted task.
NotifyDelegateOfHeadersCompleteLater(headers, frame_len);
NotifyDelegateOfHeadersCompleteLater(std::move(headers), frame_len);
}
}

Expand All @@ -81,15 +83,15 @@ void QuicChromiumClientStream::OnInitialHeadersComplete(
session_->OnInitialHeadersComplete(id(), header_block);

// The delegate will read the headers via a posted task.
NotifyDelegateOfHeadersCompleteLater(header_block, frame_len);
NotifyDelegateOfHeadersCompleteLater(std::move(header_block), frame_len);
}

void QuicChromiumClientStream::OnTrailingHeadersComplete(
bool fin,
size_t frame_len,
const QuicHeaderList& header_list) {
QuicSpdyStream::OnTrailingHeadersComplete(fin, frame_len, header_list);
NotifyDelegateOfHeadersCompleteLater(received_trailers(), frame_len);
NotifyDelegateOfHeadersCompleteLater(received_trailers().Clone(), frame_len);
}

void QuicChromiumClientStream::OnPromiseHeadersComplete(
Expand Down Expand Up @@ -263,11 +265,11 @@ bool QuicChromiumClientStream::CanWrite(const CompletionCallback& callback) {
}

void QuicChromiumClientStream::NotifyDelegateOfHeadersCompleteLater(
const SpdyHeaderBlock& headers,
SpdyHeaderBlock headers,
size_t frame_len) {
RunOrBuffer(
base::Bind(&QuicChromiumClientStream::NotifyDelegateOfHeadersComplete,
weak_factory_.GetWeakPtr(), headers, frame_len));
RunOrBuffer(base::Bind(
&QuicChromiumClientStream::NotifyDelegateOfHeadersComplete,
weak_factory_.GetWeakPtr(), base::Passed(std::move(headers)), frame_len));
}

void QuicChromiumClientStream::NotifyDelegateOfHeadersComplete(
Expand Down
2 changes: 1 addition & 1 deletion net/quic/quic_chromium_client_stream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientStream : public QuicSpdyStream {
using QuicSpdyStream::HasBufferedData;

private:
void NotifyDelegateOfHeadersCompleteLater(const SpdyHeaderBlock& headers,
void NotifyDelegateOfHeadersCompleteLater(SpdyHeaderBlock headers,
size_t frame_len);
void NotifyDelegateOfHeadersComplete(SpdyHeaderBlock headers,
size_t frame_len);
Expand Down
Loading

0 comments on commit 94893a7

Please sign in to comment.