Improve NetworkStateHandler API

chromeos/network/network_state_handler_observer.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_OBSERVER_H_
 #define CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_OBSERVER_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 16 matching lines @@
   // Called when one or more network manager properties changes.
   virtual void NetworkManagerChanged();
 
   // The list of networks changed.
   virtual void NetworkListChanged(const NetworkStateList& networks);
 
   // The list of devices changed. Typically we don't care about the list
   // 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);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_OBSERVER_H_