tray: Show warning in network tray when VPN or proxy used.

tray: Show warning in network tray when VPN or proxy used.

Let the user know they may be monitored if a VPN or proxy is being used.

TEST=manual
BUG=646698
Review-Url: https://codereview.chromium.org/2836453003
Cr-Commit-Position: refs/heads/master@{#466442}
Committed: https://chromium.googlesource.com/chromium/src/+/ab3bee6aed8f51dab8a8980baae0de0223698bf1

[sammiequon, 2017-04-20 23:28:37]

Description was changed from

==========
tray: Show warning in network tray when VPN or proxy used.

Let the user know they may be monitored if a VPN or proxy is being used.

TEST=manual
BUG=646698
==========

to

==========
tray: Show warning in network tray when VPN or proxy used.

Let the user know they may be monitored if a VPN or proxy is being used.

TEST=manual
BUG=646698
==========

[sammiequon, 2017-04-20 23:28:37]

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

[sammiequon, 2017-04-20 23:28:53]

stevenjb@ - Please take a look. Thanks!

https://codereview.chromium.org/2836453003/diff/1/chrome/browser/ui/ash/netwo...
File chrome/browser/ui/ash/networking_config_delegate_chromeos.h (right):

https://codereview.chromium.org/2836453003/diff/1/chrome/browser/ui/ash/netwo...
chrome/browser/ui/ash/networking_config_delegate_chromeos.h:27: bool
HasDefaultNetworkProxyConfigured() override;
Steven, do you know where this should go? I was thinking of making a new
delegate but wonder if you knew of a good existing place.

[stevenjb, 2017-04-20 23:44:56]

https://codereview.chromium.org/2836453003/diff/1/ash/system/network/network_...
File ash/system/network/network_list.cc (right):

https://codereview.chromium.org/2836453003/diff/1/ash/system/network/network_...
ash/system/network/network_list.cc:506: if (!connection_warning_) {
Move this code to a helper:
connection_warning_ = CreateConnectionWarning();

https://codereview.chromium.org/2836453003/diff/1/chrome/browser/ui/ash/netwo...
File chrome/browser/ui/ash/networking_config_delegate_chromeos.cc (right):

https://codereview.chromium.org/2836453003/diff/1/chrome/browser/ui/ash/netwo...
chrome/browser/ui/ash/networking_config_delegate_chromeos.cc:56: return
NetworkStateInformer::HasDefaultNetworkProxyConfigured();
It looks like the code in
NetworkStateInformer::HasDefaultNetworkProxyConfigured() could be moved to
src/chromeos/network, maybe src/chromeos/network/network_util.cc ? Then we could
avoid using a delegate.

[sammiequon, 2017-04-21 00:05:32]

https://codereview.chromium.org/2836453003/diff/1/ash/system/network/network_...
File ash/system/network/network_list.cc (right):

https://codereview.chromium.org/2836453003/diff/1/ash/system/network/network_...
ash/system/network/network_list.cc:506: if (!connection_warning_) {
On 2017/04/20 23:44:55, stevenjb wrote:
> Move this code to a helper:
> connection_warning_ = CreateConnectionWarning();

Done.

https://codereview.chromium.org/2836453003/diff/1/chrome/browser/ui/ash/netwo...
File chrome/browser/ui/ash/networking_config_delegate_chromeos.cc (right):

https://codereview.chromium.org/2836453003/diff/1/chrome/browser/ui/ash/netwo...
chrome/browser/ui/ash/networking_config_delegate_chromeos.cc:56: return
NetworkStateInformer::HasDefaultNetworkProxyConfigured();
On 2017/04/20 23:44:55, stevenjb wrote:
> It looks like the code in
> NetworkStateInformer::HasDefaultNetworkProxyConfigured() could be moved to
> src/chromeos/network, maybe src/chromeos/network/network_util.cc ? Then we
could
> avoid using a delegate.

I think the problem is the g_browser_process in
NetworkStateInformer::HasDefaultNetworkProxyConfigured(). We can't get
chrome/browser code into ash/ or chromeos/ right?

[stevenjb, 2017-04-21 17:30:52]

https://codereview.chromium.org/2836453003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/network_state_informer.cc (right):

https://codereview.chromium.org/2836453003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/network_state_informer.cc:97: NULL,
g_browser_process->local_state(), *network, &onc_source);
Ugh, yeah, this is a pain. However, we deal with this already in
UIProxyConfigService.

So, you can move this to UIProxyConfigService, and use local_state_prefs_
instead of g_browser_process->local_state(), then access it through
NetworkHandler::Get()->ui_proxy_config_service()->HasDefaultNetworkProxyConfigured().

[stevenjb, 2017-04-21 18:38:25]

Nice, thanks! lgtm w/ 2 nits.

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
File chromeos/network/proxy/ui_proxy_config_service.cc (right):

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
chromeos/network/proxy/ui_proxy_config_service.cc:163:
proxy_config::GetProxyConfigForNetwork(NULL, local_state_prefs_, *network,
nullptr

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
File chromeos/network/proxy/ui_proxy_config_service.h (right):

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
chromeos/network/proxy/ui_proxy_config_service.h:53: bool
HasDefaultNetworkProxyConfigured();
Please document:

// Returns true if there is a default network and it has a proxy
// configuration with mode == MODE_FIXED_SERVERS.

[sammiequon, 2017-04-21 19:02:53]

Thanks!

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
File chromeos/network/proxy/ui_proxy_config_service.cc (right):

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
chromeos/network/proxy/ui_proxy_config_service.cc:163:
proxy_config::GetProxyConfigForNetwork(NULL, local_state_prefs_, *network,
On 2017/04/21 18:38:24, stevenjb wrote:
> nullptr

Done.

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
File chromeos/network/proxy/ui_proxy_config_service.h (right):

https://codereview.chromium.org/2836453003/diff/40001/chromeos/network/proxy/...
chromeos/network/proxy/ui_proxy_config_service.h:53: bool
HasDefaultNetworkProxyConfigured();
On 2017/04/21 18:38:25, stevenjb wrote:
> Please document:
> 
> // Returns true if there is a default network and it has a proxy
> // configuration with mode == MODE_FIXED_SERVERS.

Done.

[sammiequon, 2017-04-21 19:03:07]

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

[commit-bot: I haz the power, 2017-04-21 19:03:36]

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-04-21 19:41:24]

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

[commit-bot: I haz the power, 2017-04-21 19:41:24]

Dry run: This issue passed the CQ dry run.

[sammiequon, 2017-04-21 20:49:40]

The CQ bit was checked by sammiequon@chromium.org

[sammiequon, 2017-04-21 20:49:40]

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/2836453003/#ps60001
(title: "Fixed patch set 3 errors.")

[commit-bot: I haz the power, 2017-04-21 20:50:16]

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-04-21 20:57:05]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492807780624780,
"parent_rev": "7d8aac7b2e42174063748f68741f2a681df2e7b9", "commit_rev":
"ab3bee6aed8f51dab8a8980baae0de0223698bf1"}

[commit-bot: I haz the power, 2017-04-21 20:57:15]

Description was changed from

==========
tray: Show warning in network tray when VPN or proxy used.

Let the user know they may be monitored if a VPN or proxy is being used.

TEST=manual
BUG=646698
==========

to

==========
tray: Show warning in network tray when VPN or proxy used.

Let the user know they may be monitored if a VPN or proxy is being used.

TEST=manual
BUG=646698

Review-Url: https://codereview.chromium.org/2836453003
Cr-Commit-Position: refs/heads/master@{#466442}
Committed:
https://chromium.googlesource.com/chromium/src/+/ab3bee6aed8f51dab8a8980baae0...
==========

[commit-bot: I haz the power, 2017-04-21 20:57:16]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ab3bee6aed8f51dab8a8980baae0...