Re-enable HTTP/2 over NPN (for OpenSSL).

Re-enable HTTP/2 over NPN (for OpenSSL).

BUG=557197
Committed: https://crrev.com/0eb771aa98ba2bd4db0e2b82241afe8f5760c32e
Cr-Commit-Position: refs/heads/master@{#361053}

Committed: https://crrev.com/f4588407e26be81fda6f2507423fe42ca6d8894b
Cr-Commit-Position: refs/heads/master@{#361330}

[Bence, 2015-11-18 12:26:48]

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

[Bence, 2015-11-18 12:26:48]

Chris: PTAL.
rsleevi, davidben: FYI.

[cbentzel, 2015-11-18 17:24:47]

On 2015/11/18 12:26:48, Bence wrote:
> Chris: PTAL.
> rsleevi, davidben: FYI.

Are there associated tests?

[davidben, 2015-11-18 17:37:03]

bnc: Can I assume you'll be taking point on getting Ubuntu or OpenSSL to
backport ALPN to the current LTS and 1.0.1? Without that, I don't think
deferring this by a few release cycles will do anything. Even come April, people
aren't likely to update to the next LTSs immediately. 14.04 will be supported
into 2019. (Even 12.04 is supported into 2017.)

[Bence, 2015-11-18 21:19:08]

On 2015/11/18 17:37:03, davidben (slow) wrote:
> bnc: Can I assume you'll be taking point on getting Ubuntu or OpenSSL to
> backport ALPN to the current LTS and 1.0.1? Without that, I don't think
> deferring this by a few release cycles will do anything. Even come April,
people
> aren't likely to update to the next LTSs immediately. 14.04 will be supported
> into 2019. (Even 12.04 is supported into 2017.)

davidben: no, I do not have the bandwidth for that.  Does anyone have evangelism
experience and free cycles for this?

[cbentzel, 2015-11-20 13:53:44]

On 2015/11/18 21:19:08, Bence wrote:
> On 2015/11/18 17:37:03, davidben (slow) wrote:
> > bnc: Can I assume you'll be taking point on getting Ubuntu or OpenSSL to
> > backport ALPN to the current LTS and 1.0.1? Without that, I don't think
> > deferring this by a few release cycles will do anything. Even come April,
> people
> > aren't likely to update to the next LTSs immediately. 14.04 will be
supported
> > into 2019. (Even 12.04 is supported into 2017.)
> 
> davidben: no, I do not have the bandwidth for that.  Does anyone have
evangelism
> experience and free cycles for this?

I'm still interested in whether there are tests associated with this or whether
there could be.

Let's try to resolve the policy issue today as well.

[Bence, 2015-11-20 18:11:40]

> I'm still interested in whether there are tests associated with this or
whether
> there could be.

Yes, there should have been, I just added them in Patch Set 2.

[cbentzel, 2015-11-20 20:45:42]

LGTM, assuming issues below are resolved.

https://codereview.chromium.org/1453903002/diff/20001/net/http/http_network_t...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1453903002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:15480:
testing::ElementsAre(kProtoHTTP2, kProtoSPDY31, kProtoHTTP11));
Have you confirmed that this fails when your actual code change is not in place?

https://codereview.chromium.org/1453903002/diff/20001/net/spdy/spdy_test_util...
File net/spdy/spdy_test_util_common.cc (right):

https://codereview.chromium.org/1453903002/diff/20001/net/spdy/spdy_test_util...
net/spdy/spdy_test_util_common.cc:380:
enable_user_alternate_protocol_ports(false),
enable_npn needs to be initialized in the constructors

[cbentzel, 2015-11-20 20:45:43]

LGTM, assuming issues below are resolved.

[Bence, 2015-11-22 21:14:59]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-11-22 21:14:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbentzel@chromium.org
Link to the patchset: https://codereview.chromium.org/1453903002/#ps40001
(title: "Initialize |enable_npn| in SpdySessionDependencies constructor.")

[Bence, 2015-11-22 21:15:09]

Thank you.

https://codereview.chromium.org/1453903002/diff/20001/net/http/http_network_t...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1453903002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:15480:
testing::ElementsAre(kProtoHTTP2, kProtoSPDY31, kProtoHTTP11));
On 2015/11/20 20:45:42, cbentzel wrote:
> Have you confirmed that this fails when your actual code change is not in
place?
> 

Yes, I confirm that this test fails for me when I revert Patch Set 1.

https://codereview.chromium.org/1453903002/diff/20001/net/spdy/spdy_test_util...
File net/spdy/spdy_test_util_common.cc (right):

https://codereview.chromium.org/1453903002/diff/20001/net/spdy/spdy_test_util...
net/spdy/spdy_test_util_common.cc:380:
enable_user_alternate_protocol_ports(false),
On 2015/11/20 20:45:42, cbentzel wrote:
> enable_npn needs to be initialized in the constructors

Done.

[Bence, 2015-11-22 21:15:09]

Thank you.

https://codereview.chromium.org/1453903002/diff/20001/net/http/http_network_t...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1453903002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:15480:
testing::ElementsAre(kProtoHTTP2, kProtoSPDY31, kProtoHTTP11));
On 2015/11/20 20:45:42, cbentzel wrote:
> Have you confirmed that this fails when your actual code change is not in
place?
> 

Yes, I confirm that this test fails for me when I revert Patch Set 1.

https://codereview.chromium.org/1453903002/diff/20001/net/spdy/spdy_test_util...
File net/spdy/spdy_test_util_common.cc (right):

https://codereview.chromium.org/1453903002/diff/20001/net/spdy/spdy_test_util...
net/spdy/spdy_test_util_common.cc:380:
enable_user_alternate_protocol_ports(false),
On 2015/11/20 20:45:42, cbentzel wrote:
> enable_npn needs to be initialized in the constructors

Done.

[commit-bot: I haz the power, 2015-11-22 21:15:10]

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

[commit-bot: I haz the power, 2015-11-22 22:22:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-22 22:23:41]

Patchset 3 (id:??) landed as
https://crrev.com/0eb771aa98ba2bd4db0e2b82241afe8f5760c32e
Cr-Commit-Position: refs/heads/master@{#361053}

[Alexander Potapenko, 2015-11-23 11:28:13]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1472743002/ by glider@chromium.org.

The reason for reverting is: The CL has introduced massive MSan and Valgrind
reports, see the bug.

BUG=557197.

[Bence, 2015-11-23 14:19:26]

Description was changed from

==========
Re-enable HTTP/2 over NPN (for OpenSSL).

BUG=557197

Committed: https://crrev.com/0eb771aa98ba2bd4db0e2b82241afe8f5760c32e
Cr-Commit-Position: refs/heads/master@{#361053}
==========

to

==========
Re-enable HTTP/2 over NPN (for OpenSSL).

BUG=557197

Committed: https://crrev.com/0eb771aa98ba2bd4db0e2b82241afe8f5760c32e
Cr-Commit-Position: refs/heads/master@{#361053}
==========

[Bence, 2015-11-24 11:57:37]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-11-24 11:57:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbentzel@chromium.org
Link to the patchset: https://codereview.chromium.org/1453903002/#ps80001
(title: "Initialize |enable_npn| in other SpdySessionDependencies constructor.")

[commit-bot: I haz the power, 2015-11-24 11:57:46]

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

[Bence, 2015-11-24 12:02:27]

1. I didn't notice the plural in cbentzel's feedback on initializing
|enable_npn| in *constructors*, so Patch Set 3 has issues with uninitialized
values, and was reverted due to MSan failures.  I am sorry for the oversight.  I
fixed this in Patch Set 5.

2. I am not able to reproduce the MSan errors either locally or using
linux_chromium_msan_* trybots: there seems to be a large number of unrelated
use-of-uninitialized-value errors, even on trunk when I compile locally.  That
is, I am not able to verify that Patch Set 5 is sufficient to fix the issue.  I
decided to enqueue this CL anyway, and keep a close eye on the MSan continuous
builder.  Memory sheriff: if there is a problem, feel free to revert without
asking.  Thanks.

[commit-bot: I haz the power, 2015-11-24 13:33:29]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-24 13:34:34]

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