Improve NetworkStateHandler API

chromeos/network/network_state_handler.h

 // 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.
 
 #ifndef CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_H_
 #define CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 13 matching lines @@
 class Value;
 }
 
 namespace chromeos {
 
 class DeviceState;
 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
     : public internal::ShillPropertyHandler::Listener {
  public:
   typedef std::vector<ManagedState*> ManagedStateList;
   typedef std::vector<const NetworkState*> NetworkStateList;
 
   virtual ~NetworkStateHandler();
 
   // Sets the global instance. Must be called before any calls to Get().
   static void Initialize();
 
   // Destroys the global instance.
   static void Shutdown();
 
   // Gets the global instance. Initialize() must be called first.
   static NetworkStateHandler* Get();
 
   // Add/remove observers.
   void AddObserver(NetworkStateHandlerObserver* observer);
   void RemoveObserver(NetworkStateHandlerObserver* observer);
 
   // Returns true if |technology| is enabled / available.
   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,
       const network_handler::ErrorCallback& error_callback);
 
   // Finds and returns a device state by |device_path| or NULL if not found.
   const DeviceState* GetDeviceState(const std::string& device_path) const;
 
   // Finds and returns a device state by |type|. Returns NULL if not found.
   const DeviceState* GetDeviceStateByType(const std::string& type) const;
 
   // 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
   // matching |type|, or an empty string if none.
   std::string HardwareAddressForType(const std::string& type) const;
   // Same as above but in aa:bb format.
   std::string FormattedHardwareAddressForType(const std::string& type) const;
 
   // Sets |list| to contain the list of networks.  The returned list contains
   // a copy of NetworkState pointers which should not be stored or used beyond
@@ skipping 40 matching lines @@
 
   // Sets the IP Address for the network associated with |service_path|.
   virtual void UpdateNetworkServiceIPAddress(
       const std::string& service_path,
       const std::string& ip_address) OVERRIDE;
 
   // Sends NetworkManagerChanged() to observers.
   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;
 
   // Called in Initialize(). Called explicitly by tests after adding
   // test observers.
   void InitShillPropertyHandler();
 
  private:
   friend class NetworkStateHandlerTest;
   FRIEND_TEST_ALL_PREFIXES(NetworkStateHandlerTest, NetworkStateHandlerStub);
 
   // Non-const getters for managed entries. These are const so that they can
   // be called by Get[Network|Device]State, even though they return non-const
   // pointers.
   DeviceState* GetModifiableDeviceState(const std::string& device_path) const;
   NetworkState* GetModifiableNetworkState(
       const std::string& service_path) const;
   ManagedState* GetModifiableManagedState(const ManagedStateList* managed_list,
                                           const std::string& path) const;
 
   // 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_;
 
   // Observer list
   ObserverList<NetworkStateHandlerObserver> observers_;
 
   // Lists of managed states
   ManagedStateList network_list_;
   ManagedStateList device_list_;
 
   // Lists of available / enabled technologies
   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);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_H_