From 94893a702549bf5d2e1f7c3cfafc904206f1cb57 Mon Sep 17 00:00:00 2001 From: bnc Date: Thu, 30 Jun 2016 06:45:25 -0700 Subject: [PATCH] Make SpdyHeaderBlock non-copyable. 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} --- .../cronet/ios/test/quic_test_server.cc | 5 +- net/http/bidirectional_stream_unittest.cc | 18 +-- net/http/http_network_transaction_unittest.cc | 4 +- net/http/http_stream_factory_impl_unittest.cc | 2 +- ...bidirectional_stream_quic_impl_unittest.cc | 22 ++-- net/quic/quic_chromium_client_stream.cc | 18 +-- net/quic/quic_chromium_client_stream.h | 2 +- net/quic/quic_chromium_client_stream_test.cc | 62 +++++++---- net/quic/quic_client_promised_info.cc | 6 +- net/quic/quic_client_promised_info_test.cc | 2 +- net/quic/quic_headers_stream_test.cc | 37 ++++--- net/quic/quic_http_stream_test.cc | 10 +- net/quic/quic_network_transaction_unittest.cc | 10 +- net/quic/quic_session_test.cc | 5 +- net/quic/quic_spdy_stream_test.cc | 25 ++--- net/quic/test_tools/quic_test_utils.h | 19 +++- ...bidirectional_stream_spdy_impl_unittest.cc | 8 +- net/spdy/buffered_spdy_framer_unittest.cc | 16 +-- net/spdy/hpack/hpack_decoder_test.cc | 47 ++++---- net/spdy/spdy_framer_test.cc | 21 ++-- net/spdy/spdy_header_block.cc | 23 ++-- net/spdy/spdy_header_block.h | 7 +- net/spdy/spdy_header_block_test.cc | 8 +- net/spdy/spdy_protocol.h | 4 +- net/spdy/spdy_session_unittest.cc | 9 +- net/spdy/spdy_stream.cc | 4 +- net/spdy/spdy_stream_test_util.cc | 2 +- net/spdy/spdy_stream_unittest.cc | 2 +- net/tools/flip_server/spdy_interface.cc | 5 +- net/tools/flip_server/spdy_interface_test.cc | 104 +++++++++++------- net/tools/quic/end_to_end_test.cc | 12 +- net/tools/quic/quic_in_memory_cache.cc | 60 +++++----- net/tools/quic/quic_in_memory_cache.h | 20 ++-- net/tools/quic/quic_in_memory_cache_test.cc | 10 +- net/tools/quic/quic_simple_server_session.cc | 11 +- .../quic/quic_simple_server_session_test.cc | 54 ++++++--- net/tools/quic/quic_simple_server_stream.cc | 6 +- .../quic/quic_simple_server_stream_test.cc | 98 +++++++++-------- net/tools/quic/test_tools/quic_test_client.cc | 2 +- 39 files changed, 435 insertions(+), 345 deletions(-) diff --git a/components/cronet/ios/test/quic_test_server.cc b/components/cronet/ios/test/quic_test_server.cc index 8a8dacd73716b5..59a7c13f657dd0 100644 --- a/components/cronet/ios/test/quic_test_server.cc +++ b/components/cronet/ios/test/quic_test_server.cc @@ -4,6 +4,8 @@ #include "components/cronet/ios/test/quic_test_server.h" +#include + #include "base/bind.h" #include "base/files/file_path.h" #include "base/files/file_util.h" @@ -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, diff --git a/net/http/bidirectional_stream_unittest.cc b/net/http/bidirectional_stream_unittest.cc index ef937d82fd2ba0..b515195da05d5e 100644 --- a/net/http/bidirectional_stream_unittest.cc +++ b/net/http/bidirectional_stream_unittest.cc @@ -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(); } @@ -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(); } @@ -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()); @@ -981,7 +981,7 @@ TEST_F(BidirectionalStreamTest, TestBuffering) { EXPECT_EQ(kUploadDataSize * 3, static_cast(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()); @@ -1063,7 +1063,7 @@ TEST_F(BidirectionalStreamTest, TestBufferingWithTrailers) { EXPECT_EQ(1, delegate->on_data_read_count()); EXPECT_EQ(kUploadDataSize * 3, static_cast(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); @@ -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()); @@ -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, @@ -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); @@ -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()); diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index a4b3592fff8152..db94e0faecb336 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -5017,7 +5017,7 @@ TEST_P(HttpNetworkTransactionTest, connect2_block[spdy_util_.GetPathKey()] = "mail.example.org:443"; } std::unique_ptr connect2( - spdy_util_.ConstructSpdySyn(3, connect2_block, LOWEST, false)); + spdy_util_.ConstructSpdySyn(3, std::move(connect2_block), LOWEST, false)); std::unique_ptr conn_resp2( spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 3)); @@ -13697,7 +13697,7 @@ TEST_P(HttpNetworkTransactionTest, DoNotUseSpdySessionForHttpOverTunnel) { req2_block[spdy_util_.GetSchemeKey()] = "http"; req2_block[spdy_util_.GetPathKey()] = "/"; std::unique_ptr 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), diff --git a/net/http/http_stream_factory_impl_unittest.cc b/net/http/http_stream_factory_impl_unittest.cc index e90078acff4d4e..5666b7cf732299 100644 --- a/net/http/http_stream_factory_impl_unittest.cc +++ b/net/http/http_stream_factory_impl_unittest.cc @@ -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(); } diff --git a/net/quic/bidirectional_stream_quic_impl_unittest.cc b/net/quic/bidirectional_stream_quic_impl_unittest.cc index f7a512314eab6e..abad19b225b98a 100644 --- a/net/quic/bidirectional_stream_quic_impl_unittest.cc +++ b/net/quic/bidirectional_stream_quic_impl_unittest.cc @@ -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(); } @@ -117,7 +117,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate { CHECK(!on_failed_called_); CHECK(!not_expect_callback_); - trailers_ = trailers; + trailers_ = trailers.Clone(); loop_->Quit(); } @@ -493,11 +493,11 @@ class BidirectionalStreamQuicImplTest std::unique_ptr 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); } @@ -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()); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/net/quic/quic_chromium_client_stream.cc b/net/quic/quic_chromium_client_stream.cc index 1075ad3852da68..405db0f89a3c4d 100644 --- a/net/quic/quic_chromium_client_stream.cc +++ b/net/quic/quic_chromium_client_stream.cc @@ -6,6 +6,7 @@ #include +#include "base/bind_helpers.h" #include "base/callback_helpers.h" #include "base/location.h" #include "base/threading/thread_task_runner_handle.h" @@ -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; @@ -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); } } @@ -81,7 +83,7 @@ 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( @@ -89,7 +91,7 @@ void QuicChromiumClientStream::OnTrailingHeadersComplete( 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( @@ -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( diff --git a/net/quic/quic_chromium_client_stream.h b/net/quic/quic_chromium_client_stream.h index 5bffe3a5838288..976db2a8b89c85 100644 --- a/net/quic/quic_chromium_client_stream.h +++ b/net/quic/quic_chromium_client_stream.h @@ -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); diff --git a/net/quic/quic_chromium_client_stream_test.cc b/net/quic/quic_chromium_client_stream_test.cc index 6792fbe47ebc4c..7a1e5a6d4797e9 100644 --- a/net/quic/quic_chromium_client_stream_test.cc +++ b/net/quic/quic_chromium_client_stream_test.cc @@ -39,7 +39,12 @@ class MockDelegate : public QuicChromiumClientStream::Delegate { MOCK_METHOD0(OnSendData, int()); MOCK_METHOD2(OnSendDataComplete, int(int, bool*)); - MOCK_METHOD2(OnHeadersAvailable, + void OnHeadersAvailable(const SpdyHeaderBlock& headers, + size_t frame_len) override { + headers_ = headers.Clone(); + OnHeadersAvailableMock(headers, frame_len); + } + MOCK_METHOD2(OnHeadersAvailableMock, void(const SpdyHeaderBlock& headers, size_t frame_len)); MOCK_METHOD2(OnDataReceived, int(const char*, int)); MOCK_METHOD0(OnDataAvailable, void()); @@ -47,6 +52,8 @@ class MockDelegate : public QuicChromiumClientStream::Delegate { MOCK_METHOD1(OnError, void(int)); MOCK_METHOD0(HasSendHeadersComplete, bool()); + SpdyHeaderBlock headers_; + private: DISALLOW_COPY_AND_ASSIGN(MockDelegate); }; @@ -92,9 +99,19 @@ class MockQuicClientSessionBase : public QuicClientSessionBase { QuicStreamId promised_stream_id, size_t frame_len)); MOCK_METHOD0(IsCryptoHandshakeConfirmed, bool()); - MOCK_METHOD5(WriteHeaders, + // Methods taking non-copyable types like SpdyHeaderBlock by value cannot be + // mocked directly. + size_t WriteHeaders( + QuicStreamId id, + SpdyHeaderBlock headers, + bool fin, + SpdyPriority priority, + QuicAckListenerInterface* ack_notifier_delegate) override { + return WriteHeadersMock(id, headers, fin, priority, ack_notifier_delegate); + } + MOCK_METHOD5(WriteHeadersMock, size_t(QuicStreamId id, - SpdyHeaderBlock headers, + const SpdyHeaderBlock& headers, bool fin, SpdyPriority priority, QuicAckListenerInterface* ack_notifier_delegate)); @@ -218,8 +235,9 @@ TEST_P(QuicChromiumClientStreamTest, OnFinRead) { stream_->OnStreamHeadersComplete(false, uncompressed_headers.length()); EXPECT_CALL(delegate_, - OnHeadersAvailable(headers_, uncompressed_headers.length())); + OnHeadersAvailableMock(_, uncompressed_headers.length())); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(headers_, delegate_.headers_); EXPECT_TRUE(stream_->decompressed_headers().empty()); QuicStreamFrame frame2(kTestStreamId, true, offset, StringPiece()); @@ -242,8 +260,9 @@ TEST_P(QuicChromiumClientStreamTest, OnDataAvailable) { stream_->OnStreamHeadersComplete(false, uncompressed_headers.length()); EXPECT_CALL(delegate_, - OnHeadersAvailable(headers_, uncompressed_headers.length())); + OnHeadersAvailableMock(_, uncompressed_headers.length())); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(headers_, delegate_.headers_); EXPECT_TRUE(stream_->decompressed_headers().empty()); const char data[] = "hello world!"; @@ -281,8 +300,9 @@ TEST_P(QuicChromiumClientStreamTest, OnDataAvailableWithError) { stream_->OnStreamHeadersComplete(false, uncompressed_headers.length()); EXPECT_CALL(delegate_, - OnHeadersAvailable(headers_, uncompressed_headers.length())); + OnHeadersAvailableMock(_, uncompressed_headers.length())); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(headers_, delegate_.headers_); EXPECT_TRUE(stream_->decompressed_headers().empty()); const char data[] = "hello world!"; @@ -312,8 +332,9 @@ TEST_P(QuicChromiumClientStreamTest, OnTrailers) { stream_->OnStreamHeadersComplete(false, uncompressed_headers.length()); EXPECT_CALL(delegate_, - OnHeadersAvailable(headers_, uncompressed_headers.length())); + OnHeadersAvailableMock(_, uncompressed_headers.length())); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(headers_, delegate_.headers_); EXPECT_TRUE(stream_->decompressed_headers().empty()); const char data[] = "hello world!"; @@ -334,18 +355,15 @@ TEST_P(QuicChromiumClientStreamTest, OnTrailers) { stream_->OnStreamHeaders(uncompressed_trailers); stream_->OnStreamHeadersComplete(true, uncompressed_trailers.length()); - SpdyHeaderBlock actual_trailers; - base::RunLoop run_loop; - EXPECT_CALL(delegate_, OnHeadersAvailable(_, uncompressed_trailers.length())) - .WillOnce(testing::DoAll( - testing::SaveArg<0>(&actual_trailers), - testing::InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); }))); + EXPECT_CALL(delegate_, + OnHeadersAvailableMock(_, uncompressed_trailers.length())) + .WillOnce(testing::InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); run_loop.Run(); // Make sure kFinalOffsetHeaderKey is gone from the delivered actual trailers. trailers.erase(kFinalOffsetHeaderKey); - EXPECT_EQ(trailers, actual_trailers); + EXPECT_EQ(trailers, delegate_.headers_); base::RunLoop().RunUntilIdle(); EXPECT_CALL(delegate_, OnClose()); } @@ -360,8 +378,9 @@ TEST_P(QuicChromiumClientStreamTest, MarkTrailersConsumedWhenNotifyDelegate) { stream_->OnStreamHeadersComplete(false, uncompressed_headers.length()); EXPECT_CALL(delegate_, - OnHeadersAvailable(headers_, uncompressed_headers.length())); + OnHeadersAvailableMock(_, uncompressed_headers.length())); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(headers_, delegate_.headers_); EXPECT_TRUE(stream_->decompressed_headers().empty()); const char data[] = "hello world!"; @@ -410,12 +429,10 @@ TEST_P(QuicChromiumClientStreamTest, MarkTrailersConsumedWhenNotifyDelegate) { // The OnHeadersAvailable call should follow. base::RunLoop run_loop3; - SpdyHeaderBlock actual_trailers; EXPECT_CALL(delegate_, - OnHeadersAvailable(_, uncompressed_trailers.length())) - .WillOnce(testing::DoAll( - testing::SaveArg<0>(&actual_trailers), - testing::InvokeWithoutArgs([&run_loop3]() { run_loop3.Quit(); }))); + OnHeadersAvailableMock(_, uncompressed_trailers.length())) + .WillOnce( + testing::InvokeWithoutArgs([&run_loop3]() { run_loop3.Quit(); })); run_loop3.Run(); // Make sure the stream is properly closed since trailers and data are all @@ -423,7 +440,7 @@ TEST_P(QuicChromiumClientStreamTest, MarkTrailersConsumedWhenNotifyDelegate) { EXPECT_TRUE(stream_->IsDoneReading()); // Make sure kFinalOffsetHeaderKey is gone from the delivered actual trailers. trailers.erase(kFinalOffsetHeaderKey); - EXPECT_EQ(trailers, actual_trailers); + EXPECT_EQ(trailers, delegate_.headers_); base::RunLoop().RunUntilIdle(); EXPECT_CALL(delegate_, OnClose()); @@ -528,9 +545,10 @@ TEST_P(QuicChromiumClientStreamTest, HeadersBeforeDelegate) { EXPECT_TRUE(stream->decompressed_headers().empty()); EXPECT_CALL(delegate_, - OnHeadersAvailable(headers_, uncompressed_headers.length())); + OnHeadersAvailableMock(_, uncompressed_headers.length())); stream->SetDelegate(&delegate_); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(headers_, delegate_.headers_); // Times(2) because OnClose will be called for stream and stream_. EXPECT_CALL(delegate_, OnClose()).Times(2); diff --git a/net/quic/quic_client_promised_info.cc b/net/quic/quic_client_promised_info.cc index 14cba80ee14e66..4de7adf83f3239 100644 --- a/net/quic/quic_client_promised_info.cc +++ b/net/quic/quic_client_promised_info.cc @@ -56,11 +56,11 @@ void QuicClientPromisedInfo::OnPromiseHeaders(const SpdyHeaderBlock& headers) { Reset(QUIC_UNAUTHORIZED_PROMISE_URL); return; } - request_headers_.reset(new SpdyHeaderBlock(headers)); + request_headers_.reset(new SpdyHeaderBlock(headers.Clone())); } void QuicClientPromisedInfo::OnResponseHeaders(const SpdyHeaderBlock& headers) { - response_headers_.reset(new SpdyHeaderBlock(headers)); + response_headers_.reset(new SpdyHeaderBlock(headers.Clone())); if (client_request_delegate_) { // We already have a client request waiting. FinalValidation(); @@ -107,7 +107,7 @@ QuicAsyncStatus QuicClientPromisedInfo::HandleClientRequest( return QUIC_FAILURE; } client_request_delegate_ = delegate; - client_request_headers_.reset(new SpdyHeaderBlock(request_headers)); + client_request_headers_.reset(new SpdyHeaderBlock(request_headers.Clone())); if (!response_headers_) { return QUIC_PENDING; } diff --git a/net/quic/quic_client_promised_info_test.cc b/net/quic/quic_client_promised_info_test.cc index f80f0ae8ec3af0..fef467f72a9914 100644 --- a/net/quic/quic_client_promised_info_test.cc +++ b/net/quic/quic_client_promised_info_test.cc @@ -101,7 +101,7 @@ class QuicClientPromisedInfoTest : public ::testing::Test { serialized_push_promise_ = SpdyUtils::SerializeUncompressedHeaders(push_promise_); - client_request_ = SpdyHeaderBlock(push_promise_); + client_request_ = push_promise_.Clone(); } class StreamVisitor : public QuicSpdyClientStream::Visitor { diff --git a/net/quic/quic_headers_stream_test.cc b/net/quic/quic_headers_stream_test.cc index 2d0b7d8b2f5290..2a606856bb04e3 100644 --- a/net/quic/quic_headers_stream_test.cc +++ b/net/quic/quic_headers_stream_test.cc @@ -233,7 +233,8 @@ class QuicHeadersStreamTest : public ::testing::TestWithParam { EXPECT_CALL(session_, WritevData(headers_stream_, kHeadersStreamId, _, _, false, nullptr)) .WillOnce(WithArgs<2>(Invoke(this, &QuicHeadersStreamTest::SaveIov))); - headers_stream_->WriteHeaders(stream_id, headers_, fin, priority, nullptr); + headers_stream_->WriteHeaders(stream_id, headers_.Clone(), fin, priority, + nullptr); // Parse the outgoing data and check that it matches was was written. if (type == SYN_STREAM) { @@ -339,8 +340,8 @@ TEST_P(QuicHeadersStreamTest, WritePushPromises) { EXPECT_CALL(session_, WritevData(headers_stream_, kHeadersStreamId, _, _, false, nullptr)) .WillOnce(WithArgs<2>(Invoke(this, &QuicHeadersStreamTest::SaveIov))); - headers_stream_->WritePushPromise(stream_id, promised_stream_id, headers_, - nullptr); + headers_stream_->WritePushPromise(stream_id, promised_stream_id, + headers_.Clone(), nullptr); // Parse the outgoing data and check that it matches was was written. EXPECT_CALL(visitor_, @@ -355,9 +356,10 @@ TEST_P(QuicHeadersStreamTest, WritePushPromises) { CheckHeaders(); saved_data_.clear(); } else { - EXPECT_DFATAL(headers_stream_->WritePushPromise( - stream_id, promised_stream_id, headers_, nullptr), - "Client shouldn't send PUSH_PROMISE"); + EXPECT_DFATAL( + headers_stream_->WritePushPromise(stream_id, promised_stream_id, + headers_.Clone(), nullptr), + "Client shouldn't send PUSH_PROMISE"); } } } @@ -370,14 +372,14 @@ TEST_P(QuicHeadersStreamTest, ProcessRawData) { // Replace with "WriteHeadersAndSaveData" SpdySerializedFrame frame; if (perspective() == Perspective::IS_SERVER) { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); headers_frame.set_has_priority(true); headers_frame.set_weight(Spdy3PriorityToHttp2Weight(0)); frame = framer_->SerializeFrame(headers_frame); EXPECT_CALL(session_, OnStreamHeadersPriority(stream_id, 0)); } else { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); frame = framer_->SerializeFrame(headers_frame); } @@ -400,7 +402,8 @@ TEST_P(QuicHeadersStreamTest, ProcessPushPromise) { for (QuicStreamId stream_id = kClientDataStreamId1; stream_id < kClientDataStreamId3; stream_id += 2) { QuicStreamId promised_stream_id = NextPromisedStreamId(); - SpdyPushPromiseIR push_promise(stream_id, promised_stream_id, headers_); + SpdyPushPromiseIR push_promise(stream_id, promised_stream_id, + headers_.Clone()); SpdySerializedFrame frame(framer_->SerializeFrame(push_promise)); if (perspective() == Perspective::IS_SERVER) { EXPECT_CALL(*connection_, @@ -436,14 +439,14 @@ TEST_P(QuicHeadersStreamTest, EmptyHeaderHOLBlockedTime) { // Replace with "WriteHeadersAndSaveData" SpdySerializedFrame frame; if (perspective() == Perspective::IS_SERVER) { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); headers_frame.set_has_priority(true); headers_frame.set_weight(Spdy3PriorityToHttp2Weight(0)); frame = framer_->SerializeFrame(headers_frame); EXPECT_CALL(session_, OnStreamHeadersPriority(stream_id, 0)); } else { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); frame = framer_->SerializeFrame(headers_frame); } @@ -471,14 +474,14 @@ TEST_P(QuicHeadersStreamTest, NonEmptyHeaderHOLBlockedTime) { for (int stream_num = 0; stream_num < 10; ++stream_num) { stream_id = QuicClientDataStreamId(stream_num); if (perspective() == Perspective::IS_SERVER) { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); headers_frame.set_has_priority(true); headers_frame.set_weight(Spdy3PriorityToHttp2Weight(0)); frames[stream_num] = framer_->SerializeFrame(headers_frame); EXPECT_CALL(session_, OnStreamHeadersPriority(stream_id, 0)).Times(1); } else { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); frames[stream_num] = framer_->SerializeFrame(headers_frame); } @@ -518,14 +521,14 @@ TEST_P(QuicHeadersStreamTest, ProcessLargeRawData) { // Replace with "WriteHeadersAndSaveData" SpdySerializedFrame frame; if (perspective() == Perspective::IS_SERVER) { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); headers_frame.set_has_priority(true); headers_frame.set_weight(Spdy3PriorityToHttp2Weight(0)); frame = framer_->SerializeFrame(headers_frame); EXPECT_CALL(session_, OnStreamHeadersPriority(stream_id, 0)); } else { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); frame = framer_->SerializeFrame(headers_frame); } @@ -723,14 +726,14 @@ TEST_P(QuicHeadersStreamTest, HpackDecoderDebugVisitor) { // Replace with "WriteHeadersAndSaveData" SpdySerializedFrame frame; if (perspective() == Perspective::IS_SERVER) { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); headers_frame.set_has_priority(true); headers_frame.set_weight(Spdy3PriorityToHttp2Weight(0)); frame = framer_->SerializeFrame(headers_frame); EXPECT_CALL(session_, OnStreamHeadersPriority(stream_id, 0)); } else { - SpdyHeadersIR headers_frame(stream_id, headers_); + SpdyHeadersIR headers_frame(stream_id, headers_.Clone()); headers_frame.set_fin(fin); frame = framer_->SerializeFrame(headers_frame); } diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc index c665be7b61e90b..26f16eef7112ef 100644 --- a/net/quic/quic_http_stream_test.cc +++ b/net/quic/quic_http_stream_test.cc @@ -1368,7 +1368,7 @@ TEST_P(QuicHttpStreamTest, ServerPushGetRequest) { callback_.callback())); // Receive the promised response headers. - response_headers_ = promised_response_; + response_headers_ = promised_response_.Clone(); size_t spdy_response_headers_frame_length; ProcessPacket(InnerConstructResponseHeadersPacket( 1, promise_id_, false, &spdy_response_headers_frame_length)); @@ -1438,7 +1438,7 @@ TEST_P(QuicHttpStreamTest, ServerPushGetRequestSlowResponse) { headers_, &response_, callback_.callback())); // Receive the promised response headers. - response_headers_ = promised_response_; + response_headers_ = promised_response_.Clone(); size_t spdy_response_headers_frame_length; ProcessPacket(InnerConstructResponseHeadersPacket( 1, promise_id_, false, &spdy_response_headers_frame_length)); @@ -1511,7 +1511,7 @@ TEST_P(QuicHttpStreamTest, ServerPushCrossOriginOK) { callback_.callback())); // Receive the promised response headers. - response_headers_ = promised_response_; + response_headers_ = promised_response_.Clone(); size_t spdy_response_headers_frame_length; ProcessPacket(InnerConstructResponseHeadersPacket( 1, promise_id_, false, &spdy_response_headers_frame_length)); @@ -1613,7 +1613,7 @@ TEST_P(QuicHttpStreamTest, ServerPushVaryCheckOK) { // Receive the promised response headers. promised_response_["vary"] = "accept-encoding"; - response_headers_ = promised_response_; + response_headers_ = promised_response_.Clone(); size_t spdy_response_headers_frame_length; ProcessPacket(InnerConstructResponseHeadersPacket( 1, promise_id_, false, &spdy_response_headers_frame_length)); @@ -1703,7 +1703,7 @@ TEST_P(QuicHttpStreamTest, ServerPushVaryCheckFail) { // Receive the promised response headers. promised_response_["vary"] = "accept-encoding"; - response_headers_ = promised_response_; + response_headers_ = promised_response_.Clone(); size_t spdy_response_headers_frame_length; ProcessPacket(InnerConstructResponseHeadersPacket( 1, promise_id_, false, &spdy_response_headers_frame_length)); diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc index d440e96b428381..3d4bce6ed6badc 100644 --- a/net/quic/quic_network_transaction_unittest.cc +++ b/net/quic/quic_network_transaction_unittest.cc @@ -423,7 +423,7 @@ class QuicNetworkTransactionTest ConvertRequestPriorityToQuicPriority(DEFAULT_PRIORITY); return client_maker_.MakeRequestHeadersPacketWithOffsetTracking( packet_number, stream_id, should_include_version, fin, priority, - headers, offset); + std::move(headers), offset); } std::unique_ptr ConstructClientRequestHeadersPacket( @@ -438,7 +438,7 @@ class QuicNetworkTransactionTest ConvertRequestPriorityToQuicPriority(DEFAULT_PRIORITY); return client_maker_.MakeRequestHeadersPacketWithOffsetTracking( packet_number, stream_id, should_include_version, fin, priority, - headers, offset); + std::move(headers), offset); } std::unique_ptr ConstructClientRequestHeadersPacket( @@ -507,7 +507,8 @@ class QuicNetworkTransactionTest SpdyHeaderBlock headers, QuicStreamOffset* offset) { return server_maker_.MakeResponseHeadersPacketWithOffsetTracking( - packet_number, stream_id, should_include_version, fin, headers, offset); + packet_number, stream_id, should_include_version, fin, + std::move(headers), offset); } std::unique_ptr ConstructServerResponseHeadersPacket( @@ -519,7 +520,8 @@ class QuicNetworkTransactionTest QuicStreamOffset* offset, QuicTestPacketMaker* maker) { return server_maker_.MakeResponseHeadersPacketWithOffsetTracking( - packet_number, stream_id, should_include_version, fin, headers, offset); + packet_number, stream_id, should_include_version, fin, + std::move(headers), offset); } void CreateSession() { diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc index a8a21d4eb847ea..9ad209c98fd909 100644 --- a/net/quic/quic_session_test.cc +++ b/net/quic/quic_session_test.cc @@ -5,6 +5,7 @@ #include "net/quic/quic_session.h" #include +#include #include "base/rand_util.h" #include "base/stl_util.h" @@ -815,12 +816,12 @@ TEST_P(QuicSessionTestServer, headers["header"] = base::Uint64ToString(base::RandUint64()) + base::Uint64ToString(base::RandUint64()) + base::Uint64ToString(base::RandUint64()); - headers_stream->WriteHeaders(stream_id, headers, true, 0, nullptr); + headers_stream->WriteHeaders(stream_id, headers.Clone(), true, 0, nullptr); stream_id += 2; } // Write once more to ensure that the headers stream has buffered data. The // random headers may have exactly filled the flow control window. - headers_stream->WriteHeaders(stream_id, headers, true, 0, nullptr); + headers_stream->WriteHeaders(stream_id, std::move(headers), true, 0, nullptr); EXPECT_TRUE(headers_stream->HasBufferedData()); EXPECT_TRUE(headers_stream->flow_controller()->IsBlocked()); diff --git a/net/quic/quic_spdy_stream_test.cc b/net/quic/quic_spdy_stream_test.cc index d08ff4145f7bba..a869ded8911beb 100644 --- a/net/quic/quic_spdy_stream_test.cc +++ b/net/quic/quic_spdy_stream_test.cc @@ -887,14 +887,13 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersSendsAFin) { .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); // Write the initial headers, without a FIN. - EXPECT_CALL(*session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, _, _, _)); stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr); // Writing trailers implicitly sends a FIN. SpdyHeaderBlock trailers; trailers["trailer key"] = "trailer value"; - EXPECT_CALL(*session_, WriteHeaders(_, _, - /*fin=*/true, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, true, _, _)); stream_->WriteTrailers(std::move(trailers), nullptr); EXPECT_TRUE(stream_->fin_sent()); } @@ -908,7 +907,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersFinalOffset) { .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); // Write the initial headers. - EXPECT_CALL(*session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, _, _, _)); stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr); // Write non-zero body data to force a non-zero final offset. @@ -919,11 +918,11 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersFinalOffset) { // number of body bytes written (including queued bytes). SpdyHeaderBlock trailers; trailers["trailer key"] = "trailer value"; - SpdyHeaderBlock trailers_with_offset = trailers; + SpdyHeaderBlock trailers_with_offset(trailers.Clone()); trailers_with_offset[kFinalOffsetHeaderKey] = base::IntToString(kBodySize); - EXPECT_CALL(*session_, WriteHeaders(_, testing::Eq(trailers_with_offset), - /*fin=*/true, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, true, _, _)); stream_->WriteTrailers(std::move(trailers), nullptr); + EXPECT_EQ(trailers_with_offset, session_->GetWriteHeaders()); } TEST_P(QuicSpdyStreamTest, WritingTrailersClosesWriteSide) { @@ -935,7 +934,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersClosesWriteSide) { .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); // Write the initial headers. - EXPECT_CALL(*session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, _, _, _)); stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr); // Write non-zero body data. @@ -945,8 +944,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersClosesWriteSide) { // Headers and body have been fully written, there is no queued data. Writing // trailers marks the end of this stream, and thus the write side is closed. - EXPECT_CALL(*session_, WriteHeaders(_, _, - /*fin=*/true, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, true, _, _)); stream_->WriteTrailers(SpdyHeaderBlock(), nullptr); EXPECT_TRUE(stream_->write_side_closed()); } @@ -960,7 +958,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersWithQueuedBytes) { .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); // Write the initial headers. - EXPECT_CALL(*session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, _, _, _)); stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr); // Write non-zero body data, but only consume partially, ensuring queueing. @@ -972,8 +970,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersWithQueuedBytes) { // Writing trailers will send a FIN, but not close the write side of the // stream as there are queued bytes. - EXPECT_CALL(*session_, WriteHeaders(_, _, - /*fin=*/true, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, true, _, _)); stream_->WriteTrailers(SpdyHeaderBlock(), nullptr); EXPECT_TRUE(stream_->fin_sent()); EXPECT_FALSE(stream_->write_side_closed()); @@ -987,7 +984,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersAfterFIN) { .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); // Write the initial headers, with a FIN. - EXPECT_CALL(*session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(*session_, WriteHeadersMock(_, _, _, _, _)); stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/true, nullptr); EXPECT_TRUE(stream_->fin_sent()); diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index 25e09731d26334..dd0d83d2b21ac2 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "base/macros.h" @@ -557,6 +558,7 @@ class MockQuicSpdySession : public QuicSpdySession { ~MockQuicSpdySession() override; QuicCryptoStream* GetCryptoStream() override { return crypto_stream_.get(); } + const SpdyHeaderBlock& GetWriteHeaders() { return write_headers_; } // From QuicSession. MOCK_METHOD3(OnConnectionClosed, @@ -604,9 +606,21 @@ class MockQuicSpdySession : public QuicSpdySession { QuicStreamId promised_stream_id, size_t frame_len, const QuicHeaderList& header_list)); - MOCK_METHOD5(WriteHeaders, + // Methods taking non-copyable types like SpdyHeaderBlock by value cannot be + // mocked directly. + size_t WriteHeaders( + QuicStreamId id, + SpdyHeaderBlock headers, + bool fin, + SpdyPriority priority, + QuicAckListenerInterface* ack_notifier_delegate) override { + write_headers_ = std::move(headers); + return WriteHeadersMock(id, write_headers_, fin, priority, + ack_notifier_delegate); + } + MOCK_METHOD5(WriteHeadersMock, size_t(QuicStreamId id, - SpdyHeaderBlock headers, + const SpdyHeaderBlock& headers, bool fin, SpdyPriority priority, QuicAckListenerInterface* ack_notifier_delegate)); @@ -616,6 +630,7 @@ class MockQuicSpdySession : public QuicSpdySession { private: std::unique_ptr crypto_stream_; + SpdyHeaderBlock write_headers_; DISALLOW_COPY_AND_ASSIGN(MockQuicSpdySession); }; diff --git a/net/spdy/bidirectional_stream_spdy_impl_unittest.cc b/net/spdy/bidirectional_stream_spdy_impl_unittest.cc index e0340172187856..62572dc3e458d9 100644 --- a/net/spdy/bidirectional_stream_spdy_impl_unittest.cc +++ b/net/spdy/bidirectional_stream_spdy_impl_unittest.cc @@ -63,7 +63,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate { void OnHeadersReceived(const SpdyHeaderBlock& response_headers) override { CHECK(!on_failed_called_); CHECK(!not_expect_callback_); - response_headers_ = response_headers; + response_headers_ = response_headers.Clone(); if (!do_not_start_read_) StartOrContinueReading(); } @@ -87,7 +87,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate { void OnTrailersReceived(const SpdyHeaderBlock& trailers) override { CHECK(!on_failed_called_); - trailers_ = trailers; + trailers_ = trailers.Clone(); if (run_until_completion_) loop_->Quit(); } @@ -163,8 +163,8 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate { const std::string& data_received() const { return data_received_; } int bytes_read() const { return bytes_read_; } int error() const { return error_; } - const SpdyHeaderBlock response_headers() const { return response_headers_; } - const SpdyHeaderBlock trailers() const { return trailers_; } + const SpdyHeaderBlock& response_headers() const { return response_headers_; } + const SpdyHeaderBlock& trailers() const { return trailers_; } int on_data_read_count() const { return on_data_read_count_; } int on_data_sent_count() const { return on_data_sent_count_; } bool on_failed_called() const { return on_failed_called_; } diff --git a/net/spdy/buffered_spdy_framer_unittest.cc b/net/spdy/buffered_spdy_framer_unittest.cc index f70a6bcc50a6b5..3e96481df23388 100644 --- a/net/spdy/buffered_spdy_framer_unittest.cc +++ b/net/spdy/buffered_spdy_framer_unittest.cc @@ -50,7 +50,7 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface { header_stream_id_ = stream_id; EXPECT_NE(header_stream_id_, SpdyFramer::kInvalidStream); syn_frame_count_++; - headers_ = headers; + headers_ = headers.Clone(); } void OnSynReply(SpdyStreamId stream_id, @@ -59,7 +59,7 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface { header_stream_id_ = stream_id; EXPECT_NE(header_stream_id_, SpdyFramer::kInvalidStream); syn_reply_frame_count_++; - headers_ = headers; + headers_ = headers.Clone(); } void OnHeaders(SpdyStreamId stream_id, @@ -72,7 +72,7 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface { header_stream_id_ = stream_id; EXPECT_NE(header_stream_id_, SpdyFramer::kInvalidStream); headers_frame_count_++; - headers_ = headers; + headers_ = headers.Clone(); } void OnDataFrameHeader(SpdyStreamId stream_id, @@ -132,7 +132,7 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface { push_promise_frame_count_++; promised_stream_id_ = promised_stream_id; EXPECT_NE(promised_stream_id_, SpdyFramer::kInvalidStream); - headers_ = headers; + headers_ = headers.Clone(); } void OnAltSvc(SpdyStreamId stream_id, @@ -244,7 +244,7 @@ TEST_P(BufferedSpdyFramerTest, ReadSynStreamHeaderBlock) { framer.CreateSynStream(1, // stream_id 0, // associated_stream_id 1, // priority - CONTROL_FLAG_NONE, headers)); + CONTROL_FLAG_NONE, headers.Clone())); EXPECT_TRUE(control_frame.get() != NULL); TestBufferedSpdyVisitor visitor(spdy_version()); @@ -295,7 +295,7 @@ TEST_P(BufferedSpdyFramerTest, ReadSynReplyHeaderBlock) { BufferedSpdyFramer framer(spdy_version()); std::unique_ptr control_frame( framer.CreateSynReply(1, // stream_id - CONTROL_FLAG_NONE, headers)); + CONTROL_FLAG_NONE, headers.Clone())); EXPECT_TRUE(control_frame.get() != NULL); TestBufferedSpdyVisitor visitor(spdy_version()); @@ -324,7 +324,7 @@ TEST_P(BufferedSpdyFramerTest, ReadHeadersHeaderBlock) { framer.CreateHeaders(1, // stream_id CONTROL_FLAG_NONE, 255, // weight - headers)); + headers.Clone())); EXPECT_TRUE(control_frame.get() != NULL); TestBufferedSpdyVisitor visitor(spdy_version()); @@ -347,7 +347,7 @@ TEST_P(BufferedSpdyFramerTest, ReadPushPromiseHeaderBlock) { headers["gamma"] = "delta"; BufferedSpdyFramer framer(spdy_version()); std::unique_ptr control_frame( - framer.CreatePushPromise(1, 2, headers)); + framer.CreatePushPromise(1, 2, headers.Clone())); EXPECT_TRUE(control_frame.get() != NULL); TestBufferedSpdyVisitor visitor(spdy_version()); diff --git a/net/spdy/hpack/hpack_decoder_test.cc b/net/spdy/hpack/hpack_decoder_test.cc index 9313a73713be72..8657a354ca42ff 100644 --- a/net/spdy/hpack/hpack_decoder_test.cc +++ b/net/spdy/hpack/hpack_decoder_test.cc @@ -310,14 +310,14 @@ TEST_P(HpackDecoderTest, DecodeNextNameInvalidIndex) { // Decoding indexed static table field should work. TEST_P(HpackDecoderTest, IndexedHeaderStatic) { // Reference static table entries #2 and #5. - SpdyHeaderBlock header_set1 = DecodeBlockExpectingSuccess("\x82\x85"); + const SpdyHeaderBlock& header_set1 = DecodeBlockExpectingSuccess("\x82\x85"); SpdyHeaderBlock expected_header_set1; expected_header_set1[":method"] = "GET"; expected_header_set1[":path"] = "/index.html"; EXPECT_EQ(expected_header_set1, header_set1); // Reference static table entry #2. - SpdyHeaderBlock header_set2 = DecodeBlockExpectingSuccess("\x82"); + const SpdyHeaderBlock& header_set2 = DecodeBlockExpectingSuccess("\x82"); SpdyHeaderBlock expected_header_set2; expected_header_set2[":method"] = "GET"; EXPECT_EQ(expected_header_set2, header_set2); @@ -325,7 +325,7 @@ TEST_P(HpackDecoderTest, IndexedHeaderStatic) { TEST_P(HpackDecoderTest, IndexedHeaderDynamic) { // First header block: add an entry to header table. - SpdyHeaderBlock header_set1 = DecodeBlockExpectingSuccess( + const SpdyHeaderBlock& header_set1 = DecodeBlockExpectingSuccess( "\x40\x03" "foo" "\x03" @@ -335,7 +335,7 @@ TEST_P(HpackDecoderTest, IndexedHeaderDynamic) { EXPECT_EQ(expected_header_set1, header_set1); // Second header block: add another entry to header table. - SpdyHeaderBlock header_set2 = DecodeBlockExpectingSuccess( + const SpdyHeaderBlock& header_set2 = DecodeBlockExpectingSuccess( "\xbe\x40\x04" "spam" "\x04" @@ -346,7 +346,7 @@ TEST_P(HpackDecoderTest, IndexedHeaderDynamic) { EXPECT_EQ(expected_header_set2, header_set2); // Third header block: refer to most recently added entry. - SpdyHeaderBlock header_set3 = DecodeBlockExpectingSuccess("\xbe"); + const SpdyHeaderBlock& header_set3 = DecodeBlockExpectingSuccess("\xbe"); SpdyHeaderBlock expected_header_set3; expected_header_set3["spam"] = "eggs"; EXPECT_EQ(expected_header_set3, header_set3); @@ -484,7 +484,7 @@ TEST_P(HpackDecoderTest, LiteralHeaderNoIndexing) { // First header with indexed name, second header with string literal // name. const char input[] = "\x04\x0c/sample/path\x00\x06:path2\x0e/sample/path/2"; - SpdyHeaderBlock header_set = + const SpdyHeaderBlock& header_set = DecodeBlockExpectingSuccess(StringPiece(input, arraysize(input) - 1)); SpdyHeaderBlock expected_header_set; @@ -497,7 +497,7 @@ TEST_P(HpackDecoderTest, LiteralHeaderNoIndexing) { // indexing and string literal names should work. TEST_P(HpackDecoderTest, LiteralHeaderIncrementalIndexing) { const char input[] = "\x44\x0c/sample/path\x40\x06:path2\x0e/sample/path/2"; - SpdyHeaderBlock header_set = + const SpdyHeaderBlock& header_set = DecodeBlockExpectingSuccess(StringPiece(input, arraysize(input) - 1)); SpdyHeaderBlock expected_header_set; @@ -572,8 +572,6 @@ TEST_P(HpackDecoderTest, BasicE21) { } TEST_P(HpackDecoderTest, SectionD4RequestHuffmanExamples) { - SpdyHeaderBlock header_set; - // 82 | == Indexed - Add == // | idx = 2 // | -> :method: GET @@ -592,13 +590,11 @@ TEST_P(HpackDecoderTest, SectionD4RequestHuffmanExamples) { // | Decoded: // | www.example.com // | -> :authority: www.example.com - string first = a2b_hex( - "828684418cf1e3c2e5f23a6ba0ab90f4" - "ff"); - header_set = DecodeBlockExpectingSuccess(first); + string first = a2b_hex("828684418cf1e3c2e5f23a6ba0ab90f4ff"); + const SpdyHeaderBlock& first_header_set = DecodeBlockExpectingSuccess(first); EXPECT_THAT( - header_set, + first_header_set, ElementsAre(Pair(":method", "GET"), Pair(":scheme", "http"), Pair(":path", "/"), Pair(":authority", "www.example.com"))); @@ -628,10 +624,11 @@ TEST_P(HpackDecoderTest, SectionD4RequestHuffmanExamples) { // | -> cache-control: no-cache string second = a2b_hex("828684be5886a8eb10649cbf"); - header_set = DecodeBlockExpectingSuccess(second); + const SpdyHeaderBlock& second_header_set = + DecodeBlockExpectingSuccess(second); EXPECT_THAT( - header_set, + second_header_set, ElementsAre(Pair(":method", "GET"), Pair(":scheme", "http"), Pair(":path", "/"), Pair(":authority", "www.example.com"), Pair("cache-control", "no-cache"))); @@ -667,9 +664,9 @@ TEST_P(HpackDecoderTest, SectionD4RequestHuffmanExamples) { string third = a2b_hex( "828785bf408825a849e95ba97d7f89" "25a849e95bb8e8b4bf"); - header_set = DecodeBlockExpectingSuccess(third); + const SpdyHeaderBlock& third_header_set = DecodeBlockExpectingSuccess(third); - EXPECT_THAT(header_set, + EXPECT_THAT(third_header_set, ElementsAre(Pair(":method", "GET"), Pair(":scheme", "https"), Pair(":path", "/index.html"), Pair(":authority", "www.example.com"), @@ -682,7 +679,6 @@ TEST_P(HpackDecoderTest, SectionD4RequestHuffmanExamples) { } TEST_P(HpackDecoderTest, SectionD6ResponseHuffmanExamples) { - SpdyHeaderBlock header_set; decoder_.ApplyHeaderTableSizeSetting(256); // 48 | == Literal indexed == @@ -732,10 +728,10 @@ TEST_P(HpackDecoderTest, SectionD6ResponseHuffmanExamples) { "941054d444a8200595040b8166e082a6" "2d1bff6e919d29ad171863c78f0b97c8" "e9ae82ae43d3"); - header_set = DecodeBlockExpectingSuccess(first); + const SpdyHeaderBlock& first_header_set = DecodeBlockExpectingSuccess(first); EXPECT_THAT( - header_set, + first_header_set, ElementsAre(Pair(":status", "302"), Pair("cache-control", "private"), Pair("date", "Mon, 21 Oct 2013 20:13:21 GMT"), Pair("location", "https://www.example.com"))); @@ -768,10 +764,11 @@ TEST_P(HpackDecoderTest, SectionD6ResponseHuffmanExamples) { // | -> location: // | https://www.example.com string second = a2b_hex("4883640effc1c0bf"); - header_set = DecodeBlockExpectingSuccess(second); + const SpdyHeaderBlock& second_header_set = + DecodeBlockExpectingSuccess(second); EXPECT_THAT( - header_set, + second_header_set, ElementsAre(Pair(":status", "307"), Pair("cache-control", "private"), Pair("date", "Mon, 21 Oct 2013 20:13:21 GMT"), Pair("location", "https://www.example.com"))); @@ -841,10 +838,10 @@ TEST_P(HpackDecoderTest, SectionD6ResponseHuffmanExamples) { "77ad94e7821dd7f2e6c7b335dfdfcd5b" "3960d5af27087f3672c1ab270fb5291f" "9587316065c003ed4ee5b1063d5007"); - header_set = DecodeBlockExpectingSuccess(third); + const SpdyHeaderBlock& third_header_set = DecodeBlockExpectingSuccess(third); EXPECT_THAT( - header_set, + third_header_set, ElementsAre(Pair(":status", "200"), Pair("cache-control", "private"), Pair("date", "Mon, 21 Oct 2013 20:13:22 GMT"), Pair("location", "https://www.example.com"), diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc index 1bbdf91490a59c..0c8b071d14e4d6 100644 --- a/net/spdy/spdy_framer_test.cc +++ b/net/spdy/spdy_framer_test.cc @@ -92,7 +92,7 @@ class SpdyFramerTestUtil { void OnHeaderFrameEnd(SpdyStreamId stream_id, bool end_headers) override { CHECK(!finished_); - frame_->set_header_block(headers_handler_->decoded_block()); + frame_->set_header_block(headers_handler_->decoded_block().Clone()); finished_ = true; if (end_headers) { headers_handler_.reset(); @@ -494,7 +494,7 @@ class TestSpdyVisitor : public SpdyFramerVisitorInterface, void OnHeaderFrameEnd(SpdyStreamId stream_id, bool end_headers) override { CHECK(headers_handler_ != nullptr); - headers_ = headers_handler_->decoded_block(); + headers_ = headers_handler_->decoded_block().Clone(); header_bytes_received_ = headers_handler_->header_bytes_parsed(); if (end_headers) { headers_handler_.reset(); @@ -1958,7 +1958,7 @@ TEST_P(SpdyFramerTest, HeaderCompression) { SpdyHeaderBlock block; block[kHeader1] = kValue1; block[kHeader2] = kValue2; - SpdySynStreamIR syn_ir_1(1, block); + SpdySynStreamIR syn_ir_1(1, block.Clone()); SpdySerializedFrame syn_frame_1(send_framer.SerializeFrame(syn_ir_1)); // SYN_STREAM #2 @@ -3969,7 +3969,6 @@ TEST_P(SpdyFramerTest, ReadCompressedSynStreamHeaderBlock) { syn_stream.set_priority(1); syn_stream.SetHeader("aa", "vv"); syn_stream.SetHeader("bb", "ww"); - SpdyHeaderBlock headers = syn_stream.header_block(); SpdySerializedFrame control_frame(framer.SerializeSynStream(syn_stream)); TestSpdyVisitor visitor(spdy_version_); visitor.use_compression_ = true; @@ -3977,7 +3976,7 @@ TEST_P(SpdyFramerTest, ReadCompressedSynStreamHeaderBlock) { reinterpret_cast(control_frame.data()), control_frame.size()); EXPECT_EQ(1, visitor.syn_frame_count_); - EXPECT_EQ(headers, visitor.headers_); + EXPECT_EQ(syn_stream.header_block(), visitor.headers_); } TEST_P(SpdyFramerTest, ReadCompressedSynReplyHeaderBlock) { @@ -3989,7 +3988,6 @@ TEST_P(SpdyFramerTest, ReadCompressedSynReplyHeaderBlock) { SpdySynReplyIR syn_reply(1); syn_reply.SetHeader("alpha", "beta"); syn_reply.SetHeader("gamma", "delta"); - SpdyHeaderBlock headers = syn_reply.header_block(); SpdySerializedFrame control_frame(framer.SerializeSynReply(syn_reply)); TestSpdyVisitor visitor(spdy_version_); visitor.use_compression_ = true; @@ -3998,7 +3996,7 @@ TEST_P(SpdyFramerTest, ReadCompressedSynReplyHeaderBlock) { control_frame.size()); EXPECT_EQ(1, visitor.syn_reply_frame_count_); EXPECT_EQ(0, visitor.headers_frame_count_); - EXPECT_EQ(headers, visitor.headers_); + EXPECT_EQ(syn_reply.header_block(), visitor.headers_); } TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlock) { @@ -4006,7 +4004,6 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlock) { SpdyHeadersIR headers_ir(1); headers_ir.SetHeader("alpha", "beta"); headers_ir.SetHeader("gamma", "delta"); - SpdyHeaderBlock headers = headers_ir.header_block(); SpdySerializedFrame control_frame(framer.SerializeHeaders(headers_ir)); TestSpdyVisitor visitor(spdy_version_); visitor.use_compression_ = true; @@ -4017,7 +4014,7 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlock) { EXPECT_EQ(0, visitor.control_frame_header_data_count_); EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_); EXPECT_EQ(0, visitor.end_of_stream_count_); - EXPECT_EQ(headers, visitor.headers_); + EXPECT_EQ(headers_ir.header_block(), visitor.headers_); } TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlockWithHalfClose) { @@ -4026,7 +4023,6 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlockWithHalfClose) { headers_ir.set_fin(true); headers_ir.SetHeader("alpha", "beta"); headers_ir.SetHeader("gamma", "delta"); - SpdyHeaderBlock headers = headers_ir.header_block(); SpdySerializedFrame control_frame(framer.SerializeHeaders(headers_ir)); TestSpdyVisitor visitor(spdy_version_); visitor.use_compression_ = true; @@ -4037,7 +4033,7 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlockWithHalfClose) { EXPECT_EQ(0, visitor.control_frame_header_data_count_); EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_); EXPECT_EQ(1, visitor.end_of_stream_count_); - EXPECT_EQ(headers, visitor.headers_); + EXPECT_EQ(headers_ir.header_block(), visitor.headers_); } TEST_P(SpdyFramerTest, ControlFrameAtMaxSizeLimit) { @@ -4620,7 +4616,6 @@ TEST_P(SpdyFramerTest, ReadCompressedPushPromise) { SpdyPushPromiseIR push_promise(42, 57); push_promise.SetHeader("foo", "bar"); push_promise.SetHeader("bar", "foofoo"); - SpdyHeaderBlock headers = push_promise.header_block(); SpdySerializedFrame frame(framer.SerializePushPromise(push_promise)); TestSpdyVisitor visitor(spdy_version_); visitor.use_compression_ = true; @@ -4628,7 +4623,7 @@ TEST_P(SpdyFramerTest, ReadCompressedPushPromise) { frame.size()); EXPECT_EQ(42u, visitor.last_push_promise_stream_); EXPECT_EQ(57u, visitor.last_push_promise_promised_stream_); - EXPECT_EQ(headers, visitor.headers_); + EXPECT_EQ(push_promise.header_block(), visitor.headers_); } TEST_P(SpdyFramerTest, ReadHeadersWithContinuation) { diff --git a/net/spdy/spdy_header_block.cc b/net/spdy/spdy_header_block.cc index 61ca9af575137b..09640c83ed6fda 100644 --- a/net/spdy/spdy_header_block.cc +++ b/net/spdy/spdy_header_block.cc @@ -120,13 +120,6 @@ SpdyHeaderBlock::StringPieceProxy::operator StringPiece() const { SpdyHeaderBlock::SpdyHeaderBlock() : storage_(new Storage) {} -SpdyHeaderBlock::SpdyHeaderBlock(const SpdyHeaderBlock& other) - : storage_(new Storage) { - for (auto iter : other) { - AppendHeader(iter.first, iter.second); - } -} - SpdyHeaderBlock::SpdyHeaderBlock(SpdyHeaderBlock&& other) : storage_(std::move(other.storage_)) { // |block_| is linked_hash_map, which does not have move constructor. @@ -135,14 +128,6 @@ SpdyHeaderBlock::SpdyHeaderBlock(SpdyHeaderBlock&& other) SpdyHeaderBlock::~SpdyHeaderBlock() {} -SpdyHeaderBlock& SpdyHeaderBlock::operator=(const SpdyHeaderBlock& other) { - clear(); - for (auto iter : other) { - AppendHeader(iter.first, iter.second); - } - return *this; -} - SpdyHeaderBlock& SpdyHeaderBlock::operator=(SpdyHeaderBlock&& other) { storage_ = std::move(other.storage_); // |block_| is linked_hash_map, which does not have move assignment @@ -151,6 +136,14 @@ SpdyHeaderBlock& SpdyHeaderBlock::operator=(SpdyHeaderBlock&& other) { return *this; } +SpdyHeaderBlock SpdyHeaderBlock::Clone() const { + SpdyHeaderBlock copy; + for (auto iter : *this) { + copy.AppendHeader(iter.first, iter.second); + } + return copy; +} + bool SpdyHeaderBlock::operator==(const SpdyHeaderBlock& other) const { return size() == other.size() && std::equal(begin(), end(), other.begin()); } diff --git a/net/spdy/spdy_header_block.h b/net/spdy/spdy_header_block.h index 3260c81ac51d31..0afde3159f542e 100644 --- a/net/spdy/spdy_header_block.h +++ b/net/spdy/spdy_header_block.h @@ -11,6 +11,7 @@ #include #include +#include "base/macros.h" #include "base/strings/string_piece.h" #include "net/base/linked_hash_map.h" #include "net/base/net_export.h" @@ -52,12 +53,14 @@ class NET_EXPORT SpdyHeaderBlock { class StringPieceProxy; SpdyHeaderBlock(); - SpdyHeaderBlock(const SpdyHeaderBlock& other); + SpdyHeaderBlock(const SpdyHeaderBlock& other) = delete; SpdyHeaderBlock(SpdyHeaderBlock&& other); ~SpdyHeaderBlock(); - SpdyHeaderBlock& operator=(const SpdyHeaderBlock& other); + SpdyHeaderBlock& operator=(const SpdyHeaderBlock& other) = delete; SpdyHeaderBlock& operator=(SpdyHeaderBlock&& other); + SpdyHeaderBlock Clone() const; + bool operator==(const SpdyHeaderBlock& other) const; bool operator!=(const SpdyHeaderBlock& other) const; diff --git a/net/spdy/spdy_header_block_test.cc b/net/spdy/spdy_header_block_test.cc index b0a533d3dace85..b4273af5c77290 100644 --- a/net/spdy/spdy_header_block_test.cc +++ b/net/spdy/spdy_header_block_test.cc @@ -96,17 +96,15 @@ TEST(SpdyHeaderBlockTest, AddHeaders) { EXPECT_EQ("", block1.GetHeader("key")); } -// This test verifies that SpdyHeaderBlock can be copied. +// This test verifies that SpdyHeaderBlock can be copied using Clone(). TEST(SpdyHeaderBlockTest, CopyBlocks) { SpdyHeaderBlock block1; block1["foo"] = string(300, 'x'); block1["bar"] = "baz"; block1.ReplaceOrAppendHeader("qux", "qux1"); - SpdyHeaderBlock block2; - block2 = block1; - - SpdyHeaderBlock block3(block1); + SpdyHeaderBlock block2 = block1.Clone(); + SpdyHeaderBlock block3(block1.Clone()); EXPECT_EQ(block1, block2); EXPECT_EQ(block1, block3); diff --git a/net/spdy/spdy_protocol.h b/net/spdy/spdy_protocol.h index 2777ad1e02e9f6..ff492e293e8a77 100644 --- a/net/spdy/spdy_protocol.h +++ b/net/spdy/spdy_protocol.h @@ -718,9 +718,9 @@ class NET_EXPORT_PRIVATE SpdyFrameWithHeaderBlockIR ~SpdyFrameWithHeaderBlockIR() override; const SpdyHeaderBlock& header_block() const { return header_block_; } - void set_header_block(const SpdyHeaderBlock& header_block) { + void set_header_block(SpdyHeaderBlock header_block) { // Deep copy. - header_block_ = header_block; + header_block_ = std::move(header_block); } void SetHeader(base::StringPiece name, base::StringPiece value) { header_block_[name] = value; diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc index e3a10bd5d77236..1aafe5ef60e565 100644 --- a/net/spdy/spdy_session_unittest.cc +++ b/net/spdy/spdy_session_unittest.cc @@ -412,7 +412,8 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreams) { std::unique_ptr headers( new SpdyHeaderBlock(spdy_util_.ConstructGetHeaderBlock(kDefaultUrl))); - std::unique_ptr headers2(new SpdyHeaderBlock(*headers)); + std::unique_ptr headers2( + new SpdyHeaderBlock(headers->Clone())); spdy_stream1->SendRequestHeaders(std::move(headers), NO_MORE_DATA_TO_SEND); EXPECT_TRUE(spdy_stream1->HasUrlFromHeaders()); @@ -551,7 +552,8 @@ TEST_P(SpdySessionTest, GoAwayTwice) { std::unique_ptr headers( new SpdyHeaderBlock(spdy_util_.ConstructGetHeaderBlock(kDefaultUrl))); - std::unique_ptr headers2(new SpdyHeaderBlock(*headers)); + std::unique_ptr headers2( + new SpdyHeaderBlock(headers->Clone())); spdy_stream1->SendRequestHeaders(std::move(headers), NO_MORE_DATA_TO_SEND); EXPECT_TRUE(spdy_stream1->HasUrlFromHeaders()); @@ -625,7 +627,8 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreamsThenClose) { std::unique_ptr headers( new SpdyHeaderBlock(spdy_util_.ConstructGetHeaderBlock(kDefaultUrl))); - std::unique_ptr headers2(new SpdyHeaderBlock(*headers)); + std::unique_ptr headers2( + new SpdyHeaderBlock(headers->Clone())); spdy_stream1->SendRequestHeaders(std::move(headers), NO_MORE_DATA_TO_SEND); EXPECT_TRUE(spdy_stream1->HasUrlFromHeaders()); diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc index 078ea22e9c3014..09083a00f28d6e 100644 --- a/net/spdy/spdy_stream.cc +++ b/net/spdy/spdy_stream.cc @@ -208,7 +208,7 @@ std::unique_ptr SpdyStream::ProduceSynStreamFrame() { (pending_send_status_ == NO_MORE_DATA_TO_SEND) ? CONTROL_FLAG_FIN : CONTROL_FLAG_NONE; std::unique_ptr frame(session_->CreateSynStream( - stream_id_, priority_, flags, *request_headers_)); + stream_id_, priority_, flags, request_headers_->Clone())); send_time_ = base::TimeTicks::Now(); return frame; } @@ -470,7 +470,7 @@ void SpdyStream::OnPushPromiseHeadersReceived(const SpdyHeaderBlock& headers) { DCHECK(!delegate_); io_state_ = STATE_RESERVED_REMOTE; - request_headers_.reset(new SpdyHeaderBlock(headers)); + request_headers_.reset(new SpdyHeaderBlock(headers.Clone())); } void SpdyStream::OnDataReceived(std::unique_ptr buffer) { diff --git a/net/spdy/spdy_stream_test_util.cc b/net/spdy/spdy_stream_test_util.cc index e052b881b8180f..cbc53c2db4c51c 100644 --- a/net/spdy/spdy_stream_test_util.cc +++ b/net/spdy/spdy_stream_test_util.cc @@ -61,7 +61,7 @@ void StreamDelegateBase::OnRequestHeadersSent() { SpdyResponseHeadersStatus StreamDelegateBase::OnResponseHeadersUpdated( const SpdyHeaderBlock& response_headers) { EXPECT_EQ(stream_->type() != SPDY_PUSH_STREAM, send_headers_completed_); - response_headers_ = response_headers; + response_headers_ = response_headers.Clone(); return RESPONSE_HEADERS_ARE_COMPLETE; } diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc index 23e3c4c3812b07..f21d14b692a558 100644 --- a/net/spdy/spdy_stream_unittest.cc +++ b/net/spdy/spdy_stream_unittest.cc @@ -226,7 +226,7 @@ class StreamDelegateWithTrailers : public test::StreamDelegateWithBody { ~StreamDelegateWithTrailers() override {} void OnTrailers(const SpdyHeaderBlock& trailers) override { - trailers_ = trailers; + trailers_ = trailers.Clone(); } const SpdyHeaderBlock& trailers() const { return trailers_; } diff --git a/net/tools/flip_server/spdy_interface.cc b/net/tools/flip_server/spdy_interface.cc index 67139934686ea6..247cc2c546f07f 100644 --- a/net/tools/flip_server/spdy_interface.cc +++ b/net/tools/flip_server/spdy_interface.cc @@ -6,6 +6,7 @@ #include #include +#include #include "net/spdy/spdy_framer.h" #include "net/spdy/spdy_protocol.h" @@ -472,7 +473,7 @@ size_t SpdySM::SendSynStreamImpl(uint32_t stream_id, DCHECK(buffered_spdy_framer_); SpdySerializedFrame* fsrcf = buffered_spdy_framer_->CreateSynStream( - stream_id, 0, 0, CONTROL_FLAG_NONE, block); + stream_id, 0, 0, CONTROL_FLAG_NONE, std::move(block)); size_t df_size = fsrcf->size(); EnqueueDataFrame(new SpdyFrameDataFrame(fsrcf)); @@ -491,7 +492,7 @@ size_t SpdySM::SendSynReplyImpl(uint32_t stream_id, DCHECK(buffered_spdy_framer_); SpdySerializedFrame* fsrcf = buffered_spdy_framer_->CreateSynReply( - stream_id, CONTROL_FLAG_NONE, block); + stream_id, CONTROL_FLAG_NONE, std::move(block)); size_t df_size = fsrcf->size(); EnqueueDataFrame(new SpdyFrameDataFrame(fsrcf)); diff --git a/net/tools/flip_server/spdy_interface_test.cc b/net/tools/flip_server/spdy_interface_test.cc index d5d657586ecfca..a62da42dd6befe 100644 --- a/net/tools/flip_server/spdy_interface_test.cc +++ b/net/tools/flip_server/spdy_interface_test.cc @@ -49,15 +49,44 @@ class SpdyFramerVisitor : public BufferedSpdyFramerVisitorInterface { virtual ~SpdyFramerVisitor() {} MOCK_METHOD1(OnError, void(SpdyFramer::SpdyError)); MOCK_METHOD2(OnStreamError, void(SpdyStreamId, const std::string&)); - MOCK_METHOD6(OnSynStream, + // SaveArg cannot be used on non-copyable types like SpdyHeaderBlock. + void OnSynStream(SpdyStreamId stream_id, + SpdyStreamId associated_stream_id, + SpdyPriority priority, + bool fin, + bool unidirectional, + const SpdyHeaderBlock& headers) override { + actual_header_block_ = headers.Clone(); + OnSynStreamMock(stream_id, associated_stream_id, priority, fin, + unidirectional, headers); + } + MOCK_METHOD6(OnSynStreamMock, void(SpdyStreamId, SpdyStreamId, SpdyPriority, bool, bool, const SpdyHeaderBlock&)); - MOCK_METHOD3(OnSynReply, void(SpdyStreamId, bool, const SpdyHeaderBlock&)); - MOCK_METHOD7(OnHeaders, + void OnSynReply(SpdyStreamId stream_id, + bool fin, + const SpdyHeaderBlock& headers) override { + actual_header_block_ = headers.Clone(); + OnSynReplyMock(stream_id, fin, headers); + } + MOCK_METHOD3(OnSynReplyMock, + void(SpdyStreamId, bool, const SpdyHeaderBlock&)); + void OnHeaders(SpdyStreamId stream_id, + bool has_priority, + int weight, + SpdyStreamId parent_stream_id, + bool exclusive, + bool fin, + const SpdyHeaderBlock& headers) override { + actual_header_block_ = headers.Clone(); + OnHeadersMock(stream_id, has_priority, weight, parent_stream_id, exclusive, + fin, headers); + } + MOCK_METHOD7(OnHeadersMock, void(SpdyStreamId stream_id, bool has_priority, int weight, @@ -86,6 +115,8 @@ class SpdyFramerVisitor : public BufferedSpdyFramerVisitorInterface { base::StringPiece, const SpdyAltSvcWireFormat::AlternativeServiceVector&)); MOCK_METHOD2(OnUnknownFrame, bool(SpdyStreamId stream_id, int frame_type)); + + SpdyHeaderBlock actual_header_block_; }; class FakeSMConnection : public SMConnection { @@ -366,7 +397,6 @@ TEST_P(SpdySMProxyTest, AddToOutputOrder) { TEST_P(SpdySMProxyTest, SendErrorNotFound) { uint32_t stream_id = 82; - SpdyHeaderBlock actual_header_block; const char* actual_data; size_t actual_size; testing::MockFunction checkpoint; // NOLINT @@ -378,14 +408,11 @@ TEST_P(SpdySMProxyTest, SendErrorNotFound) { { InSequence s; if (GetParam() < HTTP2) { - EXPECT_CALL(*spdy_framer_visitor_, - OnSynReply(stream_id, false, _)) - .WillOnce(SaveArg<2>(&actual_header_block)); + EXPECT_CALL(*spdy_framer_visitor_, OnSynReplyMock(stream_id, false, _)); } else { EXPECT_CALL(*spdy_framer_visitor_, - OnHeaders(stream_id, /*has_priority=*/false, _, _, _, - /*fin=*/false, _)) - .WillOnce(SaveArg<6>(&actual_header_block)); + OnHeadersMock(stream_id, /*has_priority=*/false, _, _, _, + /*fin=*/false, _)); } EXPECT_CALL(checkpoint, Call(0)); EXPECT_CALL(*spdy_framer_visitor_, @@ -405,16 +432,16 @@ TEST_P(SpdySMProxyTest, SendErrorNotFound) { spdy_framer_->ProcessInput(df->data, df->size); ASSERT_EQ(2, spdy_framer_->frames_received()); - ASSERT_EQ(2u, actual_header_block.size()); - ASSERT_EQ("404 Not Found", actual_header_block[":status"]); - ASSERT_EQ("HTTP/1.1", actual_header_block[":version"]); + ASSERT_EQ(2u, spdy_framer_visitor_->actual_header_block_.size()); + ASSERT_EQ("404 Not Found", + spdy_framer_visitor_->actual_header_block_[":status"]); + ASSERT_EQ("HTTP/1.1", spdy_framer_visitor_->actual_header_block_[":version"]); ASSERT_EQ("wtf?", StringPiece(actual_data, actual_size)); } TEST_P(SpdySMProxyTest, SendSynStream) { uint32_t stream_id = 82; BalsaHeaders headers; - SpdyHeaderBlock actual_header_block; headers.AppendHeader("key1", "value1"); headers.AppendHeader("Host", "www.example.com"); headers.SetRequestFirstlineFromStringPieces("GET", "/path", "HTTP/1.1"); @@ -428,24 +455,23 @@ TEST_P(SpdySMProxyTest, SendSynStream) { { InSequence s; EXPECT_CALL(*spdy_framer_visitor_, - OnSynStream(stream_id, 0, _, false, false, _)) - .WillOnce(SaveArg<5>(&actual_header_block)); + OnSynStreamMock(stream_id, 0, _, false, false, _)); } spdy_framer_->ProcessInput(df->data, df->size); ASSERT_EQ(1, spdy_framer_->frames_received()); - ASSERT_EQ(5u, actual_header_block.size()); - ASSERT_EQ("GET", actual_header_block[":method"]); - ASSERT_EQ("HTTP/1.1", actual_header_block[":version"]); - ASSERT_EQ("/path", actual_header_block[":path"]); - ASSERT_EQ("www.example.com", actual_header_block[":host"]); - ASSERT_EQ("value1", actual_header_block["key1"]); + ASSERT_EQ(5u, spdy_framer_visitor_->actual_header_block_.size()); + ASSERT_EQ("GET", spdy_framer_visitor_->actual_header_block_[":method"]); + ASSERT_EQ("HTTP/1.1", spdy_framer_visitor_->actual_header_block_[":version"]); + ASSERT_EQ("/path", spdy_framer_visitor_->actual_header_block_[":path"]); + ASSERT_EQ("www.example.com", + spdy_framer_visitor_->actual_header_block_[":host"]); + ASSERT_EQ("value1", spdy_framer_visitor_->actual_header_block_["key1"]); } TEST_P(SpdySMProxyTest, SendSynReply) { uint32_t stream_id = 82; BalsaHeaders headers; - SpdyHeaderBlock actual_header_block; headers.AppendHeader("key1", "value1"); headers.SetResponseFirstlineFromStringPieces("HTTP/1.1", "200", "OK"); @@ -458,22 +484,20 @@ TEST_P(SpdySMProxyTest, SendSynReply) { { InSequence s; if (GetParam() < HTTP2) { - EXPECT_CALL(*spdy_framer_visitor_, OnSynReply(stream_id, false, _)) - .WillOnce(SaveArg<2>(&actual_header_block)); + EXPECT_CALL(*spdy_framer_visitor_, OnSynReplyMock(stream_id, false, _)); } else { EXPECT_CALL(*spdy_framer_visitor_, - OnHeaders(stream_id, /*has_priority=*/false, _, _, _, - /*fin=*/false, _)) - .WillOnce(SaveArg<6>(&actual_header_block)); + OnHeadersMock(stream_id, /*has_priority=*/false, _, _, _, + /*fin=*/false, _)); } } spdy_framer_->ProcessInput(df->data, df->size); ASSERT_EQ(1, spdy_framer_->frames_received()); - ASSERT_EQ(3u, actual_header_block.size()); - ASSERT_EQ("200 OK", actual_header_block[":status"]); - ASSERT_EQ("HTTP/1.1", actual_header_block[":version"]); - ASSERT_EQ("value1", actual_header_block["key1"]); + ASSERT_EQ(3u, spdy_framer_visitor_->actual_header_block_.size()); + ASSERT_EQ("200 OK", spdy_framer_visitor_->actual_header_block_[":status"]); + ASSERT_EQ("HTTP/1.1", spdy_framer_visitor_->actual_header_block_[":version"]); + ASSERT_EQ("value1", spdy_framer_visitor_->actual_header_block_["key1"]); } TEST_P(SpdySMProxyTest, SendDataFrame) { @@ -571,7 +595,6 @@ TEST_P(SpdySMServerTest, NewStream) { TEST_P(SpdySMServerTest, NewStreamError) { uint32_t stream_id = 82; - SpdyHeaderBlock actual_header_block; const char* actual_data; size_t actual_size; testing::MockFunction checkpoint; // NOLINT @@ -583,13 +606,11 @@ TEST_P(SpdySMServerTest, NewStreamError) { { InSequence s; if (GetParam() < HTTP2) { - EXPECT_CALL(*spdy_framer_visitor_, OnSynReply(stream_id, false, _)) - .WillOnce(SaveArg<2>(&actual_header_block)); + EXPECT_CALL(*spdy_framer_visitor_, OnSynReplyMock(stream_id, false, _)); } else { EXPECT_CALL(*spdy_framer_visitor_, - OnHeaders(stream_id, /*has_priority=*/false, _, _, _, - /*fin=*/false, _)) - .WillOnce(SaveArg<6>(&actual_header_block)); + OnHeadersMock(stream_id, /*has_priority=*/false, _, _, _, + /*fin=*/false, _)); } EXPECT_CALL(checkpoint, Call(0)); EXPECT_CALL(*spdy_framer_visitor_, @@ -609,9 +630,10 @@ TEST_P(SpdySMServerTest, NewStreamError) { spdy_framer_->ProcessInput(df->data, df->size); ASSERT_EQ(2, spdy_framer_->frames_received()); - ASSERT_EQ(2u, actual_header_block.size()); - ASSERT_EQ("404 Not Found", actual_header_block["status"]); - ASSERT_EQ("HTTP/1.1", actual_header_block["version"]); + ASSERT_EQ(2u, spdy_framer_visitor_->actual_header_block_.size()); + ASSERT_EQ("404 Not Found", + spdy_framer_visitor_->actual_header_block_["status"]); + ASSERT_EQ("HTTP/1.1", spdy_framer_visitor_->actual_header_block_["version"]); ASSERT_EQ("wtf?", StringPiece(actual_data, actual_size)); } diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index 8a95dea48e9556..7f4451c94cc24b 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "base/memory/singleton.h" @@ -2110,7 +2111,7 @@ class ServerStreamWithErrorResponseBody : public QuicSimpleServerStream { // This method must call CloseReadSide to cause the test case, StopReading // is not sufficient. ReliableQuicStreamPeer::CloseReadSide(this); - SendHeadersAndBody(headers, response_body_); + SendHeadersAndBody(std::move(headers), response_body_); } string response_body_; @@ -2197,8 +2198,8 @@ class ServerStreamThatSendsHugeResponse : public QuicSimpleServerStream { string body; test::GenerateBody(&body, body_bytes_); response.set_body(body); - SendHeadersAndBodyAndTrailers(response.headers(), response.body(), - response.trailers()); + SendHeadersAndBodyAndTrailers(response.headers().Clone(), response.body(), + response.trailers().Clone()); } private: @@ -2448,7 +2449,8 @@ TEST_P(EndToEndTest, Trailers) { trailers["some-trailing-header"] = "trailing-header-value"; QuicInMemoryCache::GetInstance()->AddResponse( - "www.google.com", "/trailer_url", headers, kBody, trailers); + "www.google.com", "/trailer_url", std::move(headers), kBody, + trailers.Clone()); EXPECT_EQ(kBody, client_->SendSynchronousRequest("/trailer_url")); EXPECT_EQ(200u, client_->response_headers()->parsed_response_code()); @@ -2498,7 +2500,7 @@ class EndToEndTestServerPush : public EndToEndTest { response_headers[":status"] = "200"; response_headers["content-length"] = IntToString(body.size()); push_resources.push_back(QuicInMemoryCache::ServerPushInfo( - resource_url, response_headers, kV3LowestPriority, body)); + resource_url, std::move(response_headers), kV3LowestPriority, body)); } QuicInMemoryCache::GetInstance()->AddSimpleResponseWithServerPushResources( diff --git a/net/tools/quic/quic_in_memory_cache.cc b/net/tools/quic/quic_in_memory_cache.cc index 4efd73d88fa8ac..3b2f9e00b57d8a 100644 --- a/net/tools/quic/quic_in_memory_cache.cc +++ b/net/tools/quic/quic_in_memory_cache.cc @@ -4,6 +4,8 @@ #include "net/tools/quic/quic_in_memory_cache.h" +#include + #include "base/files/file_enumerator.h" #include "base/files/file_util.h" #include "base/stl_util.h" @@ -80,18 +82,20 @@ class ResourceFileImpl : public net::QuicInMemoryCache::ResourceFile { } // namespace -QuicInMemoryCache::ServerPushInfo::ServerPushInfo( - GURL request_url, - const SpdyHeaderBlock& headers, - net::SpdyPriority priority, - string body) +QuicInMemoryCache::ServerPushInfo::ServerPushInfo(GURL request_url, + SpdyHeaderBlock headers, + net::SpdyPriority priority, + string body) : request_url(request_url), - headers(headers), + headers(std::move(headers)), priority(priority), body(body) {} -QuicInMemoryCache::ServerPushInfo::ServerPushInfo(const ServerPushInfo& other) = - default; +QuicInMemoryCache::ServerPushInfo::ServerPushInfo(const ServerPushInfo& other) + : request_url(other.request_url), + headers(other.headers.Clone()), + priority(other.priority), + body(other.body) {} QuicInMemoryCache::Response::Response() : response_type_(REGULAR_RESPONSE) {} @@ -160,7 +164,7 @@ void QuicInMemoryCache::AddSimpleResponse(StringPiece host, response_headers[":status"] = IntToString(response_code); response_headers["content-length"] = IntToString(static_cast(body.length())); - AddResponse(host, path, response_headers, body); + AddResponse(host, path, std::move(response_headers), body); } void QuicInMemoryCache::AddSimpleResponseWithServerPushResources( @@ -179,19 +183,19 @@ void QuicInMemoryCache::AddDefaultResponse(Response* response) { void QuicInMemoryCache::AddResponse(StringPiece host, StringPiece path, - const SpdyHeaderBlock& response_headers, + SpdyHeaderBlock response_headers, StringPiece response_body) { - AddResponseImpl(host, path, REGULAR_RESPONSE, response_headers, response_body, - SpdyHeaderBlock()); + AddResponseImpl(host, path, REGULAR_RESPONSE, std::move(response_headers), + response_body, SpdyHeaderBlock()); } void QuicInMemoryCache::AddResponse(StringPiece host, StringPiece path, - const SpdyHeaderBlock& response_headers, + SpdyHeaderBlock response_headers, StringPiece response_body, - const SpdyHeaderBlock& response_trailers) { - AddResponseImpl(host, path, REGULAR_RESPONSE, response_headers, response_body, - response_trailers); + SpdyHeaderBlock response_trailers) { + AddResponseImpl(host, path, REGULAR_RESPONSE, std::move(response_headers), + response_body, std::move(response_trailers)); } void QuicInMemoryCache::AddSpecialResponse(StringPiece host, @@ -239,7 +243,7 @@ void QuicInMemoryCache::InitializeFromDirectory(const string& cache_directory) { resource_file->Read(); AddResponse(resource_file->host(), resource_file->path(), - resource_file->spdy_headers(), resource_file->body()); + resource_file->spdy_headers().Clone(), resource_file->body()); resource_files.push_back(std::move(resource_file)); } @@ -253,7 +257,7 @@ void QuicInMemoryCache::InitializeFromDirectory(const string& cache_directory) { QUIC_BUG << "Push URL '" << push_url << "' not found."; return; } - push_resources.push_back(ServerPushInfo(url, response->headers(), + push_resources.push_back(ServerPushInfo(url, response->headers().Clone(), net::kV3LowestPriority, response->body().as_string())); } @@ -278,13 +282,12 @@ QuicInMemoryCache::~QuicInMemoryCache() { STLDeleteValues(&responses_); } -void QuicInMemoryCache::AddResponseImpl( - StringPiece host, - StringPiece path, - SpecialResponseType response_type, - const SpdyHeaderBlock& response_headers, - StringPiece response_body, - const SpdyHeaderBlock& response_trailers) { +void QuicInMemoryCache::AddResponseImpl(StringPiece host, + StringPiece path, + SpecialResponseType response_type, + SpdyHeaderBlock response_headers, + StringPiece response_body, + SpdyHeaderBlock response_trailers) { DCHECK(!host.empty()) << "Host must be populated, e.g. \"www.google.com\""; string key = GetKey(host, path); if (ContainsKey(responses_, key)) { @@ -293,9 +296,9 @@ void QuicInMemoryCache::AddResponseImpl( } Response* new_response = new Response(); new_response->set_response_type(response_type); - new_response->set_headers(response_headers); + new_response->set_headers(std::move(response_headers)); new_response->set_body(response_body); - new_response->set_trailers(response_trailers); + new_response->set_trailers(std::move(response_trailers)); DVLOG(1) << "Add response with key " << key; responses_[key] = new_response; } @@ -326,11 +329,10 @@ void QuicInMemoryCache::MaybeAddServerPushResources( string path = push_resource.request_url.path(); if (responses_.find(GetKey(host, path)) == responses_.end()) { // Add a server push response to responses map, if it is not in the map. - SpdyHeaderBlock headers = push_resource.headers; StringPiece body = push_resource.body; DVLOG(1) << "Add response for push resource: host " << host << " path " << path; - AddResponse(host, path, headers, body); + AddResponse(host, path, push_resource.headers.Clone(), body); } } } diff --git a/net/tools/quic/quic_in_memory_cache.h b/net/tools/quic/quic_in_memory_cache.h index 21477c3f40d461..2747c9303be7c2 100644 --- a/net/tools/quic/quic_in_memory_cache.h +++ b/net/tools/quic/quic_in_memory_cache.h @@ -45,7 +45,7 @@ class QuicInMemoryCache { // comprising a response for the push request. struct ServerPushInfo { ServerPushInfo(GURL request_url, - const SpdyHeaderBlock& headers, + SpdyHeaderBlock headers, SpdyPriority priority, std::string body); ServerPushInfo(const ServerPushInfo& other); @@ -75,8 +75,10 @@ class QuicInMemoryCache { void set_response_type(SpecialResponseType response_type) { response_type_ = response_type; } - void set_headers(const SpdyHeaderBlock& headers) { headers_ = headers; } - void set_trailers(const SpdyHeaderBlock& trailers) { trailers_ = trailers; } + void set_headers(SpdyHeaderBlock headers) { headers_ = std::move(headers); } + void set_trailers(SpdyHeaderBlock trailers) { + trailers_ = std::move(trailers); + } void set_body(base::StringPiece body) { body.CopyToString(&body_); } private: @@ -108,7 +110,7 @@ class QuicInMemoryCache { base::StringPiece path() { return path_; } void set_path(base::StringPiece path) { path_ = path; } - SpdyHeaderBlock spdy_headers() { return spdy_headers_; } + const SpdyHeaderBlock& spdy_headers() { return spdy_headers_; } base::StringPiece body() { return body_; } @@ -167,15 +169,15 @@ class QuicInMemoryCache { // Add a response to the cache. void AddResponse(base::StringPiece host, base::StringPiece path, - const SpdyHeaderBlock& response_headers, + SpdyHeaderBlock response_headers, base::StringPiece response_body); // Add a response, with trailers, to the cache. void AddResponse(base::StringPiece host, base::StringPiece path, - const SpdyHeaderBlock& response_headers, + SpdyHeaderBlock response_headers, base::StringPiece response_body, - const SpdyHeaderBlock& response_trailers); + SpdyHeaderBlock response_trailers); // Simulate a special behavior at a particular path. void AddSpecialResponse(base::StringPiece host, @@ -206,9 +208,9 @@ class QuicInMemoryCache { void AddResponseImpl(base::StringPiece host, base::StringPiece path, SpecialResponseType response_type, - const SpdyHeaderBlock& response_headers, + SpdyHeaderBlock response_headers, base::StringPiece response_body, - const SpdyHeaderBlock& response_trailers); + SpdyHeaderBlock response_trailers); std::string GetKey(base::StringPiece host, base::StringPiece path) const; diff --git a/net/tools/quic/quic_in_memory_cache_test.cc b/net/tools/quic/quic_in_memory_cache_test.cc index 4f547bcf21afca..d31e8fd813e868 100644 --- a/net/tools/quic/quic_in_memory_cache_test.cc +++ b/net/tools/quic/quic_in_memory_cache_test.cc @@ -90,8 +90,8 @@ TEST_F(QuicInMemoryCacheTest, AddResponse) { response_trailers["key-3"] = "value-3"; QuicInMemoryCache* cache = QuicInMemoryCache::GetInstance(); - cache->AddResponse(kRequestHost, "/", response_headers, kResponseBody, - response_trailers); + cache->AddResponse(kRequestHost, "/", response_headers.Clone(), kResponseBody, + response_trailers.Clone()); const QuicInMemoryCache::Response* response = cache->GetResponse(kRequestHost, kRequestPath); @@ -158,7 +158,7 @@ TEST_F(QuicInMemoryCacheTest, DefaultResponse) { response_headers["content-length"] = "0"; QuicInMemoryCache::Response* default_response = new QuicInMemoryCache::Response; - default_response->set_headers(response_headers); + default_response->set_headers(std::move(response_headers)); cache->AddDefaultResponse(default_response); // Now we should get the default response for the original request. @@ -198,7 +198,7 @@ TEST_F(QuicInMemoryCacheTest, AddSimpleResponseWithServerPushResources) { response_headers[":status"] = "200"; response_headers["content-length"] = base::UintToString(body.size()); push_resources.push_back( - ServerPushInfo(resource_url, response_headers, i, body)); + ServerPushInfo(resource_url, response_headers.Clone(), i, body)); } QuicInMemoryCache* cache = QuicInMemoryCache::GetInstance(); @@ -233,7 +233,7 @@ TEST_F(QuicInMemoryCacheTest, GetServerPushResourcesAndPushResponses) { response_headers[":status"] = push_response_status[i]; response_headers["content-length"] = base::UintToString(body.size()); push_resources.push_back( - ServerPushInfo(resource_url, response_headers, i, body)); + ServerPushInfo(resource_url, response_headers.Clone(), i, body)); } QuicInMemoryCache* cache = QuicInMemoryCache::GetInstance(); cache->AddSimpleResponseWithServerPushResources( diff --git a/net/tools/quic/quic_simple_server_session.cc b/net/tools/quic/quic_simple_server_session.cc index 429c6fdd440cd7..3db89a4cafeb0d 100644 --- a/net/tools/quic/quic_simple_server_session.cc +++ b/net/tools/quic/quic_simple_server_session.cc @@ -77,7 +77,8 @@ void QuicSimpleServerSession::PromisePushResources( SpdyHeaderBlock headers = SynthesizePushRequestHeaders( request_url, resource, original_request_headers); highest_promised_stream_id_ += 2; - SendPushPromise(original_stream_id, highest_promised_stream_id_, headers); + SendPushPromise(original_stream_id, highest_promised_stream_id_, + headers.Clone()); promised_streams_.push_back(PromisedStreamInfo( std::move(headers), highest_promised_stream_id_, resource.priority)); } @@ -151,7 +152,7 @@ SpdyHeaderBlock QuicSimpleServerSession::SynthesizePushRequestHeaders( GURL push_request_url = resource.request_url; string path = push_request_url.path(); - SpdyHeaderBlock spdy_headers = original_request_headers; + SpdyHeaderBlock spdy_headers = original_request_headers.Clone(); // :authority could be different from original request. spdy_headers.ReplaceOrAppendHeader(":authority", push_request_url.host()); spdy_headers.ReplaceOrAppendHeader(":path", path); @@ -179,7 +180,7 @@ void QuicSimpleServerSession::SendPushPromise(QuicStreamId original_stream_id, void QuicSimpleServerSession::HandlePromisedPushRequests() { while (!promised_streams_.empty() && ShouldCreateOutgoingDynamicStream()) { - const PromisedStreamInfo& promised_info = promised_streams_.front(); + PromisedStreamInfo& promised_info = promised_streams_.front(); DCHECK_EQ(next_outgoing_stream_id(), promised_info.stream_id); if (promised_info.is_cancelled) { @@ -196,10 +197,10 @@ void QuicSimpleServerSession::HandlePromisedPushRequests() { DCHECK_EQ(promised_info.stream_id, promised_stream->id()); DVLOG(1) << "created server push stream " << promised_stream->id(); - const SpdyHeaderBlock request_headers(promised_info.request_headers); + SpdyHeaderBlock request_headers(std::move(promised_info.request_headers)); promised_streams_.pop_front(); - promised_stream->PushResponse(request_headers); + promised_stream->PushResponse(std::move(request_headers)); } } diff --git a/net/tools/quic/quic_simple_server_session_test.cc b/net/tools/quic/quic_simple_server_session_test.cc index 76e21be01ba89f..8e93f9bb37f5cc 100644 --- a/net/tools/quic/quic_simple_server_session_test.cc +++ b/net/tools/quic/quic_simple_server_session_test.cc @@ -66,17 +66,33 @@ class MockQuicHeadersStream : public QuicHeadersStream { explicit MockQuicHeadersStream(QuicSpdySession* session) : QuicHeadersStream(session) {} - MOCK_METHOD4(WritePushPromise, + // Methods taking non-copyable types like SpdyHeaderBlock by value cannot be + // mocked directly. + size_t WritePushPromise(QuicStreamId original_stream_id, + QuicStreamId promised_stream_id, + SpdyHeaderBlock headers, + QuicAckListenerInterface* ack_listener) override { + return WritePushPromiseMock(original_stream_id, promised_stream_id, headers, + ack_listener); + } + MOCK_METHOD4(WritePushPromiseMock, size_t(QuicStreamId original_stream_id, QuicStreamId promised_stream_id, - SpdyHeaderBlock headers, + const SpdyHeaderBlock& headers, QuicAckListenerInterface* ack_listener)); - MOCK_METHOD5(WriteHeaders, - size_t(QuicStreamId stream_id, + size_t WriteHeaders(QuicStreamId stream_id, SpdyHeaderBlock headers, bool fin, SpdyPriority priority, + QuicAckListenerInterface* ack_listener) override { + return WriteHeadersMock(stream_id, headers, fin, priority, ack_listener); + } + MOCK_METHOD5(WriteHeadersMock, + size_t(QuicStreamId stream_id, + const SpdyHeaderBlock& headers, + bool fin, + SpdyPriority priority, QuicAckListenerInterface* ack_listener)); }; @@ -462,18 +478,19 @@ class QuicSimpleServerSessionServerPushTest GURL resource_url = GURL(url); string body; GenerateBody(&body, body_size); - SpdyHeaderBlock response_headers; QuicInMemoryCache::GetInstance()->AddSimpleResponse(resource_host, path, 200, body); push_resources.push_back(QuicInMemoryCache::ServerPushInfo( - resource_url, response_headers, kDefaultPriority, body)); + resource_url, SpdyHeaderBlock(), kDefaultPriority, body)); // PUSH_PROMISED are sent for all the resources. - EXPECT_CALL(*headers_stream_, WritePushPromise(kClientDataStreamId1, - stream_id, _, nullptr)); + EXPECT_CALL( + *headers_stream_, + WritePushPromiseMock(kClientDataStreamId1, stream_id, _, nullptr)); if (i <= kMaxStreamsForTest) { // |kMaxStreamsForTest| promised responses should be sent. - EXPECT_CALL(*headers_stream_, WriteHeaders(stream_id, _, false, - kDefaultPriority, nullptr)); + EXPECT_CALL( + *headers_stream_, + WriteHeadersMock(stream_id, _, false, kDefaultPriority, nullptr)); // Since flow control window is smaller than response body, not the // whole body will be sent. EXPECT_CALL(*connection_, @@ -511,8 +528,9 @@ TEST_P(QuicSimpleServerSessionServerPushTest, // After an open stream is marked draining, a new stream is expected to be // created and a response sent on the stream. - EXPECT_CALL(*headers_stream_, WriteHeaders(next_out_going_stream_id, _, false, - kDefaultPriority, nullptr)); + EXPECT_CALL(*headers_stream_, + WriteHeadersMock(next_out_going_stream_id, _, false, + kDefaultPriority, nullptr)); EXPECT_CALL(*connection_, SendStreamData(next_out_going_stream_id, _, 0, false, nullptr)) .WillOnce(Return(QuicConsumedData(kStreamFlowControlWindowSize, false))); @@ -545,14 +563,14 @@ TEST_P(QuicSimpleServerSessionServerPushTest, // only one queued resource will be sent out. QuicStreamId stream_not_reset = (kMaxStreamsForTest + 1) * 2; InSequence s; - EXPECT_CALL(*headers_stream_, WriteHeaders(stream_not_reset, _, false, - kDefaultPriority, nullptr)); + EXPECT_CALL(*headers_stream_, WriteHeadersMock(stream_not_reset, _, false, + kDefaultPriority, nullptr)); EXPECT_CALL(*connection_, SendStreamData(stream_not_reset, _, 0, false, nullptr)) .WillOnce(Return(QuicConsumedData(kStreamFlowControlWindowSize, false))); EXPECT_CALL(*connection_, SendBlocked(stream_not_reset)); - EXPECT_CALL(*headers_stream_, WriteHeaders(stream_got_reset, _, false, - kDefaultPriority, nullptr)) + EXPECT_CALL(*headers_stream_, WriteHeadersMock(stream_got_reset, _, false, + kDefaultPriority, nullptr)) .Times(0); session_->StreamDraining(2); @@ -572,8 +590,8 @@ TEST_P(QuicSimpleServerSessionServerPushTest, QuicStreamId stream_got_reset = 2; EXPECT_CALL(*connection_, SendRstStream(stream_got_reset, QUIC_RST_ACKNOWLEDGEMENT, _)); - EXPECT_CALL(*headers_stream_, WriteHeaders(stream_to_open, _, false, - kDefaultPriority, nullptr)); + EXPECT_CALL(*headers_stream_, WriteHeadersMock(stream_to_open, _, false, + kDefaultPriority, nullptr)); EXPECT_CALL(*connection_, SendStreamData(stream_to_open, _, 0, false, nullptr)) .WillOnce(Return(QuicConsumedData(kStreamFlowControlWindowSize, false))); diff --git a/net/tools/quic/quic_simple_server_stream.cc b/net/tools/quic/quic_simple_server_stream.cc index d882b72a12e2a3..dda6a420d180bf 100644 --- a/net/tools/quic/quic_simple_server_stream.cc +++ b/net/tools/quic/quic_simple_server_stream.cc @@ -127,7 +127,7 @@ void QuicSimpleServerStream::PushResponse( return; } // Change the stream state to emulate a client request. - request_headers_ = push_request_headers; + request_headers_ = std::move(push_request_headers); content_length_ = 0; DVLOG(1) << "Stream " << id() << ": Ready to receive server push response."; @@ -203,8 +203,8 @@ void QuicSimpleServerStream::SendResponse() { } DVLOG(1) << "Sending response for stream " << id(); - SendHeadersAndBodyAndTrailers(response->headers(), response->body(), - response->trailers()); + SendHeadersAndBodyAndTrailers(response->headers().Clone(), response->body(), + response->trailers().Clone()); } void QuicSimpleServerStream::SendNotFoundResponse() { diff --git a/net/tools/quic/quic_simple_server_stream_test.cc b/net/tools/quic/quic_simple_server_stream_test.cc index 191f5d229d3e44..5161c9f223cc8f 100644 --- a/net/tools/quic/quic_simple_server_stream_test.cc +++ b/net/tools/quic/quic_simple_server_stream_test.cc @@ -4,6 +4,8 @@ #include "net/tools/quic/quic_simple_server_stream.h" +#include + #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" #include "net/quic/quic_connection.h" @@ -120,9 +122,19 @@ class MockQuicSimpleServerSession : public QuicSimpleServerSession { void(QuicStreamId stream_id, SpdyPriority priority)); MOCK_METHOD3(OnStreamHeadersComplete, void(QuicStreamId stream_id, bool fin, size_t frame_len)); - MOCK_METHOD5(WriteHeaders, + // Methods taking non-copyable types like SpdyHeaderBlock by value cannot be + // mocked directly. + size_t WriteHeaders( + QuicStreamId id, + SpdyHeaderBlock headers, + bool fin, + SpdyPriority priority, + QuicAckListenerInterface* ack_notifier_delegate) override { + return WriteHeadersMock(id, headers, fin, priority, ack_notifier_delegate); + } + MOCK_METHOD5(WriteHeadersMock, size_t(QuicStreamId id, - SpdyHeaderBlock headers, + const SpdyHeaderBlock& headers, bool fin, SpdyPriority priority, QuicAckListenerInterface* ack_notifier_delegate)); @@ -131,7 +143,17 @@ class MockQuicSimpleServerSession : public QuicSimpleServerSession { QuicRstStreamErrorCode error, QuicStreamOffset bytes_written)); MOCK_METHOD1(OnHeadersHeadOfLineBlocking, void(QuicTime::Delta delta)); - MOCK_METHOD4(PromisePushResources, + // Matchers cannot be used on non-copyable types like SpdyHeaderBlock. + void PromisePushResources( + const string& request_url, + const std::list& resources, + QuicStreamId original_stream_id, + const SpdyHeaderBlock& original_request_headers) override { + original_request_headers_ = original_request_headers.Clone(); + PromisePushResourcesMock(request_url, resources, original_stream_id, + original_request_headers); + } + MOCK_METHOD4(PromisePushResourcesMock, void(const string&, const std::list&, QuicStreamId, @@ -139,6 +161,8 @@ class MockQuicSimpleServerSession : public QuicSimpleServerSession { using QuicSession::ActivateStream; + SpdyHeaderBlock original_request_headers_; + private: DISALLOW_COPY_AND_ASSIGN(MockQuicSimpleServerSession); }; @@ -274,7 +298,7 @@ TEST_P(QuicSimpleServerStreamTest, TestFramingExtraData) { string large_body = "hello world!!!!!!"; // We'll automatically write out an error (headers + body) - EXPECT_CALL(session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(session_, WriteHeadersMock(_, _, _, _, _)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .WillOnce(Invoke(MockQuicSession::ConsumeAllData)); EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); @@ -305,13 +329,13 @@ TEST_P(QuicSimpleServerStreamTest, SendResponseWithIllegalResponseStatus) { response_headers_[":status"] = "200 OK"; response_headers_["content-length"] = "5"; string body = "Yummm"; - QuicInMemoryCache::GetInstance()->AddResponse("www.google.com", "/bar", - response_headers_, body); + QuicInMemoryCache::GetInstance()->AddResponse( + "www.google.com", "/bar", std::move(response_headers_), body); stream_->set_fin_received(true); InSequence s; - EXPECT_CALL(session_, WriteHeaders(stream_->id(), _, false, _, nullptr)); + EXPECT_CALL(session_, WriteHeadersMock(stream_->id(), _, false, _, nullptr)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData( @@ -336,13 +360,13 @@ TEST_P(QuicSimpleServerStreamTest, SendResponseWithIllegalResponseStatus2) { response_headers_[":status"] = "+200"; response_headers_["content-length"] = "5"; string body = "Yummm"; - QuicInMemoryCache::GetInstance()->AddResponse("www.google.com", "/bar", - response_headers_, body); + QuicInMemoryCache::GetInstance()->AddResponse( + "www.google.com", "/bar", std::move(response_headers_), body); stream_->set_fin_received(true); InSequence s; - EXPECT_CALL(session_, WriteHeaders(stream_->id(), _, false, _, nullptr)); + EXPECT_CALL(session_, WriteHeadersMock(stream_->id(), _, false, _, nullptr)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData( @@ -372,8 +396,8 @@ TEST_P(QuicSimpleServerStreamTest, SendPushResponseWith404Response) { response_headers_[":status"] = "404"; response_headers_["content-length"] = "8"; string body = "NotFound"; - QuicInMemoryCache::GetInstance()->AddResponse("www.google.com", "/bar", - response_headers_, body); + QuicInMemoryCache::GetInstance()->AddResponse( + "www.google.com", "/bar", std::move(response_headers_), body); InSequence s; EXPECT_CALL(session_, @@ -394,12 +418,12 @@ TEST_P(QuicSimpleServerStreamTest, SendResponseWithValidHeaders) { response_headers_[":status"] = "200"; response_headers_["content-length"] = "5"; string body = "Yummm"; - QuicInMemoryCache::GetInstance()->AddResponse("www.google.com", "/bar", - response_headers_, body); + QuicInMemoryCache::GetInstance()->AddResponse( + "www.google.com", "/bar", std::move(response_headers_), body); stream_->set_fin_received(true); InSequence s; - EXPECT_CALL(session_, WriteHeaders(stream_->id(), _, false, _, nullptr)); + EXPECT_CALL(session_, WriteHeadersMock(stream_->id(), _, false, _, nullptr)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData(body.length(), true))); @@ -418,9 +442,8 @@ TEST_P(QuicSimpleServerStreamTest, SendReponseWithPushResources) { string host = "www.google.com"; string request_path = "/foo"; string body = "Yummm"; - SpdyHeaderBlock response_headers; string url = host + "/bar"; - QuicInMemoryCache::ServerPushInfo push_info(GURL(url), response_headers, + QuicInMemoryCache::ServerPushInfo push_info(GURL(url), SpdyHeaderBlock(), kDefaultPriority, "Push body"); std::list push_resources; push_resources.push_back(push_info); @@ -435,21 +458,21 @@ TEST_P(QuicSimpleServerStreamTest, SendReponseWithPushResources) { stream_->set_fin_received(true); InSequence s; - EXPECT_CALL(session_, PromisePushResources(host + request_path, _, - ::net::test::kClientDataStreamId1, - *request_headers)); - EXPECT_CALL(session_, WriteHeaders(stream_->id(), _, false, _, nullptr)); + EXPECT_CALL(session_, + PromisePushResourcesMock(host + request_path, _, + ::net::test::kClientDataStreamId1, _)); + EXPECT_CALL(session_, WriteHeadersMock(stream_->id(), _, false, _, nullptr)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData(body.length(), true))); QuicSimpleServerStreamPeer::SendResponse(stream_); + EXPECT_EQ(*request_headers, session_.original_request_headers_); } TEST_P(QuicSimpleServerStreamTest, PushResponseOnClientInitiatedStream) { // Calling PushResponse() on a client initialted stream is never supposed to // happen. - SpdyHeaderBlock headers; - EXPECT_DFATAL(stream_->PushResponse(headers), + EXPECT_DFATAL(stream_->PushResponse(SpdyHeaderBlock()), "Client initiated stream" " shouldn't be used as promised stream."); } @@ -477,18 +500,18 @@ TEST_P(QuicSimpleServerStreamTest, PushResponseOnServerInitiatedStream) { response_headers_[":status"] = "200"; response_headers_["content-length"] = "5"; const string kBody = "Hello"; - QuicInMemoryCache::GetInstance()->AddResponse(kHost, kPath, response_headers_, - kBody); + QuicInMemoryCache::GetInstance()->AddResponse( + kHost, kPath, std::move(response_headers_), kBody); // Call PushResponse() should trigger stream to fetch response from cache // and send it back. EXPECT_CALL(session_, - WriteHeaders(kServerInitiatedStreamId, _, false, - server_initiated_stream->priority(), nullptr)); + WriteHeadersMock(kServerInitiatedStreamId, _, false, + server_initiated_stream->priority(), nullptr)); EXPECT_CALL(session_, WritevData(_, kServerInitiatedStreamId, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData(kBody.size(), true))); - server_initiated_stream->PushResponse(headers); + server_initiated_stream->PushResponse(std::move(headers)); EXPECT_EQ(kPath, QuicSimpleServerStreamPeer::headers( server_initiated_stream)[":path"] .as_string()); @@ -500,13 +523,10 @@ TEST_P(QuicSimpleServerStreamTest, PushResponseOnServerInitiatedStream) { TEST_P(QuicSimpleServerStreamTest, TestSendErrorResponse) { EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); - response_headers_[":version"] = "HTTP/1.1"; - response_headers_[":status"] = "500 Server Error"; - response_headers_["content-length"] = "3"; stream_->set_fin_received(true); InSequence s; - EXPECT_CALL(session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(session_, WriteHeadersMock(_, _, _, _, _)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData(3, true))); @@ -526,7 +546,7 @@ TEST_P(QuicSimpleServerStreamTest, InvalidMultipleContentLength) { headers_string_ = SpdyUtils::SerializeUncompressedHeaders(request_headers); - EXPECT_CALL(session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(session_, WriteHeadersMock(_, _, _, _, _)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(AnyNumber()) .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); @@ -547,7 +567,7 @@ TEST_P(QuicSimpleServerStreamTest, InvalidLeadingNullContentLength) { headers_string_ = SpdyUtils::SerializeUncompressedHeaders(request_headers); - EXPECT_CALL(session_, WriteHeaders(_, _, _, _, _)); + EXPECT_CALL(session_, WriteHeadersMock(_, _, _, _, _)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(AnyNumber()) .WillRepeatedly(Invoke(MockQuicSession::ConsumeAllData)); @@ -576,12 +596,8 @@ TEST_P(QuicSimpleServerStreamTest, ValidMultipleContentLength) { } TEST_P(QuicSimpleServerStreamTest, SendQuicRstStreamNoErrorWithEarlyResponse) { - response_headers_[":version"] = "HTTP/1.1"; - response_headers_[":status"] = "500 Server Error"; - response_headers_["content-length"] = "3"; - InSequence s; - EXPECT_CALL(session_, WriteHeaders(stream_->id(), _, _, _, _)); + EXPECT_CALL(session_, WriteHeadersMock(stream_->id(), _, false, _, nullptr)); EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) .Times(1) .WillOnce(Return(QuicConsumedData(3, true))); @@ -598,10 +614,6 @@ TEST_P(QuicSimpleServerStreamTest, SendQuicRstStreamNoErrorWithEarlyResponse) { TEST_P(QuicSimpleServerStreamTest, DoNotSendQuicRstStreamNoErrorWithRstReceived) { - response_headers_[":version"] = "HTTP/1.1"; - response_headers_[":status"] = "500 Server Error"; - response_headers_["content-length"] = "3"; - InSequence s; EXPECT_FALSE(stream_->reading_stopped()); diff --git a/net/tools/quic/test_tools/quic_test_client.cc b/net/tools/quic/test_tools/quic_test_client.cc index a5ed02488b72a2..914c4f8ab03f15 100644 --- a/net/tools/quic/test_tools/quic_test_client.cc +++ b/net/tools/quic/test_tools/quic_test_client.cc @@ -582,7 +582,7 @@ void QuicTestClient::OnClose(QuicSpdyStream* stream) { response_headers_complete_ = stream_->headers_decompressed(); SpdyBalsaUtils::SpdyHeadersToResponseHeaders(stream_->response_headers(), &response_headers_); - response_trailers_ = stream_->received_trailers(); + response_trailers_ = stream_->received_trailers().Clone(); stream_error_ = stream_->stream_error(); bytes_read_ = stream_->stream_bytes_read() + stream_->header_bytes_read(); bytes_written_ =