NSH cleanup + logging fixes (Take 2, with MSan fix)

This is mostly formatting changes and changes from NET_LOG_TYPE ->
NET_LOG, plus some additional logging to help track down errors.

I separated this out to make actual logic changes easier to review
in an upcoming CL.

BUG=none
TBR=pneubeck@chromium.org

Review URL: https://codereview.chromium.org/1000403003

Cr-Commit-Position: refs/heads/master@{#320551}

6 files changed, 175 insertions, 164 deletions

diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index bb806a5..d095e57 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -64,7 +64,7 @@ void LogConfigProperties(const std::string& desc,
   for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
        iter.Advance()) {
     std::string v = "******";
-    if (!shill_property_util::IsPassphraseKey(iter.key()))
+    if (shill_property_util::IsLoggableShillProperty(iter.key()))
       base::JSONWriter::Write(&iter.value(), &v);
     NET_LOG(USER) << desc << ": " << path + "." + iter.key() + "=" + v;
   }
diff --git a/chromeos/network/network_device_handler_impl.cc b/chromeos/network/network_device_handler_impl.cc
index 3e8c629..cb3ff2c 100644
--- a/chromeos/network/network_device_handler_impl.cc
+++ b/chromeos/network/network_device_handler_impl.cc
@@ -128,6 +128,7 @@ void SetDevicePropertyInternal(
     const base::Value& value,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.SetProperty: " << property_name;
   DBusThreadManager::Get()->GetShillDeviceClient()->SetProperty(
       dbus::ObjectPath(device_path),
       property_name,
@@ -340,6 +341,8 @@ void NetworkDeviceHandlerImpl::RegisterCellularNetwork(
     const std::string& network_id,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.RegisterCellularNetwork: " << device_path
+                << " Id: " << network_id;
   DBusThreadManager::Get()->GetShillDeviceClient()->Register(
       dbus::ObjectPath(device_path),
       network_id,
@@ -352,6 +355,8 @@ void NetworkDeviceHandlerImpl::SetCarrier(
     const std::string& carrier,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.SetCarrier: " << device_path
+                << " carrier: " << carrier;
   DBusThreadManager::Get()->GetShillDeviceClient()->SetCarrier(
       dbus::ObjectPath(device_path),
       carrier,
@@ -365,6 +370,7 @@ void NetworkDeviceHandlerImpl::RequirePin(
     const std::string& pin,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.RequirePin: " << device_path << ": " << require_pin;
   DBusThreadManager::Get()->GetShillDeviceClient()->RequirePin(
       dbus::ObjectPath(device_path),
       pin,
@@ -378,6 +384,7 @@ void NetworkDeviceHandlerImpl::EnterPin(
     const std::string& pin,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.EnterPin: " << device_path;
   DBusThreadManager::Get()->GetShillDeviceClient()->EnterPin(
       dbus::ObjectPath(device_path),
       pin,
@@ -391,6 +398,7 @@ void NetworkDeviceHandlerImpl::UnblockPin(
     const std::string& new_pin,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.UnblockPin: " << device_path;
   DBusThreadManager::Get()->GetShillDeviceClient()->UnblockPin(
       dbus::ObjectPath(device_path),
       puk,
@@ -405,6 +413,7 @@ void NetworkDeviceHandlerImpl::ChangePin(
     const std::string& new_pin,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "Device.ChangePin: " << device_path;
   DBusThreadManager::Get()->GetShillDeviceClient()->ChangePin(
       dbus::ObjectPath(device_path),
       old_pin,
@@ -459,6 +468,7 @@ void NetworkDeviceHandlerImpl::AddWifiWakeOnPacketConnection(
   if (!device_state)
     return;
 
+  NET_LOG(USER) << "Device.AddWakeOnWifi: " << device_state->path();
   DBusThreadManager::Get()->GetShillDeviceClient()->AddWakeOnPacketConnection(
       dbus::ObjectPath(device_state->path()),
       ip_endpoint,
@@ -476,6 +486,7 @@ void NetworkDeviceHandlerImpl::RemoveWifiWakeOnPacketConnection(
   if (!device_state)
     return;
 
+  NET_LOG(USER) << "Device.RemoveWakeOnWifi: " << device_state->path();
   DBusThreadManager::Get()
       ->GetShillDeviceClient()
       ->RemoveWakeOnPacketConnection(dbus::ObjectPath(device_state->path()),
@@ -493,6 +504,7 @@ void NetworkDeviceHandlerImpl::RemoveAllWifiWakeOnPacketConnections(
   if (!device_state)
     return;
 
+  NET_LOG(USER) << "Device.RemoveAllWakeOnWifi: " << device_state->path();
   DBusThreadManager::Get()
       ->GetShillDeviceClient()
       ->RemoveAllWakeOnPacketConnections(dbus::ObjectPath(device_state->path()),
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index a3aca7c..7703b10 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -65,8 +65,7 @@ std::string ValueAsString(const base::Value& value) {
 const char NetworkStateHandler::kDefaultCheckPortalList[] =
     "ethernet,wifi,cellular";
 
-NetworkStateHandler::NetworkStateHandler()
-    : network_list_sorted_(false) {
+NetworkStateHandler::NetworkStateHandler() : network_list_sorted_(false) {
 }
 
 NetworkStateHandler::~NetworkStateHandler() {
@@ -131,15 +130,15 @@ void NetworkStateHandler::SetTechnologyEnabled(
     const network_handler::ErrorCallback& error_callback) {
   ScopedVector<std::string> technologies = GetTechnologiesForType(type);
   for (ScopedVector<std::string>::iterator it = technologies.begin();
-      it != technologies.end(); ++it) {
+       it != technologies.end(); ++it) {
     std::string* technology = *it;
     DCHECK(technology);
     if (!shill_property_handler_->IsTechnologyAvailable(*technology))
       continue;
     NET_LOG_USER("SetTechnologyEnabled",
                  base::StringPrintf("%s:%d", technology->c_str(), enabled));
-    shill_property_handler_->SetTechnologyEnabled(
-        *technology, enabled, error_callback);
+    shill_property_handler_->SetTechnologyEnabled(*technology, enabled,
+                                                  error_callback);
   }
   // Signal Device/Technology state changed.
   NotifyDeviceListChanged();
@@ -149,7 +148,7 @@ const DeviceState* NetworkStateHandler::GetDeviceState(
     const std::string& device_path) const {
   const DeviceState* device = GetModifiableDeviceState(device_path);
   if (device && !device->update_received())
-    return NULL;
+    return nullptr;
   return device;
 }
 
@@ -163,7 +162,7 @@ const DeviceState* NetworkStateHandler::GetDeviceStateByType(
     if (device->Matches(type))
       return device->AsDeviceState();
   }
-  return NULL;
+  return nullptr;
 }
 
 bool NetworkStateHandler::GetScanningByType(
@@ -184,13 +183,13 @@ const NetworkState* NetworkStateHandler::GetNetworkState(
     const std::string& service_path) const {
   const NetworkState* network = GetModifiableNetworkState(service_path);
   if (network && !network->update_received())
-    return NULL;
+    return nullptr;
   return network;
 }
 
 const NetworkState* NetworkStateHandler::DefaultNetwork() const {
   if (default_network_path_.empty())
-    return NULL;
+    return nullptr;
   return GetNetworkState(default_network_path_);
 }
 
@@ -208,7 +207,7 @@ const NetworkState* NetworkStateHandler::ConnectedNetworkByType(
     if (network->Matches(type))
       return network;
   }
-  return NULL;
+  return nullptr;
 }
 
 const NetworkState* NetworkStateHandler::ConnectingNetworkByType(
@@ -225,7 +224,7 @@ const NetworkState* NetworkStateHandler::ConnectingNetworkByType(
     if (network->Matches(type))
       return network;
   }
-  return NULL;
+  return nullptr;
 }
 
 const NetworkState* NetworkStateHandler::FirstNetworkByType(
@@ -243,12 +242,12 @@ const NetworkState* NetworkStateHandler::FirstNetworkByType(
     if (network->Matches(type))
       return network;
   }
-  return NULL;
+  return nullptr;
 }
 
 std::string NetworkStateHandler::FormattedHardwareAddressForType(
     const NetworkTypePattern& type) const {
-  const DeviceState* device = NULL;
+  const DeviceState* device = nullptr;
   const NetworkState* network = ConnectedNetworkByType(type);
   if (network)
     device = GetDeviceState(network->device_path());
@@ -262,11 +261,8 @@ std::string NetworkStateHandler::FormattedHardwareAddressForType(
 void NetworkStateHandler::GetVisibleNetworkListByType(
     const NetworkTypePattern& type,
     NetworkStateList* list) {
-  GetNetworkListByType(type,
-                       false /* configured_only */,
-                       true /* visible_only */,
-                       0 /* no limit */,
-                       list);
+  GetNetworkListByType(type, false /* configured_only */,
+                       true /* visible_only */, 0 /* no limit */, list);
 }
 
 void NetworkStateHandler::GetVisibleNetworkList(NetworkStateList* list) {
@@ -306,12 +302,12 @@ const NetworkState* NetworkStateHandler::GetNetworkStateFromServicePath(
   ManagedState* managed =
       GetModifiableManagedState(&network_list_, service_path);
   if (!managed)
-    return NULL;
+    return nullptr;
   const NetworkState* network = managed->AsNetworkState();
   DCHECK(network);
   if (!network->update_received() ||
       (configured_only && !network->IsInProfile())) {
-    return NULL;
+    return nullptr;
   }
   return network;
 }
@@ -325,7 +321,7 @@ const NetworkState* NetworkStateHandler::GetNetworkStateFromGuid(
     if (network->guid() == guid)
       return network;
   }
-  return NULL;
+  return nullptr;
 }
 
 void NetworkStateHandler::GetDeviceList(DeviceStateList* list) const {
@@ -356,8 +352,8 @@ void NetworkStateHandler::RequestUpdateForNetwork(
   if (network)
     network->set_update_requested(true);
   NET_LOG_EVENT("RequestUpdate", service_path);
-  shill_property_handler_->RequestProperties(
-      ManagedState::MANAGED_TYPE_NETWORK, service_path);
+  shill_property_handler_->RequestProperties(ManagedState::MANAGED_TYPE_NETWORK,
+                                             service_path);
 }
 
 void NetworkStateHandler::ClearLastErrorForNetwork(
@@ -384,43 +380,39 @@ const NetworkState* NetworkStateHandler::GetEAPForEthernet(
   const NetworkState* network = GetNetworkState(service_path);
   if (!network) {
     NET_LOG_ERROR("GetEAPForEthernet", "Unknown service path " + service_path);
-    return NULL;
+    return nullptr;
   }
   if (network->type() != shill::kTypeEthernet) {
     NET_LOG_ERROR("GetEAPForEthernet", "Not of type Ethernet: " + service_path);
-    return NULL;
+    return nullptr;
   }
   if (!network->IsConnectedState())
-    return NULL;
+    return nullptr;
 
   // The same EAP service is shared for all ethernet services/devices.
   // However EAP is used/enabled per device and only if the connection was
   // successfully established.
   const DeviceState* device = GetDeviceState(network->device_path());
   if (!device) {
-    NET_LOG_ERROR(
-        "GetEAPForEthernet",
-        base::StringPrintf("Unknown device %s of connected ethernet service %s",
-                           network->device_path().c_str(),
-                           service_path.c_str()));
-    return NULL;
+    NET_LOG(ERROR) << "GetEAPForEthernet: Unknown device "
+                   << network->device_path()
+                   << " for connected ethernet service: " << service_path;
+    return nullptr;
   }
   if (!device->eap_authentication_completed())
-    return NULL;
+    return nullptr;
 
   NetworkStateList list;
   GetNetworkListByType(NetworkTypePattern::Primitive(shill::kTypeEthernetEap),
-                       true /* configured_only */,
-                       false /* visible_only */,
-                       1 /* limit */,
-                       &list);
+                       true /* configured_only */, false /* visible_only */,
+                       1 /* limit */, &list);
   if (list.empty()) {
     NET_LOG_ERROR("GetEAPForEthernet",
                   base::StringPrintf(
                       "Ethernet service %s connected using EAP, but no "
                       "EAP service found.",
                       service_path.c_str()));
-    return NULL;
+    return nullptr;
   }
   return list.front();
 }
@@ -504,8 +496,8 @@ void NetworkStateHandler::UpdateManagedStateProperties(
     UpdateNetworkStateProperties(managed->AsNetworkState(), properties);
   } else {
     // Device
-    for (base::DictionaryValue::Iterator iter(properties);
-         !iter.IsAtEnd(); iter.Advance()) {
+    for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
+         iter.Advance()) {
       managed->PropertyChanged(iter.key(), iter.value());
     }
     managed->InitialPropertiesReceived(properties);
@@ -520,8 +512,8 @@ void NetworkStateHandler::UpdateNetworkStateProperties(
   bool network_property_updated = false;
   std::string prev_connection_state = network->connection_state();
   bool prev_is_captive_portal = network->is_captive_portal();
-  for (base::DictionaryValue::Iterator iter(properties);
-       !iter.IsAtEnd(); iter.Advance()) {
+  for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
+       iter.Advance()) {
     if (network->PropertyChanged(iter.key(), iter.value()))
       network_property_updated = true;
   }
@@ -628,10 +620,8 @@ void NetworkStateHandler::UpdateDeviceProperty(const std::string& device_path,
     // Notify a change for each Ethernet service using this device.
     NetworkStateList ethernet_services;
     GetNetworkListByType(NetworkTypePattern::Ethernet(),
-                         false /* configured_only */,
-                         false /* visible_only */,
-                         0 /* no limit */,
-                         &ethernet_services);
+                         false /* configured_only */, false /* visible_only */,
+                         0 /* no limit */, &ethernet_services);
     for (NetworkStateList::const_iterator it = ethernet_services.begin();
          it != ethernet_services.end(); ++it) {
       const NetworkState* ethernet_service = *it;
@@ -648,7 +638,7 @@ void NetworkStateHandler::UpdateIPConfigProperties(
     ManagedState::ManagedType type,
     const std::string& path,
     const std::string& ip_config_path,
-    const base::DictionaryValue& properties)  {
+    const base::DictionaryValue& properties) {
   if (type == ManagedState::MANAGED_TYPE_NETWORK) {
     NetworkState* network = GetModifiableNetworkState(path);
     if (!network)
@@ -736,13 +726,13 @@ void NetworkStateHandler::SortNetworkList() {
   }
   network_list_.clear();
   network_list_.insert(network_list_.end(), active.begin(), active.end());
-  network_list_.insert(
-      network_list_.end(), non_wifi_visible.begin(), non_wifi_visible.end());
-  network_list_.insert(
-      network_list_.end(), wifi_visible.begin(), wifi_visible.end());
+  network_list_.insert(network_list_.end(), non_wifi_visible.begin(),
+                       non_wifi_visible.end());
+  network_list_.insert(network_list_.end(), wifi_visible.begin(),
+                       wifi_visible.end());
   network_list_.insert(network_list_.end(), hidden.begin(), hidden.end());
-  network_list_.insert(
-      network_list_.end(), new_networks.begin(), new_networks.end());
+  network_list_.insert(network_list_.end(), new_networks.begin(),
+                       new_networks.end());
   network_list_sorted_ = true;
 }
 
@@ -776,7 +766,7 @@ void NetworkStateHandler::DefaultNetworkServiceChanged(
 
   default_network_path_ = service_path;
   NET_LOG_EVENT("DefaultNetworkServiceChanged:", default_network_path_);
-  const NetworkState* network = NULL;
+  const NetworkState* network = nullptr;
   if (!default_network_path_.empty()) {
     network = GetNetworkState(default_network_path_);
     if (!network) {
@@ -836,7 +826,7 @@ DeviceState* NetworkStateHandler::GetModifiableDeviceState(
     const std::string& device_path) const {
   ManagedState* managed = GetModifiableManagedState(&device_list_, device_path);
   if (!managed)
-    return NULL;
+    return nullptr;
   return managed->AsDeviceState();
 }
 
@@ -845,7 +835,7 @@ NetworkState* NetworkStateHandler::GetModifiableNetworkState(
   ManagedState* managed =
       GetModifiableManagedState(&network_list_, service_path);
   if (!managed)
-    return NULL;
+    return nullptr;
   return managed->AsNetworkState();
 }
 
@@ -858,7 +848,7 @@ ManagedState* NetworkStateHandler::GetModifiableManagedState(
     if (managed->path() == path)
       return managed;
   }
-  return NULL;
+  return nullptr;
 }
 
 NetworkStateHandler::ManagedStateList* NetworkStateHandler::GetManagedList(
@@ -870,7 +860,7 @@ NetworkStateHandler::ManagedStateList* NetworkStateHandler::GetManagedList(
       return &device_list_;
   }
   NOTREACHED();
-  return NULL;
+  return nullptr;
 }
 
 void NetworkStateHandler::OnNetworkConnectionStateChanged(
@@ -886,7 +876,7 @@ void NetworkStateHandler::OnNetworkConnectionStateChanged(
           network->path());
       default_network_path_.clear();
       SortNetworkList();
-      NotifyDefaultNetworkChanged(NULL);
+      NotifyDefaultNetworkChanged(nullptr);
     }
   }
   NET_LOG_EVENT("NOTIFY:" + event + ": " + network->connection_state(),
diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc
index 39d161d..02b620d 100644
--- a/chromeos/network/shill_property_handler.cc
+++ b/chromeos/network/shill_property_handler.cc
@@ -9,8 +9,6 @@
 #include "base/bind.h"
 #include "base/format_macros.h"
 #include "base/stl_util.h"
-#include "base/strings/string_util.h"
-#include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
@@ -18,8 +16,8 @@
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_profile_client.h"
 #include "chromeos/dbus/shill_service_client.h"
-#include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
+#include "components/device_event_log/device_event_log.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -57,17 +55,17 @@ class ShillPropertyObserver : public ShillPropertyChangedObserver {
   ShillPropertyObserver(ManagedState::ManagedType type,
                         const std::string& path,
                         const Handler& handler)
-      : type_(type),
-        path_(path),
-        handler_(handler) {
+      : type_(type), path_(path), handler_(handler) {
     if (type_ == ManagedState::MANAGED_TYPE_NETWORK) {
       DVLOG(2) << "ShillPropertyObserver: Network: " << path;
-      DBusThreadManager::Get()->GetShillServiceClient()->
-          AddPropertyChangedObserver(dbus::ObjectPath(path_), this);
+      DBusThreadManager::Get()
+          ->GetShillServiceClient()
+          ->AddPropertyChangedObserver(dbus::ObjectPath(path_), this);
     } else if (type_ == ManagedState::MANAGED_TYPE_DEVICE) {
       DVLOG(2) << "ShillPropertyObserver: Device: " << path;
-      DBusThreadManager::Get()->GetShillDeviceClient()->
-          AddPropertyChangedObserver(dbus::ObjectPath(path_), this);
+      DBusThreadManager::Get()
+          ->GetShillDeviceClient()
+          ->AddPropertyChangedObserver(dbus::ObjectPath(path_), this);
     } else {
       NOTREACHED();
     }
@@ -75,11 +73,13 @@ class ShillPropertyObserver : public ShillPropertyChangedObserver {
 
   ~ShillPropertyObserver() override {
     if (type_ == ManagedState::MANAGED_TYPE_NETWORK) {
-      DBusThreadManager::Get()->GetShillServiceClient()->
-          RemovePropertyChangedObserver(dbus::ObjectPath(path_), this);
+      DBusThreadManager::Get()
+          ->GetShillServiceClient()
+          ->RemovePropertyChangedObserver(dbus::ObjectPath(path_), this);
     } else if (type_ == ManagedState::MANAGED_TYPE_DEVICE) {
-      DBusThreadManager::Get()->GetShillDeviceClient()->
-          RemovePropertyChangedObserver(dbus::ObjectPath(path_), this);
+      DBusThreadManager::Get()
+          ->GetShillDeviceClient()
+          ->RemovePropertyChangedObserver(dbus::ObjectPath(path_), this);
     } else {
       NOTREACHED();
     }
@@ -109,10 +109,10 @@ ShillPropertyHandler::ShillPropertyHandler(Listener* listener)
 
 ShillPropertyHandler::~ShillPropertyHandler() {
   // Delete network service observers.
-  STLDeleteContainerPairSecondPointers(
-      observed_networks_.begin(), observed_networks_.end());
-  STLDeleteContainerPairSecondPointers(
-      observed_devices_.begin(), observed_devices_.end());
+  STLDeleteContainerPairSecondPointers(observed_networks_.begin(),
+                                       observed_networks_.end());
+  STLDeleteContainerPairSecondPointers(observed_devices_.begin(),
+                                       observed_devices_.end());
   CHECK(shill_manager_ == DBusThreadManager::Get()->GetShillManagerClient());
   shill_manager_->RemovePropertyChangedObserver(this);
 }
@@ -123,10 +123,9 @@ void ShillPropertyHandler::Init() {
 }
 
 void ShillPropertyHandler::UpdateManagerProperties() {
-  NET_LOG_EVENT("UpdateManagerProperties", "");
-  shill_manager_->GetProperties(
-      base::Bind(&ShillPropertyHandler::ManagerPropertiesCallback,
-                 AsWeakPtr()));
+  NET_LOG(EVENT) << "UpdateManagerProperties";
+  shill_manager_->GetProperties(base::Bind(
+      &ShillPropertyHandler::ManagerPropertiesCallback, AsWeakPtr()));
 }
 
 bool ShillPropertyHandler::IsTechnologyAvailable(
@@ -156,20 +155,17 @@ void ShillPropertyHandler::SetTechnologyEnabled(
   if (enabled) {
     enabling_technologies_.insert(technology);
     shill_manager_->EnableTechnology(
-        technology,
-        base::Bind(&base::DoNothing),
-        base::Bind(&ShillPropertyHandler::EnableTechnologyFailed,
-                   AsWeakPtr(), technology, error_callback));
+        technology, base::Bind(&base::DoNothing),
+        base::Bind(&ShillPropertyHandler::EnableTechnologyFailed, AsWeakPtr(),
+                   technology, error_callback));
   } else {
     // Immediately clear locally from enabled and enabling lists.
     enabled_technologies_.erase(technology);
     enabling_technologies_.erase(technology);
     shill_manager_->DisableTechnology(
-        technology,
-        base::Bind(&base::DoNothing),
+        technology, base::Bind(&base::DoNothing),
         base::Bind(&network_handler::ShillErrorCallbackFunction,
-                   "SetTechnologyEnabled Failed",
-                   technology, error_callback));
+                   "SetTechnologyEnabled Failed", technology, error_callback));
   }
 }
 
@@ -177,34 +173,26 @@ void ShillPropertyHandler::SetCheckPortalList(
     const std::string& check_portal_list) {
   base::StringValue value(check_portal_list);
   shill_manager_->SetProperty(
-      shill::kCheckPortalListProperty,
-      value,
-      base::Bind(&base::DoNothing),
+      shill::kCheckPortalListProperty, value, base::Bind(&base::DoNothing),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
-                 "SetCheckPortalList Failed",
-                 "Manager",
+                 "SetCheckPortalList Failed", "Manager",
                  network_handler::ErrorCallback()));
 }
 
 void ShillPropertyHandler::SetWakeOnLanEnabled(bool enabled) {
   base::FundamentalValue value(enabled);
   shill_manager_->SetProperty(
-      shill::kWakeOnLanEnabledProperty,
-      value,
-      base::Bind(&base::DoNothing),
+      shill::kWakeOnLanEnabledProperty, value, base::Bind(&base::DoNothing),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
-                 "SetWakeOnLanEnabled Failed",
-                 "Manager",
+                 "SetWakeOnLanEnabled Failed", "Manager",
                  network_handler::ErrorCallback()));
 }
 
 void ShillPropertyHandler::RequestScan() const {
   shill_manager_->RequestScan(
-      "",
-      base::Bind(&base::DoNothing),
+      "", base::Bind(&base::DoNothing),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
-                 "RequestScan Failed",
-                 "", network_handler::ErrorCallback()));
+                 "RequestScan Failed", "", network_handler::ErrorCallback()));
 }
 
 void ShillPropertyHandler::RequestProperties(ManagedState::ManagedType type,
@@ -212,19 +200,19 @@ void ShillPropertyHandler::RequestProperties(ManagedState::ManagedType type,
   if (pending_updates_[type].find(path) != pending_updates_[type].end())
     return;  // Update already requested.
 
-  NET_LOG_DEBUG("Request Properties: " + ManagedState::TypeToString(type),
-                path);
+  NET_LOG(DEBUG) << "Request Properties: " << ManagedState::TypeToString(type)
+                 << " For: " << path;
   pending_updates_[type].insert(path);
   if (type == ManagedState::MANAGED_TYPE_NETWORK) {
     DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
         dbus::ObjectPath(path),
-        base::Bind(&ShillPropertyHandler::GetPropertiesCallback,
-                   AsWeakPtr(), type, path));
+        base::Bind(&ShillPropertyHandler::GetPropertiesCallback, AsWeakPtr(),
+                   type, path));
   } else if (type == ManagedState::MANAGED_TYPE_DEVICE) {
     DBusThreadManager::Get()->GetShillDeviceClient()->GetProperties(
         dbus::ObjectPath(path),
-        base::Bind(&ShillPropertyHandler::GetPropertiesCallback,
-                   AsWeakPtr(), type, path));
+        base::Bind(&ShillPropertyHandler::GetPropertiesCallback, AsWeakPtr(),
+                   type, path));
   } else {
     NOTREACHED();
   }
@@ -243,13 +231,12 @@ void ShillPropertyHandler::ManagerPropertiesCallback(
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& properties) {
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    NET_LOG_ERROR("ManagerPropertiesCallback",
-                  base::StringPrintf("Failed: %d", call_status));
+    NET_LOG(ERROR) << "ManagerPropertiesCallback Failed: " << call_status;
     return;
   }
-  NET_LOG_EVENT("ManagerPropertiesCallback", "Success");
-  for (base::DictionaryValue::Iterator iter(properties);
-       !iter.IsAtEnd(); iter.Advance()) {
+  NET_LOG(EVENT) << "ManagerPropertiesCallback: Success";
+  for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
+       iter.Advance()) {
     ManagerPropertyChanged(iter.key(), iter.value());
   }
 
@@ -271,12 +258,15 @@ void ShillPropertyHandler::CheckPendingStateListUpdates(
 
 void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
                                                   const base::Value& value) {
-  NET_LOG_DEBUG("ManagerPropertyChanged", key);
   if (key == shill::kDefaultServiceProperty) {
     std::string service_path;
     value.GetAsString(&service_path);
+    NET_LOG(EVENT) << "Manager.DefaultService = " << service_path;
     listener_->DefaultNetworkServiceChanged(service_path);
-  } else if (key == shill::kServiceCompleteListProperty) {
+    return;
+  }
+  NET_LOG(DEBUG) << "ManagerPropertyChanged: " << key;
+  if (key == shill::kServiceCompleteListProperty) {
     const base::ListValue* vlist = GetListValue(key, value);
     if (vlist) {
       listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
@@ -317,8 +307,8 @@ void ShillPropertyHandler::UpdateProperties(ManagedState::ManagedType type,
                                             const base::ListValue& entries) {
   std::set<std::string>& requested_updates = requested_updates_[type];
   std::set<std::string> new_requested_updates;
-  NET_LOG_DEBUG("UpdateProperties: " + ManagedState::TypeToString(type),
-                base::StringPrintf("%" PRIuS, entries.GetSize()));
+  NET_LOG(DEBUG) << "UpdateProperties: " << ManagedState::TypeToString(type)
+                 << ": " << entries.GetSize();
   for (base::ListValue::const_iterator iter = entries.begin();
        iter != entries.end(); ++iter) {
     std::string path;
@@ -341,10 +331,10 @@ void ShillPropertyHandler::UpdateProperties(ManagedState::ManagedType type,
 void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type,
                                           const base::ListValue& entries) {
   ShillPropertyObserverMap& observer_map =
-      (type == ManagedState::MANAGED_TYPE_NETWORK)
-      ? observed_networks_ : observed_devices_;
+      (type == ManagedState::MANAGED_TYPE_NETWORK) ? observed_networks_
+                                                   : observed_devices_;
   ShillPropertyObserverMap new_observed;
-  for (auto* entry: entries) {
+  for (auto* entry : entries) {
     std::string path;
     entry->GetAsString(&path);
     if (path.empty())
@@ -356,8 +346,8 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type,
     } else {
       // Create an observer for future updates.
       observer = new ShillPropertyObserver(
-          type, path, base::Bind(
-              &ShillPropertyHandler::PropertyChangedCallback, AsWeakPtr()));
+          type, path, base::Bind(&ShillPropertyHandler::PropertyChangedCallback,
+                                 AsWeakPtr()));
     }
     auto result = new_observed.insert(std::make_pair(path, observer));
     if (!result.second) {
@@ -370,7 +360,7 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type,
       break;
   }
   // Delete network service observers still in observer_map.
-  for (auto& observer: observer_map) {
+  for (auto& observer : observer_map) {
     delete observer.second;
   }
   observer_map.swap(new_observed);
@@ -378,9 +368,7 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type,
 
 void ShillPropertyHandler::UpdateAvailableTechnologies(
     const base::ListValue& technologies) {
-  std::stringstream technologies_str;
-  technologies_str << technologies;
-  NET_LOG_EVENT("AvailableTechnologies:", technologies_str.str());
+  NET_LOG(EVENT) << "AvailableTechnologies:" << technologies;
   available_technologies_.clear();
   for (base::ListValue::const_iterator iter = technologies.begin();
        iter != technologies.end(); ++iter) {
@@ -394,9 +382,7 @@ void ShillPropertyHandler::UpdateAvailableTechnologies(
 
 void ShillPropertyHandler::UpdateEnabledTechnologies(
     const base::ListValue& technologies) {
-  std::stringstream technologies_str;
-  technologies_str << technologies;
-  NET_LOG_EVENT("EnabledTechnologies:", technologies_str.str());
+  NET_LOG(EVENT) << "EnabledTechnologies:" << technologies;
   enabled_technologies_.clear();
   for (base::ListValue::const_iterator iter = technologies.begin();
        iter != technologies.end(); ++iter) {
@@ -411,9 +397,7 @@ void ShillPropertyHandler::UpdateEnabledTechnologies(
 
 void ShillPropertyHandler::UpdateUninitializedTechnologies(
     const base::ListValue& technologies) {
-  std::stringstream technologies_str;
-  technologies_str << technologies;
-  NET_LOG_EVENT("UninitializedTechnologies:", technologies_str.str());
+  NET_LOG(EVENT) << "UninitializedTechnologies:" << technologies;
   uninitialized_technologies_.clear();
   for (base::ListValue::const_iterator iter = technologies.begin();
        iter != technologies.end(); ++iter) {
@@ -432,9 +416,8 @@ void ShillPropertyHandler::EnableTechnologyFailed(
     const std::string& dbus_error_message) {
   enabling_technologies_.erase(technology);
   network_handler::ShillErrorCallbackFunction(
-      "EnableTechnology Failed",
-      technology, error_callback,
-      dbus_error_name, dbus_error_message);
+      "EnableTechnology Failed", technology, error_callback, dbus_error_name,
+      dbus_error_message);
 }
 
 void ShillPropertyHandler::GetPropertiesCallback(
@@ -442,14 +425,14 @@ void ShillPropertyHandler::GetPropertiesCallback(
     const std::string& path,
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& properties) {
-  NET_LOG_DEBUG("GetPropertiesCallback: " + ManagedState::TypeToString(type),
-                path);
+  NET_LOG(DEBUG) << "GetPropertiesCallback: "
+                 << ManagedState::TypeToString(type) << " For: " << path;
   pending_updates_[type].erase(path);
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
     // The shill service no longer exists.  This can happen when a network
     // has been removed.
-    NET_LOG_DEBUG("Failed to get properties",
-                  base::StringPrintf("%s: %d", path.c_str(), call_status));
+    NET_LOG(DEBUG) << "Failed to get properties for: " << path << ": "
+                   << call_status;
     return;
   }
   listener_->UpdateManagedStateProperties(type, path, properties);
@@ -480,7 +463,7 @@ void ShillPropertyHandler::PropertyChangedCallback(
       key == shill::kIPConfigProperty) {
     RequestIPConfig(type, path, value);
   } else if (type == ManagedState::MANAGED_TYPE_DEVICE &&
-      key == shill::kIPConfigsProperty) {
+             key == shill::kIPConfigsProperty) {
     RequestIPConfigsList(type, path, value);
   }
 
@@ -499,13 +482,13 @@ void ShillPropertyHandler::RequestIPConfig(
   std::string ip_config_path;
   if (!ip_config_path_value.GetAsString(&ip_config_path) ||
       ip_config_path.empty()) {
-    NET_LOG_ERROR("Invalid IPConfig", path);
+    NET_LOG(ERROR) << "Invalid IPConfig: " << path;
     return;
   }
   DBusThreadManager::Get()->GetShillIPConfigClient()->GetProperties(
       dbus::ObjectPath(ip_config_path),
-      base::Bind(&ShillPropertyHandler::GetIPConfigCallback,
-                 AsWeakPtr(), type, path, ip_config_path));
+      base::Bind(&ShillPropertyHandler::GetIPConfigCallback, AsWeakPtr(), type,
+                 path, ip_config_path));
 }
 
 void ShillPropertyHandler::RequestIPConfigsList(
@@ -526,16 +509,15 @@ void ShillPropertyHandler::GetIPConfigCallback(
     const std::string& path,
     const std::string& ip_config_path,
     DBusMethodCallStatus call_status,
-    const base::DictionaryValue& properties)  {
+    const base::DictionaryValue& properties) {
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
     // IP Config properties not availabe. Shill will emit a property change
     // when they are.
-    NET_LOG_EVENT(
-        base::StringPrintf("Failed to get IP Config properties: %s: %d",
-                           ip_config_path.c_str(), call_status), path);
+    NET_LOG(EVENT) << "Failed to get IP Config properties: " << ip_config_path
+                   << ": " << call_status << ", For: " << path;
     return;
   }
-  NET_LOG_EVENT("IP Config properties received", path);
+  NET_LOG(EVENT) << "IP Config properties received: " << path;
   listener_->UpdateIPConfigProperties(type, path, ip_config_path, properties);
 }
 
diff --git a/chromeos/network/shill_property_util.cc b/chromeos/network/shill_property_util.cc
index 2e77fb7..a53487b 100644
--- a/chromeos/network/shill_property_util.cc
+++ b/chromeos/network/shill_property_util.cc
@@ -4,6 +4,8 @@
 
 #include "chromeos/network/shill_property_util.h"
 
+#include <set>
+
 #include "base/i18n/icu_encoding_detection.h"
 #include "base/i18n/icu_string_conversions.h"
 #include "base/json/json_writer.h"
@@ -313,18 +315,38 @@ bool DoIdentifyingPropertiesMatch(const base::DictionaryValue& new_properties,
   return new_identifying.Equals(&old_identifying);
 }
 
-bool IsPassphraseKey(const std::string& key) {
-  return key == shill::kEapPrivateKeyPasswordProperty ||
-      key == shill::kEapPasswordProperty ||
-      key == shill::kL2tpIpsecPasswordProperty ||
-      key == shill::kOpenVPNPasswordProperty ||
-      key == shill::kOpenVPNAuthUserPassProperty ||
-      key == shill::kOpenVPNTLSAuthContentsProperty ||
-      key == shill::kPassphraseProperty ||
-      key == shill::kOpenVPNOTPProperty ||
-      key == shill::kEapPrivateKeyProperty ||
-      key == shill::kEapPinProperty ||
-      key == shill::kApnPasswordProperty;
+bool IsLoggableShillProperty(const std::string& key) {
+  static std::set<std::string>* s_skip_properties = nullptr;
+  if (!s_skip_properties) {
+    s_skip_properties = new std::set<std::string>;
+    s_skip_properties->insert(shill::kApnPasswordProperty);
+    s_skip_properties->insert(shill::kEapCaCertNssProperty);
+    s_skip_properties->insert(shill::kEapCaCertPemProperty);
+    s_skip_properties->insert(shill::kEapCaCertProperty);
+    s_skip_properties->insert(shill::kEapClientCertNssProperty);
+    s_skip_properties->insert(shill::kEapClientCertProperty);
+    s_skip_properties->insert(shill::kEapPasswordProperty);
+    s_skip_properties->insert(shill::kEapPinProperty);
+    s_skip_properties->insert(shill::kEapPrivateKeyPasswordProperty);
+    s_skip_properties->insert(shill::kEapPrivateKeyProperty);
+    s_skip_properties->insert(shill::kL2tpIpsecCaCertPemProperty);
+    s_skip_properties->insert(shill::kL2tpIpsecPasswordProperty);
+    s_skip_properties->insert(shill::kL2tpIpsecPinProperty);
+    s_skip_properties->insert(shill::kL2tpIpsecPskProperty);
+    s_skip_properties->insert(shill::kOpenVPNAuthUserPassProperty);
+    s_skip_properties->insert(shill::kOpenVPNCaCertNSSProperty);
+    s_skip_properties->insert(shill::kOpenVPNCaCertPemProperty);
+    s_skip_properties->insert(shill::kOpenVPNCaCertProperty);
+    s_skip_properties->insert(shill::kOpenVPNCertProperty);
+    s_skip_properties->insert(shill::kOpenVPNExtraCertPemProperty);
+    s_skip_properties->insert(shill::kOpenVPNOTPProperty);
+    s_skip_properties->insert(shill::kOpenVPNPasswordProperty);
+    s_skip_properties->insert(shill::kOpenVPNPinProperty);
+    s_skip_properties->insert(shill::kOpenVPNTLSAuthContentsProperty);
+    s_skip_properties->insert(shill::kPPPoEPasswordProperty);
+    s_skip_properties->insert(shill::kPassphraseProperty);
+  }
+  return s_skip_properties->count(key) == 0;
 }
 
 bool GetHomeProviderFromProperty(const base::Value& value,
diff --git a/chromeos/network/shill_property_util.h b/chromeos/network/shill_property_util.h
index 3637c6a..cc9da9c 100644
--- a/chromeos/network/shill_property_util.h
+++ b/chromeos/network/shill_property_util.h
@@ -82,8 +82,13 @@ bool DoIdentifyingPropertiesMatch(
     const base::DictionaryValue& new_properties,
     const base::DictionaryValue& old_properties);
 
-// Returns true if |key| corresponds to a passphrase property.
-bool IsPassphraseKey(const std::string& key);
+// Returns false if |key| is something that should not be logged either
+// because it is sensitive or noisy. Note: this is not necessarily
+// comprehensive, do not use it for anything genuinely sensitive (user logs
+// should always be treated as sensitive data, but sometimes they end up
+// attached to public issues so this helps prevent accidents, but it should not
+// be relied upon).
+bool IsLoggableShillProperty(const std::string& key);
 
 // Parses |value| (which should be a Dictionary). Returns true and sets
 // |home_provider_id| if |value| was succesfully parsed.