Improve NetworkStateHandler API

chromeos/network/network_state_handler.h

Index: chromeos/network/network_state_handler.h
diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h
index 8c2d12573c62d13aa87ee8a15bcb0b3fbaddd134..7358bfe91a1e455877ca1ac5ba1b4ffd483b60e4 100644
--- a/chromeos/network/network_state_handler.h
+++ b/chromeos/network/network_state_handler.h
@@ -31,14 +31,14 @@ class NetworkState;
 class NetworkStateHandlerObserver;
 class NetworkStateHandlerTest;
 
-// Class for tracking the list of visible networks and their state.
+// Class for tracking the list of visible networks and their properties.
 //
-// This class maps essential state from the connection manager (Shill) for
-// each visible network. It is not used to change the state of services or
-// devices, only global (manager) state.
+// This class maps essential properties from the connection manager (Shill) for
+// each visible network. It is not used to change the properties of services or
+// devices, only global (manager) properties.
 //
-// All getters return the currently cached state. This class is expected to
-// keep states up to date by managing the appropriate Shill observers.
+// All getters return the currently cached properties. This class is expected to
+// keep properties up to date by managing the appropriate Shill observers.
 // It will invoke its own more specific observer methods when the specified
 // changes occur.
 class CHROMEOS_EXPORT NetworkStateHandler
@@ -66,8 +66,8 @@ class CHROMEOS_EXPORT NetworkStateHandler
   bool TechnologyAvailable(const std::string& technology) const;
   bool TechnologyEnabled(const std::string& technology) const;
 
-  // Asynchronously sets the enabled state for |technology|.
-  // Note: Modifes Manager state. Calls |error_callback| on failure.
+  // Asynchronously sets the enabled property for |technology|.
+  // Note: Modifies Manager state. Calls |error_callback| on failure.
   void SetTechnologyEnabled(
       const std::string& technology,
       bool enabled,
@@ -82,19 +82,20 @@ class CHROMEOS_EXPORT NetworkStateHandler
   // Finds and returns a network state by |service_path| or NULL if not found.
   // Note: NetworkState is frequently updated asynchronously, i.e. properties
   // are not always updated all at once. This will contain the most recent
-  // value for each state. To receive notifications when the state changes,
-  // observer this class and implement NetworkServiceChanged().
+  // value for each property. To receive notifications when a property changes,
+  // observer this class and implement NetworkPropertyChanged().

[pneubeck (no reviews), 2012/12/19 19:40:06]

nit: observer -> observe

[stevenjb, 2012/12/19 21:36:50]

Done.

   const NetworkState* GetNetworkState(const std::string& service_path) const;
 
-  // Returns the "active" network (first network in the list if connected),
-  // NULL if none.
-  const NetworkState* ActiveNetwork() const;
+  // Returns the default connected network (which includes VPNs) or NULL.
+  // This is equivalent to ConnectedNetworkByType(kMatchTypeDefault).
+  const NetworkState* DefaultNetwork() const;
 
-  // Returns the first connected network of type |type|, otherwise NULL.
+  // Returns the primary connected network of type |type|, otherwise NULL.
+  // |type| can be a type defined in service_constants.h, or the additional
+  // match types defined in NetworkState; see NetworkState::MatchesType().
   const NetworkState* ConnectedNetworkByType(const std::string& type) const;
 
-  // Returns the first connecting network of type |type|, otherwise NULL.
-  // An empty type will return any connecting non-ethernet network.
+  // Like ConnectedNetworkByType() but returns the connecting network or NULL.
   const NetworkState* ConnectingNetworkByType(const std::string& type) const;
 
   // Returns the hardware (MAC) address for the first connected network
@@ -155,9 +156,9 @@ class CHROMEOS_EXPORT NetworkStateHandler
   virtual void ManagerPropertyChanged() OVERRIDE;
 
   // Called by |shill_property_handler_| when the service or device list has
-  // changed and all entries have been updated. If |type| == TYPE_NETWORK,
-  // this notifies observers that the network list has changed, and if the
-  // active network has changed sends that notification also.
+  // changed and all entries have been updated. This updates the list and
+  // notifies observers. If |type| == TYPE_NETWORK this also calls
+  // CheckDefaultNetworkChanged().
   virtual void ManagedStateListChanged(
       ManagedState::ManagedType type) OVERRIDE;
 
@@ -181,11 +182,19 @@ class CHROMEOS_EXPORT NetworkStateHandler
   // Gets the list specified by |type|.
   ManagedStateList* GetManagedList(ManagedState::ManagedType type);
 
-  // Helper function called to parse |network| properties.
+  // Helper function called to parse |network| properties. Also calls
+  // OnNetworkConnectionStateChanged if the connection_state property changes.

[pneubeck (no reviews), 2012/12/19 19:40:06]

nit:
OnNetworkConnectionStateChanged -> OnNetworkConnectionStateChanged()

for consistency.

[stevenjb, 2012/12/19 21:36:50]

Done.

   bool ParseNetworkServiceProperty(NetworkState* network,
                                    const std::string& key,
                                    const base::Value& value);
 
+  // Helper function to notify observers. Calls CheckDefaultNetworkChanged().
+  bool OnNetworkConnectionStateChanged(NetworkState* network);
+
+  // Called whenever the default network may have changed. If it has changed,
+  // signals observers and returns true.
+  bool CheckDefaultNetworkChanged();
+
   // Shill property handler instance, owned by this class.
   scoped_ptr<internal::ShillPropertyHandler> shill_property_handler_;
 
@@ -200,8 +209,8 @@ class CHROMEOS_EXPORT NetworkStateHandler
   std::set<std::string> available_technologies_;
   std::set<std::string> enabled_technologies_;
 
-  // Keeps track of the active network for notifying observers when it changes.
-  std::string active_network_path_;
+  // Keeps track of the default network for notifying observers when it changes.
+  std::string default_network_path_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkStateHandler);
 };