Move logic to figure out if a socket can be reused into HttpStream.

Move logic to figure out if a socket can be reused into HttpStream.

It previously was in HttpNetworkTransaction, which applied the logic
to SPDY and QUIC (where it makes no sense) as well as to HTTP.

This also removes a couple methods from HttpStream that are no longer
needed.

BUG=525809
Committed: https://crrev.com/bd84c3906ac0696c38a10aa63a01078943043e65
Cr-Commit-Position: refs/heads/master@{#346908}

[mmenke, 2015-09-01 18:59:53]

mmenke@chromium.org changed reviewers:
+ rch@chromium.org

[mmenke, 2015-09-01 18:59:54]

Ryan:  I'm sending these reviews to you because I think you're the QUIC person
who best knows the HttpNetworkTransaction / [Quic]HttpStream API, but feel free
to redirect.

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_network_t...
File net/http/http_network_transaction.cc (left):

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_network_t...
net/http/http_network_transaction.cc:176: } else if
(stream_->IsSpdyHttpStream()) {
Checking for SPDY and not QUIC here is what inspired this CL - turns out the
stream->Drain does the right thing, anyways, but it's weird to have only one
check here.

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_network_t...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_network_t...
net/http/http_network_transaction.cc:309: if (keep_alive &&
stream_->CanReuseConnection()) {
This checks a bit more than necessary, but the redundant checks don't hurt.

I'd really like to turn this into something like:

int result = stream_->CreateNewStreamForAuthRestart(callback);

And make the BasicStream do the draining and such (SPDY/QUIC just return
ERR_NEED_NEW_STREAM synchronously or something), but I'll leave that for a
future cleanup - maybe next fixit.

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_stream_fa...
File net/http/http_stream_factory_impl_unittest.cc (right):

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_stream_fa...
net/http/http_stream_factory_impl_unittest.cc:1184: MockRead
mock_read(SYNCHRONOUS, ERR_IO_PENDING);
This change was needed to keep the SPDY session open.

[Ryan Hamilton, 2015-09-02 04:42:40]

This is fantastic! LGTM

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_network_t...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/1320683003/diff/60001/net/http/http_network_t...
net/http/http_network_transaction.cc:309: if (keep_alive &&
stream_->CanReuseConnection()) {
On 2015/09/01 18:59:54, mmenke wrote:
> This checks a bit more than necessary, but the redundant checks don't hurt.
> 
> I'd really like to turn this into something like:
> 
> int result = stream_->CreateNewStreamForAuthRestart(callback);
> 
> And make the BasicStream do the draining and such (SPDY/QUIC just return
> ERR_NEED_NEW_STREAM synchronously or something), but I'll leave that for a
> future cleanup - maybe next fixit.

Hear! Hear! That would be awesome.

https://codereview.chromium.org/1320683003/diff/80001/net/http/http_stream.h
File net/http/http_stream.h (right):

https://codereview.chromium.org/1320683003/diff/80001/net/http/http_stream.h#...
net/http/http_stream.h:95: //               CanReuseConnection() and
IsResponseBodyComplete().
Boy this would be great.

[mmenke, 2015-09-02 13:50:02]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2015-09-02 13:50:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1320683003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1320683003/80001

[commit-bot: I haz the power, 2015-09-02 14:12:41]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-09-02 14:13:19]

Patchset 5 (id:??) landed as
https://crrev.com/bd84c3906ac0696c38a10aa63a01078943043e65
Cr-Commit-Position: refs/heads/master@{#346908}