QUIC - extend origin-to-force-quic-on command line option to accept list

QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org, eroman@chromium.org,

Committed: https://crrev.com/8a2f463d3726f944f5ec279664b25118fd3ccf3c
Cr-Commit-Position: refs/heads/master@{#382370}

[ramant (doing other things), 2016-03-19 04:07:55]

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

[commit-bot: I haz the power, 2016-03-19 04:08:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808303005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808303005/1

[commit-bot: I haz the power, 2016-03-19 04:09:56]

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

[commit-bot: I haz the power, 2016-03-19 04:09:57]

Dry run: Try jobs failed on following builders:
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[ramant (doing other things), 2016-03-19 04:10:55]

Description was changed from

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org
==========

to

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org
==========

[ramant (doing other things), 2016-03-19 04:10:56]

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

[ramant (doing other things), 2016-03-19 04:34:28]

Patchset #2 (id:20001) has been deleted

[ramant (doing other things), 2016-03-19 04:36:46]

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

[commit-bot: I haz the power, 2016-03-19 04:36:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808303005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808303005/40001

[ramant (doing other things), 2016-03-19 04:37:09]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-03-19 04:54:52]

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

[commit-bot: I haz the power, 2016-03-19 04:54:53]

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

[ramant (doing other things), 2016-03-19 05:13:07]

Patchset #1 (id:40001) has been deleted

[ramant (doing other things), 2016-03-19 05:13:13]

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

[commit-bot: I haz the power, 2016-03-19 05:13:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808303005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808303005/60001

[ramant (doing other things), 2016-03-19 05:41:20]

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

[commit-bot: I haz the power, 2016-03-19 05:41:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808303005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808303005/80001

[ramant (doing other things), 2016-03-19 05:41:31]

Patchset #1 (id:60001) has been deleted

[ramant (doing other things), 2016-03-19 05:42:27]

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

[ramant (doing other things), 2016-03-19 06:13:17]

Hi Ryan,
  Had couple of questions, would appreciate your opinion on using set vs
unordered_set and renaming command line option.

thanks
raman

https://codereview.chromium.org/1808303005/diff/80001/chrome/browser/io_thread.h
File chrome/browser/io_thread.h (right):

https://codereview.chromium.org/1808303005/diff/80001/chrome/browser/io_threa...
chrome/browser/io_thread.h:237: std::set<net::HostPortPair>
origins_to_force_quic_on;
Used std::set because we are storing HostPortPair? std::unordered_set requires
operator== and operator() to HostPortPair (I am guessing this option would be
used for testing only and wasn't sure if we should modify HostPortPair or not).

https://codereview.chromium.org/1808303005/diff/80001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:775: const char kOriginToForceQuicOn[]         
 = "origin-to-force-quic-on";
Should we change the name to "origins-to-force-quic-on"?

[commit-bot: I haz the power, 2016-03-19 07:03:28]

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

[commit-bot: I haz the power, 2016-03-19 07:03:29]

Dry run: This issue passed the CQ dry run.

[Ryan Hamilton, 2016-03-20 22:46:53]

LGTM, modulo a couple nits.

https://codereview.chromium.org/1808303005/diff/80001/chrome/browser/io_threa...
File chrome/browser/io_thread_unittest.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/chrome/browser/io_threa...
chrome/browser/io_thread_unittest.cc:608:
command_line_.AppendSwitchASCII(switches::kOriginToForceQuicOn,
nit: Please a string literal here because it ensure that kFoo matches what we
expect it to.

https://codereview.chromium.org/1808303005/diff/80001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:775: const char kOriginToForceQuicOn[]         
 = "origin-to-force-quic-on";
On 2016/03/19 06:13:16, ramant wrote:
> Should we change the name to "origins-to-force-quic-on"?

That's fine with me.

https://codereview.chromium.org/1808303005/diff/80001/net/http/http_network_s...
File net/http/http_network_session.h (right):

https://codereview.chromium.org/1808303005/diff/80001/net/http/http_network_s...
net/http/http_network_session.h:151: std::set<net::HostPortPair>
origins_to_force_quic_on;
This is already in net, so you can leave net:: off.

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_end_to_en...
File net/quic/quic_end_to_end_unittest.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_end_to_en...
net/quic/quic_end_to_end_unittest.cc:163:
params_.origins_to_force_quic_on.insert(HostPortPair::FromString(host));
nit: do we really need a loop here?

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_network_t...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:625:
params_.origins_to_force_quic_on.insert(HostPortPair::FromString(host));
Ditto, no need for loops here and below.

[ramant (doing other things), 2016-03-21 17:23:14]

Description was changed from

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org
==========

to

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org, eroman@chromium.org,
==========

[ramant (doing other things), 2016-03-21 17:23:14]

rtenneti@chromium.org changed reviewers:
+ eroman@chromium.org

[ramant (doing other things), 2016-03-21 17:23:48]

+ eroman@ for  chrome/browser/resources/net_internals/quic_view.html

Thanks Ryan. Made all the changes.

https://codereview.chromium.org/1808303005/diff/80001/chrome/browser/io_threa...
File chrome/browser/io_thread_unittest.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/chrome/browser/io_threa...
chrome/browser/io_thread_unittest.cc:608:
command_line_.AppendSwitchASCII(switches::kOriginToForceQuicOn,
On 2016/03/20 22:46:53, Ryan Hamilton wrote:
> nit: Please a string literal here because it ensure that kFoo matches what we
> expect it to.

Done.

https://codereview.chromium.org/1808303005/diff/80001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:775: const char kOriginToForceQuicOn[]         
 = "origin-to-force-quic-on";
On 2016/03/20 22:46:53, Ryan Hamilton wrote:
> On 2016/03/19 06:13:16, ramant wrote:
> > Should we change the name to "origins-to-force-quic-on"?
> 
> That's fine with me.

Acknowledged.

https://codereview.chromium.org/1808303005/diff/80001/net/http/http_network_s...
File net/http/http_network_session.h (right):

https://codereview.chromium.org/1808303005/diff/80001/net/http/http_network_s...
net/http/http_network_session.h:151: std::set<net::HostPortPair>
origins_to_force_quic_on;
On 2016/03/20 22:46:53, Ryan Hamilton wrote:
> This is already in net, so you can leave net:: off.

Done.

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_end_to_en...
File net/quic/quic_end_to_end_unittest.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_end_to_en...
net/quic/quic_end_to_end_unittest.cc:163:
params_.origins_to_force_quic_on.insert(HostPortPair::FromString(host));
On 2016/03/20 22:46:53, Ryan Hamilton wrote:
> nit: do we really need a loop here?

Done.

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_network_t...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1808303005/diff/80001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:625:
params_.origins_to_force_quic_on.insert(HostPortPair::FromString(host));
On 2016/03/20 22:46:53, Ryan Hamilton wrote:
> Ditto, no need for loops here and below.

Done.

[ramant (doing other things), 2016-03-21 17:23:55]

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

[commit-bot: I haz the power, 2016-03-21 17:24:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808303005/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808303005/100001

[commit-bot: I haz the power, 2016-03-21 18:34:30]

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

[commit-bot: I haz the power, 2016-03-21 18:34:31]

Dry run: This issue passed the CQ dry run.

[eroman, 2016-03-21 18:56:14]

net_internals LGTM

[ramant (doing other things), 2016-03-21 20:18:11]

The CQ bit was checked by rtenneti@chromium.org

[ramant (doing other things), 2016-03-21 20:18:12]

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/1808303005/#ps100001
(title: "Fixed comments for Patch Set 1")

[commit-bot: I haz the power, 2016-03-21 20:18:32]

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

[commit-bot: I haz the power, 2016-03-21 20:27:05]

Description was changed from

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org, eroman@chromium.org,
==========

to

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org, eroman@chromium.org,
==========

[commit-bot: I haz the power, 2016-03-21 20:27:06]

Committed patchset #2 (id:100001)

[commit-bot: I haz the power, 2016-03-21 20:27:58]

Description was changed from

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org, eroman@chromium.org,
==========

to

==========
QUIC - extend origin-to-force-quic-on command line option to accept list
of host_ports for which QUIC will be forced. A TCP job wouldn't even be
started for these origins.

R=rch@chromium.org, eroman@chromium.org,

Committed: https://crrev.com/8a2f463d3726f944f5ec279664b25118fd3ccf3c
Cr-Commit-Position: refs/heads/master@{#382370}
==========

[commit-bot: I haz the power, 2016-03-21 20:27:59]

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