Create a QuicChromiumClientStream::Handle class for allowing a stream

net/quic/chromium/quic_chromium_client_stream.h

Index: net/quic/chromium/quic_chromium_client_stream.h
diff --git a/net/quic/chromium/quic_chromium_client_stream.h b/net/quic/chromium/quic_chromium_client_stream.h
index d65f2e5abfa03bff1adafde1761faff54b89e0e0..03d58ab04c60a3cec75757415b782fa764bddb06 100644
--- a/net/quic/chromium/quic_chromium_client_stream.h
+++ b/net/quic/chromium/quic_chromium_client_stream.h
@@ -32,6 +32,8 @@ class QuicClientSessionBase;
 // are owned by the QuicClientSession which created them.
 class NET_EXPORT_PRIVATE QuicChromiumClientStream : public QuicSpdyStream {
  public:
+  // TODO(rch): Remove this class completely in favor of async methods
+  // on the Handle.
   // Delegate handles protocol specific behavior of a quic stream.
   class NET_EXPORT_PRIVATE Delegate {
    public:
@@ -61,6 +63,112 @@ class NET_EXPORT_PRIVATE QuicChromiumClientStream : public QuicSpdyStream {
     DISALLOW_COPY_AND_ASSIGN(Delegate);
   };
 
+  // Wrapper for interacting with the session in a restricted fashion.
+  class NET_EXPORT_PRIVATE Handle {
+   public:
+    ~Handle();
+
+    // Returns true if the stream is still connected.
+    bool IsOpen() { return stream_ != nullptr; }
+
+    // Writes |header_block| to the peer. Closes the write side if |fin| is
+    // true. If non-null, |ack_notifier_delegate| will be notified when the
+    // headers are ACK'd by the peer.
+    size_t WriteHeaders(SpdyHeaderBlock header_block,
+                        bool fin,
+                        QuicReferenceCountedPointer<QuicAckListenerInterface>
+                            ack_notifier_delegate);
+
+    // Writes |data| to the peer. Closes the write side if |fin| is true.
+    // If the data could not be written immediately, returns ERR_IO_PENDING
+    // and invokes |callback| asynchronously when the write completes.
+    int WriteStreamData(base::StringPiece data,
+                        bool fin,
+                        const CompletionCallback& callback);
+
+    // Same as WriteStreamData except it writes data from a vector of IOBuffers,
+    // with the length of each buffer at the corresponding index in |lengths|.
+    int WritevStreamData(const std::vector<scoped_refptr<IOBuffer>>& buffers,
+                         const std::vector<int>& lengths,
+                         bool fin,
+                         const CompletionCallback& callback);
+
+    // Reads at most |buf_len| bytes into |buf|. Returns the number of bytes
+    // read.
+    int Read(IOBuffer* buf, int buf_len);
+
+    // Called to notify the stream when the final incoming data is read.
+    void OnFinRead();
+
+    // Prevents the connection from migrating to a new network while this
+    // stream is open.
+    void DisableConnectionMigration();
+
+    // Sets the priority of the stream to |priority|.
+    void SetPriority(SpdyPriority priority);
+
+    // Sends a RST_STREAM frame to the peer and closes the streams.
+    void Reset(QuicRstStreamErrorCode error_code);
+
+    // Clears |delegate_| from this Handle, but does not disconnect the Handle
+    // from |stream_|.
+    void ClearDelegate();
+
+    QuicStreamId id() const;
+    QuicErrorCode connection_error() const;
+    QuicRstStreamErrorCode stream_error() const;
+    bool fin_sent() const;
+    bool fin_received() const;
+    uint64_t stream_bytes_read() const;
+    uint64_t stream_bytes_written() const;
+    size_t NumBytesConsumed() const;
+    bool IsDoneReading() const;
+    bool IsFirstStream() const;
+
+    // TODO(rch): Move these test-only methods to a peer, or else remove.
+    SpdyPriority priority() const;
+    bool can_migrate();
+    void OnPromiseHeaderList(QuicStreamId promised_id,

[xunjieli, 2017/05/08 19:51:00]

This one doesn't seem to be test-only.

[Ryan Hamilton, 2017/05/08 20:58:06]

I made it private, and the only error is from quic_http_stream_test.cc, so it
looks test-only to me. Do you see it used elsewhere?

[xunjieli, 2017/05/09 15:03:46]

Acknowledged. I think I mistakenly assumed that this was the same one as the
Stream's OnPromiseHeaderList().

[Ryan Hamilton, 2017/05/09 16:48:19]

I so know the feeling! I keep going back and forth between the HttpStream, the
Stream::Handle and the QuicStream. *head explodes*

+                             size_t frame_len,
+                             const QuicHeaderList& header_list);
+    Delegate* GetDelegate();
+
+   private:
+    friend class QuicChromiumClientStream;
+
+    // Constucts a new Handle for |stream| with |delegate| set to receive
+    // up calls on various events.
+    Handle(QuicChromiumClientStream* stream, Delegate* delegate);
+
+    // Methods invoked by the stream.
+    void OnInitialHeadersAvailable(const SpdyHeaderBlock& headers,
+                                   size_t frame_len);
+    void OnTrailingHeadersAvailable(const SpdyHeaderBlock& headers,
+                                    size_t frame_len);
+    void OnDataAvailable();
+    void OnClose();
+    void OnError(int error);
+
+    // Saves various fields from the stream before the stream goes away.
+    void SaveState();
+
+    QuicChromiumClientStream* stream_;  // Unowned.
+    Delegate* delegate_;                // Owns this.

[xunjieli, 2017/05/08 19:51:00]

optional: I only see this commenting style in QUIC code. If possible, can we
standardize it so that the comment is placed above the member?

[Ryan Hamilton, 2017/05/08 20:58:05]

It appears to be used in other places in net too. For example:

https://cs.chromium.org/chromium/src/net/http/http_cache_transaction.h?rcl=53...

[xunjieli, 2017/05/09 15:03:46]

Acknowledged. Good point. 

+
+    QuicStreamId id_;
+    QuicErrorCode connection_error_;
+    QuicRstStreamErrorCode stream_error_;
+    bool fin_sent_;
+    bool fin_received_;
+    uint64_t stream_bytes_read_;
+    uint64_t stream_bytes_written_;
+    bool is_done_reading_;
+    bool is_first_stream_;
+    size_t num_bytes_consumed_;
+
+    DISALLOW_COPY_AND_ASSIGN(Handle);
+  };
+
   QuicChromiumClientStream(QuicStreamId id,
                            QuicClientSessionBase* session,
                            const NetLogWithSource& net_log);
@@ -99,11 +207,15 @@ class NET_EXPORT_PRIVATE QuicChromiumClientStream : public QuicSpdyStream {
                        const std::vector<int>& lengths,
                        bool fin,
                        const CompletionCallback& callback);
-  // Set new |delegate|. |delegate| must not be NULL.
-  // If this stream has already received data, OnDataReceived() will be
-  // called on the delegate.
-  void SetDelegate(Delegate* delegate);
-  Delegate* GetDelegate() { return delegate_; }
+
+  // Creates a new Handle for this stream and sets |delegate| on the handle.
+  // Must only be called once.
+  std::unique_ptr<QuicChromiumClientStream::Handle> CreateHandle(
+      QuicChromiumClientStream::Delegate* delegate);
+
+  // Clears |handle_| from this stream.
+  void ClearHandle();
+
   void OnError(int error);
 
   // Reads at most |buf_len| bytes into |buf|. Returns the number of bytes read.
@@ -124,19 +236,19 @@ class NET_EXPORT_PRIVATE QuicChromiumClientStream : public QuicSpdyStream {
   using QuicStream::sequencer;
 
  private:
-  void NotifyDelegateOfInitialHeadersAvailableLater(SpdyHeaderBlock headers,
-                                                    size_t frame_len);
-  void NotifyDelegateOfInitialHeadersAvailable(SpdyHeaderBlock headers,
-                                               size_t frame_len);
-  void NotifyDelegateOfTrailingHeadersAvailableLater(SpdyHeaderBlock headers,
-                                                     size_t frame_len);
-  void NotifyDelegateOfTrailingHeadersAvailable(SpdyHeaderBlock headers,
-                                                size_t frame_len);
-  void NotifyDelegateOfDataAvailableLater();
-  void NotifyDelegateOfDataAvailable();
+  void NotifyHandleOfInitialHeadersAvailableLater(SpdyHeaderBlock headers,
+                                                  size_t frame_len);
+  void NotifyHandleOfInitialHeadersAvailable(SpdyHeaderBlock headers,
+                                             size_t frame_len);
+  void NotifyHandleOfTrailingHeadersAvailableLater(SpdyHeaderBlock headers,
+                                                   size_t frame_len);
+  void NotifyHandleOfTrailingHeadersAvailable(SpdyHeaderBlock headers,
+                                              size_t frame_len);
+  void NotifyHandleOfDataAvailableLater();
+  void NotifyHandleOfDataAvailable();
 
   NetLogWithSource net_log_;
-  Delegate* delegate_;
+  Handle* handle_;
 
   bool headers_delivered_;