Remove DCHECK in HttpStreamParser::ReadResponseHeaders.

Remove DCHECK in HttpStreamParser::ReadResponseHeaders.

The DCHECK dereferenced the request's HttpResponseInfo.  It turns out
that when an HttpNetworkTransaction is cancelled and an
HttpResponseDrainer is used, the HttpResponseInfo is deleted with the
HttpStreamParser's knowledge.

This CL also adds a pair of tests for the situation.

BUG=368418
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271014

[mmenke, 2014-05-14 15:53:27]

I'd also like to get rid of HttpStreamParser::GetResponseInfo, because it's
unsafe to call in the same case.  It doesn't look like it's used anywhere,
either, except by HttpPipelining, which I intend to get rid of, anyways.

[willchan no longer on Chromium, 2014-05-15 08:49:02]

https://codereview.chromium.org/284913003/diff/1/net/http/http_stream_parser.cc
File net/http/http_stream_parser.cc (left):

https://codereview.chromium.org/284913003/diff/1/net/http/http_stream_parser....
net/http/http_stream_parser.cc:349: DCHECK_NE(1,
response_->headers->response_code() / 100);
I'm confused, don't you need to delete this DCHECK?

[mmenke, 2014-05-15 12:20:07]

https://codereview.chromium.org/284913003/diff/1/net/http/http_stream_parser.cc
File net/http/http_stream_parser.cc (left):

https://codereview.chromium.org/284913003/diff/1/net/http/http_stream_parser....
net/http/http_stream_parser.cc:349: DCHECK_NE(1,
response_->headers->response_code() / 100);
On 2014/05/15 08:49:02, willchan (traveling 5.7-5.15) wrote:
> I'm confused, don't you need to delete this DCHECK?

Erm...oops.  Added it back for local testing of the tests, forgot to remove it
again (With the line, the HttpStreamParser test fails in debug mode, the other
one doesn't zero out the pointer, so may not)

Anyhow, now *really* removed.

[mmenke, 2014-05-15 12:33:48]

On 2014/05/15 12:20:07, mmenke wrote:
>
https://codereview.chromium.org/284913003/diff/1/net/http/http_stream_parser.cc
> File net/http/http_stream_parser.cc (left):
> 
>
https://codereview.chromium.org/284913003/diff/1/net/http/http_stream_parser....
> net/http/http_stream_parser.cc:349: DCHECK_NE(1,
> response_->headers->response_code() / 100);
> On 2014/05/15 08:49:02, willchan (traveling 5.7-5.15) wrote:
> > I'm confused, don't you need to delete this DCHECK?
> 
> Erm...oops.  Added it back for local testing of the tests, forgot to remove it
> again (With the line, the HttpStreamParser test fails in debug mode, the other
> one doesn't zero out the pointer, so may not)
> 
> Anyhow, now *really* removed.

Really should learn not to rely on the trybots so much... "I have tests that
work + trybots passed" obviously isn't always enough, particularly in debug only
failures.

[willchan no longer on Chromium, 2014-05-16 12:36:58]

lgtm

[mmenke, 2014-05-16 13:04:14]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-05-16 13:04:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/284913003/40001

[commit-bot: I haz the power, 2014-05-16 14:03:48]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-16 14:58:14]

Change committed as 271014