Remove the dependence on BrowserThread::FILE for

Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
TBR=jochen@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2944313003
Cr-Commit-Position: refs/heads/master@{#481691}
Committed: https://chromium.googlesource.com/chromium/src/+/0070d4146866230c1a5599be3444794ee72d47ae

[eroman, 2017-06-21 01:33:04]

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

[commit-bot: I haz the power, 2017-06-21 01:33:23]

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-21 01:42:58]

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

[commit-bot: I haz the power, 2017-06-21 01:42:59]

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...)

[eroman, 2017-06-21 20:25:20]

Description was changed from

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
==========

to

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[eroman, 2017-06-21 20:25:22]

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

[commit-bot: I haz the power, 2017-06-21 20:25: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-21 20:29:18]

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

[commit-bot: I haz the power, 2017-06-21 20:29:19]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[eroman, 2017-06-21 21:17:15]

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

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

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-21 21:27:24]

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

[commit-bot: I haz the power, 2017-06-21 21:27:24]

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...)

[eroman, 2017-06-21 22:50:10]

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

[commit-bot: I haz the power, 2017-06-21 22:50:38]

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-21 22:53:52]

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

[commit-bot: I haz the power, 2017-06-21 22:53:53]

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...)

[eroman, 2017-06-21 23:08:54]

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

[commit-bot: I haz the power, 2017-06-21 23:09: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-21 23:19:58]

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

[commit-bot: I haz the power, 2017-06-21 23:19:59]

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...)

[eroman, 2017-06-21 23:25:04]

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

[commit-bot: I haz the power, 2017-06-21 23:25:26]

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-21 23:47:25]

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

[commit-bot: I haz the power, 2017-06-21 23:47:25]

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_...)

[eroman, 2017-06-21 23:55:50]

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

[commit-bot: I haz the power, 2017-06-21 23:56:13]

Dry run: CQ is trying da patch.

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

[eroman, 2017-06-22 00:13:08]

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

[eroman, 2017-06-22 00:13:09]

... there are more refactors that can be done here, but I want to target the
narrower removal of BrowserThread::FILE first (for task scheduler migration).

[commit-bot: I haz the power, 2017-06-22 01:56:34]

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

[commit-bot: I haz the power, 2017-06-22 01:56:35]

Dry run: Try jobs failed on following builders:
  android_cronet_tester on master.tryserver.chromium.android (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[mmenke, 2017-06-22 18:07:06]

Looks great, LGTM!

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
File net/proxy/proxy_config_service_linux.cc (right):

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
net/proxy/proxy_config_service_linux.cc:517:
scoped_refptr<base::SequencedTaskRunner> task_runner_;
optional:  Should we keep the glib thread as a SingleThreadTaskRunner?

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
File net/proxy/proxy_config_service_linux.h (right):

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
net/proxy/proxy_config_service_linux.h:52: // depending on the implementation.
Could we make it clearer that we're not performing file i/o on glib_task_runner?

[eroman, 2017-06-22 19:50:33]

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

[eroman, 2017-06-22 19:50:47]

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
File net/proxy/proxy_config_service_linux.cc (right):

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
net/proxy/proxy_config_service_linux.cc:517:
scoped_refptr<base::SequencedTaskRunner> task_runner_;
On 2017/06/22 18:07:06, mmenke wrote:
> optional:  Should we keep the glib thread as a SingleThreadTaskRunner?

We have a virtual method:

  const scoped_refptr<base::SequencedTaskRunner>& GetNotificationTaskRunner()

In some implementations it returns the glib thread (SingleThreadTaskRunner) and
in the KDE implementation it returns a SequencedTaskRunner (an internal
"file_task_runner"). If it needs to downcast, I believe I would need to change
the signature to return a copy rather than a const-reference.

Changing the member to store the base class seemed like the path of least
resistance.

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
File net/proxy/proxy_config_service_linux.h (right):

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
net/proxy/proxy_config_service_linux.h:52: // depending on the implementation.
On 2017/06/22 18:07:06, mmenke wrote:
> Could we make it clearer that we're not performing file i/o on
glib_task_runner?

Done. How about this:

 50     // This interface supports both GNOME and KDE implementations. In the
 51     // case of GNOME, the glib_task_runner will be used for interacting with
 52     // gconf/gsettings as those APIs have thread affinity. Whereas in the
case
 53     // of KDE, its configuration files will be monitored at well-known
locations
 54     // and glib_task_runner will not be used. Instead, blocking file I/O
 55     // operations will be posted directly using the task scheduler.

[commit-bot: I haz the power, 2017-06-22 19:50:54]

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-22 19:52:10]

Still LGTM

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
File net/proxy/proxy_config_service_linux.h (right):

https://codereview.chromium.org/2944313003/diff/120001/net/proxy/proxy_config...
net/proxy/proxy_config_service_linux.h:52: // depending on the implementation.
On 2017/06/22 19:50:47, eroman wrote:
> On 2017/06/22 18:07:06, mmenke wrote:
> > Could we make it clearer that we're not performing file i/o on
> glib_task_runner?
> 
> Done. How about this:
> 
>  50     // This interface supports both GNOME and KDE implementations. In the
>  51     // case of GNOME, the glib_task_runner will be used for interacting
with
>  52     // gconf/gsettings as those APIs have thread affinity. Whereas in the
> case
>  53     // of KDE, its configuration files will be monitored at well-known
> locations
>  54     // and glib_task_runner will not be used. Instead, blocking file I/O
>  55     // operations will be posted directly using the task scheduler.

SGTM

[eroman, 2017-06-22 19:56:20]

The CQ bit was unchecked by eroman@chromium.org

[eroman, 2017-06-22 19:56:25]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-06-22 19:56:46]

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-22 20:04:20]

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

[commit-bot: I haz the power, 2017-06-22 20:04:21]

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

[eroman, 2017-06-22 20:06:18]

Description was changed from

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
TBR=mkwst@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[eroman, 2017-06-22 20:06:42]

TBR mkwst@chromium.org for the following files:

  content/shell/browser/layout_test/layout_test_browser_context.cc
  content/shell/browser/layout_test/layout_test_url_request_context_getter.cc
  content/shell/browser/layout_test/layout_test_url_request_context_getter.h
  content/shell/browser/shell_browser_context.cc
  content/shell/browser/shell_url_request_context_getter.cc
  content/shell/browser/shell_url_request_context_getter.h

[eroman, 2017-06-22 20:06:50]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-06-22 20:07:17]

CQ is trying da patch.

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

[eroman, 2017-06-22 20:07:46]

The CQ bit was unchecked by eroman@chromium.org

[eroman, 2017-06-22 20:08:36]

Sorry @mkwst, please disregard my TBR request!
(Updated wrong CL)

[eroman, 2017-06-22 20:08:45]

Description was changed from

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
TBR=mkwst@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
TBR=jochen@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[eroman, 2017-06-22 20:09:43]

TBR jochen for the following files:

  android_webview/browser/aw_browser_context.cc
  chrome/service/net/service_url_request_context_getter.cc
  components/cronet/android/cronet_url_request_context_adapter.cc
  components/proxy_config/ios/proxy_service_factory.cc
  content/shell/browser/shell_url_request_context_getter.cc
  content/shell/browser/shell_url_request_context_getter.h
  headless/lib/browser/headless_url_request_context_getter.cc
  ios/crnet/crnet_environment.mm
  remoting/base/url_request_context_getter.cc

[eroman, 2017-06-22 20:09:44]

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

[commit-bot: I haz the power, 2017-06-22 20:10:08]

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-22 22:04:59]

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

[commit-bot: I haz the power, 2017-06-22 22:05:00]

Dry run: This issue passed the CQ dry run.

[eroman, 2017-06-22 22:13:37]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-06-22 22:14:07]

CQ is trying da patch.

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

[eroman, 2017-06-22 22:18:02]

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

[eroman, 2017-06-22 22:18:02]

TBR jochen for the following files:

  android_webview/browser/aw_browser_context.cc
  chrome/service/net/service_url_request_context_getter.cc
  components/cronet/android/cronet_url_request_context_adapter.cc
  components/proxy_config/ios/proxy_service_factory.cc
  content/shell/browser/shell_url_request_context_getter.cc
  content/shell/browser/shell_url_request_context_getter.h
  headless/lib/browser/headless_url_request_context_getter.cc
  ios/crnet/crnet_environment.mm
  remoting/base/url_request_context_getter.cc

[commit-bot: I haz the power, 2017-06-22 22:18:27]

CQ is committing da patch.

Bot data: {"patchset_id": 130001, "attempt_start_ts": 1498169617275180,
"parent_rev": "f597670d9a6ea1298dac6048389c3a51e45d6434", "commit_rev":
"0070d4146866230c1a5599be3444794ee72d47ae"}

[commit-bot: I haz the power, 2017-06-22 22:18:43]

Description was changed from

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
TBR=jochen@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Remove the dependence on BrowserThread::FILE for
ProxyConfigServiceLinux.

BUG=689520
TBR=jochen@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2944313003
Cr-Commit-Position: refs/heads/master@{#481691}
Committed:
https://chromium.googlesource.com/chromium/src/+/0070d4146866230c1a5599be3444...
==========

[commit-bot: I haz the power, 2017-06-22 22:18:45]

Committed patchset #8 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/0070d4146866230c1a5599be3444...