Clean up NetworkState members

chrome/browser/chromeos/mobile/mobile_activator.cc

Index: chrome/browser/chromeos/mobile/mobile_activator.cc
diff --git a/chrome/browser/chromeos/mobile/mobile_activator.cc b/chrome/browser/chromeos/mobile/mobile_activator.cc
index 9475e6e8e06d78d5ee02cd41e6a80263e58f771a..a26674ea3e6fd2087ad0e795a5cb9921a9136d2e 100644
--- a/chrome/browser/chromeos/mobile/mobile_activator.cc
+++ b/chrome/browser/chromeos/mobile/mobile_activator.cc
@@ -165,7 +165,8 @@ MobileActivator::MobileActivator()
       initial_OTASP_attempts_(0),
       trying_OTASP_attempts_(0),
       final_OTASP_attempts_(0),
-      payment_reconnect_count_(0) {
+      payment_reconnect_count_(0),
+      weak_ptr_factory_(this) {
 }
 
 MobileActivator::~MobileActivator() {
@@ -268,9 +269,30 @@ void MobileActivator::InitiateActivation(const std::string& service_path) {
 }
 
 void MobileActivator::ContinueActivation() {
-  const NetworkState* network = GetNetworkState(service_path_);
-  if (!network ||
-      (network->payment_url().empty() && network->usage_url().empty()))
+  NetworkHandler::Get()->network_configuration_handler()->GetProperties(
+      service_path_,
+      base::Bind(&MobileActivator::GetPropertiesSuccess,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&MobileActivator::GetPropertiesFailure,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void MobileActivator::GetPropertiesSuccess(
+    const std::string& service_path,
+    const base::DictionaryValue& properties) {

[armansito, 2013/09/03 23:52:06]

This method is really meant for the initial property load at the start of the
state machine rather than a generic success callback for GetProperties, so could
you rename this method to reflect that? Maybe something like,
"InitialPropertiesReceived".

[stevenjb, 2013/09/04 00:04:50]

Actually it's called for ContinueActivation(), so renamed
GetPropertiesAndContinueActivation.

+  const DictionaryValue* payment_dict;
+  std::string usage_url, payment_url;
+  if (!properties.GetStringWithoutPathExpansion(

[gauravsh, 2013/09/04 00:06:52]

It shouldn't happen, but you should check for service_path == service_path_
here.

[stevenjb, 2013/09/04 01:04:11]

Done.

+          flimflam::kUsageURLProperty, &usage_url) ||
+      !properties.GetDictionaryWithoutPathExpansion(
+          flimflam::kPaymentPortalProperty, &payment_dict) ||
+      !payment_dict->GetStringWithoutPathExpansion(
+          flimflam::kPaymentPortalURL, &payment_url)) {
+    NET_LOG_ERROR("MobileActivator missing properties", service_path_);
+    return;
+  }
+
+  if (payment_url.empty() && usage_url.empty())
     return;
 
   DisableCertRevocationChecking();
@@ -279,13 +301,20 @@ void MobileActivator::ContinueActivation() {
   DictionaryValue auto_connect_property;
   auto_connect_property.SetBoolean(flimflam::kAutoConnectProperty, true);
   NetworkHandler::Get()->network_configuration_handler()->SetProperties(
-      network->path(),
+      service_path_,
       auto_connect_property,
       base::Bind(&base::DoNothing),
       network_handler::ErrorCallback());
   StartActivation();
 }
 
+void MobileActivator::GetPropertiesFailure(
+    const std::string& error_name,
+    scoped_ptr<base::DictionaryValue> error_data) {
+  NET_LOG_ERROR("MobileActivator GetProperties Failed: " + error_name,
+                service_path_);
+}
+
 void MobileActivator::OnSetTransactionStatus(bool success) {
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&MobileActivator::HandleSetTransactionStatus,