system tray ui change for AllowOnlyPolicyNetworksToConnect

system tray ui change for AllowOnlyPolicyNetworksToConnect

This commit grays out unmanaged WiFis and button to joining other WiFis
when AllowOnlyPolicyNetworksToConnect is true after user logged in, as
well as adds the tooltip to tell users why.

BUG=208378
Committed: https://crrev.com/59355922b037b0e94738980d2e8828f6ecf5c5f1
Cr-Commit-Position: refs/heads/master@{#361319}

[fqj, 2015-11-23 05:29:16]

Description was changed from

==========
system tray ui change for AllowOnlyPolicyNetworksToConnect

This commit grays out unmanaged WiFis and button to joining other WiFis
when AllowOnlyPolicyNetworksToConnect is true after user logged in, as
well as adds the tooltip to tell users why.

BUG=208378
==========

to

==========
system tray ui change for AllowOnlyPolicyNetworksToConnect

This commit grays out unmanaged WiFis and button to joining other WiFis
when AllowOnlyPolicyNetworksToConnect is true after user logged in, as
well as adds the tooltip to tell users why.

Depends: https://codereview.chromium.org/1470783002/

BUG=208378
==========

[fqj, 2015-11-23 05:29:31]

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

[fqj, 2015-11-23 05:29:32]

PTAL.

[fqj, 2015-11-23 08:48:20]

Patchset #2 (id:20001) has been deleted

[stevenjb, 2015-11-23 18:10:34]

Please move the ash/system/tray/hover_highlight_view changes to a separate CL
and create an issue for 'support tooltips in HoverHighlightView'.

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
File ash/system/chromeos/network/network_state_list_detailed_view.cc (right):

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
ash/system/chromeos/network/network_state_list_detailed_view.cc:468:
global_network_config &&
if (global_network_config) {
  ...
}

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
ash/system/chromeos/network/network_state_list_detailed_view.cc:471:
&policy_prohibites_unmanaged);
Let's wrap the above in an anonymous function:

bool policy_prohibites_unmanaged = get_policy_prohibites_unmanaged();

Also, include the login state check there.

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
File ui/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:100: &policy_prohibites_unmanaged);
Same here, let's use a helper function for this.

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:112: network->guid(),
network->profile_path()));
This too.

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:124:
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_PROHIBITED);
{}

[fqj, 2015-11-23 18:57:22]

Description was changed from

==========
system tray ui change for AllowOnlyPolicyNetworksToConnect

This commit grays out unmanaged WiFis and button to joining other WiFis
when AllowOnlyPolicyNetworksToConnect is true after user logged in, as
well as adds the tooltip to tell users why.

Depends: https://codereview.chromium.org/1470783002/

BUG=208378
==========

to

==========
system tray ui change for AllowOnlyPolicyNetworksToConnect

This commit grays out unmanaged WiFis and button to joining other WiFis
when AllowOnlyPolicyNetworksToConnect is true after user logged in, as
well as adds the tooltip to tell users why.

BUG=208378
==========

[fqj, 2015-11-23 19:21:47]

PTAL.

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
File ash/system/chromeos/network/network_state_list_detailed_view.cc (right):

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
ash/system/chromeos/network/network_state_list_detailed_view.cc:468:
global_network_config &&
On 2015/11/23 18:10:34, stevenjb wrote:
> if (global_network_config) {
>   ...
> }

Done.

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
ash/system/chromeos/network/network_state_list_detailed_view.cc:471:
&policy_prohibites_unmanaged);
On 2015/11/23 18:10:34, stevenjb wrote:
> Let's wrap the above in an anonymous function:
> 
> bool policy_prohibites_unmanaged = get_policy_prohibites_unmanaged();
> 
> Also, include the login state check there.

Done.

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
File ui/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:100: &policy_prohibites_unmanaged);
On 2015/11/23 18:10:34, stevenjb wrote:
> Same here, let's use a helper function for this.

Done.

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:112: network->guid(),
network->profile_path()));
On 2015/11/23 18:10:34, stevenjb wrote:
> This too.

Merged into one

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:124:
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_PROHIBITED);
On 2015/11/23 18:10:34, stevenjb wrote:
> {}

Done.

[stevenjb, 2015-11-23 20:39:32]

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
File ash/system/chromeos/network/network_state_list_detailed_view.cc (right):

https://codereview.chromium.org/1469733003/diff/40001/ash/system/chromeos/net...
ash/system/chromeos/network/network_state_list_detailed_view.cc:471:
&policy_prohibites_unmanaged);
On 2015/11/23 19:21:47, fqj wrote:
> On 2015/11/23 18:10:34, stevenjb wrote:
> > Let's wrap the above in an anonymous function:
> > 
> > bool policy_prohibites_unmanaged = get_policy_prohibites_unmanaged();
> > 
> > Also, include the login state check there.
> 
> Done.

Sorry, I think I confused you with "anonymous function". I meant "helper
function in an anonymous namespace", e.g. see CreateInfoBubbleLabel() at the top
of this file. The point is to make this (already pretty lengthy) function more
readable.

Also, it should be named PolicyProhibitsUnmanaged(). Sorry about the confusion.

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
File ui/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/1469733003/diff/40001/ui/chromeos/network/net...
ui/chromeos/network/network_list.cc:100: &policy_prohibites_unmanaged);
On 2015/11/23 19:21:47, fqj wrote:
> On 2015/11/23 18:10:34, stevenjb wrote:
> > Same here, let's use a helper function for this.
> 
> Done.

Same here:
namespace {

bool IsProhibitedByPolicy(...) { ... }

}  // namespace

[fqj, 2015-11-23 21:33:18]

Done PTAL.

[stevenjb, 2015-11-23 22:05:53]

lgtm w/ nits

https://codereview.chromium.org/1469733003/diff/120001/ash/system/chromeos/ne...
File ash/system/chromeos/network/network_state_list_detailed_view.cc (right):

https://codereview.chromium.org/1469733003/diff/120001/ash/system/chromeos/ne...
ash/system/chromeos/network/network_state_list_detailed_view.cc:478: bool
policy_prohibites_unmanaged = PolicyProhibitsUnmanaged();
s/prohibites/prohibits/
Also, this appears to be just used once, so inline PolicyProhibitsUnmanaged() (I
know my example used a local but I was just focused on the large chunk of logic
that shoud be moved :) )

https://codereview.chromium.org/1469733003/diff/120001/ui/chromeos/network/ne...
File ui/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/1469733003/diff/120001/ui/chromeos/network/ne...
ui/chromeos/network/network_list.cc:39:
NetworkHandler::Get()->managed_network_configuration_handler();
Move this just before it is used, after the early exits.

https://codereview.chromium.org/1469733003/diff/120001/ui/chromeos/network/ne...
ui/chromeos/network/network_list.cc:39:
NetworkHandler::Get()->managed_network_configuration_handler();
Move this just before it is used, after the early exits.

[fqj, 2015-11-23 22:11:20]

fqj@chromium.org changed reviewers:
+ sky@chromium.org

[fqj, 2015-11-23 22:11:20]

Done.

@sky: please review ash/BUILD.gn
Thanks

[sky, 2015-11-24 00:27:01]

LGTM

[fqj, 2015-11-24 10:51:58]

The CQ bit was checked by fqj@chromium.org

[fqj, 2015-11-24 10:51:58]

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/1469733003/#ps140001
(title: " ")

[commit-bot: I haz the power, 2015-11-24 10:52:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469733003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469733003/140001

[commit-bot: I haz the power, 2015-11-24 11:48:28]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-11-24 11:49:17]

Patchset 7 (id:??) landed as
https://crrev.com/59355922b037b0e94738980d2e8828f6ecf5c5f1
Cr-Commit-Position: refs/heads/master@{#361319}