Support HTTP/2 drafts 14 and 15 simultaneously.

Support HTTP/2 drafts 14 and 15 simultaneously.

BUG=432673
Committed: https://crrev.com/5b3dce0d316928cecab45b24a889554dee25dee8
Cr-Commit-Position: refs/heads/master@{#304063}

[Bence, 2014-11-12 22:53:05]

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

[Bence, 2014-11-12 22:53:05]

Ryan: PTAL.  Thank you.

[Ryan Hamilton, 2014-11-12 23:32:55]

Two minor comments about naming...

But it doesn't look like this actually treats 14 and 15 differently. Are you
planning to do something different in a subsequent CL? If not, what's the
motivation?

https://codereview.chromium.org/724523002/diff/1/net/http/http_response_info.h
File net/http/http_response_info.h (right):

https://codereview.chromium.org/724523002/diff/1/net/http/http_response_info....
net/http/http_response_info.h:42: CONNECTION_INFO_SPDY4_14 = 6,  // HTTP/2
draft-14.
Should we just name this HTTP2_14 and the other HTTP2_15?

https://codereview.chromium.org/724523002/diff/1/net/socket/next_proto.h
File net/socket/next_proto.h (right):

https://codereview.chromium.org/724523002/diff/1/net/socket/next_proto.h#newc...
net/socket/next_proto.h:30: kProtoSPDY31 = 102,
Should we consider renaming this to be 3_1 to match the identifiers below?

(If so, not as part of this CL :>)

[Bence, 2014-11-13 01:56:54]

On 2014/11/12 23:32:55, Ryan Hamilton wrote:
> 
> But it doesn't look like this actually treats 14 and 15 differently. Are you
> planning to do something different in a subsequent CL? If not, what's the
> motivation?
>

h2-14 and h2-15 are practically wire compatible.  I am aiming for a minimum safe
CL to support both simultaneously.  In order to have statistics on negotiated
versions, it's necessary to expand |enum NextProto|.  Because of
|HttpResponseInfo::ConnectionInfoToString|, it is necessary to expand |enum
HttpResponseInfo|.  It is not strictly necessary to expand |enum
AlternateProtocol|, but servers supposedly will expect clients to differentiate
between persistent h2-14 and h2-15 alternate services.

[Bence, 2014-11-13 02:05:05]

PTAL.

https://codereview.chromium.org/724523002/diff/1/net/http/http_response_info.h
File net/http/http_response_info.h (right):

https://codereview.chromium.org/724523002/diff/1/net/http/http_response_info....
net/http/http_response_info.h:42: CONNECTION_INFO_SPDY4_14 = 6,  // HTTP/2
draft-14.
On 2014/11/12 23:32:54, Ryan Hamilton wrote:
> Should we just name this HTTP2_14 and the other HTTP2_15?

Done.

https://codereview.chromium.org/724523002/diff/1/net/socket/next_proto.h
File net/socket/next_proto.h (right):

https://codereview.chromium.org/724523002/diff/1/net/socket/next_proto.h#newc...
net/socket/next_proto.h:30: kProtoSPDY31 = 102,
On 2014/11/12 23:32:55, Ryan Hamilton wrote:
> Should we consider renaming this to be 3_1 to match the identifiers below?
> 
> (If so, not as part of this CL :>)

This is not crucial: 1 is a minor version number, 14, 15 are draft numbers, not
exactly the same role.  I can do this nonetheless, whatever you prefer.

[Ryan Hamilton, 2014-11-13 04:11:46]

I'm confused about the 14 vs 15 difference. I think you're saying that you do
not intend to land a follow up CL adding 15-specific (or 14-specific)
functionality. Is that right? And this is because 15 is effectively a super-set
of 14?

[Bence, 2014-11-13 04:52:59]

On 2014/11/13 04:11:46, Ryan Hamilton wrote:
> I'm confused about the 14 vs 15 difference. I think you're saying that you do
> not intend to land a follow up CL adding 15-specific (or 14-specific)
> functionality. Is that right? And this is because 15 is effectively a
super-set
> of 14?

Yes, you are right.  Sorry for the confusion.  If you look at the diff [1]
between draft-14 and draft-15, you'll see that most changes are editorial. 
Examples of minor functional changes are that a WINDOW_UPDATE can now be
received in reserved (local) state, or that HTTP_1_1_REQUIRED error code is
introduced (which we never emit).  Therefore negotiating h2-14 and speaking
h2-15 seems okay to me, but if it turns out to be a problem, I can always change
the code to distinguish between these drafts.

[1]
https://tools.ietf.org/wg/httpbis/draft-ietf-httpbis-http2/draft-ietf-httpbis...

[Ryan Hamilton, 2014-11-13 05:15:54]

On 2014/11/13 04:52:59, Bence wrote:
> On 2014/11/13 04:11:46, Ryan Hamilton wrote:
> > I'm confused about the 14 vs 15 difference. I think you're saying that you
do
> > not intend to land a follow up CL adding 15-specific (or 14-specific)
> > functionality. Is that right? And this is because 15 is effectively a
> super-set
> > of 14?
> 
> Yes, you are right.  Sorry for the confusion.  If you look at the diff [1]
> between draft-14 and draft-15, you'll see that most changes are editorial. 
> Examples of minor functional changes are that a WINDOW_UPDATE can now be
> received in reserved (local) state, or that HTTP_1_1_REQUIRED error code is
> introduced (which we never emit).  Therefore negotiating h2-14 and speaking
> h2-15 seems okay to me, but if it turns out to be a problem, I can always
change
> the code to distinguish between these drafts.
> 
> [1]
>
https://tools.ietf.org/wg/httpbis/draft-ietf-httpbis-http2/draft-ietf-httpbis...

Great! Thanks for the clarification. Sorry for the delay! LGTM

[Bence, 2014-11-13 05:31:05]

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

[Bence, 2014-11-13 05:31:06]

ttuttle: PTAL at components/domain_reliability/util.cc.  Thanks.

[Deprecated (see juliatuttle), 2014-11-13 15:23:59]

On 2014/11/13 05:31:06, Bence wrote:
> ttuttle: PTAL at components/domain_reliability/util.cc.  Thanks.

components/domain_reliability/util.cc lgtm.

[Bence, 2014-11-13 18:35:54]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2014-11-13 18:36:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/724523002/20001

[commit-bot: I haz the power, 2014-11-13 19:57:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-13 19:58:40]

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