Chrome will stop the support on May 15th.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b2bc765abed Some spdy/h2 tests fail if I disable npn...

Node v4.x does not support ALPN :(

Chrome has now stopped supporting NPN in latest Chrome stable. Only ALPN is supported. Firefox should do the same.

We can't use ALPN until our test infrastructure has node >=5.0.0.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d762ad7a091bb5e336ee190caaf5c948359dc34 test_spdy.js still fails even with node 5. node-spdy doesn't support SPDY+ALPN?

Review commit: https://reviewboard.mozilla.org/r/64016/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/64016/

Review commit: https://reviewboard.mozilla.org/r/64018/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/64018/

https://reviewboard.mozilla.org/r/64018/#review61060 I'm actually going to redirect to :mcmanus on this one (for one thing, I'm not sure how this impacts spdy and if we want to disable that at the same time?)

Comment on attachment 8770573 [details] Bug 1248198 - Disable NPN by default https://reviewboard.mozilla.org/r/64018/#review61062

Comment on attachment 8770573 [details] Bug 1248198 - Disable NPN by default The only practical effect this will have right now is the breaking of websites. It can be done later.

(In reply to Patrick McManus [:mcmanus] from comment #16) I'll follow necko owner's decision, but > The only practical effect this will have right now is the breaking of > websites. It can be done later. No websites will break. Affected websites will just use http/1.1 instead of spdy or h2.

(In reply to Masatoshi Kimura [:emk] from comment #17) > > No websites will break. Affected websites will just use http/1.1 instead of > spdy or h2. it's true - its a particular definition of break :)

Comment on attachment 8770572 [details] Bug 1248198 - Remove the "security.ssl.enable_npn" pref. https://reviewboard.mozilla.org/r/64016/#review61444 Please add more details as to why (especially why that version) in the commit message and/or a comment in the code.

Comment on attachment 8770572 [details] Bug 1248198 - Remove the "security.ssl.enable_npn" pref. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64016/diff/1-2/

Comment on attachment 8770573 [details] Bug 1248198 - Disable NPN by default Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64018/diff/1-2/

I didn't mean to request review again (for now). I posted the updated patch for posterity. MozReview is too damn smart...

fwiw a bigger issue here than spdy may be that h2 is widely used with npn for now - largely because it took openssl a long time to ship alpn. Its definitely not in our interests to push that traffic back onto h1. on beta 48 iirc its about 5:1 in favor of alpn vs npn, but its 50:1 in favor of h2 vs spdy.. (it might be time to get rid of spdy. I'll think about that harder.)

A heads up. We disabled NPN in NSS 3.27 (which I just landed on inbound and should ride trains with FF 51) to see if anything breaks. Only the ability to enable it is removed and we plan on removing the actual code in 3.28 when things don't break too badly.

emk: can we land the hasNode patch? We had to back out NSS 3.27 because the version of node on taskcluster is too old.

(In reply to Martin Thomson [:mt:] from comment #25) > emk: can we land the hasNode patch? We had to back out NSS 3.27 because the > version of node on taskcluster is too old. I think we can. The patch is already r+'ed and does not affect NPN functionality.

Oh, I recalled why I didn't land it. MozReview does not allow partial landing :( I will have to extract and attach the patch.

try run just in case: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3dab7b72787

Pushed by VYV03354@nifty.ne.jp: https://hg.mozilla.org/integration/mozilla-inbound/rev/c6876f222943 Set "hasNode" only if the node version is >=5.0.0 because older node does not support ALPN. r=glandium

So the "hasNode" patch appears to have totally broke our h2 testing on infra, so right now we have no automated test coverage for that. Please back it out until infra gets upgraded to an appropriate node version.

Oh, just saw comment 24 - so apparently we have no NPN on trunk, and we have no ALPN available for testing, so it's literally impossible to test h2? We need to either revert these changes or pressure releng to upgrade all our nodes to 5.0 or higher, now. Also, there should be a way to say "this test should count as a failure if it's skipped", as that would've caught this situation instead of us having no h2 testing for the last 2+ weeks...

OK, from talking to people in #releng, it looks like backout of the NPN removal and the hasNode patch is the right way to go - this counts as test bustage (even though it's silent). Not an ideal situation, to be sure (I also applaud the effort to kill NPN!), but better to keep supporting old/experimental tech and have tests than to not be testing our h2 implementation (plus the things that depend on it now, like webpush).

(see comments 32-34)

I added node 5 support on taskcluster in bug 1280178. Is it effectively reverted by bug 1293737? If so, bug 1293737 should be reverted.

OK, that's a slightly better situation (and indeed, taskcluster builds seem to be running our h2 tests). However, taskcluster xpcshell tests are tier-2. All our tier-1 h2 tests (both linux and os x) are still being skipped because non-tc has not been upgraded.

Ugh, hit "save" too soon. Meant to add: So, effectively, we still have tier-1 test bustage (even though it's silent) which should be backed out.

OK, but I can't simply backout this because it will cause (non-silent) bustage. Please ask :mt or :ekr about NSS backout.

(In reply to Nicholas Hurley [:nwgh][:hurley] from comment #38) > Ugh, hit "save" too soon. Meant to add: > > So, effectively, we still have tier-1 test bustage (even though it's silent) > which should be backed out. Backing out the hasNode patch will only make things fail. I don't think that's what you want. Backing out NSS would leave Aurora with a beta version of NSS, so I don't think that's an option either. Pressuring folks into upgrading infrastructure sounds like the right approach to me.

xpcshell tests for linux64/debug and /asan are tier-1. Which are you referring to at tier-2?

(In reply to Dustin J. Mitchell [:dustin] from comment #41) > xpcshell tests for linux64/debug and /asan are tier-1. Which are you > referring to at tier-2? taskcluster are listed as tier-2 (unless I'm reading treeherder wrong). But yes, non-taskcluster xpcshell tests on linux (and os x!) are tier-1, and those are what is silently busted right now.

(In reply to Nicholas Hurley [:nwgh][:hurley] from comment #42) > (In reply to Dustin J. Mitchell [:dustin] from comment #41) > > xpcshell tests for linux64/debug and /asan are tier-1. Which are you > > referring to at tier-2? > > taskcluster are listed as tier-2 (unless I'm reading treeherder wrong). Linux x64 opt and pgo TC are listed as tier-2, but debug and asan TC are not (unless I'm reading treeherder wrong). I also found bug 1242556 was fixed (that is, at least Linux x64 debug TC is tier-1). > But yes, non-taskcluster xpcshell tests on linux (and os x!) are tier-1, and > those are what is silently busted right now. It's normal that a test is disabled on *some* tier-1 platform.

(In reply to Masatoshi Kimura [:emk] from comment #43) > > But yes, non-taskcluster xpcshell tests on linux (and os x!) are tier-1, and > > those are what is silently busted right now. > > It's normal that a test is disabled on *some* tier-1 platform. Yes, but linux and mac are the only opt tier-1 builds that test(ed) h2. And as much as it's good that we have ASAN and debug builds testing h2, we should also have the bits we actually ship testing h2, as well.

* This pref is dead sue to upstream change (bug 1288044). So there is no point in keeping the pref. * Now we have a good test platform coverage.

Comment on attachment 8770572 [details] Bug 1248198 - Remove the "security.ssl.enable_npn" pref. https://reviewboard.mozilla.org/r/64016/#review93986 Yeah, the pref's a no-op. Doesn't make much sense to keep this code around. ::: security/manager/ssl/nsNSSComponent.cpp:1887 (Diff revision 3) > > SSL_OptionSetDefault(SSL_ENABLE_FALSE_START, > Preferences::GetBool("security.ssl.enable_false_start", > FALSE_START_ENABLED_DEFAULT)); > > - // SSL_ENABLE_NPN and SSL_ENABLE_ALPN also require calling > + // SSL_ENABLE_ALPN also requires calling nit: it would be great to reformat this documentation text to be as close to 80 characters on each line as possible (in vim, you can set the text width to 80, select these lines, and use `gq` to do this automatically)

Comment on attachment 8770572 [details] Bug 1248198 - Remove the "security.ssl.enable_npn" pref. https://reviewboard.mozilla.org/r/64016/#review94056

:glandium, autoland wants to add you as a reviewer even though you didn't review this part of the patch. Is this a bug? Is it possible to remove a reviewer?

I don't know.

Pushed by VYV03354@nifty.ne.jp: https://hg.mozilla.org/integration/mozilla-inbound/rev/6e8714a6425c Remove the "security.ssl.enable_npn" pref. r=keeler,mcmanus

(In reply to Mike Hommey [:glandium] from comment #51) > I don't know. OK, thanks. I landed the patch manually to edit the commit message as expected.

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2017/npn-support-has-been-removed/

I've added a note to the Fx53 release notes: https://developer.mozilla.org/en-US/Firefox/Releases/53#HTTPNetworking