Added RefreshPolicies to PolicyService.

chrome/browser/policy/policy_service_impl.cc

Index: chrome/browser/policy/policy_service_impl.cc
diff --git a/chrome/browser/policy/policy_service_impl.cc b/chrome/browser/policy/policy_service_impl.cc
index a9111665457d6d43b39e561f87ff43af4b71de63..9ef32fd00c736f5f5ddc78bf6aa44fae2060c473 100644
--- a/chrome/browser/policy/policy_service_impl.cc
+++ b/chrome/browser/policy/policy_service_impl.cc
@@ -4,11 +4,33 @@
 
 #include "chrome/browser/policy/policy_service_impl.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/observer_list.h"
 #include "base/stl_util.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
 
 namespace policy {
 
+namespace {
+
+// Helper to invoke |callbacks| asynchronously.
+void InvokeCallbacks(std::vector<base::Closure>* callbacks) {
+  for (std::vector<base::Closure>::iterator it = callbacks->begin();
+       it != callbacks->end(); ++it) {
+    it->Run();
+  }
+}
+
+// Helper to resolve the overloading of PostTask.
+void PostTask(BrowserThread::ID id, const base::Closure& callback) {
+  BrowserThread::PostTask(id, FROM_HERE, callback);
+}
+
+}  // namespace
+
 struct PolicyServiceImpl::Entry {
   PolicyMap policies;
   ObserverList<PolicyService::Observer, true> observers;
@@ -18,6 +40,7 @@ struct PolicyServiceImpl::ProviderData {
   ConfigurationPolicyProvider* provider;
   ConfigurationPolicyObserverRegistrar registrar;
   PolicyMap policies;
+  bool refresh_pending;
 };
 
 PolicyServiceImpl::PolicyServiceImpl(const Providers& providers) {
@@ -27,6 +50,7 @@ PolicyServiceImpl::PolicyServiceImpl(const Providers& providers) {
     ProviderData* data = new ProviderData;
     data->provider = provider;
     data->registrar.Init(provider, this);
+    data->refresh_pending = false;
     if (provider->IsInitializationComplete())
       provider->Provide(&data->policies);
     else
@@ -35,7 +59,7 @@ PolicyServiceImpl::PolicyServiceImpl(const Providers& providers) {
   }
   // There are no observers yet, but calls to GetPolicies() should already get
   // the processed policy values.
-  MergeAndTriggerUpdates();
+  MergeAndTriggerUpdates(false);
 }
 
 PolicyServiceImpl::~PolicyServiceImpl() {
@@ -75,12 +99,26 @@ bool PolicyServiceImpl::IsInitializationComplete() const {
   return initialization_complete_;
 }
 
+void PolicyServiceImpl::RefreshPolicies(const base::Closure& callback) {
+  refresh_callbacks_.push_back(callback);
+
+  // Some providers might invoke OnUpdatePolicy synchronously while handling
+  // RefreshPolicies. Flag all with refresh_pending before starting to refresh.
+  ProviderList::iterator it;
+  for (it = providers_.begin(); it != providers_.end(); ++it)
+    (*it)->refresh_pending = true;
+  for (it = providers_.begin(); it != providers_.end(); ++it)
+    (*it)->provider->RefreshPolicies();
+}
+
 void PolicyServiceImpl::OnUpdatePolicy(ConfigurationPolicyProvider* provider) {
   ProviderList::iterator it = GetProviderData(provider);
   if (it == providers_.end())
     return;
   provider->Provide(&(*it)->policies);
-  MergeAndTriggerUpdates();
+  bool did_refresh = (*it)->refresh_pending;
+  (*it)->refresh_pending = false;
+  MergeAndTriggerUpdates(did_refresh);
 }
 
 void PolicyServiceImpl::OnProviderGoingAway(
@@ -88,9 +126,10 @@ void PolicyServiceImpl::OnProviderGoingAway(
   ProviderList::iterator it = GetProviderData(provider);
   if (it == providers_.end())
     return;
+  bool did_refresh = (*it)->refresh_pending;
   delete *it;
   providers_.erase(it);
-  MergeAndTriggerUpdates();
+  MergeAndTriggerUpdates(did_refresh);
 }
 
 PolicyServiceImpl::Entry* PolicyServiceImpl::GetOrCreate(
@@ -122,15 +161,21 @@ void PolicyServiceImpl::MaybeCleanup(const PolicyNamespace& ns) {
   }
 }
 
-void PolicyServiceImpl::MergeAndTriggerUpdates() {
+void PolicyServiceImpl::MergeAndTriggerUpdates(bool is_refresh) {
   // TODO(joaodasilva): do this for each namespace once the providers also
   // provide policy for more namespaces.
+
   PolicyMap policies;
+  bool refresh_pending = false;
   for (ProviderList::iterator it = providers_.begin();
        it != providers_.end(); ++it) {
     policies.MergeFrom((*it)->policies);
+    refresh_pending |= (*it)->refresh_pending;
   }
 
+  if (refresh_pending)

[Mattias Nissler (ping if slow), 2012/04/26 11:08:52]

This results in all updates being blocked until the last provider answers. This
can potentially put us into big trouble. I think we should not abort here, but
update the effective polices and send OnPolicyUpdate events. Just don't invoke
the callbacks.

[Joao da Silva, 2012/04/26 14:43:58]

Done.

+    return;
+
   Entry* entry = GetOrCreate(std::make_pair(POLICY_DOMAIN_CHROME, ""));
   if (!policies.Equals(entry->policies)) {
     // Swap first, so that observers that call GetPolicies() see the current
@@ -160,6 +205,22 @@ void PolicyServiceImpl::MergeAndTriggerUpdates() {
       }
     }
   }
+
+  // Invoke all the callbacks if a refresh has just fully completed.
+  if (is_refresh && !refresh_pending) {
+    // Some policies (e.g. URLBlacklist) post tasks to other message loops
+    // before they start enforcing updated policy values; make sure those tasks
+    // have finished after a policy update.
+    // Updates of the URLBlacklist are done on IO, after building the blacklist
+    // on FILE, which is initiated from IO.

[Mattias Nissler (ping if slow), 2012/04/26 11:08:52]

This doesn't belong here. Policies might trigger various asynchronous events
(e.g. extension installation), and this is not the right place to wait for all
these effects to take place.

[Joao da Silva, 2012/04/26 14:43:58]

Right; this was introduced to fix automation tests, so I've moved the code
there.

+    std::vector<base::Closure>* callbacks = new std::vector<base::Closure>();
+    callbacks->swap(refresh_callbacks_);
+    PostTask(BrowserThread::IO,
+        base::Bind(&PostTask, BrowserThread::FILE,
+            base::Bind(&PostTask, BrowserThread::IO,
+                base::Bind(&PostTask, BrowserThread::UI,
+                    base::Bind(InvokeCallbacks, base::Owned(callbacks))))));
+  }
 }
 
 }  // namespace policy