Defer NetworkConfigurationHandler::CreateShillConfiguration callback

chromeos/network/network_configuration_handler.cc

Index: chromeos/network/network_configuration_handler.cc
diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index 03bf1fbe0b4f9e8ee0fd5c4ee91c008654b897d2..bdc8c0c433c2bb1af630cedab88cd16358f53122 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -79,8 +79,7 @@ void LogConfigProperties(const std::string& desc,
 // Service and delete the service from each profile. Triggers either
 // |callback| on success or |error_callback| on failure, and calls
 // |handler|->ProfileEntryDeleterCompleted() on completion to delete itself.
-class NetworkConfigurationHandler::ProfileEntryDeleter
-    : public base::SupportsWeakPtr<ProfileEntryDeleter> {
+class NetworkConfigurationHandler::ProfileEntryDeleter {
  public:
   ProfileEntryDeleter(NetworkConfigurationHandler* handler,
                       const std::string& service_path,
@@ -93,7 +92,8 @@ class NetworkConfigurationHandler::ProfileEntryDeleter
         guid_(guid),
         source_(source),
         callback_(callback),
-        error_callback_(error_callback) {}
+        error_callback_(error_callback),
+        weak_ptr_factory_(this) {}
 
   void Run() {
     DBusThreadManager::Get()
@@ -101,7 +101,7 @@ class NetworkConfigurationHandler::ProfileEntryDeleter
         ->GetLoadableProfileEntries(
             dbus::ObjectPath(service_path_),
             base::Bind(&ProfileEntryDeleter::GetProfileEntriesToDeleteCallback,
-                       AsWeakPtr()));
+                       weak_ptr_factory_.GetWeakPtr()));
   }
 
  private:
@@ -138,9 +138,9 @@ class NetworkConfigurationHandler::ProfileEntryDeleter
       DBusThreadManager::Get()->GetShillProfileClient()->DeleteEntry(
           dbus::ObjectPath(profile_path), entry_path,
           base::Bind(&ProfileEntryDeleter::ProfileEntryDeletedCallback,
-                     AsWeakPtr(), profile_path, entry_path),
-          base::Bind(&ProfileEntryDeleter::ShillErrorCallback, AsWeakPtr(),
-                     profile_path, entry_path));
+                     weak_ptr_factory_.GetWeakPtr(), profile_path, entry_path),
+          base::Bind(&ProfileEntryDeleter::ShillErrorCallback,
+                     weak_ptr_factory_.GetWeakPtr(), profile_path, entry_path));
     }
   }
 
@@ -183,6 +183,8 @@ class NetworkConfigurationHandler::ProfileEntryDeleter
   // Map of pending profile entry deletions, indexed by profile path.
   std::map<std::string, std::string> profile_delete_entries_;
 
+  base::WeakPtrFactory<ProfileEntryDeleter> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ProfileEntryDeleter);
 };
 
@@ -206,7 +208,8 @@ void NetworkConfigurationHandler::GetShillProperties(
   DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
       dbus::ObjectPath(service_path),
       base::Bind(&NetworkConfigurationHandler::GetPropertiesCallback,
-                 AsWeakPtr(), callback, error_callback, service_path));
+                 weak_ptr_factory_.GetWeakPtr(), callback, error_callback,
+                 service_path));
 }
 
 void NetworkConfigurationHandler::SetShillProperties(
@@ -243,10 +246,10 @@ void NetworkConfigurationHandler::SetShillProperties(
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperties(
       dbus::ObjectPath(service_path), *properties_to_set,
       base::Bind(&NetworkConfigurationHandler::SetPropertiesSuccessCallback,
-                 AsWeakPtr(), service_path, base::Passed(&properties_copy),
-                 source, callback),
+                 weak_ptr_factory_.GetWeakPtr(), service_path,
+                 base::Passed(&properties_copy), source, callback),
       base::Bind(&NetworkConfigurationHandler::SetPropertiesErrorCallback,
-                 AsWeakPtr(), service_path, error_callback));
+                 weak_ptr_factory_.GetWeakPtr(), service_path, error_callback));
 
   // If we set the StaticIPConfig property, request an IP config refresh
   // after calling SetProperties.
@@ -272,9 +275,9 @@ void NetworkConfigurationHandler::ClearShillProperties(
   DBusThreadManager::Get()->GetShillServiceClient()->ClearProperties(
       dbus::ObjectPath(service_path), names,
       base::Bind(&NetworkConfigurationHandler::ClearPropertiesSuccessCallback,
-                 AsWeakPtr(), service_path, names, callback),
+                 weak_ptr_factory_.GetWeakPtr(), service_path, names, callback),
       base::Bind(&NetworkConfigurationHandler::ClearPropertiesErrorCallback,
-                 AsWeakPtr(), service_path, error_callback));
+                 weak_ptr_factory_.GetWeakPtr(), service_path, error_callback));
 }
 
 void NetworkConfigurationHandler::CreateShillConfiguration(
@@ -322,8 +325,8 @@ void NetworkConfigurationHandler::CreateShillConfiguration(
       properties_to_set->DeepCopy());
   manager->ConfigureServiceForProfile(
       dbus::ObjectPath(profile_path), *properties_to_set,
-      base::Bind(&NetworkConfigurationHandler::RunCreateNetworkCallback,
-                 AsWeakPtr(), profile_path, source,
+      base::Bind(&NetworkConfigurationHandler::ConfigurationCompleted,
+                 weak_ptr_factory_.GetWeakPtr(), profile_path, source,
                  base::Passed(&properties_copy), callback),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
                  "Config.CreateConfiguration Failed", "", error_callback));
@@ -367,16 +370,39 @@ void NetworkConfigurationHandler::SetNetworkProfile(
       dbus::ObjectPath(service_path), shill::kProfileProperty,
       profile_path_value,
       base::Bind(&NetworkConfigurationHandler::SetNetworkProfileCompleted,
-                 AsWeakPtr(), service_path, profile_path, source, callback),
+                 weak_ptr_factory_.GetWeakPtr(), service_path, profile_path,
+                 source, callback),
       base::Bind(&SetNetworkProfileErrorCallback, service_path, profile_path,
                  error_callback));
 }
 
+// NetworkStateHandlerObserver methods
+void NetworkConfigurationHandler::NetworkListChanged() {
+  for (auto iter = configure_callbacks_.begin();
+       iter != configure_callbacks_.end();) {
+    const std::string& service_path = iter->first;
+    const NetworkState* state =
+        network_state_handler_->GetNetworkStateFromServicePath(service_path,
+                                                               true);
+    if (!state) {
+      NET_LOG(ERROR) << "Configured network not in list: " << service_path;
+      ++iter;
+      continue;
+    }
+    network_handler::ServiceResultCallback& callback = iter->second;
+    callback.Run(service_path, state->guid());
+    iter = configure_callbacks_.erase(iter);
+  }
+}
+
+void NetworkConfigurationHandler::OnShuttingDown() {
+  network_state_handler_->RemoveObserver(this, FROM_HERE);
+}
+
 // NetworkConfigurationHandler Private methods
 
 NetworkConfigurationHandler::NetworkConfigurationHandler()
-    : network_state_handler_(NULL) {
-}
+    : network_state_handler_(nullptr), weak_ptr_factory_(this) {}
 
 NetworkConfigurationHandler::~NetworkConfigurationHandler() {
 }
@@ -386,30 +412,37 @@ void NetworkConfigurationHandler::Init(
     NetworkDeviceHandler* network_device_handler) {
   network_state_handler_ = network_state_handler;
   network_device_handler_ = network_device_handler;
+
+  // Observer is removed in OnShuttingDown() observer override.
+  network_state_handler_->AddObserver(this, FROM_HERE);
 }
 
-void NetworkConfigurationHandler::RunCreateNetworkCallback(
+void NetworkConfigurationHandler::ConfigurationCompleted(
     const std::string& profile_path,
     NetworkConfigurationObserver::Source source,
     std::unique_ptr<base::DictionaryValue> configure_properties,
     const network_handler::ServiceResultCallback& callback,
     const dbus::ObjectPath& service_path) {
-  if (!callback.is_null()) {
-    std::string guid;
-    configure_properties->GetStringWithoutPathExpansion(
-        ::onc::network_config::kGUID, &guid);
-    DCHECK(!guid.empty());
-    callback.Run(service_path.value(), guid);
-  }
+  // Shill should send a network list update, but to ensure that Shill sends
+  // the newly configured properties immediately, request an update here.
+  network_state_handler_->RequestUpdateForNetwork(service_path.value());
+
+  // Notify observers immediately. (Note: Currently this is primarily used
+  // by tests).
   for (auto& observer : observers_) {
     observer.OnConfigurationCreated(service_path.value(), profile_path,
                                     *configure_properties, source);
   }
-  // This may also get called when CreateConfiguration is used to update an
-  // existing configuration, so request a service update just in case.
-  // TODO(pneubeck): Separate 'Create' and 'Update' calls and only trigger
-  // this on an update.
-  network_state_handler_->RequestUpdateForNetwork(service_path.value());
+
+  if (callback.is_null())
+    return;
+
+  // |configure_callbacks_| will get triggered when NetworkStateHandler
+  // notifies this that a state list update has occurred. |service_path|
+  // is unique per configuration. In the unlikely case that an existing
+  // configuration is reconfigured twice without a NetworkStateHandler update,
+  // (the UI should prevent that) the first callback will not get called.
+  configure_callbacks_[service_path.value()] = callback;
 }
 
 void NetworkConfigurationHandler::ProfileEntryDeleterCompleted(