Added a PolicyDomainDescriptor.

chrome/browser/policy/cloud/component_cloud_policy_service.cc

 // Copyright (c) 2013 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/policy/cloud/component_cloud_policy_service.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/pickle.h"
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "chrome/browser/policy/cloud/component_cloud_policy_store.h"
 #include "chrome/browser/policy/cloud/component_cloud_policy_updater.h"
 #include "chrome/browser/policy/cloud/resource_cache.h"
+#include "chrome/browser/policy/policy_domain_descriptor.h"

[bartfab (slow), 2013/05/21 12:10:56]

Nit: This should be included in the header file here.

[Joao da Silva, 2013/05/21 17:50:14]

Added a forward decl.

 #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace em = enterprise_management;
 
 namespace policy {
 
+namespace {
+
+void GetComponentIds(scoped_refptr<const PolicyDomainDescriptor>& descriptor,
+                     std::set<std::string>* set) {
+  const PolicyDomainDescriptor::SchemaMap& map = descriptor->components();
+  for (PolicyDomainDescriptor::SchemaMap::const_iterator it = map.begin();
+       it != map.end(); ++it) {
+    set->insert(it->first);
+  }
+}
+
+}  // namespace
+
 const char ComponentCloudPolicyService::kComponentNamespaceCache[] =
     "component-namespace-cache";
 
 ComponentCloudPolicyService::Delegate::~Delegate() {}
 
 // Owns the objects that live on the background thread, and posts back to UI
 // to the service whenever the policy changes.
 class ComponentCloudPolicyService::Backend
     : public ComponentCloudPolicyStore::Delegate {
  public:
@@ skipping 20 matching lines @@
   void SetCredentials(const std::string& username, const std::string& dm_token);
 
   // Passes a policy protobuf to the backend, to start its validation and
   // eventual download of the policy data on the background thread.
   // This is ignored if the backend isn't connected.
   void UpdateExternalPolicy(scoped_ptr<em::PolicyFetchResponse> response);
 
   // ComponentCloudPolicyStore::Delegate implementation:
   virtual void OnComponentCloudPolicyStoreUpdated() OVERRIDE;
 
-  // Passes the current list of components in |domain|, so that the disk cache
+  // Passes the current descriptor of a domain, so that the disk cache
   // can purge components that aren't being tracked anymore.
-  void SetCurrentComponents(PolicyDomain domain, const StringSet* components);
+  void RegisterPolicyDomain(
+      scoped_refptr<const PolicyDomainDescriptor> descriptor);
 
  private:
+  typedef std::map<PolicyDomain, scoped_refptr<const PolicyDomainDescriptor> >
+      DomainMap;
+
   scoped_ptr<ComponentMap> ReadCachedComponents();
 
   base::WeakPtr<ComponentCloudPolicyService> service_;
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
   scoped_ptr<ResourceCache> cache_;
   scoped_ptr<ComponentCloudPolicyStore> store_;
   scoped_ptr<ComponentCloudPolicyUpdater> updater_;
+  DomainMap domain_map_;
 
   DISALLOW_COPY_AND_ASSIGN(Backend);
 };
 
 ComponentCloudPolicyService::Backend::Backend(
     base::WeakPtr<ComponentCloudPolicyService> service,
     scoped_refptr<base::SequencedTaskRunner> task_runner,
     scoped_ptr<ResourceCache> cache)
     : service_(service),
       task_runner_(task_runner),
       cache_(cache.Pass()) {}
 
 ComponentCloudPolicyService::Backend::~Backend() {}
 
 void ComponentCloudPolicyService::Backend::Init() {
   DCHECK(!store_);
   store_.reset(new ComponentCloudPolicyStore(this, cache_.get()));
 }
 
 void ComponentCloudPolicyService::Backend::FinalizeInit() {
-  // Read the components that were cached in the last SetCurrentComponents()
+  // Read the components that were cached in the last RegisterPolicyDomain()
   // calls for each domain.
   scoped_ptr<ComponentMap> components = ReadCachedComponents();
 
   // Read the initial policy.
   store_->Load();
   scoped_ptr<PolicyBundle> policy(new PolicyBundle);
   policy->CopyFrom(store_->policy());
 
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
@@ skipping 22 matching lines @@
 void ComponentCloudPolicyService::Backend::UpdateExternalPolicy(
     scoped_ptr<em::PolicyFetchResponse> response) {
   if (updater_)
     updater_->UpdateExternalPolicy(response.Pass());
 }
 
 void ComponentCloudPolicyService::Backend::
     OnComponentCloudPolicyStoreUpdated() {
   scoped_ptr<PolicyBundle> bundle(new PolicyBundle);
   bundle->CopyFrom(store_->policy());
+  for (DomainMap::iterator i = domain_map_.begin(); i != domain_map_.end(); ++i)

[bartfab (slow), 2013/05/21 12:10:56]

|it| is the established short name for an iterator. |i| sounds like an integer
to me.

[Joao da Silva, 2013/05/21 17:50:14]

Done.

+    i->second->FilterBundle(bundle.get());
+
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&ComponentCloudPolicyService::OnPolicyUpdated,
                  service_,
                  base::Passed(&bundle)));
 }
 
-void ComponentCloudPolicyService::Backend::SetCurrentComponents(
-    PolicyDomain domain,
-    const StringSet* components) {
+void ComponentCloudPolicyService::Backend::RegisterPolicyDomain(
+    scoped_refptr<const PolicyDomainDescriptor> descriptor) {
   // Store the current list of components in the cache.
+  StringSet ids;
   std::string policy_type;
-  if (ComponentCloudPolicyStore::GetPolicyType(domain, &policy_type)) {
+  if (ComponentCloudPolicyStore::GetPolicyType(descriptor->domain(),
+                                               &policy_type)) {
+    GetComponentIds(descriptor, &ids);
     Pickle pickle;
-    for (StringSet::const_iterator it = components->begin();
-         it != components->end(); ++it) {
+    for (StringSet::iterator it = ids.begin(); it != ids.end(); ++it)

[bartfab (slow), 2013/05/21 12:10:56]

Why is the iterator no longer const?

[Joao da Silva, 2013/05/21 17:50:14]

I don't know why this changed, reverted.

       pickle.WriteString(*it);
-    }
     std::string data(reinterpret_cast<const char*>(pickle.data()),
                      pickle.size());
     cache_->Store(kComponentNamespaceCache, policy_type, data);
   }
 
+  domain_map_[descriptor->domain()] = descriptor;
+
   // Purge any components that have been removed.
   if (store_)
-    store_->Purge(domain, *components);
+    store_->Purge(descriptor->domain(), ids);
 }
 
 scoped_ptr<ComponentCloudPolicyService::ComponentMap>
     ComponentCloudPolicyService::Backend::ReadCachedComponents() {
   scoped_ptr<ComponentMap> components(new ComponentMap);
   std::map<std::string, std::string> contents;
   cache_->LoadAllSubkeys(kComponentNamespaceCache, &contents);
   for (std::map<std::string, std::string>::iterator it = contents.begin();
        it != contents.end(); ++it) {
     PolicyDomain domain;
@@ skipping 78 matching lines @@
     client_->RemoveObserver(this);
     client_ = NULL;
 
     backend_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&Backend::Disconnect, base::Unretained(backend_)));
   }
 }
 
 void ComponentCloudPolicyService::RegisterPolicyDomain(
-    PolicyDomain domain,
-    const std::set<std::string>& current_ids) {
+    scoped_refptr<const PolicyDomainDescriptor> descriptor) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  DCHECK(SupportsDomain(domain));
+  DCHECK(SupportsDomain(descriptor->domain()));
 
-  // Send the new set to the backend, to purge the cache.
-  backend_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&Backend::SetCurrentComponents,
-                 base::Unretained(backend_),
-                 domain,
-                 base::Owned(new StringSet(current_ids))));
+  // Send the new descriptor to the backend, to purge the cache.
+  backend_task_runner_->PostTask(FROM_HERE,
+                                 base::Bind(&Backend::RegisterPolicyDomain,
+                                            base::Unretained(backend_),
+                                            descriptor));
 
   // Register the current list of components for |domain| at the |client_|.
-  StringSet& registered_ids = registered_components_[domain];
+  StringSet current_ids;
+  GetComponentIds(descriptor, &current_ids);
+  StringSet& previous_ids = registered_components_[descriptor->domain()];
   if (client_ && is_initialized()) {
-    if (UpdateClientNamespaces(domain, registered_ids, current_ids))
+    if (UpdateClientNamespaces(descriptor->domain(), previous_ids, current_ids))
       delegate_->OnComponentCloudPolicyRefreshNeeded();
   }
-  registered_ids = current_ids;
+
+  previous_ids = current_ids;

[bartfab (slow), 2013/05/21 12:10:56]

I think |registered_ids| was clearer here. Now the line looks like you are
modifying the previous set of IDs when in fact, you are modifying the entry in
the |registered_components_| map.

[Joao da Silva, 2013/05/21 17:50:14]

Done.

 }
 
 void ComponentCloudPolicyService::OnPolicyFetched(CloudPolicyClient* client) {
   DCHECK_EQ(client_, client);
   // Pass each PolicyFetchResponse whose policy type is registered to the
   // Backend.
   const CloudPolicyClient::ResponseMap& responses = client_->responses();
   for (CloudPolicyClient::ResponseMap::const_iterator it = responses.begin();
        it != responses.end(); ++it) {
     const PolicyNamespaceKey& key(it->first);
@@ skipping 142 matching lines @@
     PolicyDomain domain,
     const StringSet& set) {
   std::string policy_type;
   if (ComponentCloudPolicyStore::GetPolicyType(domain, &policy_type)) {
     for (StringSet::const_iterator it = set.begin(); it != set.end(); ++it)
       client_->RemoveNamespaceToFetch(PolicyNamespaceKey(policy_type, *it));
   }
 }
 
 }  // namespace policy