Add a new QuicChromiumClientSession::Handle class

net/quic/chromium/quic_chromium_client_session.h

Index: net/quic/chromium/quic_chromium_client_session.h
diff --git a/net/quic/chromium/quic_chromium_client_session.h b/net/quic/chromium/quic_chromium_client_session.h
index 8576051f88d37eb9b4393b7ae68d56d0befe5523..71390079089e6946d0755ae1e4898f137cdb8835 100644
--- a/net/quic/chromium/quic_chromium_client_session.h
+++ b/net/quic/chromium/quic_chromium_client_session.h
@@ -66,13 +66,106 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
       public QuicChromiumPacketReader::Visitor,
       public QuicChromiumPacketWriter::Delegate {
  public:
-  // An interface for observing events on a session.
-  class NET_EXPORT_PRIVATE Observer {
+  class StreamRequest;
+
+  // Wrapper for interacting with the session in a restricted fashion which
+  // hides the details of the underlying session's lifetime. All methods of
+  // the Handle are safe to use even after the underlying session is destroyed.
+  class NET_EXPORT_PRIVATE Handle : public MultiplexedSessionHandle {

[xunjieli, 2017/05/04 16:54:42]

Can we disallow copy and assign for this class so know where instances of Handle
are created?

[Ryan Hamilton, 2017/05/05 03:50:24]

Done.

    public:
-    virtual ~Observer() {}
-    virtual void OnCryptoHandshakeConfirmed() = 0;
-    virtual void OnSuccessfulVersionNegotiation(const QuicVersion& version) = 0;
-    virtual void OnSessionClosed(int error, bool port_migration_detected) = 0;
+    explicit Handle(const base::WeakPtr<QuicChromiumClientSession>& session);
+    Handle(const Handle& other);
+    ~Handle();
+
+    // Returns true if the session is still connected.
+    bool IsConnected() const;
+
+    // Returns true if the handshake has been confirmed.
+    // the handle is not valid.

[xunjieli, 2017/05/04 16:54:42]

nit: delete the second line.

[Ryan Hamilton, 2017/05/05 03:50:25]

Done.

+    bool IsCryptoHandshakeConfirmed() const;
+
+    // Returns a new stream request which can be used to create a new
+    // QUIC stream. If |requires_confirmation| is true, then the requested
+    // stream will not be created until the handshake as been confirmed.

[xunjieli, 2017/05/04 16:54:42]

nit: s/as/has

[Ryan Hamilton, 2017/05/05 03:50:24]

Removed this method completely. Yay! :)

+    std::unique_ptr<StreamRequest> CreateStreamRequest(
+        bool requires_confirmation) const;
+
+    // Sends Rst for the stream, and makes sure that future calls to
+    // IsClosedStream(id) return true, which ensures that any subsequent
+    // frames related to this stream will be ignored (modulo flow
+    // control accounting).
+    void ResetPromised(QuicStreamId id, QuicRstStreamErrorCode error_code);
+
+    // Returns a new packet bundler while will cause writes to be batched up
+    // until a packet is full, or the last bundler is destroyed.
+    std::unique_ptr<QuicConnection::ScopedPacketBundler> CreatePacketBundler(
+        QuicConnection::AckBundling bundling_mode);
+
+    // Populates network error details for this session.
+    void PopulateNetErrorDetails(NetErrorDetails* details) const;
+
+    // Returns the connection timing for the handshake of this session.
+    const LoadTimingInfo::ConnectTiming& GetConnectTiming();
+
+    // Signs the exported keying material used for Token Binding using key
+    // |*key| and puts the signature in |*out|. Returns a net error code.
+    Error GetTokenBindingSignature(crypto::ECPrivateKey* key,
+                                   TokenBindingType tb_type,
+                                   std::vector<uint8_t>* out);
+
+    // Returns true if |other| is a handle to the same session as this handle.
+    bool SharesSameSession(const Handle& other) const;
+
+    // Returns the QUIC version used by the session.
+    QuicVersion GetQuicVersion() const;
+
+    // Copies the remote udp address into |address| and returns a net error
+    // code.
+    int GetPeerAddress(IPEndPoint* address) const;
+
+    // Returns the push promise index associated with the session.
+    QuicClientPushPromiseIndex* GetPushPromiseIndex();
+
+    // Returns the session's server ID.
+    QuicServerId server_id() const { return server_id_; }
+
+    // Returns the session's net log.
+    const NetLogWithSource& net_log() const { return net_log_; }
+
+   private:
+    friend class QuicChromiumClientSession;
+    friend class QuicChromiumClientSession::StreamRequest;
+
+    // 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);
+
+    // Called when the handshake is confirmed.
+    void OnCryptoHandshakeConfirmed();
+
+    // Called when the session is closed with a net error.
+    void OnSessionClosed(int error, bool port_migration_detected);
+
+    // Called by |request| to create a stream.
+    int TryCreateStream(StreamRequest* request);
+
+    // Called by |request| to cancel stream request.
+    void CancelRequest(StreamRequest* request);
+
+    base::WeakPtr<QuicChromiumClientSession> session_;
+    NetLogWithSource net_log_;
+
+    // Information latched from the session when it is closed.

[xunjieli, 2017/05/04 16:54:42]

nit: consider rephrasing "latched" to "saved" or something more general.

[Ryan Hamilton, 2017/05/05 03:50:24]

Heh, sure. "latched" is pretty common vocabulary in GFE land, but this isn't GFE
code :)

+    bool was_handshake_confirmed_;
+    int error_;
+    bool port_migration_detected_;
+    QuicServerId server_id_;
+    QuicVersion quic_version_;
+    LoadTimingInfo::ConnectTiming connect_timing_;
+    QuicClientPushPromiseIndex* push_promise_index_;
   };
 
   // A helper class used to manage a request to create a stream.
@@ -102,7 +195,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
       STATE_REQUEST_STREAM_COMPLETE,
     };
 
-    StreamRequest(const base::WeakPtr<QuicChromiumClientSession>& session,
+    StreamRequest(QuicChromiumClientSession::Handle session,
                   bool requires_confirmation);
 
     void OnIOComplete(int rv);
@@ -123,7 +216,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
     // if |session_| is destroyed while the stream request is still pending.
     void OnRequestCompleteFailure(int rv);
 
-    base::WeakPtr<QuicChromiumClientSession> session_;
+    QuicChromiumClientSession::Handle session_;
     const bool requires_confirmation_;
     CompletionCallback callback_;
     QuicChromiumClientStream* stream_;
@@ -166,8 +259,8 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
 
   void Initialize() override;
 
-  void AddObserver(Observer* observer);
-  void RemoveObserver(Observer* observer);
+  void AddHandle(Handle* handle);
+  void RemoveHandle(Handle* handle);
 
   // Waits for the handshake to be confirmed and invokes |callback| when
   // that happens. If the handshake has already been confirmed, returns OK.
@@ -176,12 +269,6 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
   // be invoked with an error.
   int WaitForHandshakeConfirmation(const CompletionCallback& callback);
 
-  // Returns a new stream request which can be used to create a new
-  // QUIC stream. If |requires_confirmation| is true, then the requested
-  // stream will not be created until the handshake as been confirmed.
-  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|
@@ -264,7 +351,8 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
 
   const NetLogWithSource& net_log() const { return net_log_; }
 
-  base::WeakPtr<QuicChromiumClientSession> GetWeakPtr();
+  // Return a Handle to this session.
+  QuicChromiumClientSession::Handle GetHandle();
 
   // Returns the number of client hello messages that have been sent on the
   // crypto stream. If the handshake has completed then this is one greater
@@ -316,7 +404,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
   void OnMigrationTimeout(size_t num_sockets);
 
   // Populates network error details for this session.
-  void PopulateNetErrorDetails(NetErrorDetails* details);
+  void PopulateNetErrorDetails(NetErrorDetails* details) const;
 
   // Returns current default socket. This is the socket over which all
   // QUIC packets are sent. This default socket can change, so do not store the
@@ -360,7 +448,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
  private:
   friend class test::QuicChromiumClientSessionPeer;
 
-  typedef std::set<Observer*> ObserverSet;
+  typedef std::set<Handle*> HandleSet;
   typedef std::list<StreamRequest*> StreamRequestQueue;
 
   QuicChromiumClientStream* CreateOutgoingReliableStreamImpl();
@@ -371,7 +459,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
   void OnClosedStream();
 
   void CloseAllStreams(int net_error);
-  void CloseAllObservers(int net_error);
+  void CloseAllHandles(int net_error);
   void CancelAllRequests(int net_error);
   void NotifyRequestsOfConfirmation(int net_error);
 
@@ -398,7 +486,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
   std::unique_ptr<ct::CTVerifyResult> ct_verify_result_;
   std::string pinning_failure_log_;
   bool pkp_bypassed_;
-  ObserverSet observers_;
+  HandleSet handles_;
   StreamRequestQueue stream_requests_;
   std::vector<CompletionCallback> waiting_for_confirmation_callbacks_;
   CompletionCallback callback_;
@@ -430,6 +518,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
   // sockets_.size(). Then in MigrateSessionOnError, check to see if
   // the current sockets_.size() == the passed in value.
   bool migration_pending_;  // True while migration is underway.
+  std::unique_ptr<Handle> self_handle_;  // Handle to this session
   base::WeakPtrFactory<QuicChromiumClientSession> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(QuicChromiumClientSession);