Add a new priority level, THROTTLED, below IDLE.

Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839
Committed: https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7
Cr-Commit-Position: refs/heads/master@{#426831}

[Randy Smith (Not in Mondays), 2016-05-07 02:37:07]

rdsmith@chromium.org changed reviewers:
+ mmenke@chromium.org

[Randy Smith (Not in Mondays), 2016-05-07 02:37:07]

Matt, based on our discussion of CL granularity, I think this is ready for
review.  I don't plan to land it until I can also land in pretty much the same
time frame the CLs for adding the scheduler and implementing the special
semantics of this priority.  Having said that, would you take a look at this
implementation?

[mmenke, 2016-05-09 19:25:42]

Think one of our end-to-end tests at the URLRequest layer should probably use
THROTTLED, out of paranoia.

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:15: return
static_cast<SpdyPriority>(MAXIMUM_PRIORITY - priority);
Should run this by the SPDY/QUIC teams.  There's a constant that claims spdy
priorities range from 0 to 7, so I think we're good, but not positive.

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:19:
ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
The fact that this is not the inverse of ConvertRequestPriorityToQuicPriority is
unexpected.  Note that this method is only used for testing, making sure we send
the right priorities (And those tests should be updated to include the new
priority).

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:21: return (priority >= (MAXIMUM_PRIORITY -
MINIMUM_PRIORITY))
Hrm...The is right, but weird.

"priority - kV3HighestPriority >= MAXIMUM_PRIORITY - MINIMUM_PRIORITY" (We're
comparing two priority deltas, not a priority and a delta - same goes for the
MAXIMUM_PRIORITY line)

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
File net/spdy/spdy_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
net/spdy/spdy_http_utils.cc:179: SpdyPriority
ConvertRequestPriorityToSpdyPriority(
Need to have the QUIC/SPDY teams weigh in on changing this.

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
net/spdy/spdy_http_utils.cc:191: return (priority > (MAXIMUM_PRIORITY -
MINIMUM_PRIORITY))
This maps IDLE priority to 4, THROTTLED to 5, and THROTTLED+1 to 4?  Why is this
different from the QUIC version?

[Randy Smith (Not in Mondays), 2016-05-11 21:30:25]

Comments incorporated.  PTAL?

Matt: I'm specifically uncertain as to whether I've done what you wanted with
the end to end test.

Ryan: Matt suggested pulling in a QUIC/SPDY expert to look over the conversion
to/from that priority space.  Willing to do that?  See the notes below with your
name on them.

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:15: return
static_cast<SpdyPriority>(MAXIMUM_PRIORITY - priority);
On 2016/05/09 19:25:42, mmenke wrote:
> Should run this by the SPDY/QUIC teams.  There's a constant that claims spdy
> priorities range from 0 to 7, so I think we're good, but not positive.

Makes sense.  Ryan?

(I've also modified this to be more in line with the changes below.)

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:19:
ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
On 2016/05/09 19:25:42, mmenke wrote:
> The fact that this is not the inverse of ConvertRequestPriorityToQuicPriority
is
> unexpected.  Note that this method is only used for testing, making sure we
send
> the right priorities (And those tests should be updated to include the new
> priority).

I've updated the test.  In what ways is this not the inverse?  (I have made
changes to both the above function and this one, so it's possible it used to not
be the inverse and now is.)

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:21: return (priority >= (MAXIMUM_PRIORITY -
MINIMUM_PRIORITY))
On 2016/05/09 19:25:42, mmenke wrote:
> Hrm...The is right, but weird.
> 
> "priority - kV3HighestPriority >= MAXIMUM_PRIORITY - MINIMUM_PRIORITY" (We're
> comparing two priority deltas, not a priority and a delta - same goes for the
> MAXIMUM_PRIORITY line)

Fixed here; not sure what you meant by "the same goes for the MAXIMUM_PRIORITY
line", but if you meant that it should be relative to kV3HighestPriority, fixed.

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
File net/spdy/spdy_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
net/spdy/spdy_http_utils.cc:179: SpdyPriority
ConvertRequestPriorityToSpdyPriority(
On 2016/05/09 19:25:42, mmenke wrote:
> Need to have the QUIC/SPDY teams weigh in on changing this.

Ryan, are you willing to play a SPDY expert on TV, since Bence's out?

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
net/spdy/spdy_http_utils.cc:191: return (priority > (MAXIMUM_PRIORITY -
MINIMUM_PRIORITY))
On 2016/05/09 19:25:42, mmenke wrote:
> This maps IDLE priority to 4, THROTTLED to 5, and THROTTLED+1 to 4?  Why is
this
> different from the QUIC version?

Because of ">" rather than ">="?  Because the QUIC version was broken and I
hadn't written tests for it; both now fixed.  

I've also updated this code to be explicit about the relationship to
kV3HighestPriority.

Ryan: Any idea why these are duplicated between SPDY and QUIC?

[mmenke, 2016-05-11 21:56:09]

QUIC responses

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:19:
ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> On 2016/05/09 19:25:42, mmenke wrote:
> > The fact that this is not the inverse of
ConvertRequestPriorityToQuicPriority
> is
> > unexpected.  Note that this method is only used for testing, making sure we
> send
> > the right priorities (And those tests should be updated to include the new
> > priority).
> 
> I've updated the test.  In what ways is this not the inverse?  (I have made
> changes to both the above function and this one, so it's possible it used to
not
> be the inverse and now is.)

This never returns THROTTLED, does it?  I had assumed that was deliberate.

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:21: return (priority >= (MAXIMUM_PRIORITY -
MINIMUM_PRIORITY))
On 2016/05/11 21:30:24, Randy Smith - Not in Fridays wrote:
> On 2016/05/09 19:25:42, mmenke wrote:
> > Hrm...The is right, but weird.
> > 
> > "priority - kV3HighestPriority >= MAXIMUM_PRIORITY - MINIMUM_PRIORITY"
(We're
> > comparing two priority deltas, not a priority and a delta - same goes for
the
> > MAXIMUM_PRIORITY line)
> 
> Fixed here; not sure what you meant by "the same goes for the MAXIMUM_PRIORITY
> line", but if you meant that it should be relative to kV3HighestPriority,
fixed.

Yea, that's what I mean (Weird to do math on priorities on two different
"priority units", relying on one or more values being 0, rather than find the
delta on one scale, and apply it to the other)

[Randy Smith (Not in Mondays), 2016-05-11 22:04:28]

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:19:
ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
On 2016/05/11 21:56:08, mmenke wrote:
> On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > On 2016/05/09 19:25:42, mmenke wrote:
> > > The fact that this is not the inverse of
> ConvertRequestPriorityToQuicPriority
> > is
> > > unexpected.  Note that this method is only used for testing, making sure
we
> > send
> > > the right priorities (And those tests should be updated to include the new
> > > priority).
> > 
> > I've updated the test.  In what ways is this not the inverse?  (I have made
> > changes to both the above function and this one, so it's possible it used to
> not
> > be the inverse and now is.)
> 
> This never returns THROTTLED, does it?  I had assumed that was deliberate.

Nope; it was a mistake on my part.  I don't see a reason to treat THROTTLED
differently than other priorities WRT QUIC/SPDY.  Let me know if you do.

[mmenke, 2016-05-11 22:10:50]

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:19:
ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
On 2016/05/11 22:04:28, Randy Smith - Not in Fridays wrote:
> On 2016/05/11 21:56:08, mmenke wrote:
> > On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > > On 2016/05/09 19:25:42, mmenke wrote:
> > > > The fact that this is not the inverse of
> > ConvertRequestPriorityToQuicPriority
> > > is
> > > > unexpected.  Note that this method is only used for testing, making sure
> we
> > > send
> > > > the right priorities (And those tests should be updated to include the
new
> > > > priority).
> > > 
> > > I've updated the test.  In what ways is this not the inverse?  (I have
made
> > > changes to both the above function and this one, so it's possible it used
to
> > not
> > > be the inverse and now is.)
> > 
> > This never returns THROTTLED, does it?  I had assumed that was deliberate.
> 
> Nope; it was a mistake on my part.  I don't see a reason to treat THROTTLED
> differently than other priorities WRT QUIC/SPDY.  Let me know if you do.

Pushed streams with the QUIC/H2 priority corresponding to THROTTLED seem kinda
weird...  But we aren't using this in production code, anyways, so I guess that
doesn't matter.

[Randy Smith (Not in Mondays), 2016-05-12 18:17:21]

On 2016/05/11 22:10:50, mmenke wrote:
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> File net/quic/quic_http_utils.cc (right):
> 
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> net/quic/quic_http_utils.cc:19:
> ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
> On 2016/05/11 22:04:28, Randy Smith - Not in Fridays wrote:
> > On 2016/05/11 21:56:08, mmenke wrote:
> > > On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > > > On 2016/05/09 19:25:42, mmenke wrote:
> > > > > The fact that this is not the inverse of
> > > ConvertRequestPriorityToQuicPriority
> > > > is
> > > > > unexpected.  Note that this method is only used for testing, making
sure
> > we
> > > > send
> > > > > the right priorities (And those tests should be updated to include the
> new
> > > > > priority).
> > > > 
> > > > I've updated the test.  In what ways is this not the inverse?  (I have
> made
> > > > changes to both the above function and this one, so it's possible it
used
> to
> > > not
> > > > be the inverse and now is.)
> > > 
> > > This never returns THROTTLED, does it?  I had assumed that was deliberate.
> > 
> > Nope; it was a mistake on my part.  I don't see a reason to treat THROTTLED
> > differently than other priorities WRT QUIC/SPDY.  Let me know if you do.
> 
> Pushed streams with the QUIC/H2 priority corresponding to THROTTLED seem kinda
> weird...  But we aren't using this in production code, anyways, so I guess
that
> doesn't matter.

Say more?  This will eventually go to production code, so I don't want to lower
the bar for it.  And I don't understand why, except for the details of what's in
the throttler, we should treat this priority differently from any others.

[mmenke, 2016-05-12 18:30:57]

On 2016/05/12 18:17:21, Randy Smith - Not in Fridays wrote:
> On 2016/05/11 22:10:50, mmenke wrote:
> >
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> > File net/quic/quic_http_utils.cc (right):
> > 
> >
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> > net/quic/quic_http_utils.cc:19:
> > ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
> > On 2016/05/11 22:04:28, Randy Smith - Not in Fridays wrote:
> > > On 2016/05/11 21:56:08, mmenke wrote:
> > > > On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > > > > On 2016/05/09 19:25:42, mmenke wrote:
> > > > > > The fact that this is not the inverse of
> > > > ConvertRequestPriorityToQuicPriority
> > > > > is
> > > > > > unexpected.  Note that this method is only used for testing, making
> sure
> > > we
> > > > > send
> > > > > > the right priorities (And those tests should be updated to include
the
> > new
> > > > > > priority).
> > > > > 
> > > > > I've updated the test.  In what ways is this not the inverse?  (I have
> > made
> > > > > changes to both the above function and this one, so it's possible it
> used
> > to
> > > > not
> > > > > be the inverse and now is.)
> > > > 
> > > > This never returns THROTTLED, does it?  I had assumed that was
deliberate.
> > > 
> > > Nope; it was a mistake on my part.  I don't see a reason to treat
THROTTLED
> > > differently than other priorities WRT QUIC/SPDY.  Let me know if you do.
> > 
> > Pushed streams with the QUIC/H2 priority corresponding to THROTTLED seem
kinda
> > weird...  But we aren't using this in production code, anyways, so I guess
> that
> > doesn't matter.
> 
> Say more?  This will eventually go to production code, so I don't want to
lower
> the bar for it.  And I don't understand why, except for the details of what's
in
> the throttler, we should treat this priority differently from any others.

Throttling is hooked up to the request start event.  If we used this in
production code, for pushed streams (Which is what I first assumed the method is
for), it's weird to say requests are THROTTLED when they aren't, and it's weird
to say they're THROTTLED based on the arbitrary priority the server assigns
them.  At least I assume the server assigns a priority to pushed requests?  Or
do we respond back with a priority when the server pushed the request?  Not
sufficiently familiar with SPDY/QUIC to know.

[Randy Smith (Not in Mondays), 2016-05-12 18:44:33]

Description was changed from

==========
Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839
==========

to

==========
Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839
==========

[Randy Smith (Not in Mondays), 2016-05-12 18:44:33]

rdsmith@chromium.org changed reviewers:
+ rch@chromium.org

[Randy Smith (Not in Mondays), 2016-05-12 18:46:35]

On 2016/05/12 18:30:57, mmenke wrote:
> On 2016/05/12 18:17:21, Randy Smith - Not in Fridays wrote:
> > On 2016/05/11 22:10:50, mmenke wrote:
> > >
> >
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> > > File net/quic/quic_http_utils.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> > > net/quic/quic_http_utils.cc:19:
> > > ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
> > > On 2016/05/11 22:04:28, Randy Smith - Not in Fridays wrote:
> > > > On 2016/05/11 21:56:08, mmenke wrote:
> > > > > On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > > > > > On 2016/05/09 19:25:42, mmenke wrote:
> > > > > > > The fact that this is not the inverse of
> > > > > ConvertRequestPriorityToQuicPriority
> > > > > > is
> > > > > > > unexpected.  Note that this method is only used for testing,
making
> > sure
> > > > we
> > > > > > send
> > > > > > > the right priorities (And those tests should be updated to include
> the
> > > new
> > > > > > > priority).
> > > > > > 
> > > > > > I've updated the test.  In what ways is this not the inverse?  (I
have
> > > made
> > > > > > changes to both the above function and this one, so it's possible it
> > used
> > > to
> > > > > not
> > > > > > be the inverse and now is.)
> > > > > 
> > > > > This never returns THROTTLED, does it?  I had assumed that was
> deliberate.
> > > > 
> > > > Nope; it was a mistake on my part.  I don't see a reason to treat
> THROTTLED
> > > > differently than other priorities WRT QUIC/SPDY.  Let me know if you do.
> > > 
> > > Pushed streams with the QUIC/H2 priority corresponding to THROTTLED seem
> kinda
> > > weird...  But we aren't using this in production code, anyways, so I guess
> > that
> > > doesn't matter.
> > 
> > Say more?  This will eventually go to production code, so I don't want to
> lower
> > the bar for it.  And I don't understand why, except for the details of
what's
> in
> > the throttler, we should treat this priority differently from any others.
> 
> Throttling is hooked up to the request start event.  If we used this in
> production code, for pushed streams (Which is what I first assumed the method
is
> for), it's weird to say requests are THROTTLED when they aren't, and it's
weird
> to say they're THROTTLED based on the arbitrary priority the server assigns
> them.  At least I assume the server assigns a priority to pushed requests?  Or
> do we respond back with a priority when the server pushed the request?  Not
> sufficiently familiar with SPDY/QUIC to know.

Arggh, just realized I didn't actually include Ryan; Ryan please start reading @
c#4 which is when I meant to pull you in :-}.

So I don't believe that the priority of a network request that's joined with a
pushed stream is based on what the server tells us; I think the priority at the
HttpNetworkTransaction level will aways be based on what URLRequest says.  Ryan?

[Ryan Hamilton, 2016-05-12 22:25:47]

I think this all looks fine but I'm a bit lost with respect to throttled + push.

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:19:
ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
On 2016/05/11 22:10:50, mmenke wrote:
> On 2016/05/11 22:04:28, Randy Smith - Not in Fridays wrote:
> > On 2016/05/11 21:56:08, mmenke wrote:
> > > On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > > > On 2016/05/09 19:25:42, mmenke wrote:
> > > > > The fact that this is not the inverse of
> > > ConvertRequestPriorityToQuicPriority
> > > > is
> > > > > unexpected.  Note that this method is only used for testing, making
sure
> > we
> > > > send
> > > > > the right priorities (And those tests should be updated to include the
> new
> > > > > priority).
> > > > 
> > > > I've updated the test.  In what ways is this not the inverse?  (I have
> made
> > > > changes to both the above function and this one, so it's possible it
used
> to
> > > not
> > > > be the inverse and now is.)
> > > 
> > > This never returns THROTTLED, does it?  I had assumed that was deliberate.
> > 
> > Nope; it was a mistake on my part.  I don't see a reason to treat THROTTLED
> > differently than other priorities WRT QUIC/SPDY.  Let me know if you do.
> 
> Pushed streams with the QUIC/H2 priority corresponding to THROTTLED seem kinda
> weird...  But we aren't using this in production code, anyways, so I guess
that
> doesn't matter.

I'm not following the pushed stream == throttled comment. Can you elaborate?

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
File net/spdy/spdy_http_utils.cc (right):

https://codereview.chromium.org/1866483002/diff/20001/net/spdy/spdy_http_util...
net/spdy/spdy_http_utils.cc:191: return (priority > (MAXIMUM_PRIORITY -
MINIMUM_PRIORITY))
On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> Ryan: Any idea why these are duplicated between SPDY and QUIC?

Because QuicPriority != SpdyPriority ... except then it turns out they are the
same. Long story short, it's a wart.

https://codereview.chromium.org/1866483002/diff/60001/net/quic/quic_http_util...
File net/quic/quic_http_utils_test.cc (right):

https://codereview.chromium.org/1866483002/diff/60001/net/quic/quic_http_util...
net/quic/quic_http_utils_test.cc:21: EXPECT_EQ(4u,
ConvertRequestPriorityToQuicPriority(IDLE));
Could there be a test here for 5u => THROTTLED?

[mmenke, 2016-05-13 21:28:24]

On 2016/05/12 22:25:47, Ryan Hamilton wrote:
> I think this all looks fine but I'm a bit lost with respect to throttled +
push.
> 
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> File net/quic/quic_http_utils.cc (right):
> 
>
https://codereview.chromium.org/1866483002/diff/20001/net/quic/quic_http_util...
> net/quic/quic_http_utils.cc:19:
> ConvertQuicPriorityToRequestPriority(SpdyPriority priority) {
> On 2016/05/11 22:10:50, mmenke wrote:
> > On 2016/05/11 22:04:28, Randy Smith - Not in Fridays wrote:
> > > On 2016/05/11 21:56:08, mmenke wrote:
> > > > On 2016/05/11 21:30:25, Randy Smith - Not in Fridays wrote:
> > > > > On 2016/05/09 19:25:42, mmenke wrote:
> > > > > > The fact that this is not the inverse of
> > > > ConvertRequestPriorityToQuicPriority
> > > > > is
> > > > > > unexpected.  Note that this method is only used for testing, making
> sure
> > > we
> > > > > send
> > > > > > the right priorities (And those tests should be updated to include
the
> > new
> > > > > > priority).
> > > > > 
> > > > > I've updated the test.  In what ways is this not the inverse?  (I have
> > made
> > > > > changes to both the above function and this one, so it's possible it
> used
> > to
> > > > not
> > > > > be the inverse and now is.)
> > > > 
> > > > This never returns THROTTLED, does it?  I had assumed that was
deliberate.
> > > 
> > > Nope; it was a mistake on my part.  I don't see a reason to treat
THROTTLED
> > > differently than other priorities WRT QUIC/SPDY.  Let me know if you do.
> > 
> > Pushed streams with the QUIC/H2 priority corresponding to THROTTLED seem
kinda
> > weird...  But we aren't using this in production code, anyways, so I guess
> that
> > doesn't matter.
> 
> I'm not following the pushed stream == throttled comment. Can you elaborate?

I had assumed a function to get net priorities from H2 priorities existed
because servers set priorities when they push streams.  I guess this isn't the
case?

[Ryan Hamilton, 2016-05-13 22:11:30]

On 2016/05/13 21:28:24, mmenke wrote:
> On 2016/05/12 22:25:47, Ryan Hamilton wrote:
> > I'm not following the pushed stream == throttled comment. Can you elaborate?
> 
> I had assumed a function to get net priorities from H2 priorities existed
> because servers set priorities when they push streams.  I guess this isn't the
> case?

Ah, right. A pushed stream has a priority, but the client does not actually know
what it is, because the server does not tell the client. However, the client can
prioritize the pushed stream if it wants to. This seems like a protocol wart.

[mmenke, 2016-05-13 22:17:39]

On 2016/05/13 22:11:30, Ryan Hamilton wrote:
> On 2016/05/13 21:28:24, mmenke wrote:
> > On 2016/05/12 22:25:47, Ryan Hamilton wrote:
> > > I'm not following the pushed stream == throttled comment. Can you
elaborate?
> > 
> > I had assumed a function to get net priorities from H2 priorities existed
> > because servers set priorities when they push streams.  I guess this isn't
the
> > case?
> 
> Ah, right. A pushed stream has a priority, but the client does not actually
know
> what it is, because the server does not tell the client. However, the client
can
> prioritize the pushed stream if it wants to. This seems like a protocol wart.

Ah, so no issues, then (as far as the new priority is concerned), was just
worried we'd be translating a server priority to THROTTLED, and not actually
throttle it, which seems like it would be really weird.

[Randy Smith (Not in Mondays), 2016-05-14 18:17:40]

Ok, if I'm following the threads correctly, I think this change is the only
outstanding issue.  Ryan, Matt: PTAL?

(As noted above, I don't plan to land this until I can land the sibling CLs
together with it, so I won't be landing immediately after stamp.  But I'd like
to know that I don't have further changes to make on this CL.)

https://codereview.chromium.org/1866483002/diff/60001/net/quic/quic_http_util...
File net/quic/quic_http_utils_test.cc (right):

https://codereview.chromium.org/1866483002/diff/60001/net/quic/quic_http_util...
net/quic/quic_http_utils_test.cc:21: EXPECT_EQ(4u,
ConvertRequestPriorityToQuicPriority(IDLE));
On 2016/05/12 22:25:46, Ryan Hamilton wrote:
> Could there be a test here for 5u => THROTTLED?

Done.

[Randy Smith (Not in Mondays), 2016-05-17 23:33:12]

Ping?

[Ryan Hamilton, 2016-05-17 23:51:49]

Oh, sorry. LGTM

[mmenke, 2016-05-18 21:46:42]

On 2016/05/17 23:51:49, Ryan Hamilton wrote:
> Oh, sorry. LGTM

Sorry, forgot about this one, two days in a row (Well, more than that, actually)

[mmenke, 2016-05-18 21:49:04]

LGTM as well

[Ryan Hamilton, 2016-06-16 19:05:25]

Still LGTM, but removing myself since this CL seems idle. Feel free to add me
back.

[Ryan Hamilton, 2016-07-22 23:41:21]

rch@chromium.org changed reviewers:
- rch@chromium.org

[Randy Smith (Not in Mondays), 2016-08-10 21:08:23]

The CQ bit was checked by rdsmith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 21:09:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 01:06:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 01:06:39]

Dry run: This issue passed the CQ dry run.

[Randy Smith (Not in Mondays), 2016-08-22 00:40:03]

The CQ bit was checked by rdsmith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-22 00:40:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-22 01:48:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-22 01:48:21]

Dry run: This issue passed the CQ dry run.

[Randy Smith (Not in Mondays), 2016-09-22 22:20:56]

The CQ bit was checked by rdsmith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 22:21:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 22:38:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 22:38:52]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Randy Smith (Not in Mondays), 2016-09-23 21:13:18]

Fix merge typo.

[Randy Smith (Not in Mondays), 2016-09-23 21:13:23]

The CQ bit was checked by rdsmith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 21:13:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 21:25:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 21:25:15]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Randy Smith (Not in Mondays), 2016-10-20 20:37:58]

The CQ bit was checked by rdsmith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 20:38:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 21:46:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-20 21:46:52]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Randy Smith (Not in Mondays), 2016-10-21 15:18:51]

The CQ bit was checked by rdsmith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 15:19:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Randy Smith (Not in Mondays), 2016-10-21 15:21:08]

rdsmith@chromium.org changed reviewers:
+ lizeb@chromium.org

[Randy Smith (Not in Mondays), 2016-10-21 15:21:08]

Benoit: Could you comment on the PS 12->13 diffs (which, not coincidentally,
contains all the predictor changes)?  I'm sending this review request in
anticipation of the try bots in hope of getting your eyes on it before you leave
today :-}.

[commit-bot: I haz the power, 2016-10-21 16:36:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 16:36:31]

Dry run: This issue passed the CQ dry run.

[Benoit L, 2016-10-21 16:51:20]

predictors LGTM, thanks!

https://codereview.chromium.org/1866483002/diff/240001/chrome/browser/predict...
File chrome/browser/predictors/resource_prefetch_predictor_unittest.cc (right):

https://codereview.chromium.org/1866483002/diff/240001/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor_unittest.cc:443: //
Confirm that there's been no shift in the
Thanks for that!

[Randy Smith (Not in Mondays), 2016-10-21 17:15:02]

The CQ bit was checked by rdsmith@chromium.org

[Randy Smith (Not in Mondays), 2016-10-21 17:15:02]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org,
rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1866483002/#ps240001
(title: "Adjusted predictor priorities.")

[commit-bot: I haz the power, 2016-10-21 17:15:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 17:36:34]

Description was changed from

==========
Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839
==========

to

==========
Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839
==========

[commit-bot: I haz the power, 2016-10-21 17:36:50]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-10-21 17:41:15]

Description was changed from

==========
Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839
==========

to

==========
Add a new priority level, THROTTLED, below IDLE.

THROTTLED is used when the consumer is expecting that low priority
requests will soon be followed by higher priority ones.  It directs the
network stack to reserve resources to handle the coming requests.  In
practice this will mean that only a very small number of THROTTLED requests
may be outstanding at any given time.

BUG=600839

Committed: https://crrev.com/5eb6fbc3c9488ec131081ac9090696af47125ae7
Cr-Commit-Position: refs/heads/master@{#426831}
==========

[commit-bot: I haz the power, 2016-10-21 17:41:16]

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