Cleanup network type matching.

Cleanup network type matching.

Before, both concrete network types (like Wimax) and type patterns (like Mobile) were represented as strings. In some cases, a variable could be both a concrete type or a pattern and was compared to another pattern.
This is hard to read and operands of the comparison can easily be swapped by mistake.
Implementing the comparison of patterns didn't scale.

This change adds a new class that encapsulates network type patterns and represents them internally as a more scalable bit vector.
It's now explicit which functions accept a type pattern as an argument.

This also adds support for Shill's network type kEthernetEap.

BUG=126870
(API change, used in chrome/browser/ui/webui/{chromeos,help})
TBR=xiyuan@chromium.org

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

[stevenjb, 2013-08-28 15:51:29]

https://codereview.chromium.org/23712002/diff/1/chrome/browser/chromeos/prere...
File chrome/browser/chromeos/prerender_condition_network.cc (right):

https://codereview.chromium.org/23712002/diff/1/chrome/browser/chromeos/prere...
chrome/browser/chromeos/prerender_condition_network.cc:25: if
(default_network->IsTypeEthernet() || type == flimflam::kTypeWifi)
I think this should probably be !default_network->IsTypeMobile()

https://codereview.chromium.org/23712002/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/dial/dial_service.cc (right):

https://codereview.chromium.org/23712002/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/dial/dial_service.cc:194: for (uint i = 0; i <
arraysize(connection_types); ++i) {
I find this confusing. Can we move the logic below to a helper method and call
it explicitly for Wifi and Wired? It also seems strange to me that we would
check wifi before wired, do you know why that is, and if so can you add a
comment? If not, maybe investigate?

https://codereview.chromium.org/23712002/diff/1/chrome/browser/ui/webui/help/...
File chrome/browser/ui/webui/help/help_utils_chromeos.cc (right):

https://codereview.chromium.org/23712002/diff/1/chrome/browser/ui/webui/help/...
chrome/browser/ui/webui/help/help_utils_chromeos.cc:50: if (type ==
flimflam::kTypeEthernet || type == shill::kTypeEthernetEap)
This is where having a method like shill_property_utill::IsTypeWired(type) would
be helpful.

ash::network_util would be a good namespace / location for this and other helper
methods (maybe in a follow-up CL).

https://codereview.chromium.org/23712002/diff/1/chromeos/network/managed_state.h
File chromeos/network/managed_state.h (right):

https://codereview.chromium.org/23712002/diff/1/chromeos/network/managed_stat...
chromeos/network/managed_state.h:78: bool IsTypeEthernet() const;
This should really be IsTypeWired() and we should add Wireless and Mobile also,
or just use shill_property_util helper methods directly.

Now that I think about it, maybe TypeEthernet is better than TypeWired, but we
should be consistent one way or the other.

https://codereview.chromium.org/23712002/diff/1/chromeos/network/network_stat...
File chromeos/network/network_state_handler.cc (right):

https://codereview.chromium.org/23712002/diff/1/chromeos/network/network_stat...
chromeos/network/network_state_handler.cc:736: if (type == kMatchTypeWired ||
type == shill::kTypeEthernetEap)
Another place where IsTypeWired(type) would be helpful.

[pneubeck (no reviews), 2013-09-02 20:41:21]

PTAL
I changed the kMatchType* constants to a new NetworkTypePattern class, in
particular to avoid functions like:
  Matches(const std::string& type, const std::string& pattern)
where type can be both a concrete type or a kMatchType* constant.

It's to easy to swap the arguments by mistake.

https://codereview.chromium.org/23712002/diff/1/chrome/browser/chromeos/prere...
File chrome/browser/chromeos/prerender_condition_network.cc (right):

https://codereview.chromium.org/23712002/diff/1/chrome/browser/chromeos/prere...
chrome/browser/chromeos/prerender_condition_network.cc:25: if
(default_network->IsTypeEthernet() || type == flimflam::kTypeWifi)
On 2013/08/28 15:51:30, stevenjb (chromium) wrote:
> I think this should probably be !default_network->IsTypeMobile()

Done.

https://codereview.chromium.org/23712002/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/dial/dial_service.cc (right):

https://codereview.chromium.org/23712002/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/dial/dial_service.cc:194: for (uint i = 0; i <
arraysize(connection_types); ++i) {
On 2013/08/28 15:51:30, stevenjb (chromium) wrote:
> I find this confusing. Can we move the logic below to a helper method and call
> it explicitly for Wifi and Wired? It also seems strange to me that we would
> check wifi before wired, do you know why that is, and if so can you add a
> comment? If not, maybe investigate?
> 

I will add the author as reviewer.

https://codereview.chromium.org/23712002/diff/1/chrome/browser/ui/webui/help/...
File chrome/browser/ui/webui/help/help_utils_chromeos.cc (right):

https://codereview.chromium.org/23712002/diff/1/chrome/browser/ui/webui/help/...
chrome/browser/ui/webui/help/help_utils_chromeos.cc:50: if (type ==
flimflam::kTypeEthernet || type == shill::kTypeEthernetEap)
On 2013/08/28 15:51:30, stevenjb (chromium) wrote:
> This is where having a method like shill_property_utill::IsTypeWired(type)
would
> be helpful.
> 
> ash::network_util would be a good namespace / location for this and other
helper
> methods (maybe in a follow-up CL).

Done.

https://codereview.chromium.org/23712002/diff/1/chromeos/network/managed_state.h
File chromeos/network/managed_state.h (right):

https://codereview.chromium.org/23712002/diff/1/chromeos/network/managed_stat...
chromeos/network/managed_state.h:78: bool IsTypeEthernet() const;
On 2013/08/28 15:51:30, stevenjb (chromium) wrote:
> This should really be IsTypeWired() and we should add Wireless and Mobile
also,
> or just use shill_property_util helper methods directly.
> 
> Now that I think about it, maybe TypeEthernet is better than TypeWired, but we
> should be consistent one way or the other.

Done.

https://codereview.chromium.org/23712002/diff/1/chromeos/network/network_stat...
File chromeos/network/network_state_handler.cc (right):

https://codereview.chromium.org/23712002/diff/1/chromeos/network/network_stat...
chromeos/network/network_state_handler.cc:736: if (type == kMatchTypeWired ||
type == shill::kTypeEthernetEap)
On 2013/08/28 15:51:30, stevenjb (chromium) wrote:
> Another place where IsTypeWired(type) would be helpful.

Since the old |type| argument could be a pattern/MatchType itself and I want to
avoid that we mistakenly swap the order in match ('supertype contains subtype'
vs. 'subtype contains supertype'), I introduced this NetworkTypePattern.

This class makes the matches more explicit and the usage of MatchTypes in method
arguments type-safe (compared to everything being a string).

[pneubeck (no reviews), 2013-09-03 07:51:22]

@Justin, please take a look at dial_service.*
I changed the order of Wifi and Ethernet because that Ethernet is usually the
preferred connection at other places of ChromeOS. Is that correct in this case?

[stevenjb, 2013-09-03 16:22:43]

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
File ash/system/chromeos/network/network_connect.cc (right):

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
ash/system/chromeos/network/network_connect.cc:308: if
(technology.Matches(flimflam::kTypeCellular)) {
This should be technology.Matches(NetworkTypePattern::Mobile())

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
File ash/system/chromeos/network/network_icon.cc (right):

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
ash/system/chromeos/network/network_icon.cc:473: type ==
flimflam::kTypeCellular) {
We should use NetworkTypePattern consistently in each if() here, and this should
be Wireless.

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
File ash/system/chromeos/network/tray_network.cc (right):

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
ash/system/chromeos/network/tray_network.cc:227:
NetworkTypePattern::Primitive(flimflam::kTypeWifi));
NEtworkTypePattern::Wifi()?

https://codereview.chromium.org/23712002/diff/44001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/23712002/diff/44001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:235: kNetworkTypeBluetooth = 1 << 3,
We don't handle Bluetooth anywhere in the current Network UI code, and I don't
expect we will. Let's omit it at this point so that we don't make incorrect
assumptions (like whether or not Bluetooth would count as "Wireless").

https://codereview.chromium.org/23712002/diff/44001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:339: return (~pattern_ &
subpattern.pattern_) == 0;
I don't think this is the behavior I would expect. I would expect
Cellular.Matches(Mobile) to be true in addition to Mobile.Matches(Cellular). Do
we have a use case for the (confusing to me) asymetric behavior where we
wouldn't just use Equals()? It seems like this function should be
MatchesPattern() and Matches() should be MatchesType(), and they should just
return (pattern_ & other.pattern_).

[justinlin, 2013-09-03 17:15:57]

thanks for the cleanup! dial_service.cc lgtm

https://codereview.chromium.org/23712002/diff/44001/chrome/browser/extensions...
File chrome/browser/extensions/api/dial/dial_service.cc (right):

https://codereview.chromium.org/23712002/diff/44001/chrome/browser/extensions...
chrome/browser/extensions/api/dial/dial_service.cc:130: if
(bind_ip_address.size() != net::kIPv4AddressSize) {
I vaguely remember that this shouldn't be able to happen, but if it can, then
thanks.

https://codereview.chromium.org/23712002/diff/44001/chrome/browser/extensions...
chrome/browser/extensions/api/dial/dial_service.cc:138: }
\n

[pneubeck (no reviews), 2013-09-04 12:57:28]

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
File ash/system/chromeos/network/network_connect.cc (right):

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
ash/system/chromeos/network/network_connect.cc:308: if
(technology.Matches(flimflam::kTypeCellular)) {
On 2013/09/03 16:22:43, stevenjb (chromium) wrote:
> This should be technology.Matches(NetworkTypePattern::Mobile())

Done.

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
File ash/system/chromeos/network/network_icon.cc (right):

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
ash/system/chromeos/network/network_icon.cc:473: type ==
flimflam::kTypeCellular) {
On 2013/09/03 16:22:43, stevenjb (chromium) wrote:
> We should use NetworkTypePattern consistently in each if() here, and this
should
> be Wireless.

Adapted the other conditions here too.

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
File ash/system/chromeos/network/tray_network.cc (right):

https://codereview.chromium.org/23712002/diff/44001/ash/system/chromeos/netwo...
ash/system/chromeos/network/tray_network.cc:227:
NetworkTypePattern::Primitive(flimflam::kTypeWifi));
On 2013/09/03 16:22:43, stevenjb (chromium) wrote:
> NEtworkTypePattern::Wifi()?

Done.

https://codereview.chromium.org/23712002/diff/44001/chrome/browser/extensions...
File chrome/browser/extensions/api/dial/dial_service.cc (right):

https://codereview.chromium.org/23712002/diff/44001/chrome/browser/extensions...
chrome/browser/extensions/api/dial/dial_service.cc:138: }
On 2013/09/03 17:15:57, justinlin wrote:
> \n

Done.

https://codereview.chromium.org/23712002/diff/44001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/23712002/diff/44001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:235: kNetworkTypeBluetooth = 1 << 3,
On 2013/09/03 16:22:43, stevenjb (chromium) wrote:
> We don't handle Bluetooth anywhere in the current Network UI code, and I don't
> expect we will. Let's omit it at this point so that we don't make incorrect
> assumptions (like whether or not Bluetooth would count as "Wireless").

Done.

https://codereview.chromium.org/23712002/diff/44001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:339: return (~pattern_ &
subpattern.pattern_) == 0;
On 2013/09/03 16:22:43, stevenjb (chromium) wrote:
> I don't think this is the behavior I would expect. I would expect
> Cellular.Matches(Mobile) to be true in addition to Mobile.Matches(Cellular).
Do
> we have a use case for the (confusing to me) asymetric behavior where we
> wouldn't just use Equals()? It seems like this function should be
> MatchesPattern() and Matches() should be MatchesType(), and they should just
> return (pattern_ & other.pattern_).
> 

Done.

[stevenjb, 2013-09-04 20:40:38]

LGTM w/ minor comments

https://codereview.chromium.org/23712002/diff/80001/chrome/browser/extensions...
File chrome/browser/extensions/api/dial/dial_service.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chrome/browser/extensions...
chrome/browser/extensions/api/dial/dial_service.cc:124:
->ConnectedNetworkByType(type);
Is this from clang-format? It's kind of weird looking.

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/network_...
File chromeos/network/network_change_notifier_chromeos.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/network_...
chromeos/network/network_change_notifier_chromeos.cc:199: if (type ==
flimflam::kTypeEthernet || type == shill::kTypeEthernetEap)
We should probably use MatchType here, just to keep this logic in one place.

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util_unittest.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
chromeos/network/shill_property_util_unittest.cc:49:
EXPECT_TRUE(wireless_.MatchesType(flimflam::kTypeWifi));
We should check that wireless_ also matches Cellular and Wimax.

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
chromeos/network/shill_property_util_unittest.cc:61:
EXPECT_TRUE(MatchesPattern(wireless_, cellular_));
Maybe check symmetry too: (wireless_, mobile_), (cellular_, mobile_)

[pneubeck (no reviews), 2013-09-05 08:28:34]

The browser/interactive_ui tests failed because NetworkStateHandler propagated
NetworkState with empty type().

I'll fix that in a separate CL at first.

https://codereview.chromium.org/23712002/diff/80001/chrome/browser/extensions...
File chrome/browser/extensions/api/dial/dial_service.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chrome/browser/extensions...
chrome/browser/extensions/api/dial/dial_service.cc:124:
->ConnectedNetworkByType(type);
On 2013/09/04 20:40:39, stevenjb (chromium) wrote:
> Is this from clang-format? It's kind of weird looking.

yes. I changed it to two lines.

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/network_...
File chromeos/network/network_change_notifier_chromeos.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/network_...
chromeos/network/network_change_notifier_chromeos.cc:199: if (type ==
flimflam::kTypeEthernet || type == shill::kTypeEthernetEap)
On 2013/09/04 20:40:39, stevenjb (chromium) wrote:
> We should probably use MatchType here, just to keep this logic in one place.

Done.

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util_unittest.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
chromeos/network/shill_property_util_unittest.cc:49:
EXPECT_TRUE(wireless_.MatchesType(flimflam::kTypeWifi));
On 2013/09/04 20:40:39, stevenjb (chromium) wrote:
> We should check that wireless_ also matches Cellular and Wimax.

Done.

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
chromeos/network/shill_property_util_unittest.cc:61:
EXPECT_TRUE(MatchesPattern(wireless_, cellular_));
On 2013/09/04 20:40:39, stevenjb (chromium) wrote:
> Maybe check symmetry too: (wireless_, mobile_), (cellular_, mobile_)

That's what MatchesPattern of the test fixture does.

[stevenjb, 2013-09-05 17:17:22]

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util_unittest.cc (right):

https://codereview.chromium.org/23712002/diff/80001/chromeos/network/shill_pr...
chromeos/network/shill_property_util_unittest.cc:61:
EXPECT_TRUE(MatchesPattern(wireless_, cellular_));
On 2013/09/05 08:28:34, pneubeck wrote:
> On 2013/09/04 20:40:39, stevenjb (chromium) wrote:
> > Maybe check symmetry too: (wireless_, mobile_), (cellular_, mobile_)
> 
> That's what MatchesPattern of the test fixture does.

Oh, right, I missed that.

[commit-bot: I haz the power, 2013-09-09 09:53:45]

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

[commit-bot: I haz the power, 2013-09-09 09:53:58]

Failed to apply patch for chromeos/network/network_state_handler.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chromeos/network/network_state_handler.cc
  Hunk #5 FAILED at 143.
  Hunk #6 succeeded at 178 (offset -8 lines).
  Hunk #7 succeeded at 185 with fuzz 1 (offset -10 lines).
  Hunk #8 FAILED at 211.
  Hunk #9 succeeded at 233 (offset -12 lines).
  Hunk #10 succeeded at 247 (offset -12 lines).
  Hunk #11 FAILED at 272.
  Hunk #12 succeeded at 310 (offset -13 lines).
  Hunk #13 succeeded at 701 (offset -13 lines).
  3 out of 13 hunks FAILED -- saving rejects to file
chromeos/network/network_state_handler.cc.rej

Patch:       chromeos/network/network_state_handler.cc
Index: chromeos/network/network_state_handler.cc
diff --git a/chromeos/network/network_state_handler.cc
b/chromeos/network/network_state_handler.cc
index
3b12071fbfbc005c376549476824c13d196a0ab5..f85bd366bf0ded9e67cb7fd74bb8ef51a3d111f5
100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -19,40 +19,13 @@
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/network/shill_property_handler.h"
+#include "chromeos/network/shill_property_util.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 namespace {
 
-// Returns true if |network->type()| == |match_type|, or it matches one of the
-// following special match types:
-// * kMatchTypeDefault matches any network (i.e. the first instance)
-// * kMatchTypeNonVirtual matches non virtual networks
-// * kMatchTypeWireless matches wireless networks
-// * kMatchTypeMobile matches cellular or wimax networks
-bool ManagedStateMatchesType(const ManagedState* managed,
-                             const std::string& match_type) {
-  const std::string& type = managed->type();
-  if (match_type == NetworkStateHandler::kMatchTypeDefault)
-    return true;
-  if (match_type == type)
-    return true;
-  if (match_type == NetworkStateHandler::kMatchTypeNonVirtual &&
-      type != flimflam::kTypeVPN) {
-    return true;
-  }
-  if (match_type == NetworkStateHandler::kMatchTypeWireless &&
-      type != flimflam::kTypeEthernet && type != flimflam::kTypeVPN) {
-    return true;
-  }
-  if (match_type == NetworkStateHandler::kMatchTypeMobile &&
-      (type == flimflam::kTypeCellular || type == flimflam::kTypeWimax)) {
-    return true;
-  }
-  return false;
-}
-
 bool ConnectionStateChanged(NetworkState* network,
                             const std::string& prev_connection_state) {
   return (network->connection_state() != prev_connection_state) &&
@@ -82,10 +55,6 @@ std::string GetManagedStateLogName(const ManagedState* state)
{
 
 }  // namespace
 
-const char NetworkStateHandler::kMatchTypeDefault[] = "default";
-const char NetworkStateHandler::kMatchTypeWireless[] = "wireless";
-const char NetworkStateHandler::kMatchTypeMobile[] = "mobile";
-const char NetworkStateHandler::kMatchTypeNonVirtual[] = "non-virtual";
 const char NetworkStateHandler::kDefaultCheckPortalList[] =
     "ethernet,wifi,cellular";
 
@@ -136,7 +105,7 @@ void NetworkStateHandler::UpdateManagerProperties() {
 }
 
 NetworkStateHandler::TechnologyState NetworkStateHandler::GetTechnologyState(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   std::string technology = GetTechnologyForType(type);
   TechnologyState state;
   if (shill_property_handler_->IsTechnologyEnabled(technology))
@@ -149,12 +118,12 @@ NetworkStateHandler::TechnologyState
NetworkStateHandler::GetTechnologyState(
     state = TECHNOLOGY_AVAILABLE;
   else
     state = TECHNOLOGY_UNAVAILABLE;
-  VLOG(2) << "GetTechnologyState: " << type << " = " << state;
+  VLOG(2) << "GetTechnologyState: " << type.ToDebugString() << " = " << state;
   return state;
 }
 
 void NetworkStateHandler::SetTechnologyEnabled(
-    const std::string& type,
+    const NetworkTypePattern& type,
     bool enabled,
     const network_handler::ErrorCallback& error_callback) {
   std::string technology = GetTechnologyForType(type);
@@ -174,26 +143,27 @@ const DeviceState* NetworkStateHandler::GetDeviceState(
 }
 
 const DeviceState* NetworkStateHandler::GetDeviceStateByType(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   for (ManagedStateList::const_iterator iter = device_list_.begin();
        iter != device_list_.end(); ++iter) {
     ManagedState* device = *iter;
     if (!device->update_received())
       continue;
-    if (ManagedStateMatchesType(device, type))
+    if (device->Matches(type))
       return device->AsDeviceState();
   }
   return NULL;
 }
 
-bool NetworkStateHandler::GetScanningByType(const std::string& type) const {
+bool NetworkStateHandler::GetScanningByType(
+    const NetworkTypePattern& type) const {
   for (ManagedStateList::const_iterator iter = device_list_.begin();
        iter != device_list_.end(); ++iter) {
     const DeviceState* device = (*iter)->AsDeviceState();
     DCHECK(device);
     if (!device->update_received())
       continue;
-    if (ManagedStateMatchesType(device, type) && device->scanning())
+    if (device->Matches(type) && device->scanning())
       return true;
   }
   return false;
@@ -217,7 +187,7 @@ const NetworkState* NetworkStateHandler::DefaultNetwork()
const {
 }
 
 const NetworkState* NetworkStateHandler::ConnectedNetworkByType(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   for (ManagedStateList::const_iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
@@ -226,14 +196,14 @@ const NetworkState*
NetworkStateHandler::ConnectedNetworkByType(
       continue;
     if (!network->IsConnectedState())
       break;  // Connected networks are listed first.
-    if (ManagedStateMatchesType(network, type))
+    if (network->Matches(type))
       return network;
   }
   return NULL;
 }
 
 const NetworkState* NetworkStateHandler::ConnectingNetworkByType(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   for (ManagedStateList::const_iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
@@ -242,28 +212,28 @@ const NetworkState*
NetworkStateHandler::ConnectingNetworkByType(
       continue;
     if (!network->IsConnectingState())
       break;  // Connected and connecting networks are listed first.
-    if (ManagedStateMatchesType(network, type))
+    if (network->Matches(type))
       return network;
   }
   return NULL;
 }
 
 const NetworkState* NetworkStateHandler::FirstNetworkByType(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   for (ManagedStateList::const_iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
     DCHECK(network);
     if (!network->update_received())
       continue;
-    if (ManagedStateMatchesType(network, type))
+    if (network->Matches(type))
       return network;
   }
   return NULL;
 }
 
 std::string NetworkStateHandler::HardwareAddressForType(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   const NetworkState* network = ConnectedNetworkByType(type);
   if (!network)
     return std::string();
@@ -276,7 +246,7 @@ std::string NetworkStateHandler::HardwareAddressForType(
 }
 
 std::string NetworkStateHandler::FormattedHardwareAddressForType(
-    const std::string& type) const {
+    const NetworkTypePattern& type) const {
   std::string address = HardwareAddressForType(type);
   if (address.size() % 2 != 0)
     return address;
@@ -290,10 +260,10 @@ std::string
NetworkStateHandler::FormattedHardwareAddressForType(
 }
 
 void NetworkStateHandler::GetNetworkList(NetworkStateList* list) const {
-  GetNetworkListByType(kMatchTypeDefault, list);
+  GetNetworkListByType(NetworkTypePattern::Default(), list);
 }
 
-void NetworkStateHandler::GetNetworkListByType(const std::string& type,
+void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type,
                                                NetworkStateList* list) const {
   DCHECK(list);
   list->clear();
@@ -303,7 +273,7 @@ void NetworkStateHandler::GetNetworkListByType(const
std::string& type,
     DCHECK(network);
     if (!network->update_received())
       continue;
-    if (ManagedStateMatchesType(network, type))
+    if (network->Matches(type))
       list->push_back(network);
   }
 }
@@ -354,7 +324,7 @@ void NetworkStateHandler::RequestScan() const {
 void NetworkStateHandler::WaitForScan(const std::string& type,
                                       const base::Closure& callback) {
   scan_complete_callbacks_[type].push_back(callback);
-  if (!GetScanningByType(type))
+  if (!GetScanningByType(NetworkTypePattern::Primitive(type)))
     RequestScan();
 }
 
@@ -745,19 +715,27 @@ void NetworkStateHandler::ScanCompleted(const std::string&
type) {
 }
 
 std::string NetworkStateHandler::GetTechnologyForType(
-    const std::string& type) const {
-  if (type == kMatchTypeMobile) {
-    if (shill_property_handler_->IsTechnologyAvailable(flimflam::kTypeWimax))
-      return flimflam::kTypeWimax;
-    else
-      return flimflam::kTypeCellular;
-  }
-  if (type == kMatchTypeDefault || type == kMatchTypeNonVirtual ||
-      type == kMatchTypeWireless) {
-    NOTREACHED();
+    const NetworkTypePattern& type) const {
+  if (type.MatchesType(flimflam::kTypeEthernet))
+    return flimflam::kTypeEthernet;
+
+  if (type.MatchesType(flimflam::kTypeWifi))
     return flimflam::kTypeWifi;
+
+  if (type.Equals(NetworkTypePattern::Wimax()))
+      return flimflam::kTypeWimax;
+
+  // Prefer Wimax over Cellular only if it's available.
+  if (typ…
(message too large)

[commit-bot: I haz the power, 2013-09-11 07:59:40]

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

[commit-bot: I haz the power, 2013-09-11 08:06:30]

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

[commit-bot: I haz the power, 2013-09-11 09:12:07]

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

[commit-bot: I haz the power, 2013-09-11 09:40:02]

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

[commit-bot: I haz the power, 2013-09-11 12:43:57]

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

[commit-bot: I haz the power, 2013-09-11 16:48:25]

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

[commit-bot: I haz the power, 2013-09-11 20:38:06]

Change committed as 222620