Revert 233048 "Support policies referencing external data for de..."

The CL broke compilation on Chrome OS.

> Support policies referencing external data for device-local accounts
> 
> This CL adds a DeviceLocalAccountExternalDataService that provides each
> device-local account with a DeviceLocalAccountExternalDataManager which
> fetches, caches and retrieves external policy data.
> 
> BUG=256635
> TEST=New browser test
> R=derat@chromium.org, joaodasilva@chromium.org
> 
> Review URL: https://codereview.chromium.org/25242002

TBR=bartfab@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233051 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/policy/cloud_external_data_manager_base_test_util.cc

-// Copyright 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/chromeos/policy/cloud_external_data_manager_base_test_util.h"
-
-#include "base/callback.h"
-#include "base/memory/weak_ptr.h"
-#include "base/sha1.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/values.h"
-#include "chrome/browser/policy/cloud/cloud_external_data_manager.h"
-#include "chrome/browser/policy/cloud/cloud_policy_core.h"
-#include "chrome/browser/policy/cloud/cloud_policy_store.h"
-#include "chrome/browser/policy/external_data_fetcher.h"
-#include "chrome/browser/policy/policy_map.h"
-#include "chrome/browser/policy/policy_types.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace policy {
-namespace test {
-
-void ExternalDataFetchCallback(scoped_ptr<std::string>* destination,
-                               const base::Closure& done_callback,
-                               scoped_ptr<std::string> data) {
-  *destination = data.Pass();
-  done_callback.Run();
-}
-
-scoped_ptr<base::DictionaryValue> ConstructExternalDataReference(
-    const std::string& url,
-    const std::string& data) {
-  const std::string hash = base::SHA1HashString(data);
-  scoped_ptr<base::DictionaryValue> metadata(new base::DictionaryValue);
-  metadata->SetStringWithoutPathExpansion("url", url);
-  metadata->SetStringWithoutPathExpansion("hash", base::HexEncode(hash.c_str(),
-                                                                  hash.size()));
-  return metadata.Pass();
-}
-
-void SetExternalDataReference(CloudPolicyCore* core,
-                              const std::string& policy,
-                              scoped_ptr<base::DictionaryValue> metadata) {
-  CloudPolicyStore* store = core->store();
-  ASSERT_TRUE(store);
-  PolicyMap policy_map;
-  policy_map.Set(policy,
-                 POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
-                 metadata.release(),
-                 new ExternalDataFetcher(store->external_data_manager(),
-                                         policy));
-  store->SetPolicyMapForTesting(policy_map);
-}
-
-}  // namespace test
-}  // namespace policy

chrome/browser/chromeos/policy/cloud_external_data_manager_base_test_util.h

-// Copyright 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_MANAGER_BASE_TEST_UTIL_H_
-#define CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_MANAGER_BASE_TEST_UTIL_H_
-
-#include <string>
-
-#include "base/callback_forward.h"
-#include "base/memory/scoped_ptr.h"
-
-namespace base {
-class DictionaryValue;
-}
-
-namespace policy {
-
-class CloudPolicyCore;
-
-namespace test {
-
-// Passes |data| to |destination| and invokes |done_callback| to indicate that
-// the |data| has been retrieved.
-void ExternalDataFetchCallback(scoped_ptr<std::string>* destination,
-                               const base::Closure& done_callback,
-                               scoped_ptr<std::string> data);
-
-// Constructs a value that points a policy referencing external data at |url|
-// and sets the expected hash of the external data to that of |data|.
-scoped_ptr<base::DictionaryValue> ConstructExternalDataReference(
-    const std::string& url,
-    const std::string& data);
-
-// TODO(bartfab): Makes an arbitrary |policy| in |core| reference external data
-// as specified in |metadata|. This is only done because there are no policies
-// that reference external data yet. Once the first such policy is added, it
-// will be sufficient to set its value to |metadata| and this method should be
-// removed.
-void SetExternalDataReference(CloudPolicyCore* core,
-                              const std::string& policy,
-                              scoped_ptr<base::DictionaryValue> metadata);
-
-}  // namespace test
-}  // namespace policy
-
-#endif  // CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_MANAGER_BASE_TEST_UTIL_H_

chrome/browser/chromeos/policy/device_local_account_external_data_manager.cc

-// Copyright 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/chromeos/policy/device_local_account_external_data_manager.h"
-
-#include "base/memory/scoped_ptr.h"
-#include "base/sequenced_task_runner.h"
-#include "chrome/browser/chromeos/policy/cloud_external_data_store.h"
-#include "chrome/browser/chromeos/policy/device_local_account_external_data_service.h"
-#include "chrome/browser/policy/cloud/resource_cache.h"
-#include "policy/policy_constants.h"
-
-namespace policy {
-
-DeviceLocalAccountExternalDataManager::DeviceLocalAccountExternalDataManager(
-    const std::string& account_id,
-    const PolicyDefinitionList* policy_definitions,
-    scoped_refptr<base::SequencedTaskRunner> backend_task_runner,
-    scoped_refptr<base::SequencedTaskRunner> io_task_runner,
-    ResourceCache* resource_cache)
-    : CloudExternalDataManagerBase(policy_definitions,
-                                   backend_task_runner,
-                                   io_task_runner) {
-  SetExternalDataStore(make_scoped_ptr(new CloudExternalDataStore(
-      account_id, backend_task_runner, resource_cache)));
-}
-
-DeviceLocalAccountExternalDataManager::
-    ~DeviceLocalAccountExternalDataManager() {
-  SetExternalDataStore(scoped_ptr<CloudExternalDataStore>());
-}
-
-void DeviceLocalAccountExternalDataManager::OnPolicyStoreLoaded() {
-  CloudExternalDataManagerBase::OnPolicyStoreLoaded();
-  // Proactively try to download and cache all external data referenced by
-  // policies.
-  FetchAll();
-}
-
-}  // namespace policy

chrome/browser/chromeos/policy/device_local_account_external_data_manager.h

-// Copyright 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA_MANAGER_H_
-#define CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA_MANAGER_H_
-
-#include <string>
-
-#include "base/basictypes.h"
-#include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h"
-
-namespace base {
-class SequencedTaskRunner;
-}
-
-namespace policy {
-
-struct PolicyDefinitionList;
-class ResourceCache;
-
-// Downloads, verifies, caches and retrieves external data referenced by
-// policies.
-// This is the implementation for device-local accounts on Chrome OS.
-// The manager is created by the DeviceLocalAccountExternalDataService and must
-// not outlive it because the |resource_cache| is owned by the service. However,
-// the service is not the manager's sole owner: The manager lives as long as
-// either the service or a DeviceLocalAccountPolicyProvider references it. This
-// is necessary because a device-local account can be removed from policy (thus
-// removing it from the service) while a session is in progress and a provider
-// exists for the account. The manager is only destroyed when neither service
-// nor provider reference it anymore.
-class DeviceLocalAccountExternalDataManager
-    : public CloudExternalDataManagerBase,
-      public base::RefCounted<DeviceLocalAccountExternalDataManager> {
- private:
-  friend class DeviceLocalAccountExternalDataService;
-  friend class base::RefCounted<DeviceLocalAccountExternalDataManager>;
-
-  // The |policy_definitions| are used to determine the maximum size that the
-  // data referenced by each policy can have. Download scheduling, verification,
-  // caching and retrieval tasks are done via the |backend_task_runner|, which
-  // must support file I/O. Network I/O is done via the |io_task_runner|. The
-  // manager is responsible for external data references by policies in
-  // |policy_store|. Downloaded external data is stored in the |resource_cache|.
-  // The data is keyed by |account_id|, allowing one cache to be shared by any
-  // number of accounts. To ensure synchronization of operations on the shared
-  // cache, all its users must access the cache via |backend_task_runner| only.
-  DeviceLocalAccountExternalDataManager(
-      const std::string& account_id,
-      const PolicyDefinitionList* policy_definitions,
-      scoped_refptr<base::SequencedTaskRunner> backend_task_runner,
-      scoped_refptr<base::SequencedTaskRunner> io_task_runner,
-      ResourceCache* resource_cache);
-  virtual ~DeviceLocalAccountExternalDataManager();
-
-  // CloudExternalDataManagerBase:
-  virtual void OnPolicyStoreLoaded() OVERRIDE;
-
-  DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountExternalDataManager);
-};
-
-}  // namespace policy
-
-#endif  // CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA_MANAGER_H_

chrome/browser/chromeos/policy/device_local_account_external_data_service.cc

-// Copyright 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/chromeos/policy/device_local_account_external_data_service.h"
-
-#include <set>
-
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "base/files/file_path.h"
-#include "base/location.h"
-#include "base/logging.h"
-#include "base/path_service.h"
-#include "base/sequenced_task_runner.h"
-#include "chrome/browser/policy/cloud/cloud_policy_store.h"
-#include "chromeos/chromeos_paths.h"
-#include "policy/policy_constants.h"
-
-namespace policy {
-
-DeviceLocalAccountExternalDataService::DeviceLocalAccountExternalDataService(
-    DeviceLocalAccountPolicyService* parent,
-    scoped_refptr<base::SequencedTaskRunner> backend_task_runner,
-    scoped_refptr<base::SequencedTaskRunner> io_task_runner)
-    : parent_(parent),
-      backend_task_runner_(backend_task_runner),
-      io_task_runner_(io_task_runner) {
-  base::FilePath cache_dir;
-  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA,
-                         &cache_dir));
-  resource_cache_.reset(new ResourceCache(cache_dir, backend_task_runner));
-  parent_->AddObserver(this);
-}
-
-DeviceLocalAccountExternalDataService::
-    ~DeviceLocalAccountExternalDataService() {
-  parent_->RemoveObserver(this);
-#if !defined(NDEBUG)
-  for (ExternalDataManagerMap::const_iterator it =
-           external_data_managers_.begin();
-       it != external_data_managers_.end(); ++it) {
-    DCHECK(it->second->HasOneRef());
-  }
-#endif  // !defined(NDEBUG)
-  backend_task_runner_->DeleteSoon(FROM_HERE, resource_cache_.release());
-}
-
-void DeviceLocalAccountExternalDataService::OnPolicyUpdated(
-    const std::string& user_id) {
-}
-
-void DeviceLocalAccountExternalDataService::OnDeviceLocalAccountsChanged() {
-  std::set<std::string> account_ids;
-  for (ExternalDataManagerMap::iterator it = external_data_managers_.begin();
-       it != external_data_managers_.end(); ) {
-    if (it->second->HasOneRef()) {
-      external_data_managers_.erase(it++);
-    } else {
-      account_ids.insert(it->first);
-      ++it;
-    }
-  }
-  backend_task_runner_->PostTask(FROM_HERE, base::Bind(
-      &ResourceCache::PurgeOtherKeys,
-      base::Unretained(resource_cache_.get()),
-      account_ids));
-}
-
-scoped_refptr<DeviceLocalAccountExternalDataManager>
-    DeviceLocalAccountExternalDataService::GetExternalDataManager(
-        const std::string& account_id,
-        CloudPolicyStore* policy_store) {
-  scoped_refptr<DeviceLocalAccountExternalDataManager>& external_data_manager =
-      external_data_managers_[account_id];
-  if (!external_data_manager) {
-    external_data_manager = new DeviceLocalAccountExternalDataManager(
-        account_id,
-        GetChromePolicyDefinitionList(),
-        backend_task_runner_,
-        io_task_runner_,
-        resource_cache_.get());
-  }
-  external_data_manager->SetPolicyStore(policy_store);
-  return external_data_manager;
-}
-
-}  // namespace policy

chrome/browser/chromeos/policy/device_local_account_external_data_service.h

-// Copyright 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA_SERVICE_H_
-#define CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA_SERVICE_H_
-
-#include <map>
-#include <string>
-
-#include "base/basictypes.h"
-#include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h"
-#include "chrome/browser/chromeos/policy/device_local_account_policy_service.h"
-#include "chrome/browser/policy/cloud/resource_cache.h"
-
-namespace base {
-class SequencedTaskRunner;
-}
-
-namespace policy {
-
-class CloudPolicyStore;
-
-// Provides DeviceLocalAccountExternalDataManagers for all device-local
-// accounts. This class owns the |resource_cache_| that the managers share.
-class DeviceLocalAccountExternalDataService
-    : public DeviceLocalAccountPolicyService::Observer {
- public:
-  DeviceLocalAccountExternalDataService(
-      DeviceLocalAccountPolicyService* parent,
-      scoped_refptr<base::SequencedTaskRunner> backend_task_runner,
-      scoped_refptr<base::SequencedTaskRunner> io_task_runner);
-  virtual ~DeviceLocalAccountExternalDataService();
-
-  // DeviceLocalAccountPolicyService::Observer:
-  virtual void OnPolicyUpdated(const std::string& user_id) OVERRIDE;
-  virtual void OnDeviceLocalAccountsChanged() OVERRIDE;
-
-  scoped_refptr<DeviceLocalAccountExternalDataManager>
-      GetExternalDataManager(const std::string& account_id,
-                             CloudPolicyStore* policy_store);
-
- private:
-  typedef std::map<std::string,
-                   scoped_refptr<DeviceLocalAccountExternalDataManager> >
-      ExternalDataManagerMap;
-
-  DeviceLocalAccountPolicyService* parent_;
-  scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;
-  scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
-  scoped_ptr<ResourceCache> resource_cache_;
-
-  ExternalDataManagerMap external_data_managers_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountExternalDataService);
-};
-
-}  // namespace policy
-
-#endif  // CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA_SERVICE_H_

chrome/browser/chromeos/policy/device_local_account_policy_provider.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/chromeos/policy/device_local_account_policy_provider.h"
 
 #include "base/bind.h"
-#include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h"
 #include "chrome/browser/policy/cloud/cloud_policy_core.h"
 #include "chrome/browser/policy/cloud/cloud_policy_service.h"
 #include "chrome/browser/policy/policy_bundle.h"
@@ -76,7 +75,6 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() {
       // Copy policy from the broker.
       bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
           .CopyFrom(broker->core()->store()->policy_map());
-      external_data_manager_ = broker->external_data_manager();
     } else {
       // Wait for the refresh to finish.
       return;

chrome/browser/chromeos/policy/device_local_account_policy_provider.h

@@ -9,9 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h"
 #include "chrome/browser/chromeos/policy/device_local_account_policy_service.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 
@@ -51,8 +49,6 @@ class DeviceLocalAccountPolicyProvider
   void UpdateFromBroker();
 
   const std::string user_id_;
-  scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager_;
-
   DeviceLocalAccountPolicyService* service_;
 
   bool store_initialized_;

chrome/browser/chromeos/policy/device_local_account_policy_service.cc

@@ -17,7 +17,6 @@
 #include "base/sequenced_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
-#include "chrome/browser/chromeos/policy/device_local_account_external_data_service.h"
 #include "chrome/browser/chromeos/policy/device_local_account_policy_store.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/policy/cloud/cloud_policy_client.h"
@@ -29,7 +28,6 @@
 #include "chromeos/dbus/session_manager_client.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/settings/cros_settings_provider.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "policy/policy_constants.h"
 
 namespace em = enterprise_management;
@@ -73,7 +71,7 @@ std::string GetCacheSubdirectoryForAccountID(const std::string& account_id) {
 void DeleteOrphanedExtensionCaches(
     const std::set<std::string>& subdirectories_to_keep) {
   base::FilePath cache_root_dir;
-  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
+  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
                          &cache_root_dir));
   base::FileEnumerator enumerator(cache_root_dir,
                                   false,
@@ -93,7 +91,7 @@ void DeleteOrphanedExtensionCaches(
 // the removal is in progress.
 void DeleteObsoleteExtensionCache(const std::string& account_id_to_delete) {
   base::FilePath cache_root_dir;
-  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
+  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
                          &cache_root_dir));
   const base::FilePath path = cache_root_dir
       .Append(GetCacheSubdirectoryForAccountID(account_id_to_delete));
@@ -106,18 +104,16 @@ void DeleteObsoleteExtensionCache(const std::string& account_id_to_delete) {
 DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker(
     const DeviceLocalAccount& account,
     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),
       store_(store.Pass()),
-      external_data_manager_(external_data_manager),
       core_(PolicyNamespaceKey(dm_protocol::kChromePublicAccountPolicyType,
                                store_->account_id()),
             store_.get(),
             task_runner) {
   base::FilePath cache_root_dir;
-  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
+  CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
                          &cache_root_dir));
   extension_loader_ = new chromeos::DeviceLocalAccountExternalPolicyLoader(
       store_.get(),
@@ -126,8 +122,6 @@ DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker(
 }
 
 DeviceLocalAccountPolicyBroker::~DeviceLocalAccountPolicyBroker() {
-  external_data_manager_->SetPolicyStore(NULL);
-  external_data_manager_->Disconnect();
 }
 
 void DeviceLocalAccountPolicyBroker::Initialize() {
@@ -136,8 +130,7 @@ void DeviceLocalAccountPolicyBroker::Initialize() {
 
 void DeviceLocalAccountPolicyBroker::ConnectIfPossible(
     chromeos::DeviceSettingsService* device_settings_service,
-    DeviceManagementService* device_management_service,
-    scoped_refptr<net::URLRequestContextGetter> request_context) {
+    DeviceManagementService* device_management_service) {
   if (core_.client())
     return;
 
@@ -147,11 +140,14 @@ void DeviceLocalAccountPolicyBroker::ConnectIfPossible(
     return;
 
   core_.Connect(client.Pass());
-  external_data_manager_->Connect(request_context);
   core_.StartRefreshScheduler();
   UpdateRefreshDelay();
 }
 
+void DeviceLocalAccountPolicyBroker::Disconnect() {
+  core_.Disconnect();
+}
+
 void DeviceLocalAccountPolicyBroker::UpdateRefreshDelay() {
   if (core_.refresh_scheduler()) {
     const Value* policy_value =
@@ -176,11 +172,7 @@ DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService(
     chromeos::DeviceSettingsService* device_settings_service,
     chromeos::CrosSettings* cros_settings,
     scoped_refptr<base::SequencedTaskRunner> store_background_task_runner,
-    scoped_refptr<base::SequencedTaskRunner> extension_cache_task_runner,
-    scoped_refptr<base::SequencedTaskRunner>
-        external_data_service_backend_task_runner,
-    scoped_refptr<base::SequencedTaskRunner> io_task_runner,
-    scoped_refptr<net::URLRequestContextGetter> request_context)
+    scoped_refptr<base::SequencedTaskRunner> extension_cache_task_runner)
     : session_manager_client_(session_manager_client),
       device_settings_service_(device_settings_service),
       cros_settings_(cros_settings),
@@ -189,28 +181,16 @@ DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService(
       orphan_cache_deletion_state_(NOT_STARTED),
       store_background_task_runner_(store_background_task_runner),
       extension_cache_task_runner_(extension_cache_task_runner),
-      request_context_(request_context),
       local_accounts_subscription_(cros_settings_->AddSettingsObserver(
           chromeos::kAccountsPrefDeviceLocalAccounts,
           base::Bind(&DeviceLocalAccountPolicyService::
                          UpdateAccountListIfNonePending,
                      base::Unretained(this)))),
       weak_factory_(this) {
-  external_data_service_.reset(new DeviceLocalAccountExternalDataService(
-      this,
-      external_data_service_backend_task_runner,
-      io_task_runner));
   UpdateAccountList();
 }
 
 DeviceLocalAccountPolicyService::~DeviceLocalAccountPolicyService() {
-  DCHECK(!request_context_);
-  DCHECK(policy_brokers_.empty());
-}
-
-void DeviceLocalAccountPolicyService::Shutdown() {
-  device_management_service_ = NULL;
-  request_context_ = NULL;
   DeleteBrokers(&policy_brokers_);
 }
 
@@ -223,8 +203,18 @@ void DeviceLocalAccountPolicyService::Connect(
   for (PolicyBrokerMap::iterator it(policy_brokers_.begin());
        it != policy_brokers_.end(); ++it) {
     it->second->ConnectIfPossible(device_settings_service_,
-                                  device_management_service_,
-                                  request_context_);
+                                  device_management_service_);
+  }
+}
+
+void DeviceLocalAccountPolicyService::Disconnect() {
+  DCHECK(device_management_service_);
+  device_management_service_ = NULL;
+
+  // Disconnect the brokers.
+  for (PolicyBrokerMap::iterator it(policy_brokers_.begin());
+       it != policy_brokers_.end(); ++it) {
+    it->second->Disconnect();
   }
 }
 
@@ -397,22 +387,16 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() {
                                             device_settings_service_,
                                             store_background_task_runner_));
       store->AddObserver(this);
-      scoped_refptr<DeviceLocalAccountExternalDataManager>
-          external_data_manager =
-              external_data_service_->GetExternalDataManager(it->account_id,
-                                                             store.get());
       broker.reset(new DeviceLocalAccountPolicyBroker(
           *it,
           store.Pass(),
-          external_data_manager,
           base::MessageLoopProxy::current()));
     }
 
     // Fire up the cloud connection for fetching policy for the account from
     // the cloud if this is an enterprise-managed device.
     broker->ConnectIfPossible(device_settings_service_,
-                              device_management_service_,
-                              request_context_);
+                              device_management_service_);
 
     policy_brokers_[it->user_id] = broker.release();
     if (!broker_initialized) {

chrome/browser/chromeos/policy/device_local_account_policy_service.h

@@ -16,7 +16,6 @@
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "chrome/browser/chromeos/extensions/device_local_account_external_policy_loader.h"
-#include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/policy/cloud/cloud_policy_core.h"
 #include "chrome/browser/policy/cloud/cloud_policy_store.h"
@@ -30,14 +29,9 @@ class DeviceSettingsService;
 class SessionManagerClient;
 }
 
-namespace net {
-class URLRequestContextGetter;
-}
-
 namespace policy {
 
 struct DeviceLocalAccount;
-class DeviceLocalAccountExternalDataService;
 class DeviceLocalAccountPolicyStore;
 class DeviceManagementService;
 
@@ -49,8 +43,6 @@ class DeviceLocalAccountPolicyBroker {
   DeviceLocalAccountPolicyBroker(
       const DeviceLocalAccount& account,
       scoped_ptr<DeviceLocalAccountPolicyStore> store,
-      scoped_refptr<DeviceLocalAccountExternalDataManager>
-          external_data_manager,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
   ~DeviceLocalAccountPolicyBroker();
 
@@ -68,16 +60,14 @@ class DeviceLocalAccountPolicyBroker {
   CloudPolicyCore* core() { return &core_; }
   const CloudPolicyCore* core() const { return &core_; }
 
-  scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager() {
-    return external_data_manager_;
-  }
-
   // Fire up the cloud connection for fetching policy for the account from the
   // cloud if this is an enterprise-managed device.
   void ConnectIfPossible(
       chromeos::DeviceSettingsService* device_settings_service,
-      DeviceManagementService* device_management_service,
-      scoped_refptr<net::URLRequestContextGetter> request_context);
+      DeviceManagementService* device_management_service);
+
+  // Destroy the cloud connection, stopping policy refreshes.
+  void Disconnect();
 
   // Reads the refresh delay from policy and configures the refresh scheduler.
   void UpdateRefreshDelay();
@@ -90,7 +80,6 @@ class DeviceLocalAccountPolicyBroker {
   const std::string account_id_;
   const std::string user_id_;
   const scoped_ptr<DeviceLocalAccountPolicyStore> store_;
-  scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager_;
   scoped_refptr<chromeos::DeviceLocalAccountExternalPolicyLoader>
       extension_loader_;
   CloudPolicyCore core_;
@@ -121,19 +110,15 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
       chromeos::DeviceSettingsService* device_settings_service,
       chromeos::CrosSettings* cros_settings,
       scoped_refptr<base::SequencedTaskRunner> store_background_task_runner,
-      scoped_refptr<base::SequencedTaskRunner> extension_cache_task_runner,
-      scoped_refptr<base::SequencedTaskRunner>
-          external_data_service_backend_task_runner,
-      scoped_refptr<base::SequencedTaskRunner> io_task_runner,
-      scoped_refptr<net::URLRequestContextGetter> request_context);
+      scoped_refptr<base::SequencedTaskRunner> extension_cache_task_runner);
   virtual ~DeviceLocalAccountPolicyService();
 
-  // Shuts down the service and prevents further policy fetches from the cloud.
-  void Shutdown();
-
   // Initializes the cloud policy service connection.
   void Connect(DeviceManagementService* device_management_service);
 
+  // Prevents further policy fetches from the cloud.
+  void Disconnect();
+
   // Get the policy broker for a given |user_id|. Returns NULL if that |user_id|
   // does not belong to an existing device-local account.
   DeviceLocalAccountPolicyBroker* GetBrokerForUser(const std::string& user_id);
@@ -190,8 +175,6 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
   // Find the broker for a given |store|. Returns NULL if |store| is unknown.
   DeviceLocalAccountPolicyBroker* GetBrokerForStore(CloudPolicyStore* store);
 
-  ObserverList<Observer, true> observers_;
-
   chromeos::SessionManagerClient* session_manager_client_;
   chromeos::DeviceSettingsService* device_settings_service_;
   chromeos::CrosSettings* cros_settings_;
@@ -222,9 +205,7 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
   const scoped_refptr<base::SequencedTaskRunner> store_background_task_runner_;
   const scoped_refptr<base::SequencedTaskRunner> extension_cache_task_runner_;
 
-  scoped_ptr<DeviceLocalAccountExternalDataService> external_data_service_;
-
-  scoped_refptr<net::URLRequestContextGetter> request_context_;
+  ObserverList<Observer, true> observers_;
 
   const scoped_ptr<chromeos::CrosSettings::ObserverSubscription>
       local_accounts_subscription_;

chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc

@@ -34,7 +34,6 @@
 #include "chrome/common/chrome_paths.h"
 #include "chromeos/chromeos_paths.h"
 #include "chromeos/dbus/power_policy_controller.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "policy/policy_constants.h"
 #include "policy/proto/cloud_policy.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -169,7 +168,6 @@ void DeviceLocalAccountPolicyServiceTestBase::SetUp() {
 }
 
 void DeviceLocalAccountPolicyServiceTestBase::TearDown() {
-  service_->Shutdown();
   service_.reset();
   extension_cache_task_runner_->RunUntilIdle();
   chromeos::DeviceSettingsTestBase::TearDown();
@@ -181,10 +179,7 @@ void DeviceLocalAccountPolicyServiceTestBase::CreatePolicyService() {
       &device_settings_service_,
       &cros_settings_,
       loop_.message_loop_proxy(),
-      extension_cache_task_runner_,
-      loop_.message_loop_proxy(),
-      loop_.message_loop_proxy(),
-      NULL));
+      extension_cache_task_runner_));
 }
 
 void DeviceLocalAccountPolicyServiceTestBase::
@@ -448,6 +443,13 @@ TEST_F(DeviceLocalAccountPolicyServiceTest, FetchPolicy) {
   EXPECT_TRUE(expected_policy_map_.Equals(
       broker->core()->store()->policy_map()));
   EXPECT_TRUE(service_->IsPolicyAvailableForUser(account_1_user_id_));
+
+  EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_))
+      .Times(0);
+  service_->Disconnect();
+  EXPECT_FALSE(broker->core()->client());
+  Mock::VerifyAndClearExpectations(&service_observer_);
+  EXPECT_TRUE(service_->IsPolicyAvailableForUser(account_1_user_id_));
 }
 
 TEST_F(DeviceLocalAccountPolicyServiceTest, RefreshPolicy) {
@@ -515,7 +517,7 @@ void DeviceLocalAccountPolicyExtensionCacheTest::SetUp() {
   DeviceLocalAccountPolicyServiceTestBase::SetUp();
   ASSERT_TRUE(cache_root_dir_.CreateUniqueTempDir());
   cache_root_dir_override_.reset(new base::ScopedPathOverride(
-      chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
+      chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
       cache_root_dir_.path()));
 
   cache_dir_1_ = GetCacheDirectoryForAccountID(kAccount1);

chrome/browser/chromeos/policy/user_cloud_external_data_manager_browsertest.cc

@@ -4,22 +4,28 @@
 
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/sha1.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h"
-#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base_test_util.h"
 #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h"
+#include "chrome/browser/policy/cloud/cloud_external_data_manager.h"
 #include "chrome/browser/policy/cloud/cloud_policy_core.h"
+#include "chrome/browser/policy/cloud/cloud_policy_store.h"
 #include "chrome/browser/policy/external_data_fetcher.h"
 #include "chrome/browser/policy/policy_map.h"
 #include "chrome/browser/policy/policy_service.h"
+#include "chrome/browser/policy/policy_types.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -38,9 +44,67 @@ namespace {
 
 const char kExternalDataPath[] = "policy/blank.html";
 
+void ExternalDataFetchCallback(scoped_ptr<std::string>* destination,
+                               const base::Closure& done_callback,
+                               scoped_ptr<std::string> data) {
+  destination->reset(data.release());
+  done_callback.Run();
+}
+
 }  // namespace
 
-typedef InProcessBrowserTest UserCloudExternalDataManagerTest;
+class UserCloudExternalDataManagerTest : public InProcessBrowserTest {
+ protected:
+  UserCloudExternalDataManagerTest();
+  virtual ~UserCloudExternalDataManagerTest();
+
+  scoped_ptr<base::DictionaryValue> ConstructMetadata(const std::string& url,
+                                                      const std::string& hash);
+  void SetExternalDataReference(const std::string& policy,
+                                scoped_ptr<base::DictionaryValue> metadata);
+
+  DISALLOW_COPY_AND_ASSIGN(UserCloudExternalDataManagerTest);
+};
+
+UserCloudExternalDataManagerTest::UserCloudExternalDataManagerTest() {
+}
+
+UserCloudExternalDataManagerTest::~UserCloudExternalDataManagerTest() {
+}
+
+scoped_ptr<base::DictionaryValue>
+    UserCloudExternalDataManagerTest::ConstructMetadata(
+        const std::string& url,
+        const std::string& hash) {
+  scoped_ptr<base::DictionaryValue> metadata(new base::DictionaryValue);
+  metadata->SetStringWithoutPathExpansion("url", url);
+  metadata->SetStringWithoutPathExpansion("hash", base::HexEncode(hash.c_str(),
+                                                                  hash.size()));
+  return metadata.Pass();
+}
+
+void UserCloudExternalDataManagerTest::SetExternalDataReference(
+    const std::string& policy,
+    scoped_ptr<base::DictionaryValue> metadata) {
+#if defined(OS_CHROMEOS)
+  UserCloudPolicyManagerChromeOS* policy_manager =
+      UserCloudPolicyManagerFactoryChromeOS::GetForProfile(
+          browser()->profile());
+#else
+  UserCloudPolicyManager* policy_manager =
+      UserCloudPolicyManagerFactory::GetForProfile(browser()->profile());
+#endif
+  ASSERT_TRUE(policy_manager);
+  CloudPolicyStore* store = policy_manager->core()->store();
+  ASSERT_TRUE(store);
+  PolicyMap policy_map;
+  policy_map.Set(policy,
+                 POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+                 metadata.release(),
+                 new ExternalDataFetcher(store->external_data_manager(),
+                                         policy));
+  store->SetPolicyMapForTesting(policy_map);
+}
 
 IN_PROC_BROWSER_TEST_F(UserCloudExternalDataManagerTest, FetchExternalData) {
   CloudExternalDataManagerBase::SetMaxExternalDataSizeForTesting(1000);
@@ -57,23 +121,13 @@ IN_PROC_BROWSER_TEST_F(UserCloudExternalDataManagerTest, FetchExternalData) {
   ASSERT_FALSE(external_data.empty());
 
   scoped_ptr<base::DictionaryValue> metadata =
-      test::ConstructExternalDataReference(url.spec(), external_data);
-#if defined(OS_CHROMEOS)
-  UserCloudPolicyManagerChromeOS* policy_manager =
-      UserCloudPolicyManagerFactoryChromeOS::GetForProfile(
-          browser()->profile());
-#else
-  UserCloudPolicyManager* policy_manager =
-      UserCloudPolicyManagerFactory::GetForProfile(browser()->profile());
-#endif
-  ASSERT_TRUE(policy_manager);
+      ConstructMetadata(url.spec(), base::SHA1HashString(external_data));
   // TODO(bartfab): The test injects an ExternalDataFetcher for an arbitrary
   // policy. This is only done because there are no policies that reference
   // external data yet. Once the first such policy is added, switch the test to
   // that policy and stop injecting a manually instantiated ExternalDataFetcher.
-  test::SetExternalDataReference(policy_manager->core(),
-                                 key::kHomepageLocation,
-                                 make_scoped_ptr(metadata->DeepCopy()));
+  SetExternalDataReference(key::kHomepageLocation,
+                           make_scoped_ptr(metadata->DeepCopy()));
   content::RunAllPendingInMessageLoop();
 
   ProfilePolicyConnector* policy_connector =
@@ -89,7 +143,7 @@ IN_PROC_BROWSER_TEST_F(UserCloudExternalDataManagerTest, FetchExternalData) {
   base::RunLoop run_loop;
   scoped_ptr<std::string> fetched_external_data;
   policy_entry->external_data_fetcher->Fetch(base::Bind(
-      &test::ExternalDataFetchCallback,
+      &ExternalDataFetchCallback,
       &fetched_external_data,
       run_loop.QuitClosure()));
   run_loop.Run();

chrome/browser/policy/browser_policy_connector.cc

@@ -307,11 +307,7 @@ void BrowserPolicyConnector::Init(
             chromeos::DeviceSettingsService::Get(),
             chromeos::CrosSettings::Get(),
             GetBackgroundTaskRunner(),
-            GetBackgroundTaskRunner(),
-            GetBackgroundTaskRunner(),
-            content::BrowserThread::GetMessageLoopProxyForThread(
-                content::BrowserThread::IO),
-            request_context));
+            GetBackgroundTaskRunner()));
     device_local_account_policy_service_->Connect(
         device_management_service_.get());
   }
@@ -362,7 +358,7 @@ void BrowserPolicyConnector::Shutdown() {
   if (device_cloud_policy_manager_)
     device_cloud_policy_manager_->Shutdown();
   if (device_local_account_policy_service_)
-    device_local_account_policy_service_->Shutdown();
+    device_local_account_policy_service_->Disconnect();
   global_user_cloud_policy_provider_.Shutdown();
 #endif
 

chrome/browser/policy/cloud/cloud_external_data_manager.cc

@@ -31,7 +31,6 @@ CloudExternalDataManager::~CloudExternalDataManager() {
 }
 
 void CloudExternalDataManager::SetPolicyStore(CloudPolicyStore* policy_store) {
-  weak_factory_.InvalidateWeakPtrs();
   policy_store_ = policy_store;
   if (policy_store_)
     policy_store_->SetExternalDataManager(weak_factory_.GetWeakPtr());

chrome/chrome_browser_chromeos.gypi

@@ -679,10 +679,6 @@
         'browser/chromeos/policy/device_cloud_policy_validator.h',
         'browser/chromeos/policy/device_local_account.cc',
         'browser/chromeos/policy/device_local_account.h',
-        'browser/chromeos/policy/device_local_account_external_data_manager.cc',
-        'browser/chromeos/policy/device_local_account_external_data_manager.h',
-        'browser/chromeos/policy/device_local_account_external_data_service.cc',
-        'browser/chromeos/policy/device_local_account_external_data_service.h',
         'browser/chromeos/policy/device_local_account_policy_provider.cc',
         'browser/chromeos/policy/device_local_account_policy_provider.h',
         'browser/chromeos/policy/device_local_account_policy_service.cc',

chrome/chrome_tests.gypi

@@ -1082,8 +1082,6 @@
         'browser/chromeos/login/wizard_in_process_browser_test.cc',
         'browser/chromeos/login/wizard_in_process_browser_test.h',
         'browser/chromeos/memory/oom_priority_manager_browsertest.cc',
-        'browser/chromeos/policy/cloud_external_data_manager_base_test_util.cc',
-        'browser/chromeos/policy/cloud_external_data_manager_base_test_util.h',
         'browser/chromeos/policy/device_local_account_browsertest.cc',
         'browser/chromeos/policy/device_policy_cros_browser_test.cc',
         'browser/chromeos/policy/device_policy_cros_browser_test.h',

chromeos/chromeos_paths.cc

@@ -33,12 +33,9 @@ const base::FilePath::CharType kUptimeFileName[] =
 const base::FilePath::CharType kUpdateRebootNeededUptimeFile[] =
     FILE_PATH_LITERAL("/var/run/chrome/update_reboot_needed_uptime");
 
-const base::FilePath::CharType kDeviceLocalAccountExtensionDir[] =
+const base::FilePath::CharType kDeviceLocalAccountCacheDir[] =
     FILE_PATH_LITERAL("/var/cache/device_local_account_extensions");
 
-const base::FilePath::CharType kDeviceLocalAccountExternalDataDir[] =
-    FILE_PATH_LITERAL("/var/cache/device_local_account_external_policy_data");
-
 bool PathProvider(int key, base::FilePath* result) {
   switch (key) {
     case FILE_DEFAULT_APP_ORDER:
@@ -59,11 +56,8 @@ bool PathProvider(int key, base::FilePath* result) {
     case FILE_UPDATE_REBOOT_NEEDED_UPTIME:
       *result = base::FilePath(kUpdateRebootNeededUptimeFile);
       break;
-    case DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS:
-      *result = base::FilePath(kDeviceLocalAccountExtensionDir);
-      break;
-    case DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA:
-      *result = base::FilePath(kDeviceLocalAccountExternalDataDir);
+    case DIR_DEVICE_LOCAL_ACCOUNT_CACHE:
+      *result = base::FilePath(kDeviceLocalAccountCacheDir);
       break;
     default:
       return false;

chromeos/chromeos_paths.h

@@ -27,13 +27,10 @@ enum {
                                      // store the uptime at which an update
                                      // became necessary. The file should be
                                      // cleared on boot.
-  DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,  // Directory under which a cache of
-                                        // force-installed extensions is
-                                        // maintained for each device-local
-                                        // account.
-  DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA,  // Directory where external data
-                                           // referenced by policies is cached
-                                           // for device-local accounts.
+  DIR_DEVICE_LOCAL_ACCOUNT_CACHE,  // Directory under which a cache of
+                                   // force-installed extensions is maintained
+                                   // for each device-local account.
+
   PATH_END
 };