Move the logic for delaying 0-RTT QUIC POST from the QuicStreamFactory

net/quic/bidirectional_stream_quic_impl_unittest.cc

 // Copyright 2016 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.
 
 #include "net/quic/bidirectional_stream_quic_impl.h"
 
 #include "base/callback_helpers.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 50 matching lines @@
                    std::unique_ptr<base::Timer> timer)
       : read_buf_(read_buf),
         read_buf_len_(read_buf_len),
         timer_(std::move(timer)),
         loop_(nullptr),
         error_(OK),
         on_data_read_count_(0),
         on_data_sent_count_(0),
         not_expect_callback_(false),
         on_failed_called_(false),
-        send_request_headers_automatically_(true) {
+        send_request_headers_automatically_(true),
+        is_ready_(false) {
     loop_.reset(new base::RunLoop);
   }
 
   ~TestDelegateBase() override {}
 
   void OnStreamReady(bool request_headers_sent) override {
+    CHECK(!is_ready_);
     CHECK(!on_failed_called_);
     EXPECT_EQ(send_request_headers_automatically_, request_headers_sent);
     CHECK(!not_expect_callback_);
     loop_->Quit();
+    is_ready_ = true;

[xunjieli, 2016/06/17 18:05:42]

nit: maybe set |is_ready_| before exiting the |loop_|? 

is_ready_ = true;
loop_->Quit();

[Ryan Hamilton, 2016/06/17 18:20:35]

Done.

   }
 
   void OnHeadersReceived(const SpdyHeaderBlock& response_headers) override {
     CHECK(!on_failed_called_);
     CHECK(!not_expect_callback_);
 
     response_headers_ = response_headers;
     loop_->Quit();
   }
 
@@ skipping 95 matching lines @@
   }
 
   // Const getters for internal states.
   const std::string& data_received() const { return data_received_; }
   int error() const { return error_; }
   const SpdyHeaderBlock& response_headers() const { return response_headers_; }
   const SpdyHeaderBlock& trailers() const { return trailers_; }
   int on_data_read_count() const { return on_data_read_count_; }
   int on_data_sent_count() const { return on_data_sent_count_; }
   bool on_failed_called() const { return on_failed_called_; }
+  bool is_ready() const { return is_ready_; }
 
  protected:
   // Quits |loop_|.
   void QuitLoop() { loop_->Quit(); }
 
   // Deletes |stream_|.
   void DeleteStream() { stream_.reset(); }
 
  private:
   std::unique_ptr<BidirectionalStreamQuicImpl> stream_;
   scoped_refptr<IOBuffer> read_buf_;
   int read_buf_len_;
   std::unique_ptr<base::Timer> timer_;
   std::string data_received_;
   std::unique_ptr<base::RunLoop> loop_;
   SpdyHeaderBlock response_headers_;
   SpdyHeaderBlock trailers_;
   int error_;
   int on_data_read_count_;
   int on_data_sent_count_;
   // This is to ensure that delegate callback is not invoked synchronously when
   // calling into |stream_|.
   bool not_expect_callback_;
   bool on_failed_called_;
   CompletionCallback callback_;
   bool send_request_headers_automatically_;
+  bool is_ready_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDelegateBase);
 };
 
 // A delegate that deletes the stream in a particular callback.
 class DeleteStreamDelegate : public TestDelegateBase {
  public:
   // Specifies in which callback the stream can be deleted.
   enum Phase {
     ON_HEADERS_RECEIVED,
@@ skipping 102 matching lines @@
   void AddWrite(std::unique_ptr<QuicReceivedPacket> packet) {
     writes_.push_back(PacketToWrite(SYNCHRONOUS, packet.release()));
   }
 
   void ProcessPacket(std::unique_ptr<QuicReceivedPacket> packet) {
     connection_->ProcessUdpPacket(self_addr_, peer_addr_, *packet);
   }
 
   // Configures the test fixture to use the list of expected writes.
   void Initialize() {
+    crypto_client_stream_factory_.set_handshake_mode(
+        MockCryptoClientStream::ZERO_RTT);
     mock_writes_.reset(new MockWrite[writes_.size()]);
     for (size_t i = 0; i < writes_.size(); i++) {
       if (writes_[i].packet == nullptr) {
         mock_writes_[i] = MockWrite(writes_[i].mode, writes_[i].rv, i);
       } else {
         mock_writes_[i] = MockWrite(writes_[i].mode, writes_[i].packet->data(),
                                     writes_[i].packet->length());
       }
     }
 
@@ skipping 20 matching lines @@
         QuicServerId(kDefaultServerHostName, kDefaultServerPort,
                      PRIVACY_MODE_DISABLED),
         kQuicYieldAfterPacketsRead,
         QuicTime::Delta::FromMilliseconds(kQuicYieldAfterDurationMilliseconds),
         /*cert_verify_flags=*/0, DefaultQuicConfig(), &crypto_config_,
         "CONNECTION_UNKNOWN", base::TimeTicks::Now(), &push_promise_index_,
         base::ThreadTaskRunnerHandle::Get().get(),
         /*socket_performance_watcher=*/nullptr, net_log().bound().net_log()));
     session_->Initialize();
     session_->GetCryptoStream()->CryptoConnect();
-    EXPECT_TRUE(session_->IsCryptoHandshakeConfirmed());
+    EXPECT_TRUE(session_->IsEncryptionEstablished());
+  }
+
+  void ConfirmHandshake() {
+    crypto_client_stream_factory_.last_stream()->SendOnCryptoHandshakeEvent(
+        QuicSession::HANDSHAKE_CONFIRMED);
   }
 
   void SetRequest(const std::string& method,
                   const std::string& path,
                   RequestPriority priority) {
     request_headers_ = client_maker_.GetRequestHeaders(method, "http", path);
   }
 
   SpdyHeaderBlock ConstructResponseHeaders(const std::string& response_code) {
     return server_maker_.GetResponseHeaders(response_code);
@@ skipping 312 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->DoNotSendRequestHeadersAutomatically();
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());

[xunjieli, 2016/06/17 18:05:42]

Does QuicStreamRequest always finish synchronously? If it does, maybe worth
adding a comment here. If it doesn't, maybe add a 
base::RunLoop().RunUntilIdle() to flush posted tasks from the message loop?

[Ryan Hamilton, 2016/06/17 18:20:35]

No, it's not guaranteed to complete synchronously. For example, if there are max
streams open, it will not complete until the next stream closes. Or if handshake
confirmation is required, it won't return until the SHLO is received. I'm not
sure that adding RunUntilIdle() is a good idea though.

+  EXPECT_FALSE(delegate->is_ready());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Sends request headers separately, which causes them to be sent in a
   // separate packet.
   delegate->SendRequestHeaders();
   // Send a Data packet.
   scoped_refptr<StringIOBuffer> buf1(new StringIOBuffer(kBody1));
   scoped_refptr<StringIOBuffer> buf2(new StringIOBuffer(kBody2));
 
   std::vector<int> lengths = {buf1->size(), buf2->size()};
@@ skipping 83 matching lines @@
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->DoNotSendRequestHeadersAutomatically();
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Send a Data packet.
   scoped_refptr<StringIOBuffer> buf1(new StringIOBuffer(kBody1));
 
   delegate->SendData(buf1, buf1->size(), false);
   delegate->WaitUntilNextCallback();  // OnDataSent
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
@@ skipping 77 matching lines @@
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->DoNotSendRequestHeadersAutomatically();
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Send a Data packet.
   scoped_refptr<StringIOBuffer> buf1(new StringIOBuffer(kBody1));
   scoped_refptr<StringIOBuffer> buf2(new StringIOBuffer(kBody2));
 
   std::vector<int> lengths = {buf1->size(), buf2->size()};
   delegate->SendvData({buf1, buf2}, lengths, !kFin);
   delegate->WaitUntilNextCallback();  // OnDataSent
 
@@ skipping 70 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Send a DATA frame.
   scoped_refptr<StringIOBuffer> buf(new StringIOBuffer(kUploadData));
 
   delegate->SendData(buf, buf->size(), true);
   delegate->WaitUntilNextCallback();  // OnDataSent
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
@@ skipping 58 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
   size_t spdy_response_headers_frame_length;
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, response_headers, &spdy_response_headers_frame_length, 0));
@@ skipping 59 matching lines @@
   request.method = "GET";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = true;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
   delegate->WaitUntilNextCallback();  // OnStreamReady
+  ConfirmHandshake();
 
   // Server sends a Rst.
   ProcessPacket(ConstructServerRstStreamPacket(1));
 
   delegate->WaitUntilNextCallback();  // OnFailed
   TestCompletionCallback cb;
   EXPECT_EQ(ERR_QUIC_PROTOCOL_ERROR, delegate->ReadData(cb.callback()));
 
   base::RunLoop().RunUntilIdle();
 
@@ skipping 19 matching lines @@
   request.method = "GET";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = true;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
   delegate->WaitUntilNextCallback();  // OnStreamReady
+  ConfirmHandshake();
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
 
   size_t spdy_response_headers_frame_length;
   QuicStreamOffset offset = 0;
   ProcessPacket(ConstructResponseHeadersPacket(
@@ skipping 37 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
   size_t spdy_response_headers_frame_length;
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, response_headers, &spdy_response_headers_frame_length, 0));
@@ skipping 35 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
 
   size_t spdy_response_headers_frame_length;
   QuicStreamOffset offset = 0;
@@ skipping 38 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<TestDelegateBase> delegate(
       new TestDelegateBase(read_buffer.get(), kReadBufferSize));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
   size_t spdy_response_headers_frame_length;
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, response_headers, &spdy_response_headers_frame_length, 0));
@@ skipping 30 matching lines @@
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<DeleteStreamDelegate> delegate(
       new DeleteStreamDelegate(read_buffer.get(), kReadBufferSize,
                                DeleteStreamDelegate::ON_HEADERS_RECEIVED));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
 
   size_t spdy_response_headers_frame_length;
   ProcessPacket(ConstructResponseHeadersPacket(
@@ skipping 22 matching lines @@
   BidirectionalStreamRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = false;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<DeleteStreamDelegate> delegate(new DeleteStreamDelegate(
       read_buffer.get(), kReadBufferSize, DeleteStreamDelegate::ON_DATA_READ));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
+  ConfirmHandshake();
   delegate->WaitUntilNextCallback();  // OnStreamReady
 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
 
   size_t spdy_response_headers_frame_length;
   ProcessPacket(ConstructResponseHeadersPacket(
@@ skipping 33 matching lines @@
   request.url = GURL("http://www.google.com/");
   request.end_stream_on_headers = true;
   request.priority = DEFAULT_PRIORITY;
 
   scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
   std::unique_ptr<DeleteStreamDelegate> delegate(
       new DeleteStreamDelegate(read_buffer.get(), kReadBufferSize,
                                DeleteStreamDelegate::ON_TRAILERS_RECEIVED));
   delegate->Start(&request, net_log().bound(), session()->GetWeakPtr());
   delegate->WaitUntilNextCallback();  // OnStreamReady
+  ConfirmHandshake();

[xunjieli, 2016/06/17 18:05:43]

You have examples here to demonstrate that GET doesn't require handshake
confirmation before sending the headers. 
Could you make a new unit test where we exercise PUT? You can duplicate the code
from BidirectionalStreamQuicImplTest.PostRequest, and change the method to PUT.
We won't gain much from having that test, but it serves as an example that PUT
also does not require handshake confirmation in 0RTT.

[Ryan Hamilton, 2016/06/17 18:20:35]

Done.

 
   // Server acks the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0));
 
   // Server sends the response headers.
   SpdyHeaderBlock response_headers = ConstructResponseHeaders("200");
 
   QuicStreamOffset offset = 0;
   size_t spdy_response_headers_frame_length;
   ProcessPacket(ConstructResponseHeadersPacket(
@@ skipping 29 matching lines @@
 
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(1, delegate->on_data_read_count());
   EXPECT_EQ(0, delegate->on_data_sent_count());
 }
 
 }  // namespace test
 
 }  // namespace net