Wire up component cloud policy to device local accounts.

chrome/browser/chromeos/policy/device_local_account_policy_service.cc

Index: chrome/browser/chromeos/policy/device_local_account_policy_service.cc
diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_service.cc b/chrome/browser/chromeos/policy/device_local_account_policy_service.cc
index c42c4fce30aac9a71d896980bb31fd072041f2e4..f9604fce8b0dda91be78514ebc97ca3b0be2089f 100644
--- a/chrome/browser/chromeos/policy/device_local_account_policy_service.cc
+++ b/chrome/browser/chromeos/policy/device_local_account_policy_service.cc
@@ -9,12 +9,12 @@
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
-#include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/path_service.h"
 #include "base/sequenced_task_runner.h"
+#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
@@ -31,6 +31,7 @@
 #include "components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h"
 #include "components/policy/core/common/cloud/device_management_service.h"
 #include "components/policy/core/common/cloud/system_policy_request_context.h"
+#include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "policy/policy_constants.h"
 #include "policy/proto/device_management_backend.pb.h"
@@ -72,29 +73,26 @@ scoped_ptr<CloudPolicyClient> CreateClient(
 }
 
 // Get the subdirectory of the cache directory in which force-installed
-// extensions are cached for |account_id|.
-std::string GetCacheSubdirectoryForAccountID(const std::string& account_id) {
+// extensions are cached for |account_id|. This is also used for the
+// component policy cache.
+std::string EncodeAccountId(const std::string& account_id) {
   return base::HexEncode(account_id.c_str(), account_id.size());
 }
 
 // Cleans up the cache directory by removing subdirectories that are not found
 // in |subdirectories_to_keep|. Only caches whose cache directory is found in
 // |subdirectories_to_keep| may be running while the clean-up is in progress.
-void DeleteOrphanedExtensionCaches(
+void DeleteOrphanedCaches(
+    const base::FilePath& cache_root_dir,
     const std::set<std::string>& subdirectories_to_keep) {
-  base::FilePath cache_root_dir;
-  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
-                         &cache_root_dir));
   base::FileEnumerator enumerator(cache_root_dir,
                                   false,
                                   base::FileEnumerator::DIRECTORIES);
   for (base::FilePath path = enumerator.Next(); !path.empty();
        path = enumerator.Next()) {
     const std::string subdirectory(path.BaseName().MaybeAsASCII());
-    if (subdirectories_to_keep.find(subdirectory) ==
-        subdirectories_to_keep.end()) {
+    if (!ContainsKey(subdirectories_to_keep, subdirectory))
       base::DeleteFile(path, true);
-    }
   }
 }
 
@@ -105,8 +103,8 @@ void DeleteObsoleteExtensionCache(const std::string& account_id_to_delete) {
   base::FilePath cache_root_dir;
   CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
                          &cache_root_dir));
-  const base::FilePath path = cache_root_dir
-      .Append(GetCacheSubdirectoryForAccountID(account_id_to_delete));
+  const base::FilePath path =
+      cache_root_dir.Append(EncodeAccountId(account_id_to_delete));
   if (base::DirectoryExists(path))
     base::DeleteFile(path, true);
 }
@@ -115,11 +113,13 @@ void DeleteObsoleteExtensionCache(const std::string& account_id_to_delete) {
 
 DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker(
     const DeviceLocalAccount& account,
+    const base::FilePath& component_policy_cache_path,
     scoped_ptr<DeviceLocalAccountPolicyStore> store,
     scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner)
     : account_id_(account.account_id),
       user_id_(account.user_id),
+      component_policy_cache_path_(component_policy_cache_path),
       store_(store.Pass()),
       external_data_manager_(external_data_manager),
       core_(PolicyNamespaceKey(dm_protocol::kChromePublicAccountPolicyType,
@@ -130,9 +130,7 @@ DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker(
   CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
                          &cache_root_dir));
   extension_loader_ = new chromeos::DeviceLocalAccountExternalPolicyLoader(
-      store_.get(),
-      cache_root_dir.Append(
-          GetCacheSubdirectoryForAccountID(account.account_id)));
+      store_.get(), cache_root_dir.Append(EncodeAccountId(account.account_id)));
 }
 
 DeviceLocalAccountPolicyBroker::~DeviceLocalAccountPolicyBroker() {
@@ -182,6 +180,11 @@ std::string DeviceLocalAccountPolicyBroker::GetDisplayName() const {
   return display_name;
 }
 
+base::FilePath DeviceLocalAccountPolicyBroker::GetComponentPolicyCachePath()
+    const {
+  return component_policy_cache_path_;
+}
+
 DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService(
     chromeos::SessionManagerClient* session_manager_client,
     chromeos::DeviceSettingsService* device_settings_service,
@@ -207,6 +210,8 @@ DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService(
                          UpdateAccountListIfNonePending,
                      base::Unretained(this)))),
       weak_factory_(this) {
+  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_COMPONENT_POLICY,
+                         &component_policy_cache_root_));
   external_data_service_.reset(new DeviceLocalAccountExternalDataService(
       this,
       external_data_service_backend_task_runner,
@@ -414,6 +419,7 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() {
                                                              store.get());
       broker.reset(new DeviceLocalAccountPolicyBroker(
           *it,
+          component_policy_cache_root_.Append(EncodeAccountId(it->account_id)),
           store.Pass(),
           external_data_manager,
           base::MessageLoopProxy::current()));
@@ -432,16 +438,7 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() {
       policy_brokers_[it->user_id]->Initialize();
     }
 
-    if (orphan_cache_deletion_state_ == NOT_STARTED) {
-      subdirectories_to_keep.insert(
-          GetCacheSubdirectoryForAccountID(it->account_id));
-    }
-  }
-
-  std::set<std::string> obsolete_account_ids;
-  for (PolicyBrokerMap::const_iterator it = old_policy_brokers.begin();
-       it != old_policy_brokers.end(); ++it) {
-    obsolete_account_ids.insert(it->second->account_id());
+    subdirectories_to_keep.insert(EncodeAccountId(it->account_id));
   }
 
   if (orphan_cache_deletion_state_ == NOT_STARTED) {
@@ -453,12 +450,17 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() {
     // orphaned cache directories not found in |subdirectories_to_keep| from the
     // cache directory.
     orphan_cache_deletion_state_ = IN_PROGRESS;
+
+    base::FilePath cache_root_dir;
+    CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,

[Bernhard Bauer, 2014/06/19 16:59:27]

Why a CHECK instead of a DCHECK?

[Joao da Silva, 2014/06/19 17:04:56]

Because this can't possibly fail, but if it does then we can't proceed. Also, a
DCHECK will skip the Get() call in release builds.

[Bernhard Bauer, 2014/06/19 17:22:44]

Eh, "Can't possibly fail" is pretty much the definition of DCHECK :) (see
http://dev.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-)

The usual way to make sure the code is compiled in in Release builds as well is
to extract the result to a variable and then DCHECK that.

+                           &cache_root_dir));
     extension_cache_task_runner_->PostTaskAndReply(
         FROM_HERE,
-        base::Bind(&DeleteOrphanedExtensionCaches, subdirectories_to_keep),
-        base::Bind(&DeviceLocalAccountPolicyService::
-                       OnOrphanedExtensionCachesDeleted,
-                   weak_factory_.GetWeakPtr()));
+        base::Bind(
+            &DeleteOrphanedCaches, cache_root_dir, subdirectories_to_keep),
+        base::Bind(
+            &DeviceLocalAccountPolicyService::OnOrphanedExtensionCachesDeleted,
+            weak_factory_.GetWeakPtr()));
 
     // Start the extension caches for all brokers. These belong to accounts in
     // |account_ids| and are not affected by the clean-up.
@@ -476,6 +478,16 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() {
     }
   }
 
+  // Purge the component policy caches of any accounts that have been removed.
+  // Do this only after any obsolete brokers have been destroyed.
+  // TODO(joaodasilva): for now this must be posted to the FILE thread,
+  // to avoid racing with the ComponentCloudPolicyStore. Use a task runner
+  // once that class supports another background thread too.
+  content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE,
+                                   base::Bind(&DeleteOrphanedCaches,
+                                              component_policy_cache_root_,
+                                              subdirectories_to_keep));
+
   FOR_EACH_OBSERVER(Observer, observers_, OnDeviceLocalAccountsChanged());
 }
 
@@ -492,6 +504,7 @@ void DeviceLocalAccountPolicyService::DeleteBrokers(PolicyBrokerMap* map) {
           weak_factory_.GetWeakPtr(),
           it->second->account_id()));
     }
+    FOR_EACH_OBSERVER(Observer, observers_, OnBrokerShutdown(it->second));
     delete it->second;
   }
   map->clear();