diff options
author | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-20 22:43:23 +0000 |
---|---|---|
committer | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-20 22:43:23 +0000 |
commit | db25a7e9bf34e638a1cbb4c8974ca566c22eb62c (patch) | |
tree | 30e1a51b52b8d4e1acee5429de32a63f69e8b6c6 | |
parent | d84d57bc5729866a1b287356089b6574bd2d8b28 (diff) | |
download | chromium_src-db25a7e9bf34e638a1cbb4c8974ca566c22eb62c.zip chromium_src-db25a7e9bf34e638a1cbb4c8974ca566c22eb62c.tar.gz chromium_src-db25a7e9bf34e638a1cbb4c8974ca566c22eb62c.tar.bz2 |
Move ownership of the ComponentCloudPolicyService to the broker.
The ComponentCloudPolicyService needs to work with the SchemaRegistry,
so it was initially owned by the DeviceLocalAccountPolicyProvider
(which always has a valid reference to the registry). However, this
means that the ComponentCloudPolicyService can't precache any policies
until the session is started.
To enable precaching at enrollment time and at the login screen, the
ComponentCloudPolicyService has been moved to the broker instead.
This allows some code simplifications too, but requires changing the
ownership of the SchemaRegistry for device local accounts.
This CL is just moving code around; a subsequent CL will trigger
the precaching.
TBR=zelidrag@chromium.org
BUG=224596
Review URL: https://codereview.chromium.org/342233005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278847 0039d316-1c4b-4281-b951-d872f2087c98
10 files changed, 218 insertions, 176 deletions
diff --git a/chrome/browser/chromeos/policy/device_local_account_browsertest.cc b/chrome/browser/chromeos/policy/device_local_account_browsertest.cc index 59ad468..c58f8fe 100644 --- a/chrome/browser/chromeos/policy/device_local_account_browsertest.cc +++ b/chrome/browser/chromeos/policy/device_local_account_browsertest.cc @@ -421,10 +421,13 @@ class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest, EXPECT_EQ(chromeos::User::USER_TYPE_PUBLIC_ACCOUNT, user->GetType()); } - base::FilePath GetCacheDirectoryForAccountID(const std::string& account_id) { + base::FilePath GetExtensionCacheDirectoryForAccountID( + const std::string& account_id) { base::FilePath extension_cache_root_dir; - PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS, - &extension_cache_root_dir); + if (!PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS, + &extension_cache_root_dir)) { + ADD_FAILURE(); + } return extension_cache_root_dir.Append( base::HexEncode(account_id.c_str(), account_id.size())); } @@ -432,7 +435,7 @@ class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest, base::FilePath GetCacheCRXFile(const std::string& account_id, const std::string& id, const std::string& version) { - return GetCacheDirectoryForAccountID(account_id) + return GetExtensionCacheDirectoryForAccountID(account_id) .Append(base::StringPrintf("%s-%s.crx", id.c_str(), version.c_str())); } @@ -764,7 +767,7 @@ IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, ExtensionsCached) { // Pre-populate the device local account's extension cache with a hosted app // and an extension. EXPECT_TRUE(base::CreateDirectory( - GetCacheDirectoryForAccountID(kAccountId1))); + GetExtensionCacheDirectoryForAccountID(kAccountId1))); base::FilePath test_dir; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir)); const base::FilePath cached_hosted_app = diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_provider.cc b/chrome/browser/chromeos/policy/device_local_account_policy_provider.cc index 837eb99..ca40ec6 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_provider.cc +++ b/chrome/browser/chromeos/policy/device_local_account_policy_provider.cc @@ -5,19 +5,16 @@ #include "chrome/browser/chromeos/policy/device_local_account_policy_provider.h" #include "base/bind.h" -#include "base/command_line.h" #include "base/values.h" #include "chrome/browser/chromeos/policy/device_local_account.h" #include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h" #include "chromeos/dbus/power_policy_controller.h" #include "components/policy/core/common/cloud/cloud_policy_core.h" #include "components/policy/core/common/cloud/cloud_policy_service.h" +#include "components/policy/core/common/cloud/component_cloud_policy_service.h" #include "components/policy/core/common/policy_bundle.h" #include "components/policy/core/common/policy_map.h" #include "components/policy/core/common/policy_namespace.h" -#include "components/policy/core/common/policy_switches.h" -#include "content/public/browser/browser_thread.h" -#include "net/url_request/url_request_context_getter.h" #include "policy/policy_constants.h" namespace policy { @@ -98,18 +95,13 @@ DeviceLocalAccountPolicyProvider::Create( return provider.Pass(); } -void DeviceLocalAccountPolicyProvider::Init(SchemaRegistry* schema_registry) { - ConfigurationPolicyProvider::Init(schema_registry); - MaybeCreateComponentPolicyService(); -} - bool DeviceLocalAccountPolicyProvider::IsInitializationComplete( PolicyDomain domain) const { if (domain == POLICY_DOMAIN_CHROME) return store_initialized_; if (ComponentCloudPolicyService::SupportsDomain(domain) && - component_policy_service_) { - return component_policy_service_->is_initialized(); + GetBroker() && GetBroker()->component_policy_service()) { + return GetBroker()->component_policy_service()->is_initialized(); } return true; } @@ -126,38 +118,18 @@ void DeviceLocalAccountPolicyProvider::RefreshPolicies() { } } -void DeviceLocalAccountPolicyProvider::Shutdown() { - component_policy_service_.reset(); - ConfigurationPolicyProvider::Shutdown(); -} - void DeviceLocalAccountPolicyProvider::OnPolicyUpdated( const std::string& user_id) { - if (user_id == user_id_) { - MaybeCreateComponentPolicyService(); + if (user_id == user_id_) UpdateFromBroker(); - } } void DeviceLocalAccountPolicyProvider::OnDeviceLocalAccountsChanged() { - MaybeCreateComponentPolicyService(); UpdateFromBroker(); } -void DeviceLocalAccountPolicyProvider::OnBrokerShutdown( - DeviceLocalAccountPolicyBroker* broker) { - if (broker->user_id() == user_id_) { - // The |component_policy_service_| relies on the broker's CloudPolicyCore, - // so destroy it if the broker is going away. - component_policy_service_.reset(); - } -} - -void DeviceLocalAccountPolicyProvider::OnComponentCloudPolicyUpdated() { - UpdateFromBroker(); -} - -DeviceLocalAccountPolicyBroker* DeviceLocalAccountPolicyProvider::GetBroker() { +DeviceLocalAccountPolicyBroker* DeviceLocalAccountPolicyProvider::GetBroker() + const { return service_->GetBrokerForUser(user_id_); } @@ -176,6 +148,9 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() { bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) .CopyFrom(broker->core()->store()->policy_map()); external_data_manager_ = broker->external_data_manager(); + + if (broker->component_policy_service()) + bundle->MergeFrom(broker->component_policy_service()->policy()); } else { // Wait for the refresh to finish. return; @@ -187,9 +162,6 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() { bundle->CopyFrom(policies()); } - if (component_policy_service_) - bundle->MergeFrom(component_policy_service_->policy()); - // Apply overrides. if (chrome_policy_overrides_) { PolicyMap& chrome_policy = @@ -206,35 +178,4 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() { UpdatePolicy(bundle.Pass()); } -void DeviceLocalAccountPolicyProvider::MaybeCreateComponentPolicyService() { - if (component_policy_service_) - return; // Already started. - - if (CommandLine::ForCurrentProcess()->HasSwitch( - switches::kDisableComponentCloudPolicy)) { - // Disabled via the command line. - return; - } - - DeviceLocalAccountPolicyBroker* broker = GetBroker(); - if (!broker || !schema_registry()) - return; // Missing broker or not initialized yet. - - scoped_ptr<ResourceCache> resource_cache( - new ResourceCache(broker->GetComponentPolicyCachePath(), - content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::FILE))); - - component_policy_service_.reset(new ComponentCloudPolicyService( - this, - schema_registry(), - broker->core(), - resource_cache.Pass(), - service_->request_context(), - content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::FILE), - content::BrowserThread::GetMessageLoopProxyForThread( - content::BrowserThread::IO))); -} - } // namespace policy diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_provider.h b/chrome/browser/chromeos/policy/device_local_account_policy_provider.h index 079f931..7807976 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_provider.h +++ b/chrome/browser/chromeos/policy/device_local_account_policy_provider.h @@ -14,8 +14,6 @@ #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 "components/policy/core/common/cloud/component_cloud_policy_service.h" -#include "components/policy/core/common/cloud/resource_cache.h" #include "components/policy/core/common/configuration_policy_provider.h" namespace policy { @@ -26,11 +24,11 @@ class PolicyMap; // DeviceLocalAccountPolicyService. Note that this implementation keeps // functioning when the device-local account disappears from // DeviceLocalAccountPolicyService. The current policy will be kept in that case -// and RefreshPolicies becomes a no-op. +// and RefreshPolicies becomes a no-op. Policies for any installed extensions +// will be kept as well in that case. class DeviceLocalAccountPolicyProvider : public ConfigurationPolicyProvider, - public DeviceLocalAccountPolicyService::Observer, - public ComponentCloudPolicyService::Delegate { + public DeviceLocalAccountPolicyService::Observer { public: DeviceLocalAccountPolicyProvider( const std::string& user_id, @@ -46,23 +44,16 @@ class DeviceLocalAccountPolicyProvider DeviceLocalAccountPolicyService* service); // ConfigurationPolicyProvider: - virtual void Init(SchemaRegistry* registry) OVERRIDE; virtual bool IsInitializationComplete(PolicyDomain domain) const OVERRIDE; virtual void RefreshPolicies() OVERRIDE; - virtual void Shutdown() OVERRIDE; // DeviceLocalAccountPolicyService::Observer: virtual void OnPolicyUpdated(const std::string& user_id) OVERRIDE; virtual void OnDeviceLocalAccountsChanged() OVERRIDE; - virtual void OnBrokerShutdown( - DeviceLocalAccountPolicyBroker* broker) OVERRIDE; - - // ComponentCloudPolicyService::Delegate: - virtual void OnComponentCloudPolicyUpdated() OVERRIDE; private: // Returns the broker for |user_id_| or NULL if not available. - DeviceLocalAccountPolicyBroker* GetBroker(); + DeviceLocalAccountPolicyBroker* GetBroker() const; // Handles completion of policy refreshes and triggers the update callback. // |success| is true if the policy refresh was successful. @@ -72,10 +63,6 @@ class DeviceLocalAccountPolicyProvider // policy from the broker if available or keeping the current policy. void UpdateFromBroker(); - // Creates the |component_policy_service_| if it hasn't been created yet - // and all the dependencies are in place. - void MaybeCreateComponentPolicyService(); - const std::string user_id_; scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager_; @@ -89,8 +76,6 @@ class DeviceLocalAccountPolicyProvider bool store_initialized_; bool waiting_for_policy_refresh_; - scoped_ptr<ComponentCloudPolicyService> component_policy_service_; - base::WeakPtrFactory<DeviceLocalAccountPolicyProvider> weak_factory_; DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountPolicyProvider); 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 9d853b8..3009c9f 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_service.cc +++ b/chrome/browser/chromeos/policy/device_local_account_policy_service.cc @@ -7,6 +7,8 @@ #include <vector> #include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/command_line.h" #include "base/file_util.h" #include "base/files/file_enumerator.h" #include "base/logging.h" @@ -30,7 +32,9 @@ #include "components/policy/core/common/cloud/cloud_policy_constants.h" #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/resource_cache.h" #include "components/policy/core/common/cloud/system_policy_request_context.h" +#include "components/policy/core/common/policy_switches.h" #include "content/public/browser/browser_thread.h" #include "net/url_request/url_request_context_getter.h" #include "policy/policy_constants.h" @@ -72,10 +76,9 @@ scoped_ptr<CloudPolicyClient> CreateClient( return client.Pass(); } -// Get the subdirectory of the cache directory in which force-installed -// extensions are cached for |account_id|. This is also used for the -// component policy cache. -std::string EncodeAccountId(const std::string& account_id) { +// Get the subdirectory of the force-installed extension cache and the component +// policy cache used for |account_id|. +std::string GetCacheSubdirectoryForAccountID(const std::string& account_id) { return base::HexEncode(account_id.c_str(), account_id.size()); } @@ -103,8 +106,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(EncodeAccountId(account_id_to_delete)); + const base::FilePath path = cache_root_dir.Append( + GetCacheSubdirectoryForAccountID(account_id_to_delete)); if (base::DirectoryExists(path)) base::DeleteFile(path, true); } @@ -116,6 +119,7 @@ DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker( const base::FilePath& component_policy_cache_path, scoped_ptr<DeviceLocalAccountPolicyStore> store, scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager, + const base::Closure& policy_update_callback, const scoped_refptr<base::SequencedTaskRunner>& task_runner) : account_id_(account.account_id), user_id_(account.user_id), @@ -125,15 +129,25 @@ DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker( core_(PolicyNamespaceKey(dm_protocol::kChromePublicAccountPolicyType, store_->account_id()), store_.get(), - task_runner) { + task_runner), + policy_update_callback_(policy_update_callback) { base::FilePath cache_root_dir; CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS, &cache_root_dir)); extension_loader_ = new chromeos::DeviceLocalAccountExternalPolicyLoader( - store_.get(), cache_root_dir.Append(EncodeAccountId(account.account_id))); + store_.get(), + cache_root_dir.Append( + GetCacheSubdirectoryForAccountID(account.account_id))); + store_->AddObserver(this); + + // Unblock the |schema_registry_| so that the |component_policy_service_| + // starts using it. + schema_registry_.SetReady(POLICY_DOMAIN_CHROME); + schema_registry_.SetReady(POLICY_DOMAIN_EXTENSIONS); } DeviceLocalAccountPolicyBroker::~DeviceLocalAccountPolicyBroker() { + store_->RemoveObserver(this); external_data_manager_->SetPolicyStore(NULL); external_data_manager_->Disconnect(); } @@ -159,6 +173,7 @@ void DeviceLocalAccountPolicyBroker::ConnectIfPossible( external_data_manager_->Connect(request_context); core_.StartRefreshScheduler(); UpdateRefreshDelay(); + CreateComponentCloudPolicyService(request_context); } void DeviceLocalAccountPolicyBroker::UpdateRefreshDelay() { @@ -180,9 +195,42 @@ std::string DeviceLocalAccountPolicyBroker::GetDisplayName() const { return display_name; } -base::FilePath DeviceLocalAccountPolicyBroker::GetComponentPolicyCachePath() - const { - return component_policy_cache_path_; +void DeviceLocalAccountPolicyBroker::OnStoreLoaded(CloudPolicyStore* store) { + UpdateRefreshDelay(); + policy_update_callback_.Run(); +} + +void DeviceLocalAccountPolicyBroker::OnStoreError(CloudPolicyStore* store) { + policy_update_callback_.Run(); +} + +void DeviceLocalAccountPolicyBroker::OnComponentCloudPolicyUpdated() { + policy_update_callback_.Run(); +} + +void DeviceLocalAccountPolicyBroker::CreateComponentCloudPolicyService( + const scoped_refptr<net::URLRequestContextGetter>& request_context) { + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kDisableComponentCloudPolicy)) { + // Disabled via the command line. + return; + } + + scoped_ptr<ResourceCache> resource_cache( + new ResourceCache(component_policy_cache_path_, + content::BrowserThread::GetMessageLoopProxyForThread( + content::BrowserThread::FILE))); + + component_policy_service_.reset(new ComponentCloudPolicyService( + this, + &schema_registry_, + core(), + resource_cache.Pass(), + request_context, + content::BrowserThread::GetMessageLoopProxyForThread( + content::BrowserThread::FILE), + content::BrowserThread::GetMessageLoopProxyForThread( + content::BrowserThread::IO))); } DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService( @@ -200,7 +248,7 @@ DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService( cros_settings_(cros_settings), device_management_service_(NULL), waiting_for_cros_settings_(false), - orphan_cache_deletion_state_(NOT_STARTED), + orphan_extension_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), @@ -260,11 +308,6 @@ bool DeviceLocalAccountPolicyService::IsPolicyAvailableForUser( return broker && broker->core()->store()->is_managed(); } -scoped_refptr<net::URLRequestContextGetter> -DeviceLocalAccountPolicyService::request_context() const { - return request_context_; -} - void DeviceLocalAccountPolicyService::AddObserver(Observer* observer) { observers_.AddObserver(observer); } @@ -273,23 +316,6 @@ void DeviceLocalAccountPolicyService::RemoveObserver(Observer* observer) { observers_.RemoveObserver(observer); } -void DeviceLocalAccountPolicyService::OnStoreLoaded(CloudPolicyStore* store) { - DeviceLocalAccountPolicyBroker* broker = GetBrokerForStore(store); - DCHECK(broker); - if (!broker) - return; - broker->UpdateRefreshDelay(); - FOR_EACH_OBSERVER(Observer, observers_, OnPolicyUpdated(broker->user_id())); -} - -void DeviceLocalAccountPolicyService::OnStoreError(CloudPolicyStore* store) { - DeviceLocalAccountPolicyBroker* broker = GetBrokerForStore(store); - DCHECK(broker); - if (!broker) - return; - FOR_EACH_OBSERVER(Observer, observers_, OnPolicyUpdated(broker->user_id())); -} - bool DeviceLocalAccountPolicyService::IsExtensionCacheDirectoryBusy( const std::string& account_id) { return busy_extension_cache_directories_.find(account_id) != @@ -320,15 +346,15 @@ bool DeviceLocalAccountPolicyService::StartExtensionCacheForAccountIfPresent( } void DeviceLocalAccountPolicyService::OnOrphanedExtensionCachesDeleted() { - DCHECK_EQ(IN_PROGRESS, orphan_cache_deletion_state_); + DCHECK_EQ(IN_PROGRESS, orphan_extension_cache_deletion_state_); - orphan_cache_deletion_state_ = DONE; + orphan_extension_cache_deletion_state_ = DONE; StartExtensionCachesIfPossible(); } void DeviceLocalAccountPolicyService::OnObsoleteExtensionCacheShutdown( const std::string& account_id) { - DCHECK_NE(NOT_STARTED, orphan_cache_deletion_state_); + DCHECK_NE(NOT_STARTED, orphan_extension_cache_deletion_state_); DCHECK(IsExtensionCacheDirectoryBusy(account_id)); // The account with |account_id| was deleted and the broker for it has shut @@ -356,7 +382,7 @@ void DeviceLocalAccountPolicyService::OnObsoleteExtensionCacheShutdown( void DeviceLocalAccountPolicyService::OnObsoleteExtensionCacheDeleted( const std::string& account_id) { - DCHECK_EQ(DONE, orphan_cache_deletion_state_); + DCHECK_EQ(DONE, orphan_extension_cache_deletion_state_); DCHECK(IsExtensionCacheDirectoryBusy(account_id)); // The cache directory for |account_id| has been deleted. The directory no @@ -417,16 +443,19 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() { session_manager_client_, 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, - component_policy_cache_root_.Append(EncodeAccountId(it->account_id)), + component_policy_cache_root_.Append( + GetCacheSubdirectoryForAccountID(it->account_id)), store.Pass(), external_data_manager, + base::Bind(&DeviceLocalAccountPolicyService::NotifyPolicyUpdated, + base::Unretained(this), + it->user_id), base::MessageLoopProxy::current())); } @@ -443,10 +472,11 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() { policy_brokers_[it->user_id]->Initialize(); } - subdirectories_to_keep.insert(EncodeAccountId(it->account_id)); + subdirectories_to_keep.insert( + GetCacheSubdirectoryForAccountID(it->account_id)); } - if (orphan_cache_deletion_state_ == NOT_STARTED) { + if (orphan_extension_cache_deletion_state_ == NOT_STARTED) { DCHECK(old_policy_brokers.empty()); DCHECK(busy_extension_cache_directories_.empty()); @@ -454,7 +484,7 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() { // been started yet. Take this opportunity to do a clean-up by removing // orphaned cache directories not found in |subdirectories_to_keep| from the // cache directory. - orphan_cache_deletion_state_ = IN_PROGRESS; + orphan_extension_cache_deletion_state_ = IN_PROGRESS; base::FilePath cache_root_dir; CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS, @@ -475,7 +505,7 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() { // their extension caches and delete the brokers. DeleteBrokers(&old_policy_brokers); - if (orphan_cache_deletion_state_ == DONE) { + if (orphan_extension_cache_deletion_state_ == DONE) { // If the initial clean-up of orphaned cache directories has been // complete, start any extension caches that are not running yet but can // be started now because their cache directories are not busy. @@ -498,7 +528,6 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() { void DeviceLocalAccountPolicyService::DeleteBrokers(PolicyBrokerMap* map) { for (PolicyBrokerMap::iterator it = map->begin(); it != map->end(); ++it) { - it->second->core()->store()->RemoveObserver(this); scoped_refptr<chromeos::DeviceLocalAccountExternalPolicyLoader> extension_loader = it->second->extension_loader(); if (extension_loader->IsCacheRunning()) { @@ -509,7 +538,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(); @@ -526,4 +555,9 @@ DeviceLocalAccountPolicyBroker* return NULL; } +void DeviceLocalAccountPolicyService::NotifyPolicyUpdated( + const std::string& user_id) { + FOR_EACH_OBSERVER(Observer, observers_, OnPolicyUpdated(user_id)); +} + } // namespace policy diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_service.h b/chrome/browser/chromeos/policy/device_local_account_policy_service.h index f91fc95..3e230e6 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_service.h +++ b/chrome/browser/chromeos/policy/device_local_account_policy_service.h @@ -10,6 +10,7 @@ #include <string> #include "base/basictypes.h" +#include "base/callback.h" #include "base/compiler_specific.h" #include "base/files/file_path.h" #include "base/memory/ref_counted.h" @@ -21,6 +22,8 @@ #include "chrome/browser/chromeos/settings/cros_settings.h" #include "components/policy/core/common/cloud/cloud_policy_core.h" #include "components/policy/core/common/cloud/cloud_policy_store.h" +#include "components/policy/core/common/cloud/component_cloud_policy_service.h" +#include "components/policy/core/common/schema_registry.h" namespace base { class SequencedTaskRunner; @@ -44,8 +47,12 @@ class DeviceManagementService; // The main switching central that downloads, caches, refreshes, etc. policy for // a single device-local account. -class DeviceLocalAccountPolicyBroker { +class DeviceLocalAccountPolicyBroker + : public CloudPolicyStore::Observer, + public ComponentCloudPolicyService::Delegate { public: + // |policy_update_callback| will be invoked to notify observers that the + // policy for |account| has been updated. // |task_runner| is the runner for policy refresh tasks. DeviceLocalAccountPolicyBroker( const DeviceLocalAccount& account, @@ -53,8 +60,9 @@ class DeviceLocalAccountPolicyBroker { scoped_ptr<DeviceLocalAccountPolicyStore> store, scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager, + const base::Closure& policy_updated_callback, const scoped_refptr<base::SequencedTaskRunner>& task_runner); - ~DeviceLocalAccountPolicyBroker(); + virtual ~DeviceLocalAccountPolicyBroker(); // Initialize the broker, loading its |store_|. void Initialize(); @@ -74,6 +82,12 @@ class DeviceLocalAccountPolicyBroker { return external_data_manager_; } + ComponentCloudPolicyService* component_policy_service() const { + return component_policy_service_.get(); + } + + SchemaRegistry* schema_registry() { return &schema_registry_; } + // Fire up the cloud connection for fetching policy for the account from the // cloud if this is an enterprise-managed device. void ConnectIfPossible( @@ -88,20 +102,28 @@ class DeviceLocalAccountPolicyBroker { // empty string if the policy is not present. std::string GetDisplayName() const; - // Returns a directory where component policy for this account can be cached. - // The DeviceLocalAccountPolicyService takes care of cleaning up caches of - // accounts that have been removed. - base::FilePath GetComponentPolicyCachePath() const; + // CloudPolicyStore::Observer: + virtual void OnStoreLoaded(CloudPolicyStore* store) OVERRIDE; + virtual void OnStoreError(CloudPolicyStore* store) OVERRIDE; + + // ComponentCloudPolicyService::Delegate: + virtual void OnComponentCloudPolicyUpdated() OVERRIDE; private: + void CreateComponentCloudPolicyService( + const scoped_refptr<net::URLRequestContextGetter>& request_context); + const std::string account_id_; const std::string user_id_; const base::FilePath component_policy_cache_path_; + SchemaRegistry schema_registry_; const scoped_ptr<DeviceLocalAccountPolicyStore> store_; scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager_; scoped_refptr<chromeos::DeviceLocalAccountExternalPolicyLoader> extension_loader_; CloudPolicyCore core_; + scoped_ptr<ComponentCloudPolicyService> component_policy_service_; + base::Closure policy_update_callback_; DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountPolicyBroker); }; @@ -110,7 +132,7 @@ class DeviceLocalAccountPolicyBroker { // The actual policy blobs are brokered by session_manager (to prevent file // manipulation), and we're making signature checks on the policy blobs to // ensure they're issued by the device owner. -class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer { +class DeviceLocalAccountPolicyService { public: // Interface for interested parties to observe policy changes. class Observer { @@ -122,9 +144,6 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer { // The list of accounts has been updated. virtual void OnDeviceLocalAccountsChanged() = 0; - - // The given |broker| is about to be destroyed. - virtual void OnBrokerShutdown(DeviceLocalAccountPolicyBroker* broker) {} }; DeviceLocalAccountPolicyService( @@ -153,15 +172,9 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer { // |user_id|. bool IsPolicyAvailableForUser(const std::string& user_id); - scoped_refptr<net::URLRequestContextGetter> request_context() const; - void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); - // CloudPolicyStore::Observer: - virtual void OnStoreLoaded(CloudPolicyStore* store) OVERRIDE; - virtual void OnStoreError(CloudPolicyStore* store) OVERRIDE; - private: typedef std::map<std::string, DeviceLocalAccountPolicyBroker*> PolicyBrokerMap; @@ -203,6 +216,9 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer { // Find the broker for a given |store|. Returns NULL if |store| is unknown. DeviceLocalAccountPolicyBroker* GetBrokerForStore(CloudPolicyStore* store); + // Notifies the |observers_| that the policy for |user_id| has changed. + void NotifyPolicyUpdated(const std::string& user_id); + ObserverList<Observer, true> observers_; chromeos::SessionManagerClient* session_manager_client_; @@ -220,12 +236,12 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer { // Orphaned extension caches are removed at startup. This tracks the status of // that process. - enum OrphanCacheDeletionState { + enum OrphanExtensionCacheDeletionState { NOT_STARTED, IN_PROGRESS, DONE, }; - OrphanCacheDeletionState orphan_cache_deletion_state_; + OrphanExtensionCacheDeletionState orphan_extension_cache_deletion_state_; // Account IDs whose extension cache directories are busy, either because a // broker for the account has not shut down completely yet or because the @@ -242,8 +258,8 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer { const scoped_ptr<chromeos::CrosSettings::ObserverSubscription> local_accounts_subscription_; - // Path to the directory that contains the cached policies for components - // for device local accounts. + // Path to the directory that contains the cached policy for components + // for device-local accounts. base::FilePath component_policy_cache_root_; base::WeakPtrFactory<DeviceLocalAccountPolicyService> weak_factory_; diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc b/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc index 0f9160a..c77f9c1 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc +++ b/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc @@ -405,7 +405,9 @@ TEST_F(DeviceLocalAccountPolicyServiceTest, FetchPolicy) { device_policy_.policy_data().device_id(), _)) .WillOnce(SaveArg<6>(&request)); - EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_)); + // This will be called twice, because the ComponentCloudPolicyService will + // also become ready after flushing all the pending tasks. + EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_)).Times(2); broker->core()->client()->FetchPolicy(); FlushDeviceSettings(); Mock::VerifyAndClearExpectations(&service_observer_); @@ -449,7 +451,9 @@ TEST_F(DeviceLocalAccountPolicyServiceTest, RefreshPolicy) { .WillOnce(mock_device_management_service_.SucceedJob(response)); EXPECT_CALL(mock_device_management_service_, StartJob(_, _, _, _, _, _, _)); EXPECT_CALL(*this, OnRefreshDone(true)).Times(1); - EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_)); + // This will be called twice, because the ComponentCloudPolicyService will + // also become ready after flushing all the pending tasks. + EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_)).Times(2); broker->core()->service()->RefreshPolicy( base::Bind(&DeviceLocalAccountPolicyServiceTest::OnRefreshDone, base::Unretained(this))); diff --git a/chrome/browser/policy/schema_registry_service_factory.cc b/chrome/browser/policy/schema_registry_service_factory.cc index eaa5268..465e47d 100644 --- a/chrome/browser/policy/schema_registry_service_factory.cc +++ b/chrome/browser/policy/schema_registry_service_factory.cc @@ -11,8 +11,51 @@ #include "components/policy/core/common/schema_registry.h" #include "content/public/browser/browser_context.h" +#if defined(OS_CHROMEOS) +#include "chrome/browser/browser_process.h" +#include "chrome/browser/browser_process_platform_part_chromeos.h" +#include "chrome/browser/chromeos/login/users/user.h" +#include "chrome/browser/chromeos/login/users/user_manager.h" +#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" +#include "chrome/browser/chromeos/policy/device_local_account_policy_service.h" +#include "chrome/browser/chromeos/profiles/profile_helper.h" +#include "chrome/browser/profiles/profile.h" +#endif + namespace policy { +#if defined(OS_CHROMEOS) +namespace { + +DeviceLocalAccountPolicyBroker* GetBroker(content::BrowserContext* context) { + Profile* profile = Profile::FromBrowserContext(context); + + if (chromeos::ProfileHelper::IsSigninProfile(profile)) + return NULL; + + if (!chromeos::UserManager::IsInitialized()) { + // Bail out in unit tests that don't have a UserManager. + return NULL; + } + + chromeos::UserManager* user_manager = chromeos::UserManager::Get(); + chromeos::User* user = user_manager->GetUserByProfile(profile); + if (!user) + return NULL; + + BrowserPolicyConnectorChromeOS* connector = + g_browser_process->platform_part()->browser_policy_connector_chromeos(); + DeviceLocalAccountPolicyService* service = + connector->GetDeviceLocalAccountPolicyService(); + if (!service) + return NULL; + + return service->GetBrokerForUser(user->email()); +} + +} // namespace +#endif // OS_CHROMEOS + // static SchemaRegistryServiceFactory* SchemaRegistryServiceFactory::GetInstance() { return Singleton<SchemaRegistryServiceFactory>::get(); @@ -60,7 +103,23 @@ SchemaRegistryServiceFactory::CreateForContextInternal( CombinedSchemaRegistry* global_registry) { DCHECK(!context->IsOffTheRecord()); DCHECK(registries_.find(context) == registries_.end()); - scoped_ptr<SchemaRegistry> registry(new SchemaRegistry); + + scoped_ptr<SchemaRegistry> registry; + +#if defined(OS_CHROMEOS) + DeviceLocalAccountPolicyBroker* broker = GetBroker(context); + if (broker) { + // The SchemaRegistry for a device-local account is owned by its + // DeviceLocalAccountPolicyBroker, which uses the registry to fetch and + // cache policy even if there is no active session for that account. + // Use a ForwardingSchemaRegistry that wraps this SchemaRegistry. + registry.reset(new ForwardingSchemaRegistry(broker->schema_registry())); + } +#endif + + if (!registry) + registry.reset(new SchemaRegistry); + scoped_ptr<SchemaRegistryService> service(new SchemaRegistryService( registry.Pass(), chrome_schema, global_registry)); registries_[context] = service.get(); diff --git a/chrome/browser/policy/schema_registry_service_factory.h b/chrome/browser/policy/schema_registry_service_factory.h index daaee37..fe6964d 100644 --- a/chrome/browser/policy/schema_registry_service_factory.h +++ b/chrome/browser/policy/schema_registry_service_factory.h @@ -7,8 +7,8 @@ #include <map> -#include "base/basictypes.h" #include "base/compiler_specific.h" +#include "base/macros.h" #include "base/memory/scoped_ptr.h" #include "base/memory/singleton.h" #include "components/keyed_service/content/browser_context_keyed_base_factory.h" diff --git a/chrome/browser/policy/test/policy_testserver.py b/chrome/browser/policy/test/policy_testserver.py index 81b6bd5..a0bd33d 100644 --- a/chrome/browser/policy/test/policy_testserver.py +++ b/chrome/browser/policy/test/policy_testserver.py @@ -443,10 +443,12 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): self.server.UpdateStateKeys(token_info['device_token'], key_update_request.server_backed_state_key) - # If this is a publicaccount request then get the username now and use it - # in every PolicyFetchResponse produced. This is required to validate - # policy for extensions in public accounts. - username = self.server.GetPolicies().get('policy_user', None) + # If this is a |publicaccount| request, get the |username| now and use + # it in every PolicyFetchResponse produced. This is required to validate + # policy for extensions in device-local accounts. + # Unfortunately, the |username| can't be obtained from |msg| because that + # requires interacting with GAIA. + username = None for request in msg.policy_request.request: if request.policy_type == 'google/chromeos/publicaccount': username = request.settings_entity_id @@ -461,7 +463,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): 'google/chrome/user', 'google/ios/user')): fetch_response = response.policy_response.response.add() - self.ProcessCloudPolicy(request, token_info, fetch_response) + self.ProcessCloudPolicy(request, token_info, fetch_response, username) elif request.policy_type == 'google/chrome/extension': self.ProcessCloudPolicyForExtensions( request, response.policy_response, token_info, username) @@ -637,7 +639,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): settings.__getattribute__(field.name).CopyFrom(policy_message) def ProcessCloudPolicyForExtensions(self, request, response, token_info, - username): + username=None): """Handles a request for policy for extensions. A request for policy for extensions is slightly different from the other @@ -649,7 +651,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): response: The DevicePolicyResponse message for the response. Multiple PolicyFetchResponses will be appended to this message. token_info: The token extracted from the request. - username: The username for the response. + username: The username for the response. May be None. """ # Send one PolicyFetchResponse for each extension that has # configuration data at the server. @@ -760,8 +762,6 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): if username: policy_data.username = username - elif msg.policy_type == 'google/chromeos/publicaccount': - policy_data.username = msg.settings_entity_id else: # For regular user/device policy, there is no way for the testserver to # know the user name belonging to the GAIA auth token we received (short diff --git a/chromeos/chromeos_paths.h b/chromeos/chromeos_paths.h index db55c27..8d284de 100644 --- a/chromeos/chromeos_paths.h +++ b/chromeos/chromeos_paths.h @@ -38,11 +38,11 @@ enum { DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA, // Directory where external data // referenced by policies is cached // for device-local accounts. - DIR_DEVICE_LOCAL_ACCOUNT_COMPONENT_POLICY, // Directory where policy for the - // components of device-local - // account is stored. Currently - // this is used for policy for - // extensions. + DIR_DEVICE_LOCAL_ACCOUNT_COMPONENT_POLICY, // Directory where policy for + // components is stored for + // device-local accounts. + // Currently this is used for + // policy for extensions. PATH_END }; |