Closed Bug 1493204 Opened 11 months ago Closed 11 months ago

H2 priority dependencies are not used for pushed streams

Categories

(Core :: Networking: HTTP, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: u408661, Assigned: u408661)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

Noticed this when working on bug 1492484, pushed streams are never properly re-parented in the priority scheme. Http2Stream::AdjustPushedPriority is called when we match a pushed stream, and that works purely on a non-hierarchical priority model (all pushed streams depend on stream 0).

When the tab would change (and we would thus move things into/out of the background group), we would *try* to re-parent the pushed stream, but instead try to re-parent stream 0 (causing a PROTOCOL_ERROR, and we silently restart the connection).

We don't even put pushed streams in their own bucket when we receive them - they all become parented by stream 0 in the priority scheme.

Here's what I propose -

1. All received PUSH_PROMISE streams get parented under some slightly lower-priority group, most likely either the "Other" group, or the "Followers" group.
2. When a pushed stream gets matched for use, re-parent that pushed stream wherever is appropriate based on our internal priority, using the same group we would use for the request if it weren't matched with a pulled stream.
3. Properly re-parent pushed streams when switching tabs, instead of just ignoring them (as the code currently in bug 1492484 does).
4. Ensure that, when the priority of a matched stream is changed by gecko, we properly re-parent it, just as we would if it were a regular request.
Previously, we had not put pushed streams in the priority tree, we just
let them be top-level items in the tree. With this change, we will put
them into the tree initially based on the priority of the associated
stream. The only exception is if the associated stream is either a
Leader or Urgent Start (in which case, we will turn the pushed streams
into followers).

Once the pushed stream is matched with a request generated by gecko,
that pushed stream will be re-prioritized based on the priority gecko
has for the request, just like a regular pulled stream.

This also allows us to re-prioritize pushed streams into the background
on tab switch (we assume that, before they are matched, they belong to
the same window as the associated stream).
Pushed by hurley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/328a8076c760
Add pushed streams to the priority tree. r=dragana
https://hg.mozilla.org/mozilla-central/rev/328a8076c760
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.