Do not reset stream if HTTP/2 response contains status text.

Do not reset stream if HTTP/2 response contains status text.

* Do not reset stream if HTTP/2 response contains status text.
* Add unittest for this.
* Add histogram for HTTP/2 response ":status" header field format.

BUG=672065
Committed: https://crrev.com/50ee38e5646a4c8296620537a2e30095a9085817
Cr-Commit-Position: refs/heads/master@{#437111}

[Bence, 2016-12-07 14:52:18]

The CQ bit was checked by bnc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 14:52:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 15:42:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 15:42:03]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Bence, 2016-12-07 15:51:20]

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

[Bence, 2016-12-07 15:51:21]

Ryan: PTAL.  Thank you.

[Ryan Hamilton, 2016-12-07 19:11:39]

lgtm, but one tiny optimization.

https://codereview.chromium.org/2555183002/diff/1/net/spdy/spdy_stream.cc
File net/spdy/spdy_stream.cc (right):

https://codereview.chromium.org/2555183002/diff/1/net/spdy/spdy_stream.cc#new...
net/spdy/spdy_stream.cc:56: if (StringToInt(status_string, &unused))
nit: Instead of calling StringToInt twice, it seems like you could call it once
with the invocation on line 50. Then if it fails, return HAS_TEXT or
DOES_NOT_START based on if end == StringPiece::end(), right?

[Ryan Hamilton, 2016-12-07 19:11:50]

Thanks for doing this!

[Bence, 2016-12-07 20:33:18]

bnc@chromium.org changed reviewers:
+ holte@chromium.org

[Bence, 2016-12-07 20:33:19]

Steven: PTAL at tools/metrics/histograms/.  Thank you.

https://codereview.chromium.org/2555183002/diff/1/net/spdy/spdy_stream.cc
File net/spdy/spdy_stream.cc (right):

https://codereview.chromium.org/2555183002/diff/1/net/spdy/spdy_stream.cc#new...
net/spdy/spdy_stream.cc:56: if (StringToInt(status_string, &unused))
On 2016/12/07 19:11:39, Ryan Hamilton wrote:
> nit: Instead of calling StringToInt twice, it seems like you could call it
once
> with the invocation on line 50. Then if it fails, return HAS_TEXT or
> DOES_NOT_START based on if end == StringPiece::end(), right?

Done.

[Bence, 2016-12-07 20:33:22]

The CQ bit was checked by bnc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 20:33:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 21:39:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 21:39:39]

Dry run: This issue passed the CQ dry run.

[Steven Holte, 2016-12-08 00:31:19]

lgtm

[Bence, 2016-12-08 00:40:03]

The CQ bit was checked by bnc@chromium.org

[Bence, 2016-12-08 00:40:03]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2555183002/#ps20001
(title: "Re: #7.")

[commit-bot: I haz the power, 2016-12-08 00:40:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 01:23:58]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481157603196080,
"parent_rev": "b98b58130df7b7574daaf03f2568e7d4342203d9", "commit_rev":
"c4570988216666d91fe201ede9bc6e25798af748"}

[commit-bot: I haz the power, 2016-12-08 01:24:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-08 01:30:11]

Description was changed from

==========
Do not reset stream if HTTP/2 response contains status text.

* Do not reset stream if HTTP/2 response contains status text.
* Add unittest for this.
* Add histogram for HTTP/2 response ":status" header field format.

BUG=672065
==========

to

==========
Do not reset stream if HTTP/2 response contains status text.

* Do not reset stream if HTTP/2 response contains status text.
* Add unittest for this.
* Add histogram for HTTP/2 response ":status" header field format.

BUG=672065

Committed: https://crrev.com/50ee38e5646a4c8296620537a2e30095a9085817
Cr-Commit-Position: refs/heads/master@{#437111}
==========

[commit-bot: I haz the power, 2016-12-08 01:30:12]

Patchset 2 (id:??) landed as
https://crrev.com/50ee38e5646a4c8296620537a2e30095a9085817
Cr-Commit-Position: refs/heads/master@{#437111}