Create alternative service field trial group.

Create alternative service field trial group.

This CL creates the independent AltSvc field trial group that directly controls
|use_alternative_services|.  The default is changed to disabled, because Alternative Services is not approved for launch yet.

See https://crrev.com/1310583002 for what |use_alternative_services| does.
See https://crrev.com/1303273002 for what happened to the command line switch "--use-spdy=no-alt-protocols".

BUG=524141
Committed: https://crrev.com/d00793801e96b0f8b26c2ef7ba06fea5cc01f13e
Cr-Commit-Position: refs/heads/master@{#345944}

[Bence, 2015-08-03 23:07:50]

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

[Bence, 2015-08-03 23:07:51]

Ryan, PTAL.  Thanks.

[Bence, 2015-08-06 18:33:51]

Ping.

[Ryan Hamilton, 2015-08-10 17:09:21]

Sorry for the delay. Looking at this now. In the description, you say:

* (One can think about this as a rename, change of triggers, and change of
  defaults.)

That kinda suggests "split this into 3 different CLs". As a reviewer, it's super
easy to review a rename CL. It's super easy to review a change of defaults CL.
And it's pretty easy to read a change of triggers CL. But it's quite hard to
review all three changes at once :/

Or if 3 is hard, how 'bout a first CL which is simply the rename plus some of
the formatting cleanup (like the comments in
http_network_transaction_unittest.cc)?

[Bence, 2015-08-11 19:49:23]

On 2015/08/10 17:09:21, Ryan Hamilton wrote:
> Sorry for the delay. Looking at this now. In the description, you say:
> 
> * (One can think about this as a rename, change of triggers, and change of
>   defaults.)
> 
> That kinda suggests "split this into 3 different CLs". As a reviewer, it's
super
> easy to review a rename CL. It's super easy to review a change of defaults CL.
> And it's pretty easy to read a change of triggers CL. But it's quite hard to
> review all three changes at once :/
> 
> Or if 3 is hard, how 'bout a first CL which is simply the rename plus some of
> the formatting cleanup (like the comments in
> http_network_transaction_unittest.cc)?

SGTM.  Rename broken out to https://crrev.com/1268313004.  Thanks.

[Ryan Hamilton, 2015-08-13 17:06:34]

On 2015/08/11 19:49:23, Bence wrote:
> On 2015/08/10 17:09:21, Ryan Hamilton wrote:
> > Sorry for the delay. Looking at this now. In the description, you say:
> > 
> > * (One can think about this as a rename, change of triggers, and change of
> >   defaults.)
> > 
> > That kinda suggests "split this into 3 different CLs". As a reviewer, it's
> super
> > easy to review a rename CL. It's super easy to review a change of defaults
CL.
> > And it's pretty easy to read a change of triggers CL. But it's quite hard to
> > review all three changes at once :/
> > 
> > Or if 3 is hard, how 'bout a first CL which is simply the rename plus some
of
> > the formatting cleanup (like the comments in
> > http_network_transaction_unittest.cc)?
> 
> SGTM.  Rename broken out to https://crrev.com/1268313004.  Thanks.

Cool. Let me know when this is ready for another look.

[Bence, 2015-08-20 11:31:36]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-08-20 11:31:36]

The CQ bit was unchecked by bnc@chromium.org

[Bence, 2015-08-20 11:32:23]

PTAL.  Now this CL is only about creating the Finch trial.

On 2015/08/13 17:06:34, Ryan Hamilton wrote:
> On 2015/08/11 19:49:23, Bence wrote:
> > On 2015/08/10 17:09:21, Ryan Hamilton wrote:
> > > Sorry for the delay. Looking at this now. In the description, you say:
> > > 
> > > * (One can think about this as a rename, change of triggers, and change of
> > >   defaults.)
> > > 
> > > That kinda suggests "split this into 3 different CLs". As a reviewer, it's
> > super
> > > easy to review a rename CL. It's super easy to review a change of defaults
> CL.
> > > And it's pretty easy to read a change of triggers CL. But it's quite hard
to
> > > review all three changes at once :/
> > > 
> > > Or if 3 is hard, how 'bout a first CL which is simply the rename plus some
> of
> > > the formatting cleanup (like the comments in
> > > http_network_transaction_unittest.cc)?
> > 
> > SGTM.  Rename broken out to https://crrev.com/1268313004.  Thanks.
> 
> Cool. Let me know when this is ready for another look.

[Ryan Hamilton, 2015-08-20 17:37:31]

Hm. So it seems like this CL disables both alt-svc and alternate-protocol (since
they're now controlled via the same Param). This means that this field trial
group would disable QUIC, since we only speak QUIC via Alt-Svc. Are we sure this
is a good idea?

In my mind, I thought we were going to be able to disable Alt-Svc independently
from Alternate-Protocol. Which is to say, we would be able to support the
existing "QUIC to the same server" and then optionally (depending on finch
configs) "QUIC to a different server"

[Bence, 2015-08-21 12:57:54]

PTAL.

On 2015/08/20 17:37:31, Ryan Hamilton wrote:
> Hm. So it seems like this CL disables both alt-svc and alternate-protocol
(since
> they're now controlled via the same Param). This means that this field trial
> group would disable QUIC, since we only speak QUIC via Alt-Svc. Are we sure
this
> is a good idea?
> 
> In my mind, I thought we were going to be able to disable Alt-Svc
independently
> from Alternate-Protocol. Which is to say, we would be able to support the
> existing "QUIC to the same server" and then optionally (depending on finch
> configs) "QUIC to a different server"

Sorry for the oversight.  Addressed in https://crrev.com/1310583002.

[Ryan Hamilton, 2015-08-26 18:41:33]

Ok, now that I see this CL, I'm not sure we should land it this way. Instead, I
think that what we really want to do is add a new param to the existing QUIC
study. If a particular QUIC group says use_alternative_services = true, then we
should enable Alt-Svc, otherwise we should leave it false. The reason I think we
should go this route, is that we want to have consistent behavior within a
group. If there is an orthogonal groups which is controlling alt-svc we won't be
able to correlate Alt-Svc gains, I think. Can you tweak this CL to do that? Ping
me if you'd like a pointer for how to do that...

[Bence, 2015-08-27 12:54:33]

Patchset #7 (id:120001) has been deleted

[Bence, 2015-08-27 14:30:13]

PTAL.  Makes a lot of sense to me.

On 2015/08/26 18:41:33, Ryan Hamilton wrote:
> Ok, now that I see this CL, I'm not sure we should land it this way. Instead,
I
> think that what we really want to do is add a new param to the existing QUIC
> study. If a particular QUIC group says use_alternative_services = true, then
we
> should enable Alt-Svc, otherwise we should leave it false. The reason I think
we
> should go this route, is that we want to have consistent behavior within a
> group. If there is an orthogonal groups which is controlling alt-svc we won't
be
> able to correlate Alt-Svc gains, I think. Can you tweak this CL to do that?
Ping
> me if you'd like a pointer for how to do that...

[Ryan Hamilton, 2015-08-27 17:24:18]

lgtm

EXCELLENT. One small nit.

https://codereview.chromium.org/1271793003/diff/140001/chrome/browser/io_thre...
File chrome/browser/io_thread_unittest.cc (right):

https://codereview.chromium.org/1271793003/diff/140001/chrome/browser/io_thre...
chrome/browser/io_thread_unittest.cc:141: TEST_F(IOThreadTest,
EnableAlternativeServicesFromFieldTrialParams) {
Can you move this down into the QUIC section, just to make it obvious that it's
QUIC specific? (And so that it's after the check on line 182 which verifies that
alt-svc is off by default for QUIC.

[Bence, 2015-08-27 17:29:54]

Thanks.

https://codereview.chromium.org/1271793003/diff/140001/chrome/browser/io_thre...
File chrome/browser/io_thread_unittest.cc (right):

https://codereview.chromium.org/1271793003/diff/140001/chrome/browser/io_thre...
chrome/browser/io_thread_unittest.cc:141: TEST_F(IOThreadTest,
EnableAlternativeServicesFromFieldTrialParams) {
On 2015/08/27 17:24:18, Ryan Hamilton wrote:
> Can you move this down into the QUIC section, just to make it obvious that
it's
> QUIC specific? (And so that it's after the check on line 182 which verifies
that
> alt-svc is off by default for QUIC.

Done.

[Bence, 2015-08-27 17:29:59]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-08-27 17:29:59]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1271793003/#ps160001
(title: "Re: #15.")

[commit-bot: I haz the power, 2015-08-27 17:30:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1271793003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1271793003/160001

[commit-bot: I haz the power, 2015-08-27 18:32:16]

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

[commit-bot: I haz the power, 2015-08-27 18:32:17]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Bence, 2015-08-27 18:50:10]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2015-08-27 18:51:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1271793003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1271793003/160001

[commit-bot: I haz the power, 2015-08-27 19:18:22]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2015-08-27 19:19:42]

Patchset 8 (id:??) landed as
https://crrev.com/d00793801e96b0f8b26c2ef7ba06fea5cc01f13e
Cr-Commit-Position: refs/heads/master@{#345944}