Re-factor network_event_log

chromeos/network/managed_state.cc

 // 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.
 
 #include "chromeos/network/managed_state.h"
 
 #include "base/logging.h"
 #include "base/values.h"
 #include "chromeos/network/device_state.h"
+#include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 ManagedState::ManagedState(ManagedType type, const std::string& path)
     : managed_type_(type),
       path_(path),
       is_observed_(false) {
 }
 
 ManagedState::~ManagedState() {
 }
 
 ManagedState* ManagedState::Create(ManagedType type, const std::string& path) {
-  switch(type) {
+  switch (type) {
     case MANAGED_TYPE_NETWORK:
       return new NetworkState(path);
     case MANAGED_TYPE_DEVICE:
       return new DeviceState(path);
   }
   return NULL;
 }
 
 NetworkState* ManagedState::AsNetworkState() {
   if (managed_type() == MANAGED_TYPE_NETWORK)
@@ skipping 16 matching lines @@
     return GetStringValue(key, value, &name_);
   } else if (key == flimflam::kTypeProperty) {
     return GetStringValue(key, value, &type_);
   }
   return false;
 }
 
 bool ManagedState::GetBooleanValue(const std::string& key,
                                    const base::Value& value,
                                    bool* out_value) {
-  bool res = value.GetAsBoolean(out_value);
-  if (!res)
-    LOG(WARNING) << "Failed to parse boolean value for:" << key;
-  return res;
+  bool new_value;
+  if (!value.GetAsBoolean(&new_value)) {
+    NET_LOG_ERROR("Error parsing state value", path() + "." + key);
+    return false;
+  }
+  if (*out_value == new_value)

[pneubeck (no reviews), 2013/05/13 13:16:25]

Not related to the log changes.
Either update the issue description or split the CL. I'd prefer the latter.

[stevenjb, 2013/05/13 19:36:39]

It is related since the only place that really cares whether we set a value to
an existing one is the logging, to avoid spam.
Since this does affect the frequency of observer calls,
I'll go ahead and split it out and add a test.

+    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)
-    LOG(WARNING) << "Failed to parse integer value for:" << key;
-  return res;
+  int new_value;
+  if (!value.GetAsInteger(&new_value)) {
+    NET_LOG_ERROR("Error parsing state value", path() + "." + key);
+    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)
-    LOG(WARNING) << "Failed to parse string value for:" << key;
-  return res;
+  std::string new_value;
+  if (!value.GetAsString(&new_value)) {
+    NET_LOG_ERROR("Error parsing state value", path() + "." + key);
+    return false;
+  }
+  if (*out_value == new_value)
+    return false;
+  *out_value = new_value;
+  return true;
 }
 
 }  // namespace chromeos