Add a new QuicChromiumClientSession::Handle class

net/quic/chromium/quic_http_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.
 
 #ifndef NET_QUIC_CHROMIUM_QUIC_HTTP_STREAM_H_
 #define NET_QUIC_CHROMIUM_QUIC_HTTP_STREAM_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 18 matching lines @@
 namespace net {
 
 namespace test {
 class QuicHttpStreamPeer;
 }  // namespace test
 
 // The QuicHttpStream is a QUIC-specific HttpStream subclass.  It holds a
 // non-owning pointer to a QuicChromiumClientStream which it uses to
 // send and receive data.
 class NET_EXPORT_PRIVATE QuicHttpStream
-    : public QuicChromiumClientSession::Observer,
-      public QuicChromiumClientStream::Delegate,
+    : public QuicChromiumClientStream::Delegate,
       public QuicClientPushPromiseIndex::Delegate,
       public MultiplexedHttpStream {
  public:
-  QuicHttpStream(const base::WeakPtr<QuicChromiumClientSession>& session,
+  QuicHttpStream(std::unique_ptr<QuicChromiumClientSession::Handle> session,
                  HttpServerProperties* http_server_properties);
 
   ~QuicHttpStream() override;
 
   // HttpStream implementation.
   int InitializeStream(const HttpRequestInfo* request_info,
                        RequestPriority priority,
                        const NetLogWithSource& net_log,
                        const CompletionCallback& callback) override;
   int SendRequest(const HttpRequestHeaders& request_headers,
@@ skipping 14 matching lines @@
   void PopulateNetErrorDetails(NetErrorDetails* details) override;
   void SetPriority(RequestPriority priority) override;
 
   // QuicChromiumClientStream::Delegate implementation
   void OnHeadersAvailable(const SpdyHeaderBlock& headers,
                           size_t frame_len) override;
   void OnDataAvailable() override;
   void OnClose() override;
   void OnError(int error) override;
 
-  // QuicChromiumClientSession::Observer implementation
-  void OnCryptoHandshakeConfirmed() override;
-  void OnSuccessfulVersionNegotiation(const QuicVersion& version) override;
-  void OnSessionClosed(int error, bool port_migration_detected) override;
-
   // QuicClientPushPromiseIndex::Delegate implementation
   bool CheckVary(const SpdyHeaderBlock& client_request,
                  const SpdyHeaderBlock& promise_request,
                  const SpdyHeaderBlock& promise_response) override;
   void OnRendezvousResult(QuicSpdyStream* stream) override;
 
   static HttpResponseInfo::ConnectionInfo ConnectionInfoFromQuicVersion(
       QuicVersion quic_version);
 
  private:
@@ skipping 43 matching lines @@
   int GetResponseStatus();
   // Sets the result of |ComputeResponseStatus()| as the |response_status_|.
   void SaveResponseStatus();
   // Sets |response_status_| to |response_status| and sets
   // |has_response_status_| to true.
   void SetResponseStatus(int response_status);
   // Computes the correct response status based on the status of the handshake,
   // |session_error|, |connection_error| and |stream_error|.
   int ComputeResponseStatus() const;
 
+  QuicChromiumClientSession::Handle* quic_session() {
+    return static_cast<QuicChromiumClientSession::Handle*>(session());
+  }
+
+  const QuicChromiumClientSession::Handle* quic_session() const {

[xunjieli, 2017/05/05 14:10:01]

We have two versions of accessors? Can we get rid of one?

C++ style guide discourages function overloads
https://google.github.io/styleguide/cppguide.html#Function_Overloading

"If you want to overload a function, consider qualifying the name with some
information about the arguments, e.g., AppendString(), AppendInt() rather than
just Append(). "

[Ryan Hamilton, 2017/05/05 17:32:47]

Can't get rid of either because we need the const version when calling from
const methods, and we need the non-const one when we want to mutate the session.
 I don't think this usage violates the style guides guidance:

Use overloaded functions (including constructors) only if a reader looking at a
call site can get a good idea of what is happening without having to first
figure out exactly which overload is being called.

Since both of these accessors do exactly the same thing (return a pointer to the
underlying session) it seems pretty clear to  caller what is happening.

(As an asside that the example of AppendString()/AppendInt() is based on the
arguments being different. In this case, it's only the const-ness which is
different)

+    return static_cast<const QuicChromiumClientSession::Handle*>(session());
+  }
+
   State next_state_;
 
-  base::WeakPtr<QuicChromiumClientSession> session_;
-  const QuicServerId server_id_;  // The ID of the QUIC server for this stream.
-
   HttpServerProperties* http_server_properties_;  // Unowned.
 
-  QuicVersion quic_version_;
-  int session_error_;             // Error code from the connection shutdown.
-  bool was_handshake_confirmed_;  // True if the crypto handshake succeeded.
-  std::unique_ptr<QuicChromiumClientSession::StreamRequest> stream_request_;
   QuicChromiumClientStream* stream_;  // Non-owning.
 
   // The following three fields are all owned by the caller and must
   // outlive this object, according to the HttpStream contract.
 
   // The request to send.
   // Only valid before the response body is read.
   const HttpRequestInfo* request_info_;
 
   // The request body to send, if any, owned by the caller.
@@ skipping 38 matching lines @@
   scoped_refptr<IOBuffer> user_buffer_;
   int user_buffer_len_;
 
   // Temporary buffer used to read the request body from UploadDataStream.
   scoped_refptr<IOBufferWithSize> raw_request_body_buf_;
   // Wraps raw_request_body_buf_ to read the remaining data progressively.
   scoped_refptr<DrainableIOBuffer> request_body_buf_;
 
   NetLogWithSource stream_net_log_;
 
+  int session_error_;  // Error code from the connection shutdown.
   QuicErrorCode quic_connection_error_;       // Cached connection error code.
   QuicRstStreamErrorCode quic_stream_error_;  // Cached stream error code.
 
-  // True when this stream receives a go away from server due to port migration.
-  bool port_migration_detected_;
-
   bool found_promise_;
   // |QuicClientPromisedInfo| owns this. It will be set when |Try()|
   // is asynchronous, i.e. it returned QUIC_PENDING, and remains valid
   // until |OnRendezvouResult()| fires or |push_handle_->Cancel()| is
   // invoked.
   QuicClientPushPromiseIndex::TryHandle* push_handle_;
 
   // Set to true when DoLoop() is being executed, false otherwise.
   bool in_loop_;
 
   // Session connect timing info.
   LoadTimingInfo::ConnectTiming connect_timing_;
 
   base::WeakPtrFactory<QuicHttpStream> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(QuicHttpStream);
 };
 
 }  // namespace net
 
 #endif  // NET_QUIC_CHROMIUM_QUIC_HTTP_STREAM_H_