Cleanup network event logging, add to ManagedNetworkConfigurationHandler

chromeos/network/network_state.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/network_state.h"
 
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_profile_handler.h"
@@ skipping 140 matching lines @@
     return GetBooleanValue(key, value, &activate_over_non_cellular_networks_);
   } else if (key == shill::kOutOfCreditsProperty) {
     return GetBooleanValue(key, value, &cellular_out_of_credits_);
   }
   return false;
 }
 
 bool NetworkState::InitialPropertiesReceived(
     const base::DictionaryValue& properties) {
   NET_LOG_DEBUG("InitialPropertiesReceived", path());
-  bool changed = UpdateName(properties);
+  bool changed = false;
+  if (!properties.HasKey(shill::kTypeProperty)) {

[pneubeck (no reviews), 2014/03/21 09:33:42]

isn't in this case the NetworkState broken and will never be "repaired"?
can we ensure that such NetworkState is destructed/removed immediately? if not
and NetworkStateHandler still calls set_update_received(), we might even use
such incomplete states.

Alternatively, instead of checking (e.g. in GetNetworkState) for
update_received, we could use a new function
  bool NetworkState::IsValid() {
    return update_received() && !type_.empty();
  }

[stevenjb, 2014/03/21 17:08:51]

I put these checks in everywhere to track down the actual culprit, but yes, this
would be a major bug. It shouldn't ever happen, I don't think we need to add
special handling (certainly not in this CL), but if it ever did happen I thought
it would be better to log a specific error rather than get an error in
shill_property_util::GetNameFromProperties.

+    NET_LOG_ERROR("NetworkState has no type",
+                  shill_property_util::GetNetworkIdFromProperties(properties));
+  } else {
+    changed |= UpdateName(properties);
+  }
   bool had_ca_cert_nss = has_ca_cert_nss_;
   has_ca_cert_nss_ = IsCaCertNssSet(properties);
   changed |= had_ca_cert_nss != has_ca_cert_nss_;
   return changed;
 }
 
 void NetworkState::GetProperties(base::DictionaryValue* dictionary) const {
   // Keep care that these properties are the same as in |PropertyChanged|.
   dictionary->SetStringWithoutPathExpansion(shill::kNameProperty, name());
   dictionary->SetStringWithoutPathExpansion(shill::kTypeProperty, type());
@@ skipping 104 matching lines @@
           connection_state == shill::kStateConfiguration ||
           connection_state == shill::kStateCarrier);
 }
 
 // static
 std::string NetworkState::IPConfigProperty(const char* key) {
   return base::StringPrintf("%s.%s", shill::kIPConfigProperty, key);
 }
 
 }  // namespace chromeos