Move some of IOThread's command line logic to network_session_configurator.

Move some of IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

TBR=lcwu@chromium.org,rkc@chromium.org
BUG=727812
Review-Url: https://codereview.chromium.org/2914893002
Cr-Commit-Position: refs/heads/master@{#478740}
Committed: https://chromium.googlesource.com/chromium/src/+/68a502104896fc2bb14fc3903afe31d275e2ffda

[mmenke, 2017-05-31 20:13:10]

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

[commit-bot: I haz the power, 2017-05-31 20:14: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, 2017-05-31 20:26:12]

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

[commit-bot: I haz the power, 2017-05-31 20:26:13]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-05-31 20:43:17]

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

[commit-bot: I haz the power, 2017-05-31 20:44:32]

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, 2017-05-31 20:54:24]

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

[commit-bot: I haz the power, 2017-05-31 20:54:25]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-05-31 21:30:03]

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

[commit-bot: I haz the power, 2017-05-31 21:30:59]

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, 2017-05-31 21:30:59]

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, 2017-05-31 21:44:03]

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

[commit-bot: I haz the power, 2017-05-31 21:44:04]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[mmenke, 2017-05-31 21:52:27]

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

[commit-bot: I haz the power, 2017-05-31 21:53:20]

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, 2017-05-31 21:58:28]

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

[commit-bot: I haz the power, 2017-05-31 21:58:29]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-06-02 16:35:22]

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

[commit-bot: I haz the power, 2017-06-02 16:35:41]

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, 2017-06-02 16:39:33]

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

[commit-bot: I haz the power, 2017-06-02 16:39:34]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2017-06-02 16:51:08]

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

[commit-bot: I haz the power, 2017-06-02 16:51:20]

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, 2017-06-02 16:55:30]

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

[commit-bot: I haz the power, 2017-06-02 16:55:32]

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

[mmenke, 2017-06-02 17:00:47]

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

[commit-bot: I haz the power, 2017-06-02 17:01:06]

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, 2017-06-02 17:11:05]

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

[commit-bot: I haz the power, 2017-06-02 17:11:06]

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

[mmenke, 2017-06-02 17:17:16]

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

[commit-bot: I haz the power, 2017-06-02 17:17:39]

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, 2017-06-02 17:26:34]

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

[commit-bot: I haz the power, 2017-06-02 17:26:35]

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

[mmenke, 2017-06-02 19:44:10]

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

[commit-bot: I haz the power, 2017-06-02 19:45:03]

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, 2017-06-02 19:55:12]

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

[commit-bot: I haz the power, 2017-06-02 19:55:13]

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

[mmenke, 2017-06-02 20:01:29]

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

[commit-bot: I haz the power, 2017-06-02 20:01:58]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-02 20:18:21]

Patchset #8 (id:90058) has been deleted

[mmenke, 2017-06-02 20:23:46]

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

[mmenke, 2017-06-02 20:23:48]

[rdsmith]:  FYI

[rch]:  PTAL.  The main changes are in components/network_session_configurator/
(Which is why I'm sending this to you) and io_thread, with everything else being
fallout from that.  Have to split out the switches into their own component
because chrome/common/service_process_util.cc depends on one of them.  See
description for more context.

[commit-bot: I haz the power, 2017-06-02 20:36:01]

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

[commit-bot: I haz the power, 2017-06-02 20:36:02]

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

[Ryan Hamilton, 2017-06-02 20:49:04]

Looks good! Thanks for doing this. I think we need some tests of the new
behavior in the configurator.

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File
components/network_session_configurator/browser/network_session_configurator.h
(right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/browser/network_session_configurator.h:6:
#define
COMPONENTS_NETWORK_SESSION_CONFIGURATOR_BROWSER_NETWORK_SESSION_CONFIGURATOR_H_
Clearly we need a longer name!

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File
components/network_session_configurator/browser/network_session_configurator_unittest.cc
(right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/browser/network_session_configurator_unittest.cc:358:
}  // namespace test
Should we add some tests for the new flag handling behavior?

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File components/network_session_configurator/common/BUILD.gn (right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/common/BUILD.gn:3: # found in the
LICENSE file.
This is probably obvious, but what's the difference between browser/ and
common/? Should we add a comment about that somewhere?

https://codereview.chromium.org/2914893002/diff/170001/content/shell/DEPS
File content/shell/DEPS (right):

https://codereview.chromium.org/2914893002/diff/170001/content/shell/DEPS#new...
content/shell/DEPS:31: 
nit: remove blank line?

[mmenke, 2017-06-02 22:02:30]

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

[commit-bot: I haz the power, 2017-06-02 22:03: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, 2017-06-02 22:38:03]

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

[commit-bot: I haz the power, 2017-06-02 22:38:04]

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

[mmenke, 2017-06-02 23:30:22]

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

[commit-bot: I haz the power, 2017-06-02 23:31:08]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-02 23:36:45]

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

[commit-bot: I haz the power, 2017-06-02 23:37:20]

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, 2017-06-03 00:17:12]

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

[commit-bot: I haz the power, 2017-06-03 00:17:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-06-05 16:07:39]

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

[commit-bot: I haz the power, 2017-06-05 16:07:56]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-05 16:09:34]

Patchset #12 (id:230001) has been deleted

[commit-bot: I haz the power, 2017-06-05 16:21:17]

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

[commit-bot: I haz the power, 2017-06-05 16:21:19]

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

[mmenke, 2017-06-05 17:30:30]

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

[commit-bot: I haz the power, 2017-06-05 17:30:49]

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, 2017-06-05 17:45:27]

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

[commit-bot: I haz the power, 2017-06-05 17:45:28]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[mmenke, 2017-06-05 17:57:21]

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

[commit-bot: I haz the power, 2017-06-05 17:57:49]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-05 18:42:33]

Patchset #11 (id:210001) has been deleted

[mmenke, 2017-06-05 18:42:54]

Patchset #12 (id:270001) has been deleted

[mmenke, 2017-06-05 18:43:27]

[rch]:  Thanks for the speedy review!  PTAL.

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File
components/network_session_configurator/browser/network_session_configurator.h
(right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/browser/network_session_configurator.h:6:
#define
COMPONENTS_NETWORK_SESSION_CONFIGURATOR_BROWSER_NETWORK_SESSION_CONFIGURATOR_H_
On 2017/06/02 20:49:04, Ryan Hamilton wrote:
> Clearly we need a longer name!

Maybe throw in a couple DELEGATES?

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File
components/network_session_configurator/browser/network_session_configurator_unittest.cc
(right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/browser/network_session_configurator_unittest.cc:358:
}  // namespace test
On 2017/06/02 20:49:04, Ryan Hamilton wrote:
> Should we add some tests for the new flag handling behavior?

I wasn't sure if it was worth the effort - we often don't include tests for
command line flags.  It's easy enough to add tests, though, so I'll go ahead and
do so.

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File components/network_session_configurator/common/BUILD.gn (right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/common/BUILD.gn:3: # found in the
LICENSE file.
On 2017/06/02 20:49:04, Ryan Hamilton wrote:
> This is probably obvious, but what's the difference between browser/ and
> common/? Should we add a comment about that somewhere?

It's pretty common for components to have directories with these names
(https://cs.chromium.org/search/?q=file:components/.*/browser/$+package:%5Echr...
- looks like over 100 components do this).  In general common/ can be included
by anything, and browser/ can only be included by things that live only in the
browser process (content/browser, chrome/browser, or things only they include or
that include them).  Since HttpNetworkSession can't be used in most processes,
seems like, for example, a file in content/common shouldn't be depending on it.

This is a common enough pattern that I'm not sure if it needs to be documented
(I'm also not sure about where this should be documented, if we chose to do so).

https://codereview.chromium.org/2914893002/diff/170001/content/shell/DEPS
File content/shell/DEPS (right):

https://codereview.chromium.org/2914893002/diff/170001/content/shell/DEPS#new...
content/shell/DEPS:31: 
On 2017/06/02 20:49:04, Ryan Hamilton wrote:
> nit: remove blank line?

I'm not sure the comment applies to this include?  content/ files depend on
common, too, so the "Content Shell can depend on more components than content/"
comment does not apply to this include, since parts of content depend on it,
just content/ itself does not....Hrm...Looks like url_formatter is included
elsewhere in content/, too, though, so I guess I'll just follow suit.

[commit-bot: I haz the power, 2017-06-05 19:25:59]

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

[commit-bot: I haz the power, 2017-06-05 19:26:01]

Dry run: This issue passed the CQ dry run.

[mmenke, 2017-06-07 15:35:07]

[rch]:  Ping!

[Ryan Hamilton, 2017-06-07 18:15:44]

Sorry for the delay. I somehow completely missed the mail with your replies.

LGTM

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
File components/network_session_configurator/common/BUILD.gn (right):

https://codereview.chromium.org/2914893002/diff/170001/components/network_ses...
components/network_session_configurator/common/BUILD.gn:3: # found in the
LICENSE file.
On 2017/06/05 18:43:27, mmenke wrote:
> On 2017/06/02 20:49:04, Ryan Hamilton wrote:
> > This is probably obvious, but what's the difference between browser/ and
> > common/? Should we add a comment about that somewhere?
> 
> It's pretty common for components to have directories with these names
>
(https://cs.chromium.org/search/?q=file:components/.*/browser/$+package:%5Echr...
> - looks like over 100 components do this).  In general common/ can be included
> by anything, and browser/ can only be included by things that live only in the
> browser process (content/browser, chrome/browser, or things only they include
or
> that include them).  Since HttpNetworkSession can't be used in most processes,
> seems like, for example, a file in content/common shouldn't be depending on
it.
> 
> This is a common enough pattern that I'm not sure if it needs to be documented
> (I'm also not sure about where this should be documented, if we chose to do
so).

Ok, fair enough!

https://codereview.chromium.org/2914893002/diff/170001/content/shell/DEPS
File content/shell/DEPS (right):

https://codereview.chromium.org/2914893002/diff/170001/content/shell/DEPS#new...
content/shell/DEPS:31: 
On 2017/06/05 18:43:27, mmenke wrote:
> On 2017/06/02 20:49:04, Ryan Hamilton wrote:
> > nit: remove blank line?
> 
> I'm not sure the comment applies to this include?  content/ files depend on
> common, too, so the "Content Shell can depend on more components than
content/"
> comment does not apply to this include, since parts of content depend on it,
> just content/ itself does not....Hrm...Looks like url_formatter is included
> elsewhere in content/, too, though, so I guess I'll just follow suit.

oh, I missed that the newline was important here. *facepalm*.

[mmenke, 2017-06-07 18:25:23]

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

[mmenke, 2017-06-07 18:25:24]

[+jam]:  Please review content/ (content/browser/utility_process_host_impl.cc). 
Is it reasonable to split a component like this just to share the switches?

This doesn't hook up the switches to the network service, so the only behavior
change is in passing the wider set of switches to the service.  Should be a
one-line change to hook up the network service, but want to add a test or two
when I do that, in a followup CL.

[mmenke, 2017-06-07 19:06:21]

On 2017/06/07 18:25:24, mmenke wrote:
> [+jam]:  Please review content/
(content/browser/utility_process_host_impl.cc). 
> Is it reasonable to split a component like this just to share the switches?

Sorry, that should be "especially content/browser/utility_process_host_impl.cc",
which is the only place where behavior has been changed.

[jam, 2017-06-08 18:23:28]

lgtm

[mmenke, 2017-06-08 18:25:06]

Description was changed from

==========
Move some IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well, to enable further code
sharing.

BUG=727812
==========

to

==========
Move some IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

BUG=727812
==========

[mmenke, 2017-06-08 18:35:57]

mmenke@chromium.org changed reviewers:
+ jochen@chromium.org, lcwu@chromium.org, rkc@chromium.org,
rohitrao@chromium.org, tobiasjs@chromium.org

[mmenke, 2017-06-08 18:35:59]

[+jochen]:  Please review chrome/ and components/BUILD.gn. I'm just moving a
number of network-related command line switches into a component, but that
requires a couple changes to top level files, unfortunately.
[+tobiasjs]:  Please review android_webview/
[+lcwu]:  Please review chromecast/
[+rkc]:  Please review extensions/shell/
[+rohitrao]:  Please review ios/

[mmenke, 2017-06-08 18:36:29]

Description was changed from

==========
Move some IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

BUG=727812
==========

to

==========
Move some of IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

BUG=727812
==========

[jochen (gone - plz use gerrit), 2017-06-08 22:05:56]

components/BUILD.gn and chrome/ lgtm

[rohitrao (ping after 24h), 2017-06-09 17:45:40]

ios/ lgtm

[mmenke, 2017-06-09 17:51:59]

Thanks!

I consider the rest of this TBRable (All it is is updating
DEPS/BUILD.gns/includes for moved switches), so I'll plan to TBR early next
week, if I don't hear back from remaining owners.

[Tobias Sargeant, 2017-06-09 17:55:36]

android_webview LGTM

https://codereview.chromium.org/2914893002/diff/290001/android_webview/browse...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/2914893002/diff/290001/android_webview/browse...
android_webview/browser/net/aw_url_request_context_getter.cc:89: //
TODO(mmenke):  Should this use network_session_configurator?
I think the answer to this is at the moment no, because WebView has no way to
participate in field trials. However this may change in not too distant future.

[mmenke, 2017-06-09 18:02:35]

I'll remove the TODO, but just wanted to point this out.

https://codereview.chromium.org/2914893002/diff/290001/android_webview/browse...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/2914893002/diff/290001/android_webview/browse...
android_webview/browser/net/aw_url_request_context_getter.cc:89: //
TODO(mmenke):  Should this use network_session_configurator?
On 2017/06/09 17:55:36, Tobias Sargeant wrote:
> I think the answer to this is at the moment no, because WebView has no way to
> participate in field trials. However this may change in not too distant
future.

But it also sets up command line flags, including the ones on the next three
lines.

[mmenke, 2017-06-12 16:05:49]

Description was changed from

==========
Move some of IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

BUG=727812
==========

to

==========
Move some of IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

TBR=lcwu@chromium.org,rkc@chromium.org
BUG=727812
==========

[mmenke, 2017-06-12 16:06:19]

[lcwu,rkc]:  TBRing both of you.

[mmenke, 2017-06-12 16:06:25]

The CQ bit was checked by mmenke@chromium.org

[mmenke, 2017-06-12 16:06:26]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
jam@chromium.org, rch@chromium.org, tobiasjs@chromium.org, rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2914893002/#ps310001
(title: "Remove TODO")

[commit-bot: I haz the power, 2017-06-12 16:07:00]

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, 2017-06-12 16:10:15]

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

[commit-bot: I haz the power, 2017-06-12 16:10:19]

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-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[mmenke, 2017-06-12 18:22:57]

The CQ bit was checked by mmenke@chromium.org

[mmenke, 2017-06-12 18:22:57]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
jam@chromium.org, rch@chromium.org, tobiasjs@chromium.org, rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2914893002/#ps330001
(title: "Fix merge")

[commit-bot: I haz the power, 2017-06-12 18:24:03]

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, 2017-06-12 20:20:34]

CQ is committing da patch.

Bot data: {"patchset_id": 330001, "attempt_start_ts": 1497291777191850,
"parent_rev": "41757e52bacb2dd4ba55823e49bb16cc96347cb4", "commit_rev":
"68a502104896fc2bb14fc3903afe31d275e2ffda"}

[commit-bot: I haz the power, 2017-06-12 20:20:49]

Description was changed from

==========
Move some of IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

TBR=lcwu@chromium.org,rkc@chromium.org
BUG=727812
==========

to

==========
Move some of IOThread's command line logic to network_session_configurator.

This will allow it to be shared with the network service, and makes it
easy to pass all relevant arguments to the network service as well.

May add handling for command line arguments that affect more than just
the HttpNetworkSession::Params there as well in a future CL, to enable
further code sharing.

TBR=lcwu@chromium.org,rkc@chromium.org
BUG=727812

Review-Url: https://codereview.chromium.org/2914893002
Cr-Commit-Position: refs/heads/master@{#478740}
Committed:
https://chromium.googlesource.com/chromium/src/+/68a502104896fc2bb14fc3903afe...
==========

[commit-bot: I haz the power, 2017-06-12 20:20:51]

Committed patchset #14 (id:330001) as
https://chromium.googlesource.com/chromium/src/+/68a502104896fc2bb14fc3903afe...