Improve NetworkStateHandler API

chromeos/network/network_state_handler_observer.h

Index: chromeos/network/network_state_handler_observer.h
diff --git a/chromeos/network/network_state_handler_observer.h b/chromeos/network/network_state_handler_observer.h
index 07a4a7723413ce86fef66d5b5394b819f8656e53..c81759302c8cb1c7fed609ca0354be6a152e3955 100644
--- a/chromeos/network/network_state_handler_observer.h
+++ b/chromeos/network/network_state_handler_observer.h
@@ -34,17 +34,17 @@ class CHROMEOS_EXPORT NetworkStateHandlerObserver {
   // of devices, so they are not passed in the method.
   virtual void DeviceListChanged();
 
-  // The active network changed. |network| will be NULL if there is no longer
-  // an active network.
-  virtual void ActiveNetworkChanged(const NetworkState* network);
+  // The default network changed (includes VPNs) or its state changed.

[pneubeck (no reviews), 2012/12/19 15:22:38]

Aahh! I think we have terminology issue here. Please distinguish between state
(like NetworkState) and something like connection state (like
NetworkState::state_).

[stevenjb, 2012/12/19 18:01:07]

That's a good observation. Will replace "state" with "connection state" (which
is mostly accurate) throughout.

+  // |network| will be NULL if there is no longer a default network.
+  virtual void DefaultNetworkChanged(const NetworkState* network);
 
-  // The state of the active network changed.
-  virtual void ActiveNetworkStateChanged(const NetworkState* network);
+  // The state of |network| changed.
+  virtual void NetworkStateChanged(const NetworkState* network);
 
-  // One or more network service properties changed. Note: for the active
-  // network, this will be called in *addition* to ActiveNetworkStateChanged()
-  // if the state property changed.
-  virtual void NetworkServiceChanged(const NetworkState* network);
+  // One or more non-state network service properties changed. Note: this will

[pneubeck (no reviews), 2012/12/19 15:22:38]

missing reference to |network|?

This is actually not restricted to non-state, but triggers if any member of
NetworkState (including |state_|) changes. Please fix the comment or
implementation.

"Use this to ..." should perhaps be before the "Note: ".

"...when the _state_ property changes..." seems to contradict "One or more
_non-state_ properties changed".

[stevenjb, 2012/12/19 18:01:07]

Fixed and clarified.

+  // get called in *addition* to NetworkStateChanged() when the state property
+  // changes. Use this to track non-state properties like wifi strength.
+  virtual void NetworkPropertyChanged(const NetworkState* network);
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkStateHandlerObserver);