Migrate ProxyConfigServiceImpl to NetworkStateHandler and NetworkProfileHandler.

chromeos/network/managed_state.h

Index: chromeos/network/managed_state.h
diff --git a/chromeos/network/managed_state.h b/chromeos/network/managed_state.h
index 5dcaff2818a76cf50b38c2240e1dfa72cd3c8fe4..fb33993aa329714ca49f3a30b51e684b0fa32d81 100644
--- a/chromeos/network/managed_state.h
+++ b/chromeos/network/managed_state.h
@@ -40,9 +40,12 @@ class ManagedState {
   NetworkState* AsNetworkState();
   DeviceState* AsDeviceState();
 
-  // Called by NetworkStateHandler when a property changes. Returns false if
-  // the property was not recognized, was not parsed successfully, or is
-  // unchanged (complex properties may be assumed to have changed).
+  // Called by NetworkStateHandler when a property was received. The return

[stevenjb, 2013/05/15 17:44:14]

I think that the original 'Returns false if' clause was more precise, how about:

Called by NetworkStateHandler when Shill indicates that a property has changed.
Returns false if the property value is unchanged for any of the following
reasons:
* The property was not recognized.
* The property was not parsed successfully.
* The |value| is equal to the existing property value; this avoids unnecessary
callbacks and logging. (Note: complex properties may not be compared, so this
may return true even though the value did not actually change).

[pneubeck (no reviews), 2013/05/15 20:24:48]

Please double check my change to the return statements in the .cc file at first.

Maybe, you swapped the direction of the implication by mistake.
What I want to ensure with the comment is:

If the state changed then all observers are notified. That is the central
responsibility of NetworkStateHandler.

IMO, the details that you mention are unimportant to the one caller of this
function, which is only interested in state changes to trigger notifications; it
cannot even differentiate the parse errors.

Please, check the adapted comment which should cover all suggestions :-)

+  // value only guarantees: If false then the state wasn't modified (e.g. if the
+  // property wasn't parsed successfully).
+  // I.e. if it returns true, the state might be modified or not.
+  // For reducing the number of notifications, false positives should be
+  // avoided.
   virtual bool PropertyChanged(const std::string& key,
                                const base::Value& value) = 0;