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

net/http/http_basic_stream.cc

 // 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.
 
 #include "net/http/http_basic_stream.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_body_drainer.h"
+#include "net/http/http_response_info.h"
 #include "net/http/http_stream_parser.h"
 #include "net/socket/client_socket_handle.h"
 
 namespace net {
 
 HttpBasicStream::HttpBasicStream(ClientSocketHandle* connection,
                                  bool using_proxy)
     : state_(connection, using_proxy) {}
 
 HttpBasicStream::~HttpBasicStream() {}
@@ skipping 89 matching lines @@
     return false;
 
   return state_.connection()->socket()->GetPeerAddress(endpoint) == OK;
 }
 
 Error HttpBasicStream::GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                                    std::vector<uint8_t>* out) {
   return parser()->GetSignedEKMForTokenBinding(key, out);
 }
 
-void HttpBasicStream::Drain(HttpNetworkSession* session) {
-  HttpResponseBodyDrainer* drainer = new HttpResponseBodyDrainer(this);
+void HttpBasicStream::Drain(HttpNetworkSession* session,
+                            scoped_ptr<HttpResponseInfo> response_info) {
+  HttpResponseBodyDrainer* drainer =
+      new HttpResponseBodyDrainer(this, std::move(response_info));

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

The ownership semantics of the Drain() method are a bit surprising. Would it be
possible to raise HttpResonseBodyDrainer up so that HttpNetworkTransaction
itself creates it? That way we can explicitly pass down the HttpStream and
HttpResponseInfo via unique_ptrs and make it explicit that both of those objects
are now owned by the HttpResponseBodyDrainer. Thus the latter would be
responsible for asserting that |response_info| outlives |session|.

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

On one hand, you're right about these ownership semantics...On the other hand,
I'd really like to make it so HttpNetworkTransaction doesn't even know about
draining responses (Draining is HTTP/1.x-specific silliness, and makes no sense
for QUIC/SPDY, so doesn't seem like it should be in the common code shared by
all paths).

Anyhow, I've gone with a different solution, so I'm leaving this as it was
before this CL.

   drainer->Start(session);
   // |drainer| will delete itself.
 }
 
 void HttpBasicStream::PopulateNetErrorDetails(NetErrorDetails* details) {
   details->connection_info = HttpResponseInfo::CONNECTION_INFO_HTTP1;
   return;
 }
 
 void HttpBasicStream::SetPriority(RequestPriority priority) {
   // TODO(akalin): Plumb this through to |connection_|.
 }
 
 }  // namespace net