Refactor SSLClientSocket::SerializeNextProtos().

Refactor SSLClientSocket::SerializeNextProtos().

Break out HTTP/2 protocol removal from SSLClientSocket::SerializeNextProtos() to
SSLClientSocket::DisableHTTP2() method.  DisableHTTP2() will be used for NPN to
create a NextProtoVector that can be used by the callback function (no
serialization is necessary for this.)

BUG=527066
Committed: https://crrev.com/f76254d62e10ebd2fc9185609fde431e3232bc1f
Cr-Commit-Position: refs/heads/master@{#351210}

[Bence, 2015-09-28 20:14:08]

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

[Bence, 2015-09-28 20:14:09]

David: PTAL.  Thanks.

[davidben, 2015-09-28 22:16:23]

lgtm

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

https://codereview.chromium.org/1371263002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket.h:182: static void DisableHTTP2(NextProtoVector*
next_protos);
Do you think this should perhaps be in next_proto.h? It's purely a random
operation on NextProtoVector.

(Whereas SerializeNextProtos is serializing it in an SSL-specific encoding.)

https://codereview.chromium.org/1371263002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_nss.cc (right):

https://codereview.chromium.org/1371263002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_nss.cc:860: PK11_TokenExists(CKM_ECDH1_DERIVE);
(Ooh! Thanks for reminding me! We can rip all of this out now because we don't
have to deal with system NSS like this anymore. I'll do that separately.)

[Bence, 2015-09-28 23:55:52]

Thanks.

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

https://codereview.chromium.org/1371263002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket.h:182: static void DisableHTTP2(NextProtoVector*
next_protos);
On 2015/09/28 22:16:22, David Benjamin wrote:
> Do you think this should perhaps be in next_proto.h? It's purely a random
> operation on NextProtoVector.
> 
> (Whereas SerializeNextProtos is serializing it in an SSL-specific encoding.)

Great idea!  Especially because I might end up using it elsewhere too, like
HttpNetworkTransaction or similar, and from that angle it really does not belong
to SSLClientSocket.  At the same time, the unit test does not seem to belong
anywhere, so I'm scratching it.

Done.

[Bence, 2015-09-28 23:55:56]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-09-28 23:55:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1371263002/#ps20001
(title: "Move DisableHTTP2 to next_protos.")

[commit-bot: I haz the power, 2015-09-28 23:56:49]

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

[commit-bot: I haz the power, 2015-09-29 00:03:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-29 00:04:42]

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