[QuicTransport] Reset QUIC stream when outgoing stream is aborted

Call QuicStream::Reset when an outgoing stream is aborted.

Bug: 1086371
Change-Id: I694fc03961d15c250f3412af30d1c387a4a5e67b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2249198Reviewed-by: Victor Vasiliev <vasilvv@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Auto-Submit: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779203}

services/network/public/mojom/quic_transport.mojom

@@ -54,6 +54,9 @@ interface QuicTransport {
   // be able to write any data to the stream, but it may be able to use other
   // functions such as reading data from the stream.
   SendFin(uint32 stream_id);
+
+  // Aborts the stream for |stream_id|.
+  AbortStream(uint32 stream_id, uint64 code);
 };
 
 // A mojo interface for the client of QuicTransport.

services/network/quic_transport.cc

@@ -118,6 +118,19 @@ class QuicTransport::Stream final {
     MaySendFin();
   }
 
+  void Abort(quic::QuicRstStreamErrorCode code) {
+    auto* stream = incoming_ ? incoming_ : outgoing_;
+    if (!stream) {
+      return;
+    }
+    stream->Reset(code);
+    incoming_ = nullptr;
+    outgoing_ = nullptr;
+    readable_watcher_.Cancel();
+    readable_.reset();
+    MayDisposeLater();
+  }
+
   ~Stream() { transport_->transport_->session()->CloseStream(id_); }
 
  private:
@@ -278,7 +291,7 @@ class QuicTransport::Stream final {
 
   // This must be the last member.
   base::WeakPtrFactory<Stream> weak_factory_{this};
-};
+};  // namespace network
 
 QuicTransport::QuicTransport(
     const GURL& url,
@@ -391,6 +404,18 @@ void QuicTransport::SendFin(uint32_t stream) {
   it->second->NotifyFinFromClient();
 }
 
+void QuicTransport::AbortStream(uint32_t stream, uint64_t code) {
+  auto it = streams_.find(stream);
+  if (it == streams_.end()) {
+    return;
+  }
+  auto code_to_pass = quic::QuicRstStreamErrorCode::QUIC_STREAM_NO_ERROR;
+  if (code < quic::QuicRstStreamErrorCode::QUIC_STREAM_LAST_ERROR) {
+    code_to_pass = static_cast<quic::QuicRstStreamErrorCode>(code);
+  }
+  it->second->Abort(code_to_pass);
+}
+
 void QuicTransport::OnConnected() {
   if (torn_down_) {
     return;

services/network/quic_transport.h

@@ -62,6 +62,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) QuicTransport final
   void AcceptUnidirectionalStream(
       UnidirectionalStreamAcceptanceCallback callback) override;
   void SendFin(uint32_t stream_id) override;
+  void AbortStream(uint32_t stream_id, uint64_t code) override;
 
   // net::QuicTransportClient::Visitor implementation:
   void OnConnected() override;

third_party/blink/renderer/modules/webtransport/bidirectional_stream.cc

@@ -64,6 +64,7 @@ void BidirectionalStream::SendFin() {
 
 void BidirectionalStream::OnOutgoingStreamAbort() {
   DCHECK(!sent_fin_);
+  quic_transport_->AbortStream(stream_id_);
   quic_transport_->ForgetStream(stream_id_);
   incoming_stream_->Reset();
 }

third_party/blink/renderer/modules/webtransport/bidirectional_stream_test.cc

@@ -72,6 +72,7 @@ class StubQuicTransport : public network::mojom::blink::QuicTransport {
   }
 
   bool WasSendFinCalled() const { return was_send_fin_called_; }
+  bool WasAbortStreamCalled() const { return was_abort_stream_called_; }
 
   // Responds to an earlier call to AcceptBidirectionalStream with a new stream
   // as if it was created by the remote server. The remote handles can be
@@ -146,6 +147,11 @@ class StubQuicTransport : public network::mojom::blink::QuicTransport {
     was_send_fin_called_ = true;
   }
 
+  void AbortStream(uint32_t stream_id, uint64_t code) override {
+    EXPECT_EQ(stream_id, kDefaultStreamId);
+    was_abort_stream_called_ = true;
+  }
+
  private:
   base::OnceCallback<void(uint32_t,
                           mojo::ScopedDataPipeConsumerHandle,
@@ -157,6 +163,7 @@ class StubQuicTransport : public network::mojom::blink::QuicTransport {
   mojo::ScopedDataPipeConsumerHandle output_consumer_;
   mojo::ScopedDataPipeProducerHandle input_producer_;
   bool was_send_fin_called_ = false;
+  bool was_abort_stream_called_ = false;
 };
 
 // This class sets up a connected blink::QuicTransport object using a
@@ -404,6 +411,10 @@ TEST(BidirectionalStreamTest, OutgoingStreamAbort) {
                              bidirectional_stream->readingAborted());
   tester.WaitUntilSettled();
   EXPECT_TRUE(tester.IsFulfilled());
+
+  const auto* const stub = scoped_quic_transport.Stub();
+  EXPECT_FALSE(stub->WasSendFinCalled());
+  EXPECT_TRUE(stub->WasAbortStreamCalled());
 }
 
 TEST(BidirectionalStreamTest, OutgoingStreamCleanClose) {
@@ -429,6 +440,10 @@ TEST(BidirectionalStreamTest, OutgoingStreamCleanClose) {
                              bidirectional_stream->readingAborted());
   tester.WaitUntilSettled();
   EXPECT_TRUE(tester.IsFulfilled());
+
+  const auto* const stub = scoped_quic_transport.Stub();
+  EXPECT_TRUE(stub->WasSendFinCalled());
+  EXPECT_FALSE(stub->WasAbortStreamCalled());
 }
 
 TEST(BidirectionalStreamTest, AbortBothOutgoingFirst) {

third_party/blink/renderer/modules/webtransport/quic_transport.cc

@@ -566,6 +566,10 @@ void QuicTransport::SendFin(uint32_t stream_id) {
   quic_transport_->SendFin(stream_id);
 }
 
+void QuicTransport::AbortStream(uint32_t stream_id) {
+  quic_transport_->AbortStream(stream_id, /*code=*/0);
+}
+
 void QuicTransport::ForgetStream(uint32_t stream_id) {
   stream_map_.erase(stream_id);
 }

third_party/blink/renderer/modules/webtransport/quic_transport.h

@@ -92,6 +92,9 @@ class MODULES_EXPORT QuicTransport final
   // Forwards a SendFin() message to the mojo interface.
   void SendFin(uint32_t stream_id);
 
+  // Forwards a AbortStream() message to the mojo interface.
+  void AbortStream(uint32_t stream_id);
+
   // Removes the reference to a stream.
   void ForgetStream(uint32_t stream_id);
 

third_party/blink/renderer/modules/webtransport/quic_transport_test.cc

@@ -120,6 +120,7 @@ class MockQuicTransport : public network::mojom::blink::QuicTransport {
                     void(uint32_t, mojo::ScopedDataPipeConsumerHandle)>));
 
   void SendFin(uint32_t stream_id) override {}
+  void AbortStream(uint32_t stream_id, uint64_t code) override {}
 
  private:
   mojo::Receiver<network::mojom::blink::QuicTransport> receiver_;

third_party/blink/renderer/modules/webtransport/send_stream.cc

@@ -44,6 +44,7 @@ void SendStream::SendFin() {
 }
 
 void SendStream::OnOutgoingStreamAbort() {
+  quic_transport_->AbortStream(stream_id_);
   quic_transport_->ForgetStream(stream_id_);
 }