Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(427)

Issue 1387363004: Disable HTTP/2 over NPN (with OpenSSL). (Closed)

Created:
5 years, 2 months ago by Bence
Modified:
5 years, 2 months ago
Reviewers:
eroman, davidben
CC:
chromium-reviews, cbentzel+watch_chromium.org, eroman, arv+watch_chromium.org, mmenke
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Disable HTTP/2 over NPN (with OpenSSL). * Split SSLConfig.next_proto into two members: one for ALPN, one for NPN. * Remove HTTP/2 from NPN. * In OpenSSL, use alpn_protos for |ALPN|, and npn_protos for |NPN|. * In NSS, use |alpn_protos| for both. * In NSS, disable NPN if |npn_protos| is empty. BUG=527066 Committed: https://crrev.com/1f295377e8de70ba03ce73d9f7929e614d23df68 Cr-Commit-Position: refs/heads/master@{#355427}

Patch Set 1 #

Patch Set 2 : Nit. #

Patch Set 3 : Rebase. #

Patch Set 4 : Nit. #

Patch Set 5 : Disable NPN in NSS if npn_protos.empty(). #

Total comments: 10

Patch Set 6 : Rebase. #

Patch Set 7 : Re: #3. #

Total comments: 2

Patch Set 8 : Re: #7. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+146 lines, -57 lines) Patch
M chrome/browser/net/preconnect.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/resources/net_internals/spdy_view.html View 1 2 3 4 5 1 chunk +6 lines, -1 line 0 comments Download
M net/http/http_network_session.h View 1 2 3 4 5 1 chunk +5 lines, -2 lines 0 comments Download
M net/http/http_network_session.cc View 1 chunk +12 lines, -3 lines 0 comments Download
M net/http/http_network_transaction.cc View 1 chunk +2 lines, -1 line 0 comments Download
M net/http/http_network_transaction_unittest.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M net/http/http_server_properties.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M net/http/http_stream_factory_impl_job.cc View 1 chunk +2 lines, -1 line 0 comments Download
M net/log/net_log_util.cc View 1 2 3 4 5 6 1 chunk +17 lines, -5 lines 0 comments Download
M net/socket/socket_test_util.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M net/socket/ssl_client_socket_nss.cc View 1 2 3 4 5 6 2 chunks +13 lines, -8 lines 0 comments Download
M net/socket/ssl_client_socket_openssl.cc View 1 2 5 chunks +7 lines, -7 lines 0 comments Download
M net/socket/ssl_client_socket_unittest.cc View 1 2 3 4 5 6 7 13 chunks +56 lines, -15 lines 0 comments Download
M net/spdy/spdy_network_transaction_unittest.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M net/ssl/ssl_config.h View 1 2 3 4 5 6 1 chunk +14 lines, -7 lines 0 comments Download

Messages

Total messages: 15 (4 generated)
Bence
David: PTAL. Thanks.
5 years, 2 months ago (2015-10-13 16:08:44 UTC) #2
davidben
Thanks! I think this looks good. Just some nits below. https://codereview.chromium.org/1387363004/diff/80001/net/log/net_log_util.cc File net/log/net_log_util.cc (right): https://codereview.chromium.org/1387363004/diff/80001/net/log/net_log_util.cc#newcode427 ...
5 years, 2 months ago (2015-10-13 20:55:59 UTC) #3
Bence
PTAL. Thanks. https://codereview.chromium.org/1387363004/diff/80001/net/log/net_log_util.cc File net/log/net_log_util.cc (right): https://codereview.chromium.org/1387363004/diff/80001/net/log/net_log_util.cc#newcode427 net/log/net_log_util.cc:427: for (const NextProto proto : alpn_protos) { ...
5 years, 2 months ago (2015-10-14 14:55:59 UTC) #4
Bence
eroman: PTAL at chrome/browser/*. Thank you.
5 years, 2 months ago (2015-10-14 14:57:02 UTC) #6
davidben
lgtm https://codereview.chromium.org/1387363004/diff/120001/net/socket/ssl_client_socket_unittest.cc File net/socket/ssl_client_socket_unittest.cc (right): https://codereview.chromium.org/1387363004/diff/120001/net/socket/ssl_client_socket_unittest.cc#newcode3143 net/socket/ssl_client_socket_unittest.cc:3143: client_config.alpn_protos.push_back(kProtoHTTP11); The comment and #ifdef apply to all ...
5 years, 2 months ago (2015-10-14 16:36:46 UTC) #7
Bence
It sounds like ALPN should be implemented in the test server, and then all of ...
5 years, 2 months ago (2015-10-14 18:39:02 UTC) #8
davidben
On 2015/10/14 18:39:02, Bence wrote: > It sounds like ALPN should be implemented in the ...
5 years, 2 months ago (2015-10-14 18:40:59 UTC) #9
eroman
lgtm
5 years, 2 months ago (2015-10-21 21:51:39 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1387363004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1387363004/140001
5 years, 2 months ago (2015-10-21 22:06:17 UTC) #13
commit-bot: I haz the power
Committed patchset #8 (id:140001)
5 years, 2 months ago (2015-10-21 23:24:31 UTC) #14
commit-bot: I haz the power
5 years, 2 months ago (2015-10-21 23:25:15 UTC) #15
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/1f295377e8de70ba03ce73d9f7929e614d23df68
Cr-Commit-Position: refs/heads/master@{#355427}

Powered by Google App Engine
This is Rietveld 408576698