Defer NetworkConfigurationHandler::CreateShillConfiguration callback

chromeos/network/network_configuration_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 "chromeos/network/network_configuration_handler.h"
 
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/format_macros.h"
@@ skipping 61 matching lines @@
     NET_LOG(USER) << desc << ": " << path + "." + iter.key() + "=" + v;
   }
 }
 
 }  // namespace
 
 // Helper class to request from Shill the profile entries associated with a
 // 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,
                       const std::string& guid,
                       NetworkConfigurationObserver::Source source,
                       const base::Closure& callback,
                       const network_handler::ErrorCallback& error_callback)
       : owner_(handler),
         service_path_(service_path),
         guid_(guid),
         source_(source),
         callback_(callback),
-        error_callback_(error_callback) {}
+        error_callback_(error_callback),
+        weak_ptr_factory_(this) {}
 
   void Run() {
     DBusThreadManager::Get()
         ->GetShillServiceClient()
         ->GetLoadableProfileEntries(
             dbus::ObjectPath(service_path_),
             base::Bind(&ProfileEntryDeleter::GetProfileEntriesToDeleteCallback,
-                       AsWeakPtr()));
+                       weak_ptr_factory_.GetWeakPtr()));
   }
 
  private:
   void GetProfileEntriesToDeleteCallback(
       DBusMethodCallStatus call_status,
       const base::DictionaryValue& profile_entries) {
     if (call_status != DBUS_METHOD_CALL_SUCCESS) {
       InvokeErrorCallback(service_path_, error_callback_,
                           "GetLoadableProfileEntriesFailed");
       // ProfileEntryDeleterCompleted will delete this.
@@ skipping 16 matching lines @@
         NET_LOG(ERROR) << "Multiple Profile Entries: " << profile_path << ": "
                        << entry_path;
         continue;
       }
       NET_LOG(DEBUG) << "Delete Profile Entry: " << profile_path << ": "
                      << entry_path;
       profile_delete_entries_[profile_path] = entry_path;
       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));
     }
   }
 
   void ProfileEntryDeletedCallback(const std::string& profile_path,
                                    const std::string& entry) {
     NET_LOG(DEBUG) << "Profile Entry Deleted: " << profile_path << ": "
                    << entry;
     profile_delete_entries_.erase(profile_path);
     if (!profile_delete_entries_.empty())
       return;
@@ skipping 22 matching lines @@
   NetworkConfigurationHandler* owner_;  // Unowned
   std::string service_path_;
   std::string guid_;
   NetworkConfigurationObserver::Source source_;
   base::Closure callback_;
   network_handler::ErrorCallback error_callback_;
 
   // 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);
 };
 
 // NetworkConfigurationHandler
 
 void NetworkConfigurationHandler::AddObserver(
     NetworkConfigurationObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void NetworkConfigurationHandler::RemoveObserver(
     NetworkConfigurationObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 void NetworkConfigurationHandler::GetShillProperties(
     const std::string& service_path,
     const network_handler::DictionaryResultCallback& callback,
     const network_handler::ErrorCallback& error_callback) {
   NET_LOG(USER) << "GetShillProperties: " << service_path;
   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(
     const std::string& service_path,
     const base::DictionaryValue& shill_properties,
     NetworkConfigurationObserver::Source source,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
   if (shill_properties.empty()) {
     if (!callback.is_null())
@@ skipping 16 matching lines @@
                                                      guid);
   }
 
   LogConfigProperties("SetProperty", service_path, *properties_to_set);
 
   std::unique_ptr<base::DictionaryValue> properties_copy(
       properties_to_set->DeepCopy());
   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.
   if (properties_to_set->HasKey(shill::kStaticIPConfigProperty))
     RequestRefreshIPConfigs(service_path);
 }
 
 void NetworkConfigurationHandler::ClearShillProperties(
     const std::string& service_path,
     const std::vector<std::string>& names,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
   if (names.empty()) {
     if (!callback.is_null())
       callback.Run();
     return;
   }
   NET_LOG(USER) << "ClearShillProperties: " << service_path;
   for (std::vector<std::string>::const_iterator iter = names.begin();
        iter != names.end(); ++iter) {
     NET_LOG(DEBUG) << "ClearProperty: " << service_path << "." << *iter;
   }
   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(
     const base::DictionaryValue& shill_properties,
     NetworkConfigurationObserver::Source source,
     const network_handler::ServiceResultCallback& callback,
     const network_handler::ErrorCallback& error_callback) {
   ShillManagerClient* manager =
       DBusThreadManager::Get()->GetShillManagerClient();
   std::string type;
@@ skipping 27 matching lines @@
     properties_to_set->SetStringWithoutPathExpansion(
         ::onc::network_config::kGUID, guid);
   }
 
   LogConfigProperties("Configure", type, *properties_to_set);
 
   std::unique_ptr<base::DictionaryValue> properties_copy(
       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));
 }
 
 void NetworkConfigurationHandler::RemoveConfiguration(
     const std::string& service_path,
     NetworkConfigurationObserver::Source source,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
@@ skipping 23 matching lines @@
     NetworkConfigurationObserver::Source source,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) {
   NET_LOG(USER) << "SetNetworkProfile: " << service_path << ": "
                 << profile_path;
   base::StringValue profile_path_value(profile_path);
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
       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() {
 }
 
 void NetworkConfigurationHandler::Init(
     NetworkStateHandler* network_state_handler,
     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(
     const std::string& service_path,
     const std::string& guid,
     NetworkConfigurationObserver::Source source,
     bool success) {
   if (success) {
     for (auto& observer : observers_)
       observer.OnConfigurationRemoved(service_path, guid, source);
@@ skipping 137 matching lines @@
 // static
 NetworkConfigurationHandler* NetworkConfigurationHandler::InitializeForTest(
     NetworkStateHandler* network_state_handler,
     NetworkDeviceHandler* network_device_handler) {
   NetworkConfigurationHandler* handler = new NetworkConfigurationHandler();
   handler->Init(network_state_handler, network_device_handler);
   return handler;
 }
 
 }  // namespace chromeos