MD Settings: Internet: Allow Shared Proxies

MD Settings: Internet: Allow Shared Proxies

Adds the 'Allow proxies for shared networks' setting to the
proxy section in the internet detail page.

This setting affects all shared networks, so we need to show
a confirmation dialog informing the user when the setting is
changed.

(In the previois Options UI, this was exposed at the top level,
which is both confusing since there is no context, and not a
good use of top level space)

BUG=656184
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/566a686b839d4b7653e899e5bba04c3ce87d9b82
Cr-Commit-Position: refs/heads/master@{#432929}

[stevenjb, 2016-11-11 00:35:47]

Description was changed from

==========
MD Settings: Internet: Allow Shared Proxies

Adds the 'Allow proxies for shared networks' setting to the
proxy section in the internet detail page.

This setting affects all shared networks, so we need to show
a confirmation dialog informing the user when the setting is
changed.

(In the previois Options UI, this was exposed at the top level,
which is both confusing since there is no context, and not a
good use of top level space)

BUG=656184
==========

to

==========
MD Settings: Internet: Allow Shared Proxies

Adds the 'Allow proxies for shared networks' setting to the
proxy section in the internet detail page.

This setting affects all shared networks, so we need to show
a confirmation dialog informing the user when the setting is
changed.

(In the previois Options UI, this was exposed at the top level,
which is both confusing since there is no context, and not a
good use of top level space)

BUG=656184
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2016-11-11 00:40:19]

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

[stevenjb, 2016-11-11 00:40:20]

Hopefully the last of the proxy related CLs...

[michaelpg, 2016-11-15 22:22:04]

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:1040: <message
name="IDS_SETTINGS_INTERNET_NETWORK_PROXY_ALLOW_SHARED" desc="Settings >
Internet > Netwok details: Text next to the checkbox for allowing proxy settings
for shared networks.">
Same (Netwok => Network)

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:1043: <message
name="IDS_SETTINGS_INTERNET_NETWORK_PROXY_ALLOW_SHARED_WARNING_TITLE"
desc="Settings > Internet > Netwok details: Title for warning dialog when
changing the alow shared proxies setting.">
alow => allow

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:1046: <message
name="IDS_SETTINGS_INTERNET_NETWORK_PROXY_ALLOW_SHARED_WARNING_MESSAGE"
desc="Settings > Internet > Netwok details: Warning message when changing the
alow shared proxies setting.">
same

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js
(right):

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js:45:
reflectToAttribute: true,
Unnecessary unless you're styling based on this or something.

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.js (right):

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:98:
'useSharedProxiesChanged_(prefs.*)',
use "(prefs.settings.use_shared_proxies.value)", otherwise this will run every
time any pref changes...

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:210:
this.useSharedProxies_ = !!pref && !!pref.value;
You can assume pref.value is a (defined) boolean if pref is truthy (we assert
prefs in the prefs_util.cc whitelist exist, I think).

!!pref && pref.value

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:331: return /**
@type {!CrOnc.ManagedProperty|undefined}*/ (
nit: consistent spacing: add space before comment-closing asterisk

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:435: 
nit: rm blank line

[stevenjb, 2016-11-17 01:43:28]

PTAL

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:1040: <message
name="IDS_SETTINGS_INTERNET_NETWORK_PROXY_ALLOW_SHARED" desc="Settings >
Internet > Netwok details: Text next to the checkbox for allowing proxy settings
for shared networks.">
On 2016/11/15 22:22:04, michaelpg wrote:
> Same (Netwok => Network)

Done.

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:1043: <message
name="IDS_SETTINGS_INTERNET_NETWORK_PROXY_ALLOW_SHARED_WARNING_TITLE"
desc="Settings > Internet > Netwok details: Title for warning dialog when
changing the alow shared proxies setting.">
On 2016/11/15 22:22:04, michaelpg wrote:
> alow => allow

Done.

https://codereview.chromium.org/2499483002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:1046: <message
name="IDS_SETTINGS_INTERNET_NETWORK_PROXY_ALLOW_SHARED_WARNING_MESSAGE"
desc="Settings > Internet > Netwok details: Warning message when changing the
alow shared proxies setting.">
On 2016/11/15 22:22:04, michaelpg wrote:
> same

Done.

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js
(right):

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js:45:
reflectToAttribute: true,
On 2016/11/15 22:22:04, michaelpg wrote:
> Unnecessary unless you're styling based on this or something.

Done.

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.js (right):

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:98:
'useSharedProxiesChanged_(prefs.*)',
On 2016/11/15 22:22:04, michaelpg wrote:
> use "(prefs.settings.use_shared_proxies.value)", otherwise this will run every
> time any pref changes... 

Done.

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:210:
this.useSharedProxies_ = !!pref && !!pref.value;
On 2016/11/15 22:22:04, michaelpg wrote:
> You can assume pref.value is a (defined) boolean if pref is truthy (we assert
> prefs in the prefs_util.cc whitelist exist, I think).
> 
> !!pref && pref.value

Closure doesn't like that.

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:331: return /**
@type {!CrOnc.ManagedProperty|undefined}*/ (
On 2016/11/15 22:22:04, michaelpg wrote:
> nit: consistent spacing: add space before comment-closing asterisk

Done.

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:435: 
On 2016/11/15 22:22:04, michaelpg wrote:
> nit: rm blank line

Done.

[michaelpg, 2016-11-17 02:00:11]

ok, i think this LGTM!

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.js (right):

https://codereview.chromium.org/2499483002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:210:
this.useSharedProxies_ = !!pref && !!pref.value;
On 2016/11/17 01:43:28, stevenjb wrote:
> On 2016/11/15 22:22:04, michaelpg wrote:
> > You can assume pref.value is a (defined) boolean if pref is truthy (we
assert
> > prefs in the prefs_util.cc whitelist exist, I think).
> > 
> > !!pref && pref.value
> 
> Closure doesn't like that.

thanks, I always forget about closure (until I run it on my own code!)

[stevenjb, 2016-11-17 03:30:30]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2016-11-17 03:32:53]

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, 2016-11-17 04:11:48]

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

[commit-bot: I haz the power, 2016-11-17 04:11:49]

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

[stevenjb, 2016-11-17 17:41:17]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-11-17 17:41:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2499483002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-11-17 17:41:47]

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, 2016-11-17 19:08:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-17 20:28:28]

Description was changed from

==========
MD Settings: Internet: Allow Shared Proxies

Adds the 'Allow proxies for shared networks' setting to the
proxy section in the internet detail page.

This setting affects all shared networks, so we need to show
a confirmation dialog informing the user when the setting is
changed.

(In the previois Options UI, this was exposed at the top level,
which is both confusing since there is no context, and not a
good use of top level space)

BUG=656184
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Internet: Allow Shared Proxies

Adds the 'Allow proxies for shared networks' setting to the
proxy section in the internet detail page.

This setting affects all shared networks, so we need to show
a confirmation dialog informing the user when the setting is
changed.

(In the previois Options UI, this was exposed at the top level,
which is both confusing since there is no context, and not a
good use of top level space)

BUG=656184
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/566a686b839d4b7653e899e5bba04c3ce87d9b82
Cr-Commit-Position: refs/heads/master@{#432929}
==========

[commit-bot: I haz the power, 2016-11-17 20:28:29]

Patchset 3 (id:??) landed as
https://crrev.com/566a686b839d4b7653e899e5bba04c3ce87d9b82
Cr-Commit-Position: refs/heads/master@{#432929}