Enable advertising h2

RESOLVED FIXED in mozilla36

Status

()

RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: nwgh, Assigned: nwgh)

Tracking

unspecified
mozilla36
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 36+)

Details

(Whiteboard: [spdy])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
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).
(Assignee)

Comment 1

4 years ago
Created attachment 8520991 [details] [diff] [review]
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
Attachment #8520991 - Flags: review?(mcmanus)
Depends on: 1101157
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
No longer depends on: 1101157
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?
Attachment #8520991 - Flags: review?(mcmanus) → review+
(Assignee)

Comment 4

4 years ago
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.
(Assignee)

Comment 5

4 years ago
Created attachment 8528534 [details] [diff] [review]
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
Attachment #8520991 - Attachment is obsolete: true
Attachment #8528534 - Flags: review?(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.
Attachment #8528534 - Flags: review?(mcmanus) → review+
(Assignee)

Comment 7

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

Comment 8

4 years ago
And of course inbound is closed, as well... :sigh: Try has validated me, though, so whenever inbound is open, this can be checked in.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7b6fe53f601f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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.
relnote-firefox: --- → ?
Keywords: relnote
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!
Keywords: relnote
relnote-firefox: ? → 36+

Updated

a year ago
Depends on: 1381016
You need to log in before you can comment on or make changes to this bug.