ManagedNetworkConfigurationHandler: Expose status about policy application.

Before, it was hard or impossible to track whether a policy application is currently running.
Now a notification (PoliciesApplied) and the current status (AnyPolicyApplicationRunning) are exposed.

BUG=425049

Review URL: https://codereview.chromium.org/661803005

Cr-Commit-Position: refs/heads/master@{#301817}

1 files changed, 60 insertions, 36 deletions

diff --git a/chromeos/network/policy_applicator.cc b/chromeos/network/policy_applicator.cc
index 2c1e720..334c106 100644
--- a/chromeos/network/policy_applicator.cc
+++ b/chromeos/network/policy_applicator.cc
@@ -46,12 +46,12 @@ const base::DictionaryValue* GetByGUID(
 }  // namespace
 
 PolicyApplicator::PolicyApplicator(
-    base::WeakPtr<ConfigurationHandler> handler,
     const NetworkProfile& profile,
     const GuidToPolicyMap& all_policies,
     const base::DictionaryValue& global_network_config,
+    ConfigurationHandler* handler,
     std::set<std::string>* modified_policies)
-    : handler_(handler), profile_(profile) {
+    : handler_(handler), profile_(profile), weak_ptr_factory_(this) {
   global_network_config_.MergeDictionary(&global_network_config);
   remaining_policies_.swap(*modified_policies);
   for (GuidToPolicyMap::const_iterator it = all_policies.begin();
@@ -60,21 +60,30 @@ PolicyApplicator::PolicyApplicator(
   }
 }
 
+PolicyApplicator::~PolicyApplicator() {
+  STLDeleteValues(&all_policies_);
+  VLOG(1) << "Destroying PolicyApplicator for " << profile_.userhash;
+}
+
 void PolicyApplicator::Run() {
   DBusThreadManager::Get()->GetShillProfileClient()->GetProperties(
       dbus::ObjectPath(profile_.path),
-      base::Bind(&PolicyApplicator::GetProfilePropertiesCallback, this),
-      base::Bind(&LogErrorMessage, FROM_HERE));
+      base::Bind(&PolicyApplicator::GetProfilePropertiesCallback,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&PolicyApplicator::GetProfilePropertiesError,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
-void PolicyApplicator::GetProfilePropertiesCallback(
-    const base::DictionaryValue& profile_properties) {
-  if (!handler_) {
-    LOG(WARNING) << "Handler destructed during policy application to profile "
-                 << profile_.ToDebugString();
-    return;
+void PolicyApplicator::ProfileEntryFinished(const std::string& entry) {
+  pending_get_entry_calls_.erase(entry);
+  if (pending_get_entry_calls_.empty()) {
+    ApplyRemainingPolicies();
+    NotifyConfigurationHandlerAndFinish();
   }
+}
 
+void PolicyApplicator::GetProfilePropertiesCallback(
+    const base::DictionaryValue& profile_properties) {
   VLOG(2) << "Received properties for profile " << profile_.ToDebugString();
   const base::ListValue* entries = NULL;
   if (!profile_properties.GetListWithoutPathExpansion(
@@ -82,6 +91,7 @@ void PolicyApplicator::GetProfilePropertiesCallback(
     LOG(ERROR) << "Profile " << profile_.ToDebugString()
                << " doesn't contain the property "
                << shill::kEntriesProperty;
+    NotifyConfigurationHandlerAndFinish();
     return;
   }
 
@@ -90,23 +100,34 @@ void PolicyApplicator::GetProfilePropertiesCallback(
     std::string entry;
     (*it)->GetAsString(&entry);
 
+    pending_get_entry_calls_.insert(entry);
     DBusThreadManager::Get()->GetShillProfileClient()->GetEntry(
         dbus::ObjectPath(profile_.path),
         entry,
-        base::Bind(&PolicyApplicator::GetEntryCallback, this, entry),
-        base::Bind(&LogErrorMessage, FROM_HERE));
+        base::Bind(&PolicyApplicator::GetEntryCallback,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   entry),
+        base::Bind(&PolicyApplicator::GetEntryError,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   entry));
+  }
+  if (pending_get_entry_calls_.empty()) {
+    ApplyRemainingPolicies();
+    NotifyConfigurationHandlerAndFinish();
   }
 }
 
+void PolicyApplicator::GetProfilePropertiesError(
+    const std::string& error_name,
+    const std::string& error_message) {
+  LOG(ERROR) << "Could not retrieve properties of profile " << profile_.path
+             << ": " << error_message;
+  NotifyConfigurationHandlerAndFinish();
+}
+
 void PolicyApplicator::GetEntryCallback(
     const std::string& entry,
     const base::DictionaryValue& entry_properties) {
-  if (!handler_) {
-    LOG(WARNING) << "Handler destructed during policy application to profile "
-                 << profile_.ToDebugString();
-    return;
-  }
-
   VLOG(2) << "Received properties for entry " << entry << " of profile "
           << profile_.ToDebugString();
 
@@ -235,6 +256,16 @@ void PolicyApplicator::GetEntryCallback(
           entry_properties, shill_properties_to_update);
     }
   }
+
+  ProfileEntryFinished(entry);
+}
+
+void PolicyApplicator::GetEntryError(const std::string& entry,
+                                     const std::string& error_name,
+                                     const std::string& error_message) {
+  LOG(ERROR) << "Could not retrieve entry " << entry << " of profile "
+             << profile_.path << ": " << error_message;
+  ProfileEntryFinished(entry);
 }
 
 void PolicyApplicator::DeleteEntry(const std::string& entry) {
@@ -271,21 +302,8 @@ void PolicyApplicator::WriteNewShillConfiguration(
     handler_->CreateConfigurationFromPolicy(shill_dictionary);
 }
 
-PolicyApplicator::~PolicyApplicator() {
-  ApplyRemainingPolicies();
-  STLDeleteValues(&all_policies_);
-  // Notify the handler about all policies being applied, so that the network
-  // lists can be updated.
-  if (handler_)
-    handler_->OnPoliciesApplied();
-}
-
 void PolicyApplicator::ApplyRemainingPolicies() {
-  if (!handler_) {
-    LOG(WARNING) << "Handler destructed during policy application to profile "
-                 << profile_.ToDebugString();
-    return;
-  }
+  DCHECK(pending_get_entry_calls_.empty());
 
   // Write all queued configurations now.
   for (ScopedVector<base::DictionaryValue>::const_iterator it =
@@ -294,12 +312,12 @@ void PolicyApplicator::ApplyRemainingPolicies() {
        ++it) {
     handler_->CreateConfigurationFromPolicy(**it);
   }
+  new_shill_configurations_.clear();
 
-  if (remaining_policies_.empty())
-    return;
+  VLOG_IF(2, !remaining_policies_.empty())
+      << "Create new managed network configurations in profile"
+      << profile_.ToDebugString() << ".";
 
-  VLOG(2) << "Create new managed network configurations in profile"
-          << profile_.ToDebugString() << ".";
   // All profile entries were compared to policies. |remaining_policies_|
   // contains all modified policies that didn't match any entry. For these
   // remaining policies, new configurations have to be created.
@@ -320,6 +338,12 @@ void PolicyApplicator::ApplyRemainingPolicies() {
     WriteNewShillConfiguration(
         *shill_dictionary, *network_policy, false /* write now */);
   }
+  remaining_policies_.clear();
+}
+
+void PolicyApplicator::NotifyConfigurationHandlerAndFinish() {
+  weak_ptr_factory_.InvalidateWeakPtrs();
+  handler_->OnPoliciesApplied(profile_);
 }
 
 }  // namespace chromeos