Do not compress QUIC headers until it is likely that they can be sent.

Review URL: https://codereview.chromium.org/86713002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237368 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 81 insertions, 42 deletions

diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index ca3ace0..28fe9dc 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -408,6 +408,10 @@ class NET_EXPORT_PRIVATE QuicConnection
     return congestion_manager_;
   }
 
+  bool CanWrite(TransmissionType transmission_type,
+                HasRetransmittableData retransmittable,
+                IsHandshake handshake);
+
  protected:
   // Send a packet to the peer using encryption |level|. If |sequence_number|
   // is present in the |retransmission_map_|, then contents of this packet will
@@ -474,10 +478,6 @@ class NET_EXPORT_PRIVATE QuicConnection
   friend class ScopedPacketBundler;
   friend class test::QuicConnectionPeer;
 
-  bool CanWrite(TransmissionType transmission_type,
-                HasRetransmittableData retransmittable,
-                IsHandshake handshake);
-
   // Packets which have not been written to the wire.
   // Owns the QuicPacket* packet.
   struct QueuedPacket {
diff --git a/net/quic/quic_http_stream.cc b/net/quic/quic_http_stream.cc
index 2887207b..d1d3549 100644
--- a/net/quic/quic_http_stream.cc
+++ b/net/quic/quic_http_stream.cc
@@ -99,19 +99,9 @@ int QuicHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
   QuicPriority priority = ConvertRequestPriorityToQuicPriority(priority_);
   stream_->set_priority(priority);
   // Store the serialized request headers.
-  SpdyHeaderBlock headers;
-  CreateSpdyHeadersFromHttpRequest(*request_info_, request_headers,
-                                   &headers, 3, /*direct=*/true);
-  request_ = stream_->compressor()->CompressHeadersWithPriority(priority,
-                                                                headers);
-  // Log the actual request with the URL Request's net log.
-  stream_net_log_.AddEvent(
-      NetLog::TYPE_HTTP_TRANSACTION_SPDY_SEND_REQUEST_HEADERS,
-      base::Bind(&SpdyHeaderBlockNetLogCallback, &headers));
-  // Also log to the QuicSession's net log.
-  stream_->net_log().AddEvent(
-      NetLog::TYPE_QUIC_HTTP_STREAM_SEND_REQUEST_HEADERS,
-      base::Bind(&SpdyHeaderBlockNetLogCallback, &headers));
+  CreateSpdyHeadersFromHttpRequest(
+      *request_info_, request_headers, &request_headers_,
+      /*version=*/3, /*direct=*/true);
 
   // Store the request body.
   request_body_stream_ = request_info_->upload_data_stream;
@@ -278,18 +268,6 @@ void QuicHttpStream::SetPriority(RequestPriority priority) {
   priority_ = priority;
 }
 
-int QuicHttpStream::OnSendData() {
-  // TODO(rch): Change QUIC IO to provide notifications to the streams.
-  NOTREACHED();
-  return OK;
-}
-
-int QuicHttpStream::OnSendDataComplete(int status, bool* eof) {
-  // TODO(rch): Change QUIC IO to provide notifications to the streams.
-  NOTREACHED();
-  return OK;
-}
-
 int QuicHttpStream::OnDataReceived(const char* data, int length) {
   DCHECK_NE(0, length);
   // Are we still reading the response headers.
@@ -424,6 +402,26 @@ int QuicHttpStream::DoSendHeaders() {
   if (!stream_)
     return ERR_UNEXPECTED;
 
+  if (request_.empty() && !stream_->CanWrite(
+          base::Bind(&QuicHttpStream::OnIOComplete,
+                     weak_factory_.GetWeakPtr()))) {
+    // Do not compress headers unless it is likely that they can be sent.
+    next_state_ = STATE_SEND_HEADERS;
+    return ERR_IO_PENDING;
+  }
+  request_ = stream_->compressor()->CompressHeadersWithPriority(
+      ConvertRequestPriorityToQuicPriority(priority_), request_headers_);
+
+  // Log the actual request with the URL Request's net log.
+  stream_net_log_.AddEvent(
+      NetLog::TYPE_HTTP_TRANSACTION_SPDY_SEND_REQUEST_HEADERS,
+      base::Bind(&SpdyHeaderBlockNetLogCallback, &request_headers_));
+  // Also log to the QuicSession's net log.
+  stream_->net_log().AddEvent(
+      NetLog::TYPE_QUIC_HTTP_STREAM_SEND_REQUEST_HEADERS,
+      base::Bind(&SpdyHeaderBlockNetLogCallback, &request_headers_));
+  request_headers_.clear();
+
   bool has_upload_data = request_body_stream_ != NULL;
 
   next_state_ = STATE_SEND_HEADERS_COMPLETE;
diff --git a/net/quic/quic_http_stream.h b/net/quic/quic_http_stream.h
index 31e6e99..314f529 100644
--- a/net/quic/quic_http_stream.h
+++ b/net/quic/quic_http_stream.h
@@ -62,8 +62,6 @@ class NET_EXPORT_PRIVATE QuicHttpStream :
   virtual void SetPriority(RequestPriority priority) OVERRIDE;
 
   // QuicReliableClientStream::Delegate implementation
-  virtual int OnSendData() OVERRIDE;
-  virtual int OnSendDataComplete(int status, bool* eof) OVERRIDE;
   virtual int OnDataReceived(const char* data, int length) OVERRIDE;
   virtual void OnClose(QuicErrorCode error) OVERRIDE;
   virtual void OnError(int error) OVERRIDE;
@@ -131,6 +129,9 @@ class NET_EXPORT_PRIVATE QuicHttpStream :
   // response.
   int response_status_;
 
+  // Serialized request headers.
+  SpdyHeaderBlock request_headers_;
+
   bool response_headers_received_;
 
   // Serialized HTTP request.
diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc
index ce6ee69..3e58dd0 100644
--- a/net/quic/quic_http_stream_test.cc
+++ b/net/quic/quic_http_stream_test.cc
@@ -729,6 +729,37 @@ TEST_F(QuicHttpStreamTest, CheckPriorityWithNoDelegate) {
   reliable_stream->SetDelegate(delegate);
 }
 
+TEST_F(QuicHttpStreamTest, DontCompressHeadersWhenNotWritable) {
+  SetRequestString("GET", "/", MEDIUM);
+  AddWrite(SYNCHRONOUS, ConstructDataPacket(1, true, kFin, 0, request_data_));
+
+  Initialize();
+  request_.method = "GET";
+  request_.url = GURL("http://www.google.com/");
+
+  EXPECT_CALL(*send_algorithm_, TimeUntilSend(_, _, _, _)).
+      WillRepeatedly(Return(QuicTime::Delta::Infinite()));
+  EXPECT_EQ(OK, stream_->InitializeStream(&request_, MEDIUM,
+                                          net_log_, callback_.callback()));
+  EXPECT_EQ(ERR_IO_PENDING, stream_->SendRequest(headers_, &response_,
+                                                 callback_.callback()));
+
+  // Verify that the headers have not been compressed and buffered in
+  // the stream.
+  QuicReliableClientStream* reliable_stream =
+      QuicHttpStreamPeer::GetQuicReliableClientStream(stream_.get());
+  EXPECT_FALSE(reliable_stream->HasBufferedData());
+  EXPECT_FALSE(AtEof());
+
+  EXPECT_CALL(*send_algorithm_, TimeUntilSend(_, _, _, _)).
+      WillRepeatedly(Return(QuicTime::Delta::Zero()));
+
+  // Data should flush out now.
+  connection_->OnCanWrite();
+  EXPECT_FALSE(reliable_stream->HasBufferedData());
+  EXPECT_TRUE(AtEof());
+}
+
 }  // namespace test
 
 }  // namespace net
diff --git a/net/quic/quic_reliable_client_stream.cc b/net/quic/quic_reliable_client_stream.cc
index 06b3178..a78d192 100644
--- a/net/quic/quic_reliable_client_stream.cc
+++ b/net/quic/quic_reliable_client_stream.cc
@@ -92,4 +92,16 @@ void QuicReliableClientStream::OnError(int error) {
   }
 }
 
+bool QuicReliableClientStream::CanWrite(const CompletionCallback& callback) {
+  bool can_write =  session()->connection()->CanWrite(
+      NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA,
+      id() == kCryptoStreamId ? IS_HANDSHAKE : NOT_HANDSHAKE);
+  if (!can_write) {
+    session()->MarkWriteBlocked(id(), EffectivePriority());
+    DCHECK(callback_.is_null());
+    callback_ = callback;
+  }
+  return can_write;
+}
+
 }  // namespace net
diff --git a/net/quic/quic_reliable_client_stream.h b/net/quic/quic_reliable_client_stream.h
index bf3fc15..8d43ac1 100644
--- a/net/quic/quic_reliable_client_stream.h
+++ b/net/quic/quic_reliable_client_stream.h
@@ -27,17 +27,6 @@ class NET_EXPORT_PRIVATE QuicReliableClientStream : public ReliableQuicStream {
    public:
     Delegate() {}
 
-    // Called when stream is ready to send data.
-    // Returns network error code. OK when it successfully sent data.
-    // ERR_IO_PENDING when performing operation asynchronously.
-    virtual int OnSendData() = 0;
-
-    // Called when data has been sent. |status| indicates network error
-    // or number of bytes that has been sent. On return, |eof| is set to true
-    // if no more data is available to send.
-    // Returns network error code. OK when it successfully sent data.
-    virtual int OnSendDataComplete(int status, bool* eof) = 0;
-
     // Called when data is received.
     // Returns network error code. OK when it successfully receives data.
     virtual int OnDataReceived(const char* data, int length) = 0;
@@ -84,8 +73,16 @@ class NET_EXPORT_PRIVATE QuicReliableClientStream : public ReliableQuicStream {
   Delegate* GetDelegate() { return delegate_; }
   void OnError(int error);
 
+  // Returns true if the stream can possible write data.  (The socket may
+  // turn out to be write blocked, of course).  If the stream can not write,
+  // this method returns false, and |callback| will be invoked when
+  // it becomes writable.
+  bool CanWrite(const CompletionCallback& callback);
+
   const BoundNetLog& net_log() const { return net_log_; }
 
+  using ReliableQuicStream::HasBufferedData;
+
  private:
   BoundNetLog net_log_;
   Delegate* delegate_;