Ignore ManagedState changes to existing state.

Changing state to an existing state is a no-op that produces event log 
noise and extra callbacks.

BUG=239652

Review URL: https://chromiumcodereview.appspot.com/15132004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@200229 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 38 insertions, 13 deletions

diff --git a/chromeos/network/managed_state.cc b/chromeos/network/managed_state.cc
index 5d9ffe3..a4b8de5 100644
--- a/chromeos/network/managed_state.cc
+++ b/chromeos/network/managed_state.cc
@@ -59,28 +59,43 @@ bool ManagedState::ManagedStatePropertyChanged(const std::string& key,
 bool ManagedState::GetBooleanValue(const std::string& key,
                                    const base::Value& value,
                                    bool* out_value) {
-  bool res = value.GetAsBoolean(out_value);
-  if (!res)
+  bool new_value;
+  if (!value.GetAsBoolean(&new_value)) {
     LOG(WARNING) << "Failed to parse boolean value for:" << key;
-  return res;
+    return false;
+  }
+  if (*out_value == new_value)
+    return false;
+  *out_value = new_value;
+  return true;
 }
 
 bool ManagedState::GetIntegerValue(const std::string& key,
                                    const base::Value& value,
                                    int* out_value) {
-  bool res = value.GetAsInteger(out_value);
-  if (!res)
+  int new_value;
+  if (!value.GetAsInteger(&new_value)) {
     LOG(WARNING) << "Failed to parse integer value for:" << key;
-  return res;
+    return false;
+  }
+  if (*out_value == new_value)
+    return false;
+  *out_value = new_value;
+  return true;
 }
 
 bool ManagedState::GetStringValue(const std::string& key,
                                   const base::Value& value,
                                   std::string* out_value) {
-  bool res = value.GetAsString(out_value);
-  if (!res)
+  std::string new_value;
+  if (!value.GetAsString(&new_value)) {
     LOG(WARNING) << "Failed to parse string value for:" << key;
-  return res;
+    return false;
+  }
+  if (*out_value == new_value)
+    return false;
+  *out_value = new_value;
+  return true;
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/managed_state.h b/chromeos/network/managed_state.h
index c7964a3..5dcaff2 100644
--- a/chromeos/network/managed_state.h
+++ b/chromeos/network/managed_state.h
@@ -31,7 +31,7 @@ class ManagedState {
 
   virtual ~ManagedState();
 
-  // This will construct and return a new instance of the approprate class
+  // This will construct and return a new instance of the appropriate class
   // based on |type|.
   static ManagedState* Create(ManagedType type, const std::string& path);
 
@@ -40,8 +40,9 @@ class ManagedState {
   NetworkState* AsNetworkState();
   DeviceState* AsDeviceState();
 
-  // Called by NetworkStateHandler when a property changes. Returns true if
-  // the property was recognized and parsed successfully.
+  // 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).
   virtual bool PropertyChanged(const std::string& key,
                                const base::Value& value) = 0;
 
@@ -64,7 +65,8 @@ class ManagedState {
   bool ManagedStatePropertyChanged(const std::string& key,
                                    const base::Value& value);
 
-  // Helper methods that log warnings and return false if parsing failed.
+  // Helper methods that log warnings and return true if parsing succeeded and
+  // the new value does not match the existing output value.
   bool GetBooleanValue(const std::string& key,
                        const base::Value& value,
                        bool* out_value);
diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc
index c5e7040..a987ba5 100644
--- a/chromeos/network/network_state_handler_unittest.cc
+++ b/chromeos/network/network_state_handler_unittest.cc
@@ -274,6 +274,14 @@ TEST_F(NetworkStateHandlerTest, ServicePropertyChanged) {
   EXPECT_EQ("TestSecurity",
             network_state_handler_->GetNetworkState(eth0)->security());
   EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(eth0));
+
+  // Changing a service to the exsiting value should not trigger an update.
+  DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
+      dbus::ObjectPath(eth0),
+      flimflam::kSecurityProperty, security_value,
+      base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(eth0));
 }
 
 TEST_F(NetworkStateHandlerTest, NetworkConnectionStateChanged) {