Move the async write handling from QuicChromiumClientStream to the Handle

net/quic/chromium/quic_chromium_client_stream.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // NOTE: This code is not shared between Google and Chrome.
 
 #ifndef NET_QUIC_CHROMIUM_QUIC_CHROMIUM_CLIENT_STREAM_H_
 #define NET_QUIC_CHROMIUM_QUIC_CHROMIUM_CLIENT_STREAM_H_
 
 #include <stddef.h>
@@ skipping 139 matching lines @@
 
     // 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();
     void OnTrailingHeadersAvailable(const SpdyHeaderBlock& headers,
                                     size_t frame_len);
     void OnDataAvailable();
+    void OnCanWrite();
     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.
 
+    // Callback to be invoked when ReadHeaders completes asynchronously.
     CompletionCallback read_headers_callback_;
     SpdyHeaderBlock* read_headers_buffer_;
 
+    // Callback to be invoked when ReadBody completes asynchronously.
     CompletionCallback read_body_callback_;
     IOBuffer* read_body_buffer_;
     int read_body_buffer_len_;
 
+    // Callback to be invoked when WriteStreamData or WritevStreamData completes
+    // asynchronously.
+    CompletionCallback write_callback_;
+
     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_;
@@ skipping 24 matching lines @@
   size_t WriteHeaders(SpdyHeaderBlock header_block,
                       bool fin,
                       QuicReferenceCountedPointer<QuicAckListenerInterface>
                           ack_listener) override;
   SpdyPriority priority() const override;
 
   // While the server's set_priority shouldn't be called externally, the creator
   // of client-side streams should be able to set the priority.
   using QuicSpdyStream::SetPriority;
 
-  int WriteStreamData(QuicStringPiece data,
-                      bool fin,
-                      const CompletionCallback& callback);
+  // Writes |data| to the peer, loses the write side if |fin| is true and

[xunjieli, 2017/05/15 15:48:36]

minor nit: Maybe break the sentence after "and" or remove "Returns false
otherwise." 

e.g. 

Writes |data| to the peer and closes the write side if |fin| is true.
Returns true if the data have been fully written.

[xunjieli, 2017/05/15 15:48:36]

nit: loses/closes

[Ryan Hamilton, 2017/05/20 22:24:54]

Done.

[Ryan Hamilton, 2017/05/20 22:24:54]

Done.

+  // returns true if the data could actually be written. Returns false

[xunjieli, 2017/05/15 15:48:37]

nit: "could actually be written" isn't as precise. Maybe consider rephrasing?

maybe s/could actually be written/have been fully written?

Could you also mention that if the data cannot be fully written synchronously,
OnCanWrite() will be invoked later?

[Ryan Hamilton, 2017/05/20 22:24:54]

Done.

+  // otherwise.
+  bool WriteStreamData(QuicStringPiece data, bool fin);
   // 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);
+  bool WritevStreamData(const std::vector<scoped_refptr<IOBuffer>>& buffers,
+                        const std::vector<int>& lengths,
+                        bool fin);
 
   // 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);
@@ skipping 28 matching lines @@
   void NotifyHandleOfDataAvailable();
 
   NetLogWithSource net_log_;
   Handle* handle_;
 
   bool headers_delivered_;
 
   // True when initial headers have been sent.
   bool initial_headers_sent_;
 
-  // Callback to be invoked when WriteStreamData or WritevStreamData completes
-  // asynchronously.
-  CompletionCallback write_callback_;
-
   QuicClientSessionBase* session_;
 
   // Set to false if this stream to not be migrated during connection migration.
   bool can_migrate_;
 
   // Stores the initial header if they arrive before the delegate.
   SpdyHeaderBlock initial_headers_;
   // Length of the HEADERS frame containing initial headers.
   size_t initial_headers_frame_len_;
 
   base::WeakPtrFactory<QuicChromiumClientStream> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(QuicChromiumClientStream);
 };
 
 }  // namespace net
 
 #endif  // NET_QUIC_CHROMIUM_QUIC_CHROMIUM_CLIENT_STREAM_H_