chromeos: Refactor the code for showing the network UI.

chromeos: Refactor the code for showing the network UI.

This patch moves some code out of //ash into //chromeos so that it can be used
in other components (e.g. athena). NetworkListView is introduced for representing
the list of available networks to the user. It can query information through the
NetworkListDelegate. NetworkInfo is used to contain the necessary information
about a network.

BUG=387199
R=stevenjb@chromium.org

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

[stevenjb, 2014-07-24 19:13:54]

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
File ui/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
ui/chromeos/network/network_list.cc:166: if (!delegate_->ShouldListOnlyVPN()) {
I don't like embedding "OnlyVPN" in this class, it seems too implementation
specific.

Let's take the opportunity to wrap the cellular section and the wifi section
separately with something like delegate_->IncludeNetworkType, or better yet,
replace ShouldSkipNetwork() with ShouldShowNetworkType(type).

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
ui/chromeos/network/network_list.cc:214: if (delegate_->ShouldListOnlyVPN())
So, I think what we really want is delegate_->GetNetworkTypePatten(), so above
we can test
delegate_->GetNetworkTypePatten()->MatchesType(shill::kType[Wifi|Cellular]) and
here we can use delegate_->GetNetworkTypePatten() == NetworkTypePattern::VPN.

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
File ui/chromeos/network/network_list_delegate.h (right):

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
ui/chromeos/network/network_list_delegate.h:46: virtual bool
OnNetworkListIsEmpty() = 0;
The purpose of this is unclear and appears to be unused?

[sadrul, 2014-07-24 20:38:45]

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
File ui/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
ui/chromeos/network/network_list.cc:166: if (!delegate_->ShouldListOnlyVPN()) {
On 2014/07/24 19:13:54, stevenjb wrote:
> I don't like embedding "OnlyVPN" in this class, it seems too implementation
> specific.
> 
> Let's take the opportunity to wrap the cellular section and the wifi section
> separately with something like delegate_->IncludeNetworkType, or better yet,
> replace ShouldSkipNetwork() with ShouldShowNetworkType(type).

Done.

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
ui/chromeos/network/network_list.cc:214: if (delegate_->ShouldListOnlyVPN())
On 2014/07/24 19:13:54, stevenjb wrote:
> So, I think what we really want is delegate_->GetNetworkTypePatten(), so above
> we can test
> delegate_->GetNetworkTypePatten()->MatchesType(shill::kType[Wifi|Cellular])
and
> here we can use delegate_->GetNetworkTypePatten() == NetworkTypePattern::VPN.

Very nice! Done. I like this, this is much cleaner.

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
File ui/chromeos/network/network_list_delegate.h (right):

https://codereview.chromium.org/417793003/diff/60001/ui/chromeos/network/netw...
ui/chromeos/network/network_list_delegate.h:46: virtual bool
OnNetworkListIsEmpty() = 0;
On 2014/07/24 19:13:54, stevenjb wrote:
> The purpose of this is unclear and appears to be unused?

Good catch. Removed.

[stevenjb, 2014-07-24 21:07:08]

lgtm

[sadrul, 2014-07-24 23:47:22]

Committed patchset #5 manually as r285411 (presubmit successful).