HttpStreamParser: Don't reuse sockets which receive unparsed data.

net/http/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.
 //
 // HttpStream provides an abstraction for a basic http streams, SPDY, and QUIC.
 // The HttpStream subtype is expected to manage the underlying transport
 // appropriately.  For example, a basic http stream will return the transport
 // socket to the pool for reuse.  SPDY streams on the other hand leave the
 // transport socket management to the SpdySession.
 
@@ skipping 46 matching lines @@
   // Writes the headers and uploads body data to the underlying socket.
   // ERR_IO_PENDING is returned if the operation could not be completed
   // synchronously, in which case the result will be passed to the callback
   // when available. Returns OK on success.
   //
   // The callback will only be invoked once the first full set of headers have
   // been received, at which point |response| will have been populated with that
   // set of headers, and is safe to read, until/unless ReadResponseHeaders is
   // called.
   //
-  // |response| must remain valid until all sets of headers has been read, or
-  // the HttpStream is destroyed. There's typically only one set of
-  // headers, except in the case of 1xx responses (See ReadResponseHeaders).
+  // |response| must remain valid for the lifetime of the stream. There's
+  // typically only one set of headers, except in the case of 1xx responses (See
+  // ReadResponseHeaders).
+  // TODO(mmenke):  |response|'s lifetime and ownership models are somewhat
+  //     bonkers, particularly considering HttpNetworkTransaction can clobber
+  //     it. Can something better be done, preferably without keeping multiple
+  //     copies of the struct around?
   virtual int SendRequest(const HttpRequestHeaders& request_headers,
                           HttpResponseInfo* response,
                           const CompletionCallback& callback) = 0;
 
   // Reads from the underlying socket until the next set of response headers
   // have been completely received.  This may only be called on 1xx responses
   // after SendRequest has completed successfully, to read the next set of
   // headers.
   //
   // ERR_IO_PENDING is returned if the operation could not be completed
@@ skipping 80 matching lines @@
 
   // Signs the EKM value for Token Binding from the TLS layer using |*key| and
   // puts the result in |*out|. Returns OK or ERR_FAILED.
   virtual Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                             std::vector<uint8_t>* out) = 0;
 
   // In the case of an HTTP error or redirect, flush the response body (usually
   // a simple error or "this page has moved") so that we can re-use the
   // underlying connection. This stream is responsible for deleting itself when
   // draining is complete.
-  virtual void Drain(HttpNetworkSession* session) = 0;
+  virtual void Drain(HttpNetworkSession* session,
+                     scoped_ptr<HttpResponseInfo> response_info) = 0;

[asanka, 2016/05/13 15:19:52]

Should document here that |response_info| *must* be the same as |response| in
SendRequest(). I'd enforce this via DCHECKs

[mmenke, 2016/05/20 21:50:02]

The more I looked at this, the less I liked it.  I've done with a simpler
solution.

 
   // Get the network error details this stream is encountering.
   // Fills in |details| if it is available; leaves |details| unchanged if it
   // is unavailable.
   virtual void PopulateNetErrorDetails(NetErrorDetails* details) = 0;
 
   // Called when the priority of the parent transaction changes.
   virtual void SetPriority(RequestPriority priority) = 0;
 
   // Queries the UploadDataStream for its progress (bytes sent).
   virtual UploadProgress GetUploadProgress() const = 0;
 
   // Returns a new (not initialized) stream using the same underlying
   // connection and invalidates the old stream - no further methods should be
   // called on the old stream.  The caller should ensure that the response body
   // from the previous request is drained before calling this method.  If the
   // subclass does not support renewing the stream, NULL is returned.
   virtual HttpStream* RenewStreamForAuth() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(HttpStream);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_STREAM_H_