Cleanup the UseSharedProxies preference.

Cleanup the UseSharedProxies preference.

- remove the UserSharedProxies pref from local state
- fix the default value of the pref in the profile pref service
- make the conditionals, when to use or ignore proxy settings, more explicit and clearer.

BUG=258835
TEST=Manually checked all combinations of {DevicePolicy, UserPolicy, SharedNetwork, NotSharedNetwork} x {before, after login} x {enabled, disabled UseSharedProxies} for proxies of the system context, signin profile and user's profile. I checked the proxies by looking at the vlogs and the UI (like net-internals and settings page). I did not test the actual network connectivity of each context.

TBR=battre@chromium.org, nkostylev@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211954

[pneubeck (no reviews), 2013-07-11 09:29:36]

Nikita, please take a look at files you own. Should be browser/chromeos/ and
webui/

Thanks!

[pneubeck (no reviews), 2013-07-11 12:56:09]

Dominic, please owner-review chrome/browser/{net,prefs}.
Thanks.

[battre, 2013-07-11 13:31:22]

We talked face to face. Here are a couple of other comments. I'll review the
updated CL once you ping me.

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/proxy_config_handler.cc (right):

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/proxy_config_handler.cc:71:
registry->RegisterBooleanPref(
Can you please improve the comment at the definition of kUseSharedProxies? I
find "A boolean pref that uses shared proxies." pretty useless.
1) A pref does not use proxies
2) Who shares the proxies?
3) What is the meaning of this pref on Desktop

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
chrome/browser/chromeos/proxy_config_service_impl.cc:155:
use_shared_proxies_pref->GetValue()->GetAsBoolean(&use_shared_proxies);
bool use_shared_proxies = profile_prefs->GetBoolean(prefs::kUseSharedProxies);

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
File chrome/browser/chromeos/proxy_config_service_impl.h (right):

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
chrome/browser/chromeos/proxy_config_service_impl.h:98: // Wether this object is
associated to the signin profile.
ni: Whether

[pneubeck (no reviews), 2013-07-12 09:42:30]

PTAL

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/proxy_config_handler.cc (right):

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/proxy_config_handler.cc:71:
registry->RegisterBooleanPref(
On 2013/07/11 13:31:23, battre wrote:
> Can you please improve the comment at the definition of kUseSharedProxies? I
> find "A boolean pref that uses shared proxies." pretty useless.
> 1) A pref does not use proxies
> 2) Who shares the proxies?
> 3) What is the meaning of this pref on Desktop

3) doesn't apply as it's defined only for ChromeOS
Clarified the comment.

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
chrome/browser/chromeos/proxy_config_service_impl.cc:155:
use_shared_proxies_pref->GetValue()->GetAsBoolean(&use_shared_proxies);
On 2013/07/11 13:31:23, battre wrote:
> bool use_shared_proxies = profile_prefs->GetBoolean(prefs::kUseSharedProxies);

Done.

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
File chrome/browser/chromeos/proxy_config_service_impl.h (right):

https://codereview.chromium.org/18112018/diff/25001/chrome/browser/chromeos/p...
chrome/browser/chromeos/proxy_config_service_impl.h:98: // Wether this object is
associated to the signin profile.
On 2013/07/11 13:31:23, battre wrote:
> ni: Whether

Done.

[battre, 2013-07-12 12:25:44]

LGTM

https://codereview.chromium.org/18112018/diff/71001/chrome/browser/chromeos/u...
File chrome/browser/chromeos/ui_proxy_config_service.h (right):

https://codereview.chromium.org/18112018/diff/71001/chrome/browser/chromeos/u...
chrome/browser/chromeos/ui_proxy_config_service.h:31: void SetPrefs(bool
login_profile, PrefService* prefs);
add comment?
// |login_profile| indicates whether |prefs| are owned by a login profile.

https://codereview.chromium.org/18112018/diff/71001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/18112018/diff/71001/chrome/common/pref_names....
chrome/common/pref_names.cc:790: // settings or from device policy are applied.
nit: move "are applied" after "proxy settings"?

[pneubeck (no reviews), 2013-07-12 16:00:43]

https://codereview.chromium.org/18112018/diff/71001/chrome/browser/chromeos/u...
File chrome/browser/chromeos/ui_proxy_config_service.h (right):

https://codereview.chromium.org/18112018/diff/71001/chrome/browser/chromeos/u...
chrome/browser/chromeos/ui_proxy_config_service.h:31: void SetPrefs(bool
login_profile, PrefService* prefs);
On 2013/07/12 12:25:44, battre wrote:
> add comment?
> // |login_profile| indicates whether |prefs| are owned by a login profile.

Done. Also renamed login_profile to signin_screen.

https://codereview.chromium.org/18112018/diff/71001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/18112018/diff/71001/chrome/common/pref_names....
chrome/common/pref_names.cc:790: // settings or from device policy are applied.
On 2013/07/12 12:25:44, battre wrote:
> nit: move "are applied" after "proxy settings"?

No, that wasn't meant. I clarified the comment.

[Nikita (slow), 2013-07-15 08:08:46]

lgtm

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/chromeos/u...
File chrome/browser/chromeos/ui_proxy_config_service.h (right):

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/chromeos/u...
chrome/browser/chromeos/ui_proxy_config_service.h:33: void SetPrefs(bool
signin_screen, PrefService* prefs);
nit: It would be helpful to describe how specifically |signin_screen|  parameter
changes behavior.

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/net/proxy_...
File chrome/browser/net/proxy_service_factory.h (right):

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/net/proxy_...
chrome/browser/net/proxy_service_factory.h:43: static PrefProxyConfigTracker*
CreatePrefProxyConfigTrackerOfProfile(
nit: Please add comments to these methods (or extend previous comment).

[pneubeck (no reviews), 2013-07-16 08:38:05]

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/chromeos/u...
File chrome/browser/chromeos/ui_proxy_config_service.h (right):

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/chromeos/u...
chrome/browser/chromeos/ui_proxy_config_service.h:33: void SetPrefs(bool
signin_screen, PrefService* prefs);
On 2013/07/15 08:08:46, Nikita Kostylev wrote:
> nit: It would be helpful to describe how specifically |signin_screen| 
parameter
> changes behavior.

Done.

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/net/proxy_...
File chrome/browser/net/proxy_service_factory.h (right):

https://codereview.chromium.org/18112018/diff/73004/chrome/browser/net/proxy_...
chrome/browser/net/proxy_service_factory.h:43: static PrefProxyConfigTracker*
CreatePrefProxyConfigTrackerOfProfile(
On 2013/07/15 08:08:46, Nikita Kostylev wrote:
> nit: Please add comments to these methods (or extend previous comment).

Done.

[commit-bot: I haz the power, 2013-07-16 08:43:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/18112018/84001

[commit-bot: I haz the power, 2013-07-16 12:32:06]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-07-16 12:37:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/18112018/84001

[commit-bot: I haz the power, 2013-07-16 13:50:43]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-07-16 13:51:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/18112018/84001

[commit-bot: I haz the power, 2013-07-16 13:52:49]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 500: 
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>If
the problem persists, please <A
HREF="http://code.google.com/appengine/community.html">report</A> your problem
and mention this error message and the query that caused it.</h2>
<h2></h2>
</body></html>

[commit-bot: I haz the power, 2013-07-16 13:53:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/18112018/84001

[commit-bot: I haz the power, 2013-07-16 13:54:12]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 500: 
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>If
the problem persists, please <A
HREF="http://code.google.com/appengine/community.html">report</A> your problem
and mention this error message and the query that caused it.</h2>
<h2></h2>
</body></html>

[pneubeck (no reviews), 2013-07-17 05:25:50]

Committed patchset #8 manually as r211954 (presubmit successful).