Remove tests that are testing on sending only STOP_SENDING. Theses tests use QuicStream::SendStopSending() which is never used in production. Sending and receiving STOP_SENDING is well covered in unit tests. end to end STOP_SENDING communication is covered in end_to_end test such as EndToEndTest.SimpleStopSendingRstStreamTest PiperOrigin-RevId: 328835344 Change-Id: I7a9f07e3582cb9412bf7a86239a4188d335d9504
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index cf08d32..ff6d357 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -4352,86 +4352,6 @@ EXPECT_TRUE(client_->client()->EarlyDataAccepted()); } -// This observer is used to check whether stream write side is closed when -// receiving STOP_SENDING (which ends up as noop). -class StopSendingObserver : public QuicConnectionDebugVisitor { - public: - explicit StopSendingObserver(QuicTestClient* client) - : num_stop_sending_frames_(0), - client_(client), - stream_write_side_closed_before_receiving_stop_sending_(false) {} - - void OnStopSendingFrame(const QuicStopSendingFrame& /*frame*/) override { - ++num_stop_sending_frames_; - stream_write_side_closed_before_receiving_stop_sending_ = - static_cast<QuicSimpleClientStream*>(client_->latest_created_stream()) - ->write_side_closed(); - } - - size_t num_stop_sending_frames() const { return num_stop_sending_frames_; } - - bool stream_write_side_closed_before_receiving_stop_sending() const { - return stream_write_side_closed_before_receiving_stop_sending_; - } - - private: - size_t num_stop_sending_frames_; - QuicTestClient* client_; - bool stream_write_side_closed_before_receiving_stop_sending_; -}; - -// Test that STOP_SENDING makes it to the peer. Create a stream and send a -// STOP_SENDING. The receiver should get a call to QuicStream::OnStopSending. -TEST_P(EndToEndTest, SimpleStopSendingTest) { - const uint16_t kStopSendingTestCode = 123; - ASSERT_TRUE(Initialize()); - if (!version_.HasIetfQuicFrames()) { - return; - } - QuicSession* client_session = GetClientSession(); - ASSERT_TRUE(client_session); - StopSendingObserver observer(client_.get()); - QuicConnection* client_connection = client_session->connection(); - ASSERT_TRUE(client_connection); - client_connection->set_debug_visitor(&observer); - - std::string response_body(1305, 'a'); - SpdyHeaderBlock response_headers; - response_headers[":status"] = quiche::QuicheTextUtils::Uint64ToString(200); - response_headers["content-length"] = - quiche::QuicheTextUtils::Uint64ToString(response_body.length()); - memory_cache_backend_.AddStopSendingResponse( - server_hostname_, "/test_url", std::move(response_headers), response_body, - kStopSendingTestCode); - - EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable()); - client_->WaitForDelayedAcks(); - - QuicStreamId stream_id = - client_session->next_outgoing_bidirectional_stream_id(); - client_->SendRequest("/test_url"); - // Wait for the connection to become idle. - client_->WaitForDelayedAcks(); - - EXPECT_THAT(client_->connection_error(), IsQuicNoError()); - QuicSimpleClientStream* client_stream = - static_cast<QuicSimpleClientStream*>(client_->latest_created_stream()); - if (client_stream != nullptr) { - // Ensure the stream has been write closed upon receiving STOP_SENDING. - EXPECT_EQ(stream_id, client_stream->id()); - EXPECT_TRUE(client_stream->write_side_closed()); - client_->WaitUntil( - -1, [&observer]() { return observer.num_stop_sending_frames() > 0; }); - if (!observer.stream_write_side_closed_before_receiving_stop_sending()) { - EXPECT_EQ(kStopSendingTestCode, - static_cast<uint16_t>(client_stream->stream_error())); - } - } else { - ADD_FAILURE() << "Missing client stream"; - } - client_connection->set_debug_visitor(nullptr); -} - TEST_P(EndToEndTest, SimpleStopSendingRstStreamTest) { ASSERT_TRUE(Initialize());
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index 5fd6358..dfe00a6 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -1309,16 +1309,6 @@ Reset(QUIC_STREAM_TTL_EXPIRED); } -void QuicStream::SendStopSending(uint16_t code) { - if (!VersionHasIetfQuicFrames(transport_version())) { - // If the connection is not version 99, do nothing. - // Do not QUIC_BUG or anything; the application really does not need to know - // what version the connection is in. - return; - } - session_->SendStopSending(code, id_); -} - bool QuicStream::IsFlowControlBlocked() const { if (!flow_controller_.has_value()) { QUIC_BUG << "Trying to access non-existent flow controller.";
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h index 68d3f89..d6fe8f8 100644 --- a/quic/core/quic_stream.h +++ b/quic/core/quic_stream.h
@@ -338,13 +338,6 @@ StreamType type() const { return type_; } - // Creates and sends a STOP_SENDING frame. This can be called regardless of - // the version that has been negotiated. If not IETF QUIC/Version 99 then the - // method is a noop, relieving the application of the necessity of - // understanding the connection's QUIC version and knowing whether it can call - // this method or not. - void SendStopSending(uint16_t code); - // Handle received StopSending frame. Returns true if the processing finishes // gracefully. virtual bool OnStopSending(uint16_t code);
diff --git a/quic/core/quic_stream_test.cc b/quic/core/quic_stream_test.cc index 36979dc..2313852 100644 --- a/quic/core/quic_stream_test.cc +++ b/quic/core/quic_stream_test.cc
@@ -1554,30 +1554,6 @@ stream_->RetransmitStreamData(0, 100, false, PTO_RETRANSMISSION); } -// Test that QuicStream::StopSending A) is a no-op if the connection is not in -// version 99, B) that it properly invokes QuicSession::StopSending, and C) that -// the correct data is passed along, including getting the stream ID. -TEST_P(QuicStreamTest, CheckStopSending) { - Initialize(); - const int kStopSendingCode = 123; - // These must start as false. - EXPECT_FALSE(stream_->write_side_closed()); - EXPECT_FALSE(QuicStreamPeer::read_side_closed(stream_)); - // Expect to actually see a stop sending if and only if we are in version 99. - if (VersionHasIetfQuicFrames(connection_->transport_version())) { - EXPECT_CALL(*session_, SendStopSending(kStopSendingCode, stream_->id())) - .Times(1); - } else { - EXPECT_CALL(*session_, SendStopSending(_, _)).Times(0); - } - stream_->SendStopSending(kStopSendingCode); - // Sending a STOP_SENDING does not actually close the local stream. - // Our implementation waits for the responding RESET_STREAM to effect the - // closes. Therefore, read- and write-side closes should both be false. - EXPECT_FALSE(stream_->write_side_closed()); - EXPECT_FALSE(QuicStreamPeer::read_side_closed(stream_)); -} - // Test that OnStreamReset does one-way (read) closes if version 99, two way // (read and write) if not version 99. TEST_P(QuicStreamTest, OnStreamResetReadOrReadWrite) {
diff --git a/quic/tools/quic_backend_response.h b/quic/tools/quic_backend_response.h index 6537987..af6cf84 100644 --- a/quic/tools/quic_backend_response.h +++ b/quic/tools/quic_backend_response.h
@@ -40,9 +40,6 @@ INCOMPLETE_RESPONSE, // The server will act as if there is a non-empty // trailer but it will not be sent, as a result, FIN // will not be sent too. - STOP_SENDING, // Acts like INCOMPLETE_RESPONSE in that the entire - // response is not sent. After sending what is sent, - // the server will send a STOP_SENDING. GENERATE_BYTES // Sends a response with a length equal to the number // of bytes in the URL path. }; @@ -73,15 +70,12 @@ void set_body(quiche::QuicheStringPiece body) { body_.assign(body.data(), body.size()); } - uint16_t stop_sending_code() const { return stop_sending_code_; } - void set_stop_sending_code(uint16_t code) { stop_sending_code_ = code; } private: SpecialResponseType response_type_; spdy::SpdyHeaderBlock headers_; spdy::SpdyHeaderBlock trailers_; std::string body_; - uint16_t stop_sending_code_; }; } // namespace quic
diff --git a/quic/tools/quic_memory_cache_backend.cc b/quic/tools/quic_memory_cache_backend.cc index c7ac912..370eb4a 100644 --- a/quic/tools/quic_memory_cache_backend.cc +++ b/quic/tools/quic_memory_cache_backend.cc
@@ -199,8 +199,8 @@ SpdyHeaderBlock response_headers, quiche::QuicheStringPiece response_body) { AddResponseImpl(host, path, QuicBackendResponse::REGULAR_RESPONSE, - std::move(response_headers), response_body, SpdyHeaderBlock(), - 0); + std::move(response_headers), response_body, + SpdyHeaderBlock()); } void QuicMemoryCacheBackend::AddResponse( @@ -211,7 +211,7 @@ SpdyHeaderBlock response_trailers) { AddResponseImpl(host, path, QuicBackendResponse::REGULAR_RESPONSE, std::move(response_headers), response_body, - std::move(response_trailers), 0); + std::move(response_trailers)); } void QuicMemoryCacheBackend::AddSpecialResponse( @@ -219,7 +219,7 @@ quiche::QuicheStringPiece path, SpecialResponseType response_type) { AddResponseImpl(host, path, response_type, SpdyHeaderBlock(), "", - SpdyHeaderBlock(), 0); + SpdyHeaderBlock()); } void QuicMemoryCacheBackend::AddSpecialResponse( @@ -229,18 +229,7 @@ quiche::QuicheStringPiece response_body, SpecialResponseType response_type) { AddResponseImpl(host, path, response_type, std::move(response_headers), - response_body, SpdyHeaderBlock(), 0); -} - -void QuicMemoryCacheBackend::AddStopSendingResponse( - quiche::QuicheStringPiece host, - quiche::QuicheStringPiece path, - spdy::SpdyHeaderBlock response_headers, - quiche::QuicheStringPiece response_body, - uint16_t stop_sending_code) { - AddResponseImpl(host, path, SpecialResponseType::STOP_SENDING, - std::move(response_headers), response_body, SpdyHeaderBlock(), - stop_sending_code); + response_body, SpdyHeaderBlock()); } QuicMemoryCacheBackend::QuicMemoryCacheBackend() : cache_initialized_(false) {} @@ -369,8 +358,7 @@ SpecialResponseType response_type, SpdyHeaderBlock response_headers, quiche::QuicheStringPiece response_body, - SpdyHeaderBlock response_trailers, - uint16_t stop_sending_code) { + SpdyHeaderBlock response_trailers) { QuicWriterMutexLock lock(&response_mutex_); DCHECK(!host.empty()) << "Host must be populated, e.g. \"www.google.com\""; @@ -384,7 +372,6 @@ new_response->set_headers(std::move(response_headers)); new_response->set_body(response_body); new_response->set_trailers(std::move(response_trailers)); - new_response->set_stop_sending_code(stop_sending_code); QUIC_DVLOG(1) << "Add response with key " << key; responses_[key] = std::move(new_response); }
diff --git a/quic/tools/quic_memory_cache_backend.h b/quic/tools/quic_memory_cache_backend.h index 56a2323..1c56ce6 100644 --- a/quic/tools/quic_memory_cache_backend.h +++ b/quic/tools/quic_memory_cache_backend.h
@@ -124,12 +124,6 @@ quiche::QuicheStringPiece response_body, QuicBackendResponse::SpecialResponseType response_type); - void AddStopSendingResponse(quiche::QuicheStringPiece host, - quiche::QuicheStringPiece path, - spdy::SpdyHeaderBlock response_headers, - quiche::QuicheStringPiece response_body, - uint16_t stop_sending_code); - // Sets a default response in case of cache misses. Takes ownership of // 'response'. void AddDefaultResponse(QuicBackendResponse* response); @@ -159,8 +153,7 @@ QuicBackendResponse::SpecialResponseType response_type, spdy::SpdyHeaderBlock response_headers, quiche::QuicheStringPiece response_body, - spdy::SpdyHeaderBlock response_trailers, - uint16_t stop_sending_code); + spdy::SpdyHeaderBlock response_trailers); std::string GetKey(quiche::QuicheStringPiece host, quiche::QuicheStringPiece path) const;
diff --git a/quic/tools/quic_simple_server_stream.cc b/quic/tools/quic_simple_server_stream.cc index a6775cc..84ddb05 100644 --- a/quic/tools/quic_simple_server_stream.cc +++ b/quic/tools/quic_simple_server_stream.cc
@@ -254,15 +254,6 @@ return; } - if (response->response_type() == QuicBackendResponse::STOP_SENDING) { - QUIC_DVLOG(1) - << "Stream " << id() - << " sending an incomplete response, i.e. no trailer, no fin."; - SendIncompleteResponse(response->headers().Clone(), response->body()); - SendStopSending(response->stop_sending_code()); - return; - } - if (response->response_type() == QuicBackendResponse::GENERATE_BYTES) { QUIC_DVLOG(1) << "Stream " << id() << " sending a generate bytes response."; std::string path = request_headers_[":path"].as_string().substr(1);