Fix chromeos::ProxyConfigService starting with the wrong ProxyConfig.

Fix chromeos::ProxyConfigService starting with the wrong ProxyConfig.

BUG=734565
Review-Url: https://codereview.chromium.org/2946153002
Cr-Commit-Position: refs/heads/master@{#481310}
Committed: https://chromium.googlesource.com/chromium/src/+/29e5a5de469977d5ce690492dfa62d3a69f77b1c

[mmenke, 2017-06-20 23:29:30]

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

[commit-bot: I haz the power, 2017-06-20 23:29: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-20 23:44:59]

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

[commit-bot: I haz the power, 2017-06-20 23:45:00]

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-21 16:42:32]

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

[commit-bot: I haz the power, 2017-06-21 16:42:44]

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 17:20:58]

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

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

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-21 18:21:33]

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

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

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-21 18:25:29]

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

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

Patchset #3 (id:40001) has been deleted

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

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

[mmenke, 2017-06-21 19:13:20]

Ok, now this should actually fix the issue.  Was on the right track last time,
but I ended up just dropping the initial configuration a bit further down the
callstack.

https://codereview.chromium.org/2946153002/diff/60001/components/proxy_config...
File components/proxy_config/pref_proxy_config_tracker_impl.cc (right):

https://codereview.chromium.org/2946153002/diff/60001/components/proxy_config...
components/proxy_config/pref_proxy_config_tracker_impl.cc:356:
!pref_config_.Equals(new_config))) {
Removing this check broke an extensions API test, which makes me a bit concerned
about the correctness of the code, but I think that's another matter entirely.

[mmenke, 2017-06-21 19:14:20]

https://codereview.chromium.org/2946153002/diff/60001/components/proxy_config...
File components/proxy_config/pref_proxy_config_tracker_impl.cc (right):

https://codereview.chromium.org/2946153002/diff/60001/components/proxy_config...
components/proxy_config/pref_proxy_config_tracker_impl.cc:356:
!pref_config_.Equals(new_config))) {
On 2017/06/21 19:13:20, mmenke wrote:
> Removing this check broke an extensions API test, which makes me a bit
concerned
> about the correctness of the code, but I think that's another matter entirely.

The correctness of the extensions code, that is.  I'm pretty confident this CL
restores the original behavior.

[stevenjb, 2017-06-21 19:38:02]

Thanks for adding the unit test!

Again, caveat, I don't really have my head fully wrapped around this code, but
the changes lgtm.

https://codereview.chromium.org/2946153002/diff/60001/chromeos/network/proxy/...
File chromeos/network/proxy/proxy_config_service_impl_unittest.cc (right):

https://codereview.chromium.org/2946153002/diff/60001/chromeos/network/proxy/...
chromeos/network/proxy/proxy_config_service_impl_unittest.cc:26: 
I don't think we usually put the entire test suite in an anon namespace?

https://codereview.chromium.org/2946153002/diff/60001/chromeos/network/proxy/...
chromeos/network/proxy/proxy_config_service_impl_unittest.cc:36: 
nit: no blank line here

[mmenke, 2017-06-21 19:44:18]

https://codereview.chromium.org/2946153002/diff/60001/chromeos/network/proxy/...
File chromeos/network/proxy/proxy_config_service_impl_unittest.cc (right):

https://codereview.chromium.org/2946153002/diff/60001/chromeos/network/proxy/...
chromeos/network/proxy/proxy_config_service_impl_unittest.cc:26: 
On 2017/06/21 19:38:02, stevenjb wrote:
> I don't think we usually put the entire test suite in an anon namespace?

In net/ we often do, but I don't have strong feelings about it.  I've reduced
the scope of the namespace to end just above the test fixture class.

https://codereview.chromium.org/2946153002/diff/60001/chromeos/network/proxy/...
chromeos/network/proxy/proxy_config_service_impl_unittest.cc:36: 
On 2017/06/21 19:38:02, stevenjb wrote:
> nit: no blank line here

Done.

[mmenke, 2017-06-21 19:45:20]

Thanks!

[mmenke, 2017-06-21 19:45:28]

The CQ bit was checked by mmenke@chromium.org

[mmenke, 2017-06-21 19:45:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2946153002/#ps100001
(title: "Remove incorrect comment")

[commit-bot: I haz the power, 2017-06-21 19:45:43]

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:35:41]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1498074328916250,
"parent_rev": "0f29527597298df4a879ee6e8bd082993248c740", "commit_rev":
"29e5a5de469977d5ce690492dfa62d3a69f77b1c"}

[commit-bot: I haz the power, 2017-06-21 21:35:57]

Description was changed from

==========
Fix chromeos::ProxyConfigService starting with the wrong ProxyConfig.

BUG=734565
==========

to

==========
Fix chromeos::ProxyConfigService starting with the wrong ProxyConfig.

BUG=734565

Review-Url: https://codereview.chromium.org/2946153002
Cr-Commit-Position: refs/heads/master@{#481310}
Committed:
https://chromium.googlesource.com/chromium/src/+/29e5a5de469977d5ce690492dfa6...
==========

[commit-bot: I haz the power, 2017-06-21 21:35:58]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/29e5a5de469977d5ce690492dfa6...