Revert of NSH cleanup + logging fixes (patchset #2 id:20001 of https://codereview.chromium.org/1003573002/)

Reason for revert:
Broke CrOS MSan build
BUG=466995

Original issue's description:
> NSH cleanup + logging fixes
>
> 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
>
> Committed: https://crrev.com/f03f6c5c2296fc93a18e535e3f27c3ca039e20f2
> Cr-Commit-Position: refs/heads/master@{#320427}

TBR=pneubeck@chromium.org,pstew@chromium.org,armansito@chromium.org,michaelpg@chromium.org,stevenjb@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=none

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

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

6 files changed, 158 insertions, 175 deletions

diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index d095e57..bb806a5 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::IsLoggableShillProperty(iter.key()))
+    if (!shill_property_util::IsPassphraseKey(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 cb3ff2c..3e8c629 100644
--- a/chromeos/network/network_device_handler_impl.cc
+++ b/chromeos/network/network_device_handler_impl.cc
@@ -128,7 +128,6 @@ 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,
@@ -341,8 +340,6 @@ 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,
@@ -355,8 +352,6 @@ 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,
@@ -370,7 +365,6 @@ 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,
@@ -384,7 +378,6 @@ 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,
@@ -398,7 +391,6 @@ 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,
@@ -413,7 +405,6 @@ 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,
@@ -468,7 +459,6 @@ 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,
@@ -486,7 +476,6 @@ void NetworkDeviceHandlerImpl::RemoveWifiWakeOnPacketConnection(
   if (!device_state)
     return;
 
-  NET_LOG(USER) << "Device.RemoveWakeOnWifi: " << device_state->path();
   DBusThreadManager::Get()
       ->GetShillDeviceClient()
       ->RemoveWakeOnPacketConnection(dbus::ObjectPath(device_state->path()),
@@ -504,7 +493,6 @@ 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 7703b10..a3aca7c 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -65,7 +65,8 @@ 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() {
@@ -130,15 +131,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();
@@ -148,7 +149,7 @@ const DeviceState* NetworkStateHandler::GetDeviceState(
     const std::string& device_path) const {
   const DeviceState* device = GetModifiableDeviceState(device_path);
   if (device && !device->update_received())
-    return nullptr;
+    return NULL;
   return device;
 }
 
@@ -162,7 +163,7 @@ const DeviceState* NetworkStateHandler::GetDeviceStateByType(
     if (device->Matches(type))
       return device->AsDeviceState();
   }
-  return nullptr;
+  return NULL;
 }
 
 bool NetworkStateHandler::GetScanningByType(
@@ -183,13 +184,13 @@ const NetworkState* NetworkStateHandler::GetNetworkState(
     const std::string& service_path) const {
   const NetworkState* network = GetModifiableNetworkState(service_path);
   if (network && !network->update_received())
-    return nullptr;
+    return NULL;
   return network;
 }
 
 const NetworkState* NetworkStateHandler::DefaultNetwork() const {
   if (default_network_path_.empty())
-    return nullptr;
+    return NULL;
   return GetNetworkState(default_network_path_);
 }
 
@@ -207,7 +208,7 @@ const NetworkState* NetworkStateHandler::ConnectedNetworkByType(
     if (network->Matches(type))
       return network;
   }
-  return nullptr;
+  return NULL;
 }
 
 const NetworkState* NetworkStateHandler::ConnectingNetworkByType(
@@ -224,7 +225,7 @@ const NetworkState* NetworkStateHandler::ConnectingNetworkByType(
     if (network->Matches(type))
       return network;
   }
-  return nullptr;
+  return NULL;
 }
 
 const NetworkState* NetworkStateHandler::FirstNetworkByType(
@@ -242,12 +243,12 @@ const NetworkState* NetworkStateHandler::FirstNetworkByType(
     if (network->Matches(type))
       return network;
   }
-  return nullptr;
+  return NULL;
 }
 
 std::string NetworkStateHandler::FormattedHardwareAddressForType(
     const NetworkTypePattern& type) const {
-  const DeviceState* device = nullptr;
+  const DeviceState* device = NULL;
   const NetworkState* network = ConnectedNetworkByType(type);
   if (network)
     device = GetDeviceState(network->device_path());
@@ -261,8 +262,11 @@ 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) {
@@ -302,12 +306,12 @@ const NetworkState* NetworkStateHandler::GetNetworkStateFromServicePath(
   ManagedState* managed =
       GetModifiableManagedState(&network_list_, service_path);
   if (!managed)
-    return nullptr;
+    return NULL;
   const NetworkState* network = managed->AsNetworkState();
   DCHECK(network);
   if (!network->update_received() ||
       (configured_only && !network->IsInProfile())) {
-    return nullptr;
+    return NULL;
   }
   return network;
 }
@@ -321,7 +325,7 @@ const NetworkState* NetworkStateHandler::GetNetworkStateFromGuid(
     if (network->guid() == guid)
       return network;
   }
-  return nullptr;
+  return NULL;
 }
 
 void NetworkStateHandler::GetDeviceList(DeviceStateList* list) const {
@@ -352,8 +356,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(
@@ -380,39 +384,43 @@ const NetworkState* NetworkStateHandler::GetEAPForEthernet(
   const NetworkState* network = GetNetworkState(service_path);
   if (!network) {
     NET_LOG_ERROR("GetEAPForEthernet", "Unknown service path " + service_path);
-    return nullptr;
+    return NULL;
   }
   if (network->type() != shill::kTypeEthernet) {
     NET_LOG_ERROR("GetEAPForEthernet", "Not of type Ethernet: " + service_path);
-    return nullptr;
+    return NULL;
   }
   if (!network->IsConnectedState())
-    return nullptr;
+    return NULL;
 
   // 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: Unknown device "
-                   << network->device_path()
-                   << " for connected ethernet service: " << service_path;
-    return nullptr;
+    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;
   }
   if (!device->eap_authentication_completed())
-    return nullptr;
+    return NULL;
 
   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 nullptr;
+    return NULL;
   }
   return list.front();
 }
@@ -496,8 +504,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);
@@ -512,8 +520,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;
   }
@@ -620,8 +628,10 @@ 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;
@@ -638,7 +648,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)
@@ -726,13 +736,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;
 }
 
@@ -766,7 +776,7 @@ void NetworkStateHandler::DefaultNetworkServiceChanged(
 
   default_network_path_ = service_path;
   NET_LOG_EVENT("DefaultNetworkServiceChanged:", default_network_path_);
-  const NetworkState* network = nullptr;
+  const NetworkState* network = NULL;
   if (!default_network_path_.empty()) {
     network = GetNetworkState(default_network_path_);
     if (!network) {
@@ -826,7 +836,7 @@ DeviceState* NetworkStateHandler::GetModifiableDeviceState(
     const std::string& device_path) const {
   ManagedState* managed = GetModifiableManagedState(&device_list_, device_path);
   if (!managed)
-    return nullptr;
+    return NULL;
   return managed->AsDeviceState();
 }
 
@@ -835,7 +845,7 @@ NetworkState* NetworkStateHandler::GetModifiableNetworkState(
   ManagedState* managed =
       GetModifiableManagedState(&network_list_, service_path);
   if (!managed)
-    return nullptr;
+    return NULL;
   return managed->AsNetworkState();
 }
 
@@ -848,7 +858,7 @@ ManagedState* NetworkStateHandler::GetModifiableManagedState(
     if (managed->path() == path)
       return managed;
   }
-  return nullptr;
+  return NULL;
 }
 
 NetworkStateHandler::ManagedStateList* NetworkStateHandler::GetManagedList(
@@ -860,7 +870,7 @@ NetworkStateHandler::ManagedStateList* NetworkStateHandler::GetManagedList(
       return &device_list_;
   }
   NOTREACHED();
-  return nullptr;
+  return NULL;
 }
 
 void NetworkStateHandler::OnNetworkConnectionStateChanged(
@@ -876,7 +886,7 @@ void NetworkStateHandler::OnNetworkConnectionStateChanged(
           network->path());
       default_network_path_.clear();
       SortNetworkList();
-      NotifyDefaultNetworkChanged(nullptr);
+      NotifyDefaultNetworkChanged(NULL);
     }
   }
   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 5b202f0..39d161d 100644
--- a/chromeos/network/shill_property_handler.cc
+++ b/chromeos/network/shill_property_handler.cc
@@ -9,6 +9,8 @@
 #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"
@@ -16,8 +18,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"
 
@@ -55,17 +57,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();
     }
@@ -73,13 +75,11 @@ 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,9 +123,10 @@ 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(
@@ -155,17 +156,20 @@ 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));
   }
 }
 
@@ -173,26 +177,34 @@ 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,
@@ -200,19 +212,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)
-                 << " For: " << path;
+  NET_LOG_DEBUG("Request Properties: " + ManagedState::TypeToString(type),
+                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();
   }
@@ -231,12 +243,13 @@ void ShillPropertyHandler::ManagerPropertiesCallback(
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& properties) {
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    NET_LOG(ERROR) << "ManagerPropertiesCallback Failed: " << call_status;
+    NET_LOG_ERROR("ManagerPropertiesCallback",
+                  base::StringPrintf("Failed: %d", 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());
   }
 
@@ -258,15 +271,12 @@ 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);
-    return;
-  }
-  NET_LOG(DEBUG) << "ManagerPropertyChanged: " << key;
-  if (key == shill::kServiceCompleteListProperty) {
+  } else if (key == shill::kServiceCompleteListProperty) {
     const base::ListValue* vlist = GetListValue(key, value);
     if (vlist) {
       listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
@@ -307,8 +317,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)
-                 << ": " << entries.GetSize();
+  NET_LOG_DEBUG("UpdateProperties: " + ManagedState::TypeToString(type),
+                base::StringPrintf("%" PRIuS, entries.GetSize()));
   for (base::ListValue::const_iterator iter = entries.begin();
        iter != entries.end(); ++iter) {
     std::string path;
@@ -331,10 +341,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())
@@ -346,8 +356,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) {
@@ -360,7 +370,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);
@@ -370,7 +380,7 @@ void ShillPropertyHandler::UpdateAvailableTechnologies(
     const base::ListValue& technologies) {
   std::stringstream technologies_str;
   technologies_str << technologies;
-  NET_LOG(EVENT) << "AvailableTechnologies:" << technologies_str;
+  NET_LOG_EVENT("AvailableTechnologies:", technologies_str.str());
   available_technologies_.clear();
   for (base::ListValue::const_iterator iter = technologies.begin();
        iter != technologies.end(); ++iter) {
@@ -386,7 +396,7 @@ void ShillPropertyHandler::UpdateEnabledTechnologies(
     const base::ListValue& technologies) {
   std::stringstream technologies_str;
   technologies_str << technologies;
-  NET_LOG(EVENT) << "EnabledTechnologies:" << technologies_str;
+  NET_LOG_EVENT("EnabledTechnologies:", technologies_str.str());
   enabled_technologies_.clear();
   for (base::ListValue::const_iterator iter = technologies.begin();
        iter != technologies.end(); ++iter) {
@@ -403,7 +413,7 @@ void ShillPropertyHandler::UpdateUninitializedTechnologies(
     const base::ListValue& technologies) {
   std::stringstream technologies_str;
   technologies_str << technologies;
-  NET_LOG(EVENT) << "UninitializedTechnologies:" << technologies_str;
+  NET_LOG_EVENT("UninitializedTechnologies:", technologies_str.str());
   uninitialized_technologies_.clear();
   for (base::ListValue::const_iterator iter = technologies.begin();
        iter != technologies.end(); ++iter) {
@@ -422,8 +432,9 @@ 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(
@@ -431,14 +442,14 @@ void ShillPropertyHandler::GetPropertiesCallback(
     const std::string& path,
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& properties) {
-  NET_LOG(DEBUG) << "GetPropertiesCallback: "
-                 << ManagedState::TypeToString(type) << " For: " << path;
+  NET_LOG_DEBUG("GetPropertiesCallback: " + ManagedState::TypeToString(type),
+                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 for: " << path << ": "
-                   << call_status;
+    NET_LOG_DEBUG("Failed to get properties",
+                  base::StringPrintf("%s: %d", path.c_str(), call_status));
     return;
   }
   listener_->UpdateManagedStateProperties(type, path, properties);
@@ -469,7 +480,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);
   }
 
@@ -488,13 +499,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(
@@ -515,15 +526,16 @@ 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) << "Failed to get IP Config properties: " << ip_config_path
-                   << ": " << call_status << ", For: " << path;
+    NET_LOG_EVENT(
+        base::StringPrintf("Failed to get IP Config properties: %s: %d",
+                           ip_config_path.c_str(), call_status), 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 a53487b..2e77fb7 100644
--- a/chromeos/network/shill_property_util.cc
+++ b/chromeos/network/shill_property_util.cc
@@ -4,8 +4,6 @@
 
 #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"
@@ -315,38 +313,18 @@ bool DoIdentifyingPropertiesMatch(const base::DictionaryValue& new_properties,
   return new_identifying.Equals(&old_identifying);
 }
 
-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 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 GetHomeProviderFromProperty(const base::Value& value,
diff --git a/chromeos/network/shill_property_util.h b/chromeos/network/shill_property_util.h
index cc9da9c..3637c6a 100644
--- a/chromeos/network/shill_property_util.h
+++ b/chromeos/network/shill_property_util.h
@@ -82,13 +82,8 @@ bool DoIdentifyingPropertiesMatch(
     const base::DictionaryValue& new_properties,
     const base::DictionaryValue& old_properties);
 
-// 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);
+// Returns true if |key| corresponds to a passphrase property.
+bool IsPassphraseKey(const std::string& key);
 
 // Parses |value| (which should be a Dictionary). Returns true and sets
 // |home_provider_id| if |value| was succesfully parsed.