Fix ConnectedNetworkByType() results during state transitions

If wifi and ethernet are both connected, and then the ethernet adapter
is unplugged, network_list_ can end up looking like:

    network_list_[0] -> ethernet, idle
    network_list_[1] -> wifi, online

shill will generally update the service's State property before it
updates the manager's Services / ServiceCompleteList properties,
leaving the now-idle service in the list.

Since ConnectedNetworkByType() assumes that connected networks are
listed first, it won't return the newly-active wifi network.  Fix
this by sorting the network list, and changing the sort function so
that it doesn't prioritize idle ethernet services.

BUG=768053
TEST=manually verify order with debug prints
TEST=unit tests

Change-Id: I3a110da7d99bd5e6463b86e17bc557124e875b85
Reviewed-on: https://chromium-review.googlesource.com/777970Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Kevin Cernekee <cernekee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517679}

chromeos/network/network_state_handler.cc

@@ -310,10 +310,11 @@ const NetworkState* NetworkStateHandler::DefaultNetwork() const {
 }
 
 const NetworkState* NetworkStateHandler::ConnectedNetworkByType(
-    const NetworkTypePattern& type) const {
-  const NetworkState* connected_network = nullptr;
+    const NetworkTypePattern& type) {
+  if (!network_list_sorted_)
+    SortNetworkList();  // Sort to ensure visible networks are listed first.
 
-  // Active networks are always listed first by Shill so no need to sort.
+  const NetworkState* connected_network = nullptr;
   for (auto iter = network_list_.begin(); iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
     DCHECK(network);
@@ -430,7 +431,7 @@ const NetworkState* NetworkStateHandler::FirstNetworkByType(
 }
 
 std::string NetworkStateHandler::FormattedHardwareAddressForType(
-    const NetworkTypePattern& type) const {
+    const NetworkTypePattern& type) {
   const NetworkState* network = ConnectedNetworkByType(type);
   if (network && network->type() == kTypeTether) {
     // If this is a Tether network, get the MAC address corresponding to that
@@ -1303,9 +1304,7 @@ void NetworkStateHandler::SortNetworkList() {
       cellular.push_back(std::move(*iter));
       continue;
     }
-    // Ethernet networks are always considered active.
-    if (network->IsConnectingOrConnected() ||
-        NetworkTypePattern::Ethernet().MatchesType(network->type())) {
+    if (network->IsConnectingOrConnected()) {
       active.push_back(std::move(*iter));
       continue;
     }

chromeos/network/network_state_handler.h

@@ -161,8 +161,7 @@ class CHROMEOS_EXPORT NetworkStateHandler
   const NetworkState* DefaultNetwork() const;
 
   // Returns the primary connected network of matching |type|, otherwise NULL.
-  const NetworkState* ConnectedNetworkByType(
-      const NetworkTypePattern& type) const;
+  const NetworkState* ConnectedNetworkByType(const NetworkTypePattern& type);
 
   // Like ConnectedNetworkByType() but returns a connecting network or NULL.
   const NetworkState* ConnectingNetworkByType(
@@ -175,8 +174,7 @@ class CHROMEOS_EXPORT NetworkStateHandler
 
   // Returns the aa:bb formatted hardware (MAC) address for the first connected
   // network matching |type|, or an empty string if none is connected.
-  std::string FormattedHardwareAddressForType(
-      const NetworkTypePattern& type) const;
+  std::string FormattedHardwareAddressForType(const NetworkTypePattern& type);
 
   // Convenience method to call GetNetworkListByType(visible=true).
   void GetVisibleNetworkListByType(const NetworkTypePattern& type,