1097320 - Enable advertising h2

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[u408661]

Rumor has it everything will be binary compatible from this point on, so let's start advertising the final version (along with all the other versions we're advertising).

Comment 1

[u408661]

Attachment: patch

This enables advertising h2 unconditionally (still preffable). I went this route, because it's much easier to disable a feature in beta timeframe (in case we shouldn't push h2 out the door) than it is to enable it (in the case we're confident the internet at large is ready to see "h2" in ALPN advertisements).

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1ad3838df523

http://ietfmemes.tumblr.com/post/102384795594/disaster-girl-supports-the-standards-process

Comment 2

[Patrick McManus [:mcmanus]]

we can drop the blocking dep - it would be nice to see that interop, but we've got an internal test and confirmation that the (at least the first part of) the issue is on the other side

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 8520991 [details] [diff] [review]
patch

Review of attachment 8520991 [details] [diff] [review]:
-----------------------------------------------------------------

good to go with compat; lets get this on 36. {h2, h2-14, h2-15} right?

Comment 4

[u408661]

Try run seems to have hit the test_http2.js timeout twice. Retriggering a bunch just to make sure this didn't somehow make the problem worse. Will land once I'm confident.

Comment 5

[u408661]

Attachment: patch v1.1

So as discussed on irc, the current patch causes the test to timeout because of some altsvc wonkiness (seems like a bug in the altsvc code, maybe?)

This patch modifies the test to not tickle the likely altsvc bug, so we can get this landed. I'll also try to dig on on what's going on with the altsvc code, though it seems likely that it's only really a bug when the server says altsvc for 2 different protocols, only one of which it's actually advertising (which is also a server bug at that point, if you ask me).

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=bfe486ddeea6

Comment 6

[Patrick McManus [:mcmanus]]

Comment on attachment 8528534 [details] [diff] [review]
patch v1.1

Review of attachment 8528534 [details] [diff] [review]:
-----------------------------------------------------------------

oh - I suspect the issue here is that the test is not robust to alt-svc failure because it is a alt-svc positive test :)

I think gecko itself is robust to using things other than the failing alt-svc (which is what we want!) the test refuses to accept that.

So adjusting the test to reflect the server is the right thing.

Comment 7

[u408661]

Ah yeah, sounds about right! Just waiting on try to validate me... I mean my changes. I need no validation from a computer!

Comment 8

[u408661]

And of course inbound is closed, as well... :sigh: Try has validated me, though, so whenever inbound is open, this can be checked in.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b6fe53f601f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7b6fe53f601f

Comment 11

[Patrick McManus [:mcmanus]]

Release Note Request (optional, but appreciated)
[Suggested wording]: see below.
[Why is this notable]: Firefox 36 launches support for the full HTTP/2 protocol. HTTP/2 enables a faster, more scalable, and more responsive web.

[Links (documentation, blog post, etc)]:
I'm going to make a blog post but it does not exist yet.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Adding to the release notes with "Support for the full HTTP/2 protocol. HTTP/2 enables a faster, more scalable, and more responsive web." as wording. FYI, it is not necessary to write "Firefox" in the release notes because it is the Firefox release notes (ditto for the version).

By the way, we stopped using the keyword relnote. We are using the tracking flag.
Thanks!