Re-factor network_event_log

chromeos/network/network_state_handler.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/network_state_handler.h"
 
 #include "base/bind.h"
 #include "base/format_macros.h"
+#include "base/json/json_writer.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chromeos/network/device_state.h"
 #include "chromeos/network/managed_state.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/network/shill_property_handler.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace {
 
-const char kLogModule[] = "NetworkStateHandler";
-
 // Returns true if |network->type()| == |match_type|, or it matches one of the
 // following special match types:
 // * kMatchTypeDefault matches any network (i.e. the first instance)
 // * kMatchTypeNonVirtual matches non virtual networks
 // * kMatchTypeWireless matches wireless networks
 // * kMatchTypeMobile matches cellular or wimax networks
 bool ManagedStateMatchesType(const chromeos::ManagedState* managed,
                              const std::string& match_type) {
   const std::string& type = managed->type();
   if (match_type == chromeos::NetworkStateHandler::kMatchTypeDefault)
     return true;
   if (match_type == type)
     return true;
   if (match_type == chromeos::NetworkStateHandler::kMatchTypeNonVirtual &&
       type != flimflam::kTypeVPN) {
     return true;
   }
   if (match_type == chromeos::NetworkStateHandler::kMatchTypeWireless &&
       type != flimflam::kTypeEthernet && type != flimflam::kTypeVPN) {
     return true;
   }
   if (match_type == chromeos::NetworkStateHandler::kMatchTypeMobile &&
       (type == flimflam::kTypeCellular || type == flimflam::kTypeWimax)) {
     return true;
   }
   return false;
 }
 
-std::string ValueAsString(const base::Value& value) {
-  if (value.GetType() == base::Value::TYPE_BOOLEAN) {
-    bool bval = false;
-    value.GetAsBoolean(&bval);
-    return bval ? "true" : "false";
-  } else if (value.GetType() == base::Value::TYPE_INTEGER) {
-    int intval = 0;
-    value.GetAsInteger(&intval);
-    return base::StringPrintf("%d", intval);
-  } else if (value.GetType() == base::Value::TYPE_DOUBLE) {
-    double dval = 0;
-    value.GetAsDouble(&dval);
-    return base::StringPrintf("%g", dval);
-  } else if (value.GetType() == base::Value::TYPE_STRING) {
-    std::string vstr;
-    value.GetAsString(&vstr);
-    return vstr;
-  }
-  return "";
-}
-
 bool ConnectionStateChanged(chromeos::NetworkState* network,
                             const std::string& prev_connection_state) {
   return (network->connection_state() != prev_connection_state) &&
          (network->connection_state() != flimflam::kStateIdle ||
           !prev_connection_state.empty());
 }
 
+std::string GetManagedStateLogName(const chromeos::ManagedState* state) {
+  if (!state)
+    return "None";
+  return base::StringPrintf("%s (%s)", state->name().c_str(),
+                            state->path().c_str());
+}
+
 }  // namespace
 
 namespace chromeos {
 
 const char NetworkStateHandler::kMatchTypeDefault[] = "default";
 const char NetworkStateHandler::kMatchTypeWireless[] = "wireless";
 const char NetworkStateHandler::kMatchTypeMobile[] = "mobile";
 const char NetworkStateHandler::kMatchTypeNonVirtual[] = "non-virtual";
 
 static NetworkStateHandler* g_network_state_handler = NULL;
@@ skipping 62 matching lines @@
     state = TECHNOLOGY_UNAVAILABLE;
   VLOG(2) << "GetTechnologyState: " << type << " = " << state;
   return state;
 }
 
 void NetworkStateHandler::SetTechnologyEnabled(
     const std::string& type,
     bool enabled,
     const network_handler::ErrorCallback& error_callback) {
   std::string technology = GetTechnologyForType(type);
-  network_event_log::AddEntry(
-      kLogModule, "SetTechnologyEnabled",
-      base::StringPrintf("%s:%d", technology.c_str(), enabled));
+  NET_LOG_EVENT("SetTechnologyEnabled",
+                base::StringPrintf("%s:%d", technology.c_str(), enabled));
   shill_property_handler_->SetTechnologyEnabled(
       technology, enabled, error_callback);
-  ManagerPropertyChanged();  // Technology state changed -> ENABLING
+  // Signal Technology state changed -> ENABLING
+  NotifyManagerPropertyChanged();
 }
 
 const DeviceState* NetworkStateHandler::GetDeviceState(
     const std::string& device_path) const {
   return GetModifiableDeviceState(device_path);
 }
 
 const DeviceState* NetworkStateHandler::GetDeviceStateByType(
     const std::string& type) const {
   for (ManagedStateList::const_iterator iter = device_list_.begin();
@@ skipping 106 matching lines @@
   list->clear();
   for (ManagedStateList::const_iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
     DCHECK(network);
     list->push_back(network);
   }
 }
 
 void NetworkStateHandler::RequestScan() const {
-  network_event_log::AddEntry(kLogModule, "RequestScan", "");
+  NET_LOG_EVENT("RequestScan", "");
   shill_property_handler_->RequestScan();
 }
 
 void NetworkStateHandler::WaitForScan(const std::string& type,
                                       const base::Closure& callback) {
   scan_complete_callbacks_[type].push_back(callback);
   if (!GetScanningByType(type))
     RequestScan();
 }
 
 void NetworkStateHandler::ConnectToBestWifiNetwork() {
-  network_event_log::AddEntry(kLogModule, "ConnectToBestWifiNetwork", "");
+  NET_LOG_EVENT("ConnectToBestWifiNetwork", "");
   WaitForScan(flimflam::kTypeWifi,
               base::Bind(&internal::ShillPropertyHandler::ConnectToBestServices,
                          shill_property_handler_->AsWeakPtr()));
 }
 
 void NetworkStateHandler::SetConnectingNetwork(
     const std::string& service_path) {
   connecting_network_ = service_path;
-  network_event_log::AddEntry(
-      kLogModule, "SetConnectingNetwork", service_path);
+  const NetworkState* network = GetNetworkState(service_path);
+  if (network)
+    NET_LOG_EVENT("SetConnectingNetwork", GetManagedStateLogName(network));
+  else
+    NET_LOG_ERROR("SetConnectingNetwork to unknown network", service_path);
 }
 
 void NetworkStateHandler::GetNetworkStatePropertiesForTest(
     base::DictionaryValue* dictionary) const {
   for (ManagedStateList::const_iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     base::DictionaryValue* network_dict = new base::DictionaryValue;
     (*iter)->AsNetworkState()->GetProperties(network_dict);
     dictionary->SetWithoutPathExpansion((*iter)->path(), network_dict);
   }
@@ skipping 40 matching lines @@
     if (is_observing)
       managed->set_is_observed(true);
     if (request_properties)
       shill_property_handler_->RequestProperties(type, path);
   }
   // Delete any remaning entries in managed_map.
   STLDeleteContainerPairSecondPointers(managed_map.begin(), managed_map.end());
 }
 
 void NetworkStateHandler::ProfileListChanged() {
-  network_event_log::AddEntry(
-      kLogModule, "ProfileListChanged", "Re-Requesting Network Properties");
+  NET_LOG_EVENT("ProfileListChanged", "Re-Requesting Network Properties");
   for (ManagedStateList::iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     shill_property_handler_->RequestProperties(
         ManagedState::MANAGED_TYPE_NETWORK, (*iter)->path());
   }
 }
 
 void NetworkStateHandler::UpdateManagedStateProperties(
     ManagedState::ManagedType type,
     const std::string& path,
@@ skipping 10 matching lines @@
   for (base::DictionaryValue::Iterator iter(properties);
        !iter.IsAtEnd(); iter.Advance()) {
     if (type == ManagedState::MANAGED_TYPE_NETWORK) {
       if (managed->PropertyChanged(iter.key(), iter.value()))
         network_property_updated = true;
     } else {
       managed->PropertyChanged(iter.key(), iter.value());
     }
   }
   managed->InitialPropertiesReceived();
-  network_event_log::AddEntry(
-      kLogModule, "PropertiesReceived",
-      base::StringPrintf("%s (%s)", path.c_str(), managed->name().c_str()));
+  NET_LOG_DEBUG("PropertiesReceived", GetManagedStateLogName(managed));
   // Notify observers.
   if (network_property_updated) {
     NetworkState* network = managed->AsNetworkState();
     DCHECK(network);
     // Signal connection state changed after all properties have been updated.
     if (ConnectionStateChanged(network, prev_connection_state))
       OnNetworkConnectionStateChanged(network);
     NetworkPropertiesUpdated(network);
   }
 }
@@ skipping 13 matching lines @@
     if (ConnectionStateChanged(network, prev_connection_state))
       OnNetworkConnectionStateChanged(network);
   } else {
     if (network->path() == default_network_path_ &&
         key != flimflam::kSignalStrengthProperty) {
       // WiFi signal strength updates are too noisy, so don't
       // trigger default network updates for those changes.
       OnDefaultNetworkChanged();
     }
     std::string detail = network->name() + "." + key;
-    std::string vstr = ValueAsString(value);
+    std::string vstr;
+    base::JSONWriter::Write(&value, &vstr);
     if (!vstr.empty())

[pneubeck (no reviews), 2013/05/16 08:21:07]

write should never fail. empty should be a valid value, hiding it may lead to
confusion.
IIRC, write accepts an argument to use quotes for strings, which might be handy
here.

[stevenjb, 2013/05/16 17:09:53]

It could be an empty string, and I did not see that option (except 'pretty'
which could be multi-line).
I added '' for empty strings and wrapped this back in a utility function, but
using JSONWriter.

       detail += " = " + vstr;
-    network_event_log::AddEntry(kLogModule, "NetworkPropertyUpdated", detail);
+    network_event_log::LogLevel log_level =
+        (key == flimflam::kSignalStrengthProperty)
+        ? network_event_log::LOG_LEVEL_DEBUG
+        : network_event_log::LOG_LEVEL_EVENT;
+    NET_LOG_LEVEL(log_level, "NetworkPropertyUpdated", detail);
   }
   NetworkPropertiesUpdated(network);
 }
 
 void NetworkStateHandler::UpdateDeviceProperty(const std::string& device_path,
                                                const std::string& key,
                                                const base::Value& value) {
   DeviceState* device = GetModifiableDeviceState(device_path);
   if (!device)
     return;
   if (!device->PropertyChanged(key, value))
     return;
 
   std::string detail = device->name() + "." + key;
-  std::string vstr = ValueAsString(value);
+  std::string vstr;
+  base::JSONWriter::Write(&value, &vstr);
   if (!vstr.empty())

[pneubeck (no reviews), 2013/05/16 08:21:07]

same here.

     detail += " = " + vstr;
-  network_event_log::AddEntry(kLogModule, "DevicePropertyUpdated", detail);
+  NET_LOG_EVENT("DevicePropertyUpdated", detail);
 
   FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                     DeviceListChanged());
 
   if (key == flimflam::kScanningProperty && device->scanning() == false)
     ScanCompleted(device->type());
 }
 
-void NetworkStateHandler::ManagerPropertyChanged() {
-  network_event_log::AddEntry(kLogModule, "NetworkManagerChanged", "");
+void NetworkStateHandler::NotifyManagerPropertyChanged() {
+  NET_LOG_DEBUG("NotifyManagerPropertyChanged", "");
   FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                     NetworkManagerChanged());
 }
 
 void NetworkStateHandler::ManagedStateListChanged(
     ManagedState::ManagedType type) {
   if (type == ManagedState::MANAGED_TYPE_NETWORK) {
     // Notify observers that the list of networks has changed.
-    network_event_log::AddEntry(
-        kLogModule, "NetworkListChanged",
-        base::StringPrintf("Size:%"PRIuS, network_list_.size()));
+    NET_LOG_EVENT("NetworkListChanged",
+                  base::StringPrintf("Size:%"PRIuS, network_list_.size()));
     FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                       NetworkListChanged());
     // The list order may have changed, so check if the default network changed.
     if (CheckDefaultNetworkChanged())
       OnDefaultNetworkChanged();
   } else if (type == ManagedState::MANAGED_TYPE_DEVICE) {
-    network_event_log::AddEntry(
-        kLogModule, "DeviceListChanged",
-        base::StringPrintf("Size:%"PRIuS, device_list_.size()));
+    NET_LOG_DEBUG("DeviceListChanged",
+                  base::StringPrintf("Size:%"PRIuS, device_list_.size()));
     FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                       DeviceListChanged());
   } else {
     NOTREACHED();
   }
 }
 
 //------------------------------------------------------------------------------
 // Private methods
 
@@ skipping 34 matching lines @@
     case ManagedState::MANAGED_TYPE_DEVICE:
       return &device_list_;
   }
   NOTREACHED();
   return NULL;
 }
 
 void NetworkStateHandler::OnNetworkConnectionStateChanged(
     NetworkState* network) {
   DCHECK(network);
-  network_event_log::AddEntry(
-      kLogModule, "NetworkConnectionStateChanged",
-      base::StringPrintf("%s:%s", network->path().c_str(),
-                         network->connection_state().c_str()));
+  NET_LOG_EVENT("NetworkConnectionStateChanged", base::StringPrintf(
+      "%s:%s", GetManagedStateLogName(network).c_str(),
+      network->connection_state().c_str()));
   FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                     NetworkConnectionStateChanged(network));
   if (CheckDefaultNetworkChanged() || network->path() == default_network_path_)
     OnDefaultNetworkChanged();
 }
 
 bool NetworkStateHandler::CheckDefaultNetworkChanged() {
   std::string new_default_network_path;
   const NetworkState* new_default_network = DefaultNetwork();
   if (new_default_network)
     new_default_network_path = new_default_network->path();
   if (new_default_network_path == default_network_path_)
     return false;
   default_network_path_ = new_default_network_path;
   return true;
 }
 
 void NetworkStateHandler::OnDefaultNetworkChanged() {
   const NetworkState* default_network = DefaultNetwork();
-  network_event_log::AddEntry(
-      kLogModule, "DefaultNetworkChanged",
-      default_network ? default_network->path() : "None");
+  NET_LOG_EVENT("DefaultNetworkChanged",
+                GetManagedStateLogName(default_network));
   FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                     DefaultNetworkChanged(default_network));
 }
 
 void NetworkStateHandler::NetworkPropertiesUpdated(
     const NetworkState* network) {
   FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                     NetworkPropertiesUpdated(network));
   // If |connecting_network_| transitions to a non-idle, non-connecting state,
   // clear it *after* signalling observers.
   if (network->path() == connecting_network_ &&
       !network->IsConnectingState() &&
       network->connection_state() != flimflam::kStateIdle) {
     connecting_network_.clear();
-    network_event_log::AddEntry(
-        kLogModule, "ClearConnectingNetwork",
-        base::StringPrintf("%s:%s", network->path().c_str(),
-                           network->connection_state().c_str()));
+    NET_LOG_EVENT("ClearConnectingNetwork", base::StringPrintf(
+        "%s:%s", GetManagedStateLogName(network).c_str(),
+        network->connection_state().c_str()));
   }
 }
 
 void NetworkStateHandler::ScanCompleted(const std::string& type) {
   size_t num_callbacks = scan_complete_callbacks_.count(type);
-  network_event_log::AddEntry(
-      kLogModule, "ScanCompleted",
-      base::StringPrintf("%s:%"PRIuS, type.c_str(), num_callbacks));
+  NET_LOG_EVENT("ScanCompleted",
+                base::StringPrintf("%s:%"PRIuS, type.c_str(), num_callbacks));
   if (num_callbacks == 0)
     return;
   ScanCallbackList& callback_list = scan_complete_callbacks_[type];
   for (ScanCallbackList::iterator iter = callback_list.begin();
        iter != callback_list.end(); ++iter) {
     (*iter).Run();
   }
   scan_complete_callbacks_.erase(type);
 }
 
 std::string NetworkStateHandler::GetTechnologyForType(
     const std::string& type) const {
   if (type == kMatchTypeMobile) {
     if (shill_property_handler_->IsTechnologyAvailable(flimflam::kTypeWimax))
       return flimflam::kTypeWimax;
     else
       return flimflam::kTypeCellular;
   }
   if (type == kMatchTypeDefault || type == kMatchTypeNonVirtual ||
       type == kMatchTypeWireless) {
     NOTREACHED();
     return flimflam::kTypeWifi;
   }
   return type;
 }
 
 }  // namespace chromeos