This is the first pass at making GetIPConfigs asynchronous.

chrome/browser/ui/webui/options/chromeos/internet_options_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 "chrome/browser/ui/webui/options/chromeos/internet_options_handler.h"
 
 #include <ctype.h>
 
 #include <map>
 #include <string>
@@ skipping 1209 matching lines @@
   chromeos::CrosRequestNetworkServiceProperties(
       network->service_path(),
       base::Bind(&InternetOptionsHandler::PopulateDictionaryDetailsCallback,
                  weak_factory_.GetWeakPtr(), network));
 }
 
 void InternetOptionsHandler::PopulateDictionaryDetailsCallback(
     const chromeos::Network* network,
     const std::string& service_path,
     const base::DictionaryValue* shill_properties) {
+  // Have to copy the properties because the object will be out of scope when
+  // this function call completes (it's owned by the calling function).
+  base::DictionaryValue* shill_props_copy = shill_properties->DeepCopy();
+  cros_->GetIPConfigs(
+      network->device_path(),
+      chromeos::NetworkLibrary::FORMAT_COLON_SEPARATED_HEX,
+      base::Bind(&InternetOptionsHandler::PopulateIPConfigsCallback,
+                 weak_factory_.GetWeakPtr(),
+                 network,

[Dmitry Polukhin, 2012/11/12 23:02:28]

I haven't write this code initially so I might not understand how it works. But
this CL makes code even more async and I'm worry that network could be deleted
by the time when PopulateIPConfigsCallback is called. Comment in
network_library.h about FindNetworkByPath/FindNetworkByUniqueId said that we
shouldn't store it. I know that code was async even before this CL so it doesn't
do something new. But perhaps we should pass network by ID.

[stevenjb, 2012/11/12 23:09:43]

Yikes.

We should never be passing a Network* in an asynchronous call. Both
PopulateDictionaryDetailsCallback and PopulateIPConfigsCallback should be
passing service_path, not Network*. Thanks for catching this!

Let's fix this here please!

[Greg Spencer (Chromium), 2012/11/13 00:04:59]

Fixed.

+                 base::Owned(shill_props_copy)));
+}
+
+void InternetOptionsHandler::PopulateIPConfigsCallback(
+    const chromeos::Network* network,
+    base::DictionaryValue* shill_properties,
+    const chromeos::NetworkIPConfigVector& ipconfigs,
+    const std::string& hardware_address) {
   if (VLOG_IS_ON(2)) {
     std::string properties_json;
     base::JSONWriter::WriteWithOptions(shill_properties,
                                        base::JSONWriter::OPTIONS_PRETTY_PRINT,
                                        &properties_json);
     VLOG(2) << "Shill Properties: " << std::endl << properties_json;
   }
 
   Profile::FromWebUI(web_ui())->GetProxyConfigTracker()->UISetCurrentNetwork(
       network->service_path());
 
   const chromeos::NetworkUIData& ui_data = network->ui_data();
   const chromeos::NetworkPropertyUIData property_ui_data(ui_data);
   const base::DictionaryValue* onc =
       cros_->FindOncForNetwork(network->unique_id());
 
   base::DictionaryValue dictionary;
-  std::string hardware_address;
-  chromeos::NetworkIPConfigVector ipconfigs = cros_->GetIPConfigs(
-      network->device_path(), &hardware_address,
-      chromeos::NetworkLibrary::FORMAT_COLON_SEPARATED_HEX);
   if (!hardware_address.empty())
     dictionary.SetString(kTagHardwareAddress, hardware_address);
 
   // The DHCP IPConfig contains the values that are actually in use at the
   // moment, even if some are overridden by static IP values.
   scoped_ptr<DictionaryValue> ipconfig_dhcp(new DictionaryValue);
   std::string ipconfig_name_servers;
   for (chromeos::NetworkIPConfigVector::const_iterator it = ipconfigs.begin();
        it != ipconfigs.end(); ++it) {
     const chromeos::NetworkIPConfig& ipconfig = *it;
@@ skipping 36 matching lines @@
       name_server_type = kNameServerTypeUser;
     }
     if (ipconfig_name_servers == kGoogleNameServers) {
       name_server_type = kNameServerTypeGoogle;
     }
     SetValueDictionary(&dictionary,
                        kDictionaryStaticIp,
                        static_ip_dict.release(),
                        property_ui_data);
   } else {
-    LOG(ERROR) << "Unable to fetch IP configuration for " << service_path;
+    LOG(ERROR) << "Unable to fetch IP configuration for "
+               << network->service_path();
     // If we were unable to fetch shill_properties for some reason,
     // then just go with some defaults.
     dictionary.SetBoolean(kIpConfigAutoConfig, false);
     dictionary.Set(kDictionarySavedIp, new DictionaryValue);
     SetValueDictionary(&dictionary, kDictionaryStaticIp, new DictionaryValue,
                        property_ui_data);
   }
 
   chromeos::ConnectionType type = network->type();
   dictionary.SetInteger(kTagType, type);
@@ skipping 494 matching lines @@
 
   dictionary->SetBoolean(kTagWimaxEnabled, cros_->wimax_enabled());
   dictionary->SetBoolean(kTagWimaxAvailable, cros_->wimax_available());
   dictionary->SetBoolean(kTagWimaxBusy, cros_->wimax_busy());
   // TODO(kevers): The use of 'offline_mode' is not quite correct.  Update once
   // we have proper back-end support.
   dictionary->SetBoolean(kTagAirplaneMode, cros_->offline_mode());
 }
 
 }  // namespace options