[CrOS Tether] Invoke "network properties updated" observer function when Tether network properties …

chromeos/network/network_state_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/network_state_handler.h"
 
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 543 matching lines @@
   if (!tether_network_state) {
     NET_LOG(ERROR) << "UpdateTetherNetworkProperties(): No NetworkState for "
                    << "Tether network with GUID \"" << guid << "\".";
     return false;
   }
 
   tether_network_state->set_carrier(carrier);
   tether_network_state->set_battery_percentage(battery_percentage);
   tether_network_state->set_signal_strength(signal_strength);
 
-  NotifyNetworkListChanged();
+  NotifyNetworkPropertiesUpdated(tether_network_state);
   return true;
 }
 
 bool NetworkStateHandler::SetTetherNetworkHasConnectedToHost(
     const std::string& guid) {
   NetworkState* tether_network_state = GetModifiableNetworkStateFromGuid(guid);
   if (!tether_network_state) {
     NET_LOG(ERROR) << "SetTetherNetworkHasConnectedToHost(): No NetworkState "
                    << "for Tether network with GUID \"" << guid << "\".";
     return false;
   }
 
   if (tether_network_state->tether_has_connected_to_host()) {
     return false;
   }
 
   tether_network_state->set_tether_has_connected_to_host(true);
-  NotifyNetworkListChanged();
+
+  NotifyNetworkPropertiesUpdated(tether_network_state);
   return true;
 }
 
 bool NetworkStateHandler::RemoveTetherNetworkState(const std::string& guid) {
   for (auto iter = tether_network_list_.begin();
        iter != tether_network_list_.end(); ++iter) {
     if (iter->get()->AsNetworkState()->guid() == guid) {
       NetworkState* wifi_network = GetModifiableNetworkStateFromGuid(
           iter->get()->AsNetworkState()->tether_guid());
       if (wifi_network)
         wifi_network->set_tether_guid(std::string());
 
       tether_network_list_.erase(iter);
       NotifyNetworkListChanged();
 
       return true;
     }
   }
   return false;
 }
 
 bool NetworkStateHandler::DisassociateTetherNetworkStateFromWifiNetwork(
     const std::string& tether_network_guid) {
-  NetworkState* tether_network =
+  NetworkState* tether_network_state =
       GetModifiableNetworkStateFromGuid(tether_network_guid);
 
-  if (!tether_network) {
+  if (!tether_network_state) {
     NET_LOG(ERROR) << "DisassociateTetherNetworkStateWithWifiNetwork(): Tether "
                    << "network with ID " << tether_network_guid
                    << " not registered; could not remove association.";
     return false;
   }
 
-  std::string wifi_network_guid = tether_network->tether_guid();
-  NetworkState* wifi_network =
+  std::string wifi_network_guid = tether_network_state->tether_guid();
+  NetworkState* wifi_network_state =
       GetModifiableNetworkStateFromGuid(wifi_network_guid);
 
-  if (!wifi_network) {
+  if (!wifi_network_state) {
     NET_LOG(ERROR) << "DisassociateTetherNetworkStateWithWifiNetwork(): Wi-Fi "
                    << "network with ID " << wifi_network_guid
                    << " not registered; could not remove association.";
     return false;
   }
 
-  wifi_network->set_tether_guid(std::string());
-  tether_network->set_tether_guid(std::string());
+  const std::string old_wifi_network_tether_guid =
+      wifi_network_state->tether_guid();
+  const std::string old_tether_network_tether_guid =
+      tether_network_state->tether_guid();
 
-  NotifyNetworkListChanged();
+  wifi_network_state->set_tether_guid(std::string());
+  tether_network_state->set_tether_guid(std::string());
+
+  if (!old_wifi_network_tether_guid.empty())
+    NotifyNetworkPropertiesUpdated(wifi_network_state);
+  if (!old_tether_network_tether_guid.empty())
+    NotifyNetworkPropertiesUpdated(tether_network_state);

[stevenjb, 2017/06/09 15:39:31]

Can't all of this just be:

if (wifi_network_state->tether_guid().empty() &&
    tether_network_state->tether_guid()) {
  return;
}

NotifyNetworkPropertiesUpdated(wifi_network_state);
NotifyNetworkPropertiesUpdated(tether_network_state);

[Kyle Horimoto, 2017/06/09 17:18:34]

Done.

 
   return true;
 }
 
 bool NetworkStateHandler::AssociateTetherNetworkStateWithWifiNetwork(
     const std::string& tether_network_guid,
     const std::string& wifi_network_guid) {
   if (tether_technology_state_ != TECHNOLOGY_ENABLED) {
     NET_LOG(ERROR) << "AssociateTetherNetworkStateWithWifiNetwork() called "
                    << "when Tether networks are not enabled. Cannot "
                    << "associate.";
     return false;
   }
 
-  NetworkState* tether_network =
+  NetworkState* tether_network_state =
       GetModifiableNetworkStateFromGuid(tether_network_guid);
-  if (!tether_network) {
+  if (!tether_network_state) {
     NET_LOG(ERROR) << "Tether network does not exist: " << tether_network_guid;
     return false;
   }
-  if (!NetworkTypePattern::Tether().MatchesType(tether_network->type())) {
+  if (!NetworkTypePattern::Tether().MatchesType(tether_network_state->type())) {
     NET_LOG(ERROR) << "Network is not a Tether network: "
                    << tether_network_guid;
     return false;
   }
 
-  NetworkState* wifi_network =
+  NetworkState* wifi_network_state =
       GetModifiableNetworkStateFromGuid(wifi_network_guid);
-  if (!wifi_network) {
+  if (!wifi_network_state) {
     NET_LOG(ERROR) << "Wi-Fi Network does not exist: " << wifi_network_guid;
     return false;
   }
-  if (!NetworkTypePattern::WiFi().MatchesType(wifi_network->type())) {
+  if (!NetworkTypePattern::WiFi().MatchesType(wifi_network_state->type())) {
     NET_LOG(ERROR) << "Network is not a W-Fi network: " << wifi_network_guid;
     return false;
   }
 
-  tether_network->set_tether_guid(wifi_network_guid);
-  wifi_network->set_tether_guid(tether_network_guid);
-  NotifyNetworkListChanged();
+  const std::string old_wifi_network_tether_guid =
+      wifi_network_state->tether_guid();
+  const std::string old_tether_network_tether_guid =
+      tether_network_state->tether_guid();
+
+  tether_network_state->set_tether_guid(wifi_network_guid);
+  wifi_network_state->set_tether_guid(tether_network_guid);
+
+  if (old_wifi_network_tether_guid != tether_network_guid)
+    NotifyNetworkPropertiesUpdated(wifi_network_state);
+  if (old_tether_network_tether_guid != wifi_network_guid)
+    NotifyNetworkPropertiesUpdated(tether_network_state);

[stevenjb, 2017/06/09 15:39:31]

Same here? Alternately this logic could be moved to a common commented method
and shared (passing empty strings for the new guids in the above case).

[Kyle Horimoto, 2017/06/09 17:18:34]

Done, but I didn't create a shared function since I think it's a bit overkill.

+
   return true;
 }
 
 void NetworkStateHandler::SetTetherNetworkStateDisconnected(
     const std::string& guid) {
-  // TODO(khorimoto): Remove the Tether network as the default network, and
-  // send a connection status change.
+  // TODO(khorimoto): Remove the Tether network as the default network.
   SetTetherNetworkStateConnectionState(guid, shill::kStateDisconnect);
 }
 
 void NetworkStateHandler::SetTetherNetworkStateConnecting(
     const std::string& guid) {
-  // TODO(khorimoto): Set the Tether network as the default network, and send
-  // a connection status change.
+  // TODO(khorimoto): Set the Tether network as the default network.
   SetTetherNetworkStateConnectionState(guid, shill::kStateConfiguration);
 }
 
 void NetworkStateHandler::SetTetherNetworkStateConnected(
     const std::string& guid) {
   // Being connected implies that AssociateTetherNetworkStateWithWifiNetwork()
   // was already called, so ensure that the association is still intact.
   DCHECK(GetNetworkStateFromGuid(GetNetworkStateFromGuid(guid)->tether_guid())
              ->tether_guid() == guid);
 
-  // TODO(khorimoto): Send a connection status change.
   SetTetherNetworkStateConnectionState(guid, shill::kStateOnline);
 }
 
 void NetworkStateHandler::SetTetherNetworkStateConnectionState(
     const std::string& guid,
     const std::string& connection_state) {
-  NetworkState* tether_network = GetModifiableNetworkStateFromGuid(guid);
-  if (!tether_network) {
+  NetworkState* tether_network_state = GetModifiableNetworkStateFromGuid(guid);
+  if (!tether_network_state) {
     NET_LOG(ERROR) << "SetTetherNetworkStateConnectionState: Tether network "
                    << "not found: " << guid;
     return;
   }
 
-  DCHECK(NetworkTypePattern::Tether().MatchesType(tether_network->type()));
+  DCHECK(
+      NetworkTypePattern::Tether().MatchesType(tether_network_state->type()));
 
-  tether_network->set_connection_state(connection_state);
-  NotifyNetworkListChanged();
+  std::string prev_connection_state = tether_network_state->connection_state();
+  tether_network_state->set_connection_state(connection_state);
+  DCHECK(!tether_network_state->is_captive_portal());
+
+  if (ConnectionStateChanged(tether_network_state, prev_connection_state,
+                             false /* prev_is_captive_portal */)) {
+    NET_LOG(EVENT) << "Changing connection state for Tether network with GUID "
+                   << guid << ". Old state: " << prev_connection_state << ", "
+                   << "New state: " << connection_state;
+
+    OnNetworkConnectionStateChanged(tether_network_state);
+    NotifyNetworkPropertiesUpdated(tether_network_state);
+  }
 }
 
 void NetworkStateHandler::EnsureTetherDeviceState() {
   bool should_be_present =
       tether_technology_state_ != TechnologyState::TECHNOLOGY_UNAVAILABLE;
 
   for (auto it = device_list_.begin(); it < device_list_.end(); ++it) {
     std::string path = (*it)->path();
     if (path == kTetherDevicePath) {
       // If the Tether DeviceState is in the list and it should not be, remove
@@ skipping 759 matching lines @@
   if (type.MatchesType(shill::kTypeVPN))
     technologies.emplace_back(shill::kTypeVPN);
   if (type.MatchesType(kTypeTether))
     technologies.emplace_back(kTypeTether);
 
   CHECK_GT(technologies.size(), 0ul);
   return technologies;
 }
 
 }  // namespace chromeos