Migrate ProxyConfigServiceImpl to NetworkStateHandler and NetworkProfileHandler.

chromeos/network/managed_state.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_MANAGED_STATE_H_
 #define CHROMEOS_NETWORK_MANAGED_STATE_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 22 matching lines @@
 
   // This will construct and return a new instance of the appropriate class
   // based on |type|.
   static ManagedState* Create(ManagedType type, const std::string& path);
 
   // Returns the specific class pointer if this is the correct type, or
   // NULL if it is not.
   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;
 
   // Called by NetworkStateHandler after all calls to PropertyChanged for the
   // initial set of properties. Used to update state requiring multiple
   // parsed properties, e.g. name from hex_ssid in NetworkState.
   virtual void InitialPropertiesReceived();
 
   const ManagedType managed_type() const { return managed_type_; }
   const std::string& path() const { return path_; }
@@ skipping 37 matching lines @@
 
   // Tracks when the state is being observed.
   bool is_observed_;
 
   DISALLOW_COPY_AND_ASSIGN(ManagedState);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_NETWORK_MANAGED_STATE_H_