fixup! quic: extract transport logic from Application to Session

Handle mechnical cleanup review comments

Author: pimterry

src/quic/application.cc

@@ -335,7 +335,7 @@ class DefaultApplication final : public Session::Application {
     return true;
   }
 
-  int GetStreamData(StreamData* stream_data) override {
+  int GetStreamData(Session::StreamData* stream_data) override {
     // Reset the state of stream_data before proceeding...
     stream_data->id = -1;
     stream_data->count = 0;
@@ -422,7 +422,7 @@ class DefaultApplication final : public Session::Application {
     stream->Schedule(&stream_queue_);
   }
 
-  bool StreamCommit(StreamData* stream_data, size_t datalen) override {
+  bool StreamCommit(Session::StreamData* stream_data, size_t datalen) override {
     DCHECK_NOT_NULL(stream_data);
     CHECK(stream_data->stream);
     stream_data->stream->Commit(datalen, stream_data->fin);

src/quic/http3.cc

@@ -241,9 +241,9 @@ class Http3ApplicationImpl final : public Session::Application {
 
     Debug(&session(), "Creating and binding HTTP/3 control streams");
     bool ret =
-        session().OpenUni(&control_stream_id_) &&
-        session().OpenUni(&qpack_enc_stream_id_) &&
-        session().OpenUni(&qpack_dec_stream_id_) &&
+        session().OpenUnidirectionalStream(&control_stream_id_) &&
+        session().OpenUnidirectionalStream(&qpack_enc_stream_id_) &&
+        session().OpenUnidirectionalStream(&qpack_dec_stream_id_) &&
         nghttp3_conn_bind_control_stream(*this, control_stream_id_) == 0 &&
         nghttp3_conn_bind_qpack_streams(
             *this, qpack_enc_stream_id_, qpack_dec_stream_id_) == 0;
@@ -669,7 +669,7 @@ class Http3ApplicationImpl final : public Session::Application {
     return {StreamPriority::DEFAULT, StreamPriorityFlags::NON_INCREMENTAL};
   }
 
-  int GetStreamData(StreamData* data) override {
+  int GetStreamData(Session::StreamData* data) override {
     static_assert(
         sizeof(ngtcp2_vec) == sizeof(nghttp3_vec) &&
             alignof(ngtcp2_vec) == alignof(nghttp3_vec) &&
@@ -702,7 +702,7 @@ class Http3ApplicationImpl final : public Session::Application {
     return 0;
   }
 
-  bool StreamCommit(StreamData* data, size_t datalen) override {
+  bool StreamCommit(Session::StreamData* data, size_t datalen) override {
     Debug(&session(),
           "HTTP/3 application committing stream %" PRIi64 " data %zu",
           data->id,

src/quic/session.cc

@@ -1736,7 +1736,7 @@ Session::SendPendingDataScope::~SendPendingDataScope() {
 
 // ============================================================================
 
-std::string StreamData::ToString() const {
+std::string Session::StreamData::ToString() const {
   DebugIndentScope indent;
 
   size_t total_bytes = 0;
@@ -3403,7 +3403,7 @@ uint64_t Session::max_data_left() const {
   return ngtcp2_conn_get_max_data_left(*this);
 }
 
-bool Session::OpenUni(stream_id* id) {
+bool Session::OpenUnidirectionalStream(stream_id* id) {
   return ngtcp2_conn_open_uni_stream(*this, id, nullptr) == 0;
 }
 

src/quic/session.h

@@ -27,25 +27,6 @@ namespace node::quic {
 
 class Endpoint;
 
-// A block of pending outbound stream data, passed between the application
-// layer (which fills it via GetStreamData) and the send pump (which hands
-// it to ngtcp2_conn_writev_stream and commits the accepted length).
-struct StreamData final {
-  // The actual number of vectors in the struct, up to kMaxVectorCount.
-  size_t count = 0;
-  // The stream identifier. If this is a negative value then no stream is
-  // identified.
-  stream_id id = -1;
-  int fin = 0;
-  ngtcp2_vec data[kMaxVectorCount]{};
-  BaseObjectPtr<Stream> stream;
-
-  inline operator const ngtcp2_vec*() const { return data; }
-  inline operator ngtcp2_vec*() { return data; }
-
-  std::string ToString() const;
-};
-
 // A Session represents one half of a persistent connection between two QUIC
 // peers. Every Session is established first by performing a TLS handshake in
 // which the client sends an initial packet to the server containing a TLS
@@ -123,6 +104,25 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
   // of a QUIC Session.
   class Application;
 
+  // A block of pending outbound stream data, passed between the application
+  // layer (which fills it via GetStreamData) and the send pump (which hands
+  // it to ngtcp2_conn_writev_stream and commits the accepted length).
+  struct StreamData final {
+    // The actual number of vectors in the struct, up to kMaxVectorCount.
+    size_t count = 0;
+    // The stream identifier. If this is a negative value then no stream is
+    // identified.
+    stream_id id = -1;
+    int fin = 0;
+    ngtcp2_vec data[kMaxVectorCount]{};
+    BaseObjectPtr<Stream> stream;
+
+    inline operator const ngtcp2_vec*() const { return data; }
+    inline operator ngtcp2_vec*() { return data; }
+
+    std::string ToString() const;
+  };
+
   // Decode the first ALPN protocol name from wire format (length-prefixed).
   static std::string_view DecodeAlpn(std::string_view wire);
 
@@ -446,8 +446,8 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
   Packet::Ptr CreateStreamDataPacket();
 
   // Tries to pack a pending datagram into the current packet buffer.
-  // If < 0 is returned, either NGTCP2_ERR_WRITE_MORE or a fatal error is
-  // returned; the caller must check. If > 0 is returned, the packet is done
+  // If a negative value is returned, it is either NGTCP2_ERR_WRITE_MORE or
+  // fatal error; the caller must check. If > 0 is returned, the packet is done
   // and the value is the size of the finalized packet. If 0 is returned,
   // the datagram is either congestion limited or was abandoned.
   ssize_t TryWritePendingDatagram(PathStorage* path,
@@ -539,11 +539,12 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
   // the session, encapsulating ngtcp2 transport details here.
 
   // Open a unidirectional stream, setting *id on success, or returning false
-  bool OpenUni(stream_id* id);
+  bool OpenUnidirectionalStream(stream_id* id);
 
   void ExtendMaxStreams(Direction direction, uint64_t max);
 
-  // Signal that we've consumed $len bytes on stream $id to update flow control
+  // Signal that we've consumed `len` bytes on stream `id` to update flow
+  // control
   void Consume(stream_id id, size_t len);
 
   // Record an application-level error on the connection without closing it.