Move the "wait for QUIC handshake confirmation" logic to QuicChromiumClientSession::StreamRequest

net/quic/chromium/quic_chromium_client_session.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.
 //
 // A client specific QuicSession subclass.  This class owns the underlying
 // QuicConnection and QuicConnectionHelper objects.  The connection stores
 // a non-owning pointer to the helper so this session needs to ensure that
 // the helper outlives the connection.
 
 #ifndef NET_QUIC_CHROMIUM_QUIC_CHROMIUM_CLIENT_SESSION_H_
@@ skipping 76 matching lines @@
     // ERR_IO_PENDING is returned, then when the request is eventuallly
     // complete |callback| will be called.
     int StartRequest(const CompletionCallback& callback);
 
     // Releases |stream_| to the caller
     QuicChromiumClientStream* ReleaseStream();
 
    private:
     friend class QuicChromiumClientSession;
 
-    StreamRequest(const base::WeakPtr<QuicChromiumClientSession>& session);
+    enum State {
+      STATE_NONE,
+      STATE_WAIT_FOR_CONFIRMATION,
+      STATE_WAIT_FOR_CONFIRMATION_COMPLETE,
+      STATE_REQUEST_STREAM,
+      STATE_REQUEST_STREAM_COMPLETE,
+    };
+
+    StreamRequest(const base::WeakPtr<QuicChromiumClientSession>& session,
+                  bool requires_confirmation);
+
+    void OnIOComplete(int rv);
+    void DoCallback(int rv);
+
+    int DoLoop(int rv);
+    int DoWaitForConfirmation();
+    int DoWaitForConfirmationComplete(int rv);
+    int DoRequestStream();
+    int DoRequestStreamComplete(int rv);
 
     // Called by |session_| for an asynchronous request when the stream
     // request has finished successfully.
     void OnRequestCompleteSuccess(QuicChromiumClientStream* stream);
 
     // Called by |session_| for an asynchronous request when the stream
     // request has finished with an error. Also called with ERR_ABORTED
     // if |session_| is destroyed while the stream request is still pending.
     void OnRequestCompleteFailure(int rv);
 
     base::WeakPtr<QuicChromiumClientSession> session_;
+    bool requires_confirmation_;

[xunjieli, 2017/05/01 20:15:06]

nit: add a const so we know this variable is not changed after construction.

[Ryan Hamilton, 2017/05/01 21:42:35]

Done.

     CompletionCallback callback_;
     QuicChromiumClientStream* stream_;
     // For tracking how much time pending stream requests wait.
     base::TimeTicks pending_start_time_;
+    State next_state_;
+
+    base::WeakPtrFactory<StreamRequest> weak_factory_;
 
     DISALLOW_COPY_AND_ASSIGN(StreamRequest);
   };
 
   // Constructs a new session which will own |connection|, but not
   // |stream_factory|, which must outlive this session.
   // TODO(rch): decouple the factory from the session via a Delegate interface.
   QuicChromiumClientSession(
       QuicConnection* connection,
       std::unique_ptr<DatagramClientSocket> socket,
@@ skipping 17 matching lines @@
       base::TaskRunner* task_runner,
       std::unique_ptr<SocketPerformanceWatcher> socket_performance_watcher,
       NetLog* net_log);
   ~QuicChromiumClientSession() override;
 
   void Initialize() override;
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  std::unique_ptr<StreamRequest> CreateStreamRequest();
+  // Waits for the handshake to be confirmed and invokes |callback| when
+  // that happens. If the handshake has already been confirmed, returns OK.
+  // If the connection has already been closed, returns a net error. If the
+  // connection closes before the handshake is confirmed, |callback| will
+  // be invoked with an error.
+  int WaitForHandshakeConfirmation(const CompletionCallback& callback);
+
+  // Returns an new stream request which can be used to create a new

[xunjieli, 2017/05/01 20:15:06]

nit: s/an/a

[Ryan Hamilton, 2017/05/01 21:42:35]

Done.

+  // QUIC stream. If |requires_confirmation| is true, then the requested
+  // stream will no be created until the handshake as been confirmed.

[xunjieli, 2017/05/01 20:15:06]

nit: 
s/will no/will not
s/as/has

[Ryan Hamilton, 2017/05/01 21:42:35]

Done.

+  std::unique_ptr<StreamRequest> CreateStreamRequest(
+      bool requires_confirmation);
 
   // Attempts to create a new stream.  If the stream can be
   // created immediately, returns OK.  If the open stream limit
   // has been reached, returns ERR_IO_PENDING, and |request|
   // will be added to the stream requets queue and will
   // be completed asynchronously.
   // TODO(rch): remove |stream| from this and use setter on |request|
   // and fix in spdy too.
   int TryCreateStream(StreamRequest* request);
 
@@ skipping 184 matching lines @@
   //     closed, perhaps due to a timeout or a protocol error.
   // CloseSessionOnError - called from the owner of the session,
   //     the QuicStreamFactory, when there is an error.
   // OnReadComplete - when there is a read error.
   // This method closes all stream and performs any necessary cleanup.
   void CloseSessionOnErrorInner(int net_error, QuicErrorCode quic_error);
 
   void CloseAllStreams(int net_error);
   void CloseAllObservers(int net_error);
   void CancelAllRequests(int net_error);
+  void NotifyAllWaitingForConfirmation(int net_error);

[xunjieli, 2017/05/01 20:15:06]

nit: maybe mention "Request" or "StreamRequest" somewhere in the method name so
it's clearer that we are notifying StreamRequests?

e.g. NotifyRequestsOfConfirmation?

[Ryan Hamilton, 2017/05/01 21:42:35]

Done.

 
   // Notifies the factory that this session is going away and no more streams
   // should be created from it.  This needs to be called before closing any
   // streams, because closing a stream may cause a new stream to be created.
   void NotifyFactoryOfSessionGoingAway();
 
   // Posts a task to notify the factory that this session has been closed.
   void NotifyFactoryOfSessionClosedLater();
 
   // Notifies the factory that this session has been closed which will
   // delete |this|.
   void NotifyFactoryOfSessionClosed();
 
   QuicServerId server_id_;
   bool require_confirmation_;
   std::unique_ptr<QuicCryptoClientStream> crypto_stream_;
   QuicStreamFactory* stream_factory_;
   std::vector<std::unique_ptr<DatagramClientSocket>> sockets_;
   TransportSecurityState* transport_security_state_;
   std::unique_ptr<QuicServerInfo> server_info_;
   std::unique_ptr<CertVerifyResult> cert_verify_result_;
   std::unique_ptr<ct::CTVerifyResult> ct_verify_result_;
   std::string pinning_failure_log_;
   bool pkp_bypassed_;
   ObserverSet observers_;
   StreamRequestQueue stream_requests_;
+  std::vector<CompletionCallback> waiting_for_confirmation_callbacks_;
   CompletionCallback callback_;
   size_t num_total_streams_;
   base::TaskRunner* task_runner_;
   NetLogWithSource net_log_;
   std::vector<std::unique_ptr<QuicChromiumPacketReader>> packet_readers_;
   LoadTimingInfo::ConnectTiming connect_timing_;
   std::unique_ptr<QuicConnectionLogger> logger_;
   // True when the session is going away, and streams may no longer be created
   // on this session. Existing stream will continue to be processed.
   bool going_away_;
@@ skipping 17 matching lines @@
   // the current sockets_.size() == the passed in value.
   bool migration_pending_;  // True while migration is underway.
   base::WeakPtrFactory<QuicChromiumClientSession> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(QuicChromiumClientSession);
 };
 
 }  // namespace net
 
 #endif  // NET_QUIC_CHROMIUM_QUIC_CHROMIUM_CLIENT_SESSION_H_