diff options
68 files changed, 774 insertions, 937 deletions
diff --git a/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc index ff40047..0d7a926 100644 --- a/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc +++ b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc @@ -23,6 +23,7 @@ #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/proto/chromeos/chrome_device_policy.pb.h" #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h" +#include "chrome/browser/policy/schema_registry.h" #include "chrome/browser/prefs/browser_prefs.h" #include "chrome/test/base/testing_browser_process.h" #include "chromeos/cryptohome/system_salt_getter.h" @@ -98,7 +99,7 @@ class DeviceCloudPolicyManagerChromeOSTest install_attributes_.get())); chrome::RegisterLocalState(local_state_.registry()); - manager_->Init(); + manager_->Init(&schema_registry_); // DeviceOAuth2TokenService uses the system request context to fetch // OAuth tokens, then writes the token to local state, encrypting it @@ -139,6 +140,7 @@ class DeviceCloudPolicyManagerChromeOSTest chromeos::system::MockStatisticsProvider mock_statistics_provider_; DeviceCloudPolicyStoreChromeOS* store_; + SchemaRegistry schema_registry_; scoped_ptr<DeviceCloudPolicyManagerChromeOS> manager_; private: 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 462ceb2..9de5a9c 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 @@ -31,6 +31,7 @@ #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" #include "chrome/browser/policy/proto/chromeos/chrome_device_policy.pb.h" +#include "chrome/browser/policy/schema_registry.h" #include "chrome/common/chrome_paths.h" #include "chromeos/chromeos_paths.h" #include "chromeos/dbus/power_policy_controller.h" @@ -774,6 +775,7 @@ class DeviceLocalAccountPolicyProviderTest virtual void SetUp() OVERRIDE; virtual void TearDown() OVERRIDE; + SchemaRegistry schema_registry_; scoped_ptr<DeviceLocalAccountPolicyProvider> provider_; MockConfigurationPolicyObserver provider_observer_; @@ -791,7 +793,7 @@ DeviceLocalAccountPolicyProviderTest::DeviceLocalAccountPolicyProviderTest() { void DeviceLocalAccountPolicyProviderTest::SetUp() { DeviceLocalAccountPolicyServiceTestBase::SetUp(); - provider_->Init(); + provider_->Init(&schema_registry_); provider_->AddObserver(&provider_observer_); } diff --git a/chrome/browser/chromeos/policy/login_profile_policy_provider.cc b/chrome/browser/chromeos/policy/login_profile_policy_provider.cc index 9fc74fc..11f6904 100644 --- a/chrome/browser/chromeos/policy/login_profile_policy_provider.cc +++ b/chrome/browser/chromeos/policy/login_profile_policy_provider.cc @@ -65,8 +65,8 @@ LoginProfilePolicyProvider::LoginProfilePolicyProvider( LoginProfilePolicyProvider::~LoginProfilePolicyProvider() { } -void LoginProfilePolicyProvider::Init() { - ConfigurationPolicyProvider::Init(); +void LoginProfilePolicyProvider::Init(SchemaRegistry* registry) { + ConfigurationPolicyProvider::Init(registry); device_policy_service_->AddObserver(POLICY_DOMAIN_CHROME, this); if (device_policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME)) UpdateFromDevicePolicy(); diff --git a/chrome/browser/chromeos/policy/login_profile_policy_provider.h b/chrome/browser/chromeos/policy/login_profile_policy_provider.h index 3a7d3c4..c2e8e5b 100644 --- a/chrome/browser/chromeos/policy/login_profile_policy_provider.h +++ b/chrome/browser/chromeos/policy/login_profile_policy_provider.h @@ -24,7 +24,7 @@ class LoginProfilePolicyProvider : public ConfigurationPolicyProvider, virtual ~LoginProfilePolicyProvider(); // ConfigurationPolicyProvider: - virtual void Init() OVERRIDE; + virtual void Init(SchemaRegistry* registry) OVERRIDE; virtual void Shutdown() OVERRIDE; virtual void RefreshPolicies() OVERRIDE; diff --git a/chrome/browser/chromeos/policy/proxy_policy_provider.cc b/chrome/browser/chromeos/policy/proxy_policy_provider.cc index 30c879c..0154321 100644 --- a/chrome/browser/chromeos/policy/proxy_policy_provider.cc +++ b/chrome/browser/chromeos/policy/proxy_policy_provider.cc @@ -56,4 +56,9 @@ void ProxyPolicyProvider::OnUpdatePolicy( UpdatePolicy(bundle.Pass()); } +void ProxyPolicyProvider::OnSchemaRegistryUpdated(bool has_new_schemas) { + // Ignore, and don't call RefreshPolicies. Policies only have to be refreshed + // when the delegate's registry is updated. +} + } // namespace policy diff --git a/chrome/browser/chromeos/policy/proxy_policy_provider.h b/chrome/browser/chromeos/policy/proxy_policy_provider.h index 5829a9e..f8f030c2 100644 --- a/chrome/browser/chromeos/policy/proxy_policy_provider.h +++ b/chrome/browser/chromeos/policy/proxy_policy_provider.h @@ -29,6 +29,9 @@ class ProxyPolicyProvider : public ConfigurationPolicyProvider, // ConfigurationPolicyProvider::Observer: virtual void OnUpdatePolicy(ConfigurationPolicyProvider* provider) OVERRIDE; + // SchemaRegistry::Observer: + virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE; + private: ConfigurationPolicyProvider* delegate_; diff --git a/chrome/browser/chromeos/policy/proxy_policy_provider_unittest.cc b/chrome/browser/chromeos/policy/proxy_policy_provider_unittest.cc index 922c547..5008111 100644 --- a/chrome/browser/chromeos/policy/proxy_policy_provider_unittest.cc +++ b/chrome/browser/chromeos/policy/proxy_policy_provider_unittest.cc @@ -7,6 +7,7 @@ #include "chrome/browser/chromeos/policy/proxy_policy_provider.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/schema_registry.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,7 +19,7 @@ class ProxyPolicyProviderTest : public testing::Test { protected: ProxyPolicyProviderTest() { mock_provider_.Init(); - proxy_provider_.Init(); + proxy_provider_.Init(&schema_registry_); proxy_provider_.AddObserver(&observer_); } @@ -28,6 +29,7 @@ class ProxyPolicyProviderTest : public testing::Test { mock_provider_.Shutdown(); } + SchemaRegistry schema_registry_; MockConfigurationPolicyObserver observer_; MockConfigurationPolicyProvider mock_provider_; ProxyPolicyProvider proxy_provider_; diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc index 9c2f809..12658ed 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc @@ -19,7 +19,6 @@ #include "chrome/browser/policy/cloud/cloud_policy_refresh_scheduler.h" #include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "components/policy/core/common/policy_pref_names.h" #include "content/public/browser/browser_thread.h" #include "net/url_request/url_request_context_getter.h" @@ -154,12 +153,12 @@ bool UserCloudPolicyManagerChromeOS::IsInitializationComplete( return true; } -void UserCloudPolicyManagerChromeOS::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) { - if (ComponentCloudPolicyService::SupportsDomain(descriptor->domain()) && - component_policy_service_) { - component_policy_service_->RegisterPolicyDomain(descriptor); - } +void UserCloudPolicyManagerChromeOS::OnSchemaRegistryUpdated( + bool has_new_schemas) { + // Send the new map even if |has_new_schemas| is false, so that policies for + // components that have been removed can be dropped from the cache. + if (component_policy_service_) + component_policy_service_->OnSchemasUpdated(schema_map()); } scoped_ptr<PolicyBundle> UserCloudPolicyManagerChromeOS::CreatePolicyBundle() { diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h index 80360e3..8448801 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h @@ -83,8 +83,7 @@ class UserCloudPolicyManagerChromeOS // ConfigurationPolicyProvider: virtual void Shutdown() OVERRIDE; virtual bool IsInitializationComplete(PolicyDomain domain) const OVERRIDE; - virtual void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) OVERRIDE; + virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE; // CloudPolicyManager: virtual scoped_ptr<PolicyBundle> CreatePolicyBundle() OVERRIDE; diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc index bfdf432..ede5970 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc @@ -25,6 +25,7 @@ #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h" +#include "chrome/browser/policy/schema_registry.h" #include "chrome/browser/prefs/browser_prefs.h" #include "chrome/browser/prefs/pref_service_syncable.h" #include "chrome/browser/signin/profile_oauth2_token_service.h" @@ -159,7 +160,7 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { scoped_ptr<ResourceCache>(), wait_for_fetch, base::TimeDelta::FromSeconds(fetch_timeout))); - manager_->Init(); + manager_->Init(&schema_registry_); manager_->AddObserver(&observer_); manager_->Connect(&prefs_, &device_management_service_, NULL, USER_AFFILIATION_NONE); @@ -297,6 +298,7 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { MockCloudPolicyStore* store_; // Not owned. MockCloudExternalDataManager* external_data_manager_; // Not owned. scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + SchemaRegistry schema_registry_; scoped_ptr<UserCloudPolicyManagerChromeOS> manager_; scoped_ptr<UserCloudPolicyTokenForwarder> token_forwarder_; diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc index a8fdf59..4f43f89 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc @@ -24,6 +24,8 @@ #include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/device_management_service.h" #include "chrome/browser/policy/cloud/resource_cache.h" +#include "chrome/browser/policy/schema_registry_service.h" +#include "chrome/browser/policy/schema_registry_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_switches.h" #include "chromeos/chromeos_paths.h" @@ -87,7 +89,9 @@ scoped_ptr<UserCloudPolicyManagerChromeOS> UserCloudPolicyManagerFactoryChromeOS::UserCloudPolicyManagerFactoryChromeOS() : BrowserContextKeyedBaseFactory( "UserCloudPolicyManagerChromeOS", - BrowserContextDependencyManager::GetInstance()) {} + BrowserContextDependencyManager::GetInstance()) { + DependsOn(SchemaRegistryServiceFactory::GetInstance()); +} UserCloudPolicyManagerFactoryChromeOS:: ~UserCloudPolicyManagerFactoryChromeOS() {} @@ -193,7 +197,7 @@ scoped_ptr<UserCloudPolicyManagerChromeOS> resource_cache.Pass(), wait_for_initial_policy, base::TimeDelta::FromSeconds(kInitialPolicyFetchTimeoutSeconds))); - manager->Init(); + manager->Init(SchemaRegistryServiceFactory::GetForContext(profile)); manager->Connect(g_browser_process->local_state(), device_management_service, g_browser_process->system_request_context(), diff --git a/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc b/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc index 6313f76..e2ec24c 100644 --- a/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc +++ b/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc @@ -36,7 +36,6 @@ #include "testing/gtest/include/gtest/gtest.h" #include "ui/views/widget/widget.h" -using testing::AnyNumber; using testing::Return; using testing::_; using testing::WithParamInterface; @@ -62,7 +61,6 @@ class TrayAccessibilityTest virtual void SetUpInProcessBrowserTestFixture() OVERRIDE { EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); } diff --git a/chrome/browser/extensions/api/networking_private/networking_private_apitest.cc b/chrome/browser/extensions/api/networking_private/networking_private_apitest.cc index 93637c2..d966c64b 100644 --- a/chrome/browser/extensions/api/networking_private/networking_private_apitest.cc +++ b/chrome/browser/extensions/api/networking_private/networking_private_apitest.cc @@ -32,7 +32,6 @@ #include "third_party/cros_system_api/dbus/service_constants.h" #endif // OS_CHROMEOS -using testing::AnyNumber; using testing::Return; using testing::_; @@ -56,7 +55,6 @@ class ExtensionNetworkingPrivateApiTest : #if defined(OS_CHROMEOS) EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); #endif diff --git a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc index c1957f30..cf4b010 100644 --- a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc +++ b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc @@ -17,17 +17,21 @@ #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/profile_policy_connector.h" #include "chrome/browser/policy/profile_policy_connector_factory.h" +#include "chrome/browser/policy/schema_registry.h" +#include "chrome/browser/policy/schema_registry_service.h" +#include "chrome/browser/policy/schema_registry_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/value_store/value_store_change.h" #include "chrome/common/extensions/api/storage.h" #include "chrome/common/extensions/api/storage/storage_schema_manifest_handler.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_set.h" +#include "components/policy/core/common/policy_namespace.h" #include "components/policy/core/common/schema.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/notification_details.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_source.h" @@ -66,15 +70,17 @@ class ManagedValueStoreCache::ExtensionTracker const content::NotificationDetails& details) OVERRIDE; private: - // Loads the schemas of the |extensions| and passes a PolicyDomainDescriptor - // to RegisterDomain(). + bool UsesManagedStorage(const Extension* extension) const; + + // Loads the schemas of the |extensions| and passes a ComponentMap to + // Register(). static void LoadSchemas(scoped_ptr<ExtensionSet> extensions, base::WeakPtr<ExtensionTracker> self); - void RegisterDomain( - scoped_refptr<const policy::PolicyDomainDescriptor> descriptor); + void Register(const policy::ComponentMap* components); Profile* profile_; content::NotificationRegistrar registrar_; + policy::SchemaRegistry* schema_registry_; base::WeakPtrFactory<ExtensionTracker> weak_factory_; DISALLOW_COPY_AND_ASSIGN(ExtensionTracker); @@ -82,6 +88,8 @@ class ManagedValueStoreCache::ExtensionTracker ManagedValueStoreCache::ExtensionTracker::ExtensionTracker(Profile* profile) : profile_(profile), + schema_registry_( + policy::SchemaRegistryServiceFactory::GetForContext(profile)), weak_factory_(this) { registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY, @@ -98,40 +106,79 @@ void ManagedValueStoreCache::ExtensionTracker::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { + // All the extensions installed on this Profile will be loaded on startup, + // triggering multiple NOTIFICATION_EXTENSION_LOADED. + // Wait until all of them are ready before registering the schemas of managed + // extensions, so that the policy loaders only have to reload at most once. if (!ExtensionSystem::Get(profile_)->ready().is_signaled()) return; - scoped_refptr<policy::PolicyDomainDescriptor> descriptor( - new policy::PolicyDomainDescriptor(policy::POLICY_DOMAIN_EXTENSIONS)); - const ExtensionSet* set = - ExtensionSystem::Get(profile_)->extension_service()->extensions(); - scoped_ptr<ExtensionSet> managed_extensions(new ExtensionSet()); - for (ExtensionSet::const_iterator it = set->begin(); it != set->end(); ++it) { - if ((*it)->manifest()->HasPath(manifest_keys::kStorageManagedSchema)) { - managed_extensions->Insert(*it); + scoped_ptr<ExtensionSet> added(new ExtensionSet); + const Extension* removed = NULL; + switch (type) { + case chrome::NOTIFICATION_EXTENSIONS_READY: + added->InsertAll( + *ExtensionSystem::Get(profile_)->extension_service()->extensions()); + break; + case chrome::NOTIFICATION_EXTENSION_LOADED: + added->Insert(content::Details<const Extension>(details).ptr()); + break; + case chrome::NOTIFICATION_EXTENSION_UNLOADED: + removed = content::Details<UnloadedExtensionInfo>(details)->extension; + break; + default: + NOTREACHED(); + return; + } + + if (removed) { + if (UsesManagedStorage(removed)) { + schema_registry_->UnregisterComponent(policy::PolicyNamespace( + policy::POLICY_DOMAIN_EXTENSIONS, removed->id())); } + return; + } - // TODO(joaodasilva): also load extensions that use the storage API for now, - // to support the Legacy Browser Support extension. Remove this. - // http://crbug.com/240704 - if ((*it)->HasAPIPermission(APIPermission::kStorage)) - managed_extensions->Insert(*it); + ExtensionSet::const_iterator it = added->begin(); + while (it != added->end()) { + std::string to_remove; + if (!UsesManagedStorage(*it)) + to_remove = (*it)->id(); + ++it; + if (!to_remove.empty()) + added->Remove(to_remove); } + if (added->is_empty()) + return; + // Load the schema files in a background thread. BrowserThread::PostBlockingPoolSequencedTask( kLoadSchemasBackgroundTaskTokenName, FROM_HERE, base::Bind(&ExtensionTracker::LoadSchemas, - base::Passed(&managed_extensions), + base::Passed(&added), weak_factory_.GetWeakPtr())); } +bool ManagedValueStoreCache::ExtensionTracker::UsesManagedStorage( + const Extension* extension) const { + if (extension->manifest()->HasPath(manifest_keys::kStorageManagedSchema)) + return true; + + // TODO(joaodasilva): also load extensions that use the storage API for now, + // to support the Legacy Browser Support extension. Remove this. + // http://crbug.com/240704 + if (extension->HasAPIPermission(APIPermission::kStorage)) + return true; + + return false; +} + // static void ManagedValueStoreCache::ExtensionTracker::LoadSchemas( scoped_ptr<ExtensionSet> extensions, base::WeakPtr<ExtensionTracker> self) { - scoped_refptr<policy::PolicyDomainDescriptor> descriptor = - new policy::PolicyDomainDescriptor(policy::POLICY_DOMAIN_EXTENSIONS); + scoped_ptr<policy::ComponentMap> components(new policy::ComponentMap); for (ExtensionSet::const_iterator it = extensions->begin(); it != extensions->end(); ++it) { @@ -140,7 +187,7 @@ void ManagedValueStoreCache::ExtensionTracker::LoadSchemas( manifest_keys::kStorageManagedSchema, &schema_file)) { // TODO(joaodasilva): Remove this. http://crbug.com/240704 if ((*it)->HasAPIPermission(APIPermission::kStorage)) { - descriptor->RegisterComponent((*it)->id(), policy::Schema()); + (*components)[(*it)->id()] = policy::Schema(); } else { NOTREACHED(); } @@ -152,19 +199,18 @@ void ManagedValueStoreCache::ExtensionTracker::LoadSchemas( policy::Schema schema = StorageSchemaManifestHandler::GetSchema(it->get(), &error); CHECK(schema.valid()) << error; - descriptor->RegisterComponent((*it)->id(), schema); + (*components)[(*it)->id()] = schema; } - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&ExtensionTracker::RegisterDomain, self, descriptor)); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&ExtensionTracker::Register, self, + base::Owned(components.release()))); } -void ManagedValueStoreCache::ExtensionTracker::RegisterDomain( - scoped_refptr<const policy::PolicyDomainDescriptor> descriptor) { - policy::ProfilePolicyConnector* connector = - policy::ProfilePolicyConnectorFactory::GetForProfile(profile_); - connector->policy_service()->RegisterPolicyDomain(descriptor); +void ManagedValueStoreCache::ExtensionTracker::Register( + const policy::ComponentMap* components) { + schema_registry_->RegisterComponents(policy::POLICY_DOMAIN_EXTENSIONS, + *components); } ManagedValueStoreCache::ManagedValueStoreCache( diff --git a/chrome/browser/extensions/api/storage/settings_apitest.cc b/chrome/browser/extensions/api/storage/settings_apitest.cc index 00778a8..a8645f0 100644 --- a/chrome/browser/extensions/api/storage/settings_apitest.cc +++ b/chrome/browser/extensions/api/storage/settings_apitest.cc @@ -6,6 +6,7 @@ #include "base/json/json_writer.h" #include "base/memory/ref_counted.h" #include "base/run_loop.h" +#include "base/values.h" #include "chrome/browser/extensions/api/storage/settings_frontend.h" #include "chrome/browser/extensions/api/storage/settings_namespace.h" #include "chrome/browser/extensions/api/storage/settings_sync_util.h" @@ -28,22 +29,23 @@ #include "chrome/browser/policy/browser_policy_connector.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" #include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_map.h" -#include "chrome/browser/policy/policy_service.h" -#include "chrome/browser/policy/profile_policy_connector.h" -#include "chrome/browser/policy/profile_policy_connector_factory.h" +#include "chrome/browser/policy/schema_map.h" +#include "chrome/browser/policy/schema_registry.h" +#include "chrome/browser/policy/schema_registry_service.h" +#include "chrome/browser/policy/schema_registry_service_factory.h" +#include "components/policy/core/common/policy_namespace.h" +#include "components/policy/core/common/schema.h" #endif namespace extensions { -using settings_namespace::FromString; using settings_namespace::LOCAL; using settings_namespace::MANAGED; using settings_namespace::Namespace; using settings_namespace::SYNC; using settings_namespace::ToString; -using testing::AnyNumber; +using testing::Mock; using testing::Return; using testing::_; @@ -100,6 +102,14 @@ class SyncChangeProcessorDelegate : public syncer::SyncChangeProcessor { DISALLOW_COPY_AND_ASSIGN(SyncChangeProcessorDelegate); }; +class MockSchemaRegistryObserver : public policy::SchemaRegistry::Observer { + public: + MockSchemaRegistryObserver() {} + virtual ~MockSchemaRegistryObserver() {} + + MOCK_METHOD1(OnSchemaRegistryUpdated, void(bool)); +}; + } // namespace class ExtensionSettingsApiTest : public ExtensionApiTest { @@ -110,7 +120,6 @@ class ExtensionSettingsApiTest : public ExtensionApiTest { #if defined(ENABLE_CONFIGURATION_POLICY) EXPECT_CALL(policy_provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(policy_provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting( &policy_provider_); #endif @@ -445,9 +454,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, IsStorageEnabled) { #if defined(ENABLE_CONFIGURATION_POLICY) -IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, PolicyDomainDescriptor) { - // Verifies that the PolicyDomainDescriptor for the extensions domain is - // created on startup. +IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, ExtensionsSchemas) { + // Verifies that the Schemas for the extensions domain are created on startup. Profile* profile = browser()->profile(); ExtensionSystem* extension_system = ExtensionSystemFactory::GetForProfile(profile); @@ -459,12 +467,65 @@ IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, PolicyDomainDescriptor) { ASSERT_TRUE(extension_system->ready().is_signaled()); } - policy::ProfilePolicyConnector* connector = - policy::ProfilePolicyConnectorFactory::GetForProfile(profile); - policy::PolicyService* service = connector->policy_service(); - scoped_refptr<const policy::PolicyDomainDescriptor> descriptor = - service->GetPolicyDomainDescriptor(policy::POLICY_DOMAIN_EXTENSIONS); - EXPECT_TRUE(descriptor.get()); + // This test starts without any test extensions installed. + EXPECT_FALSE(GetSingleLoadedExtension()); + message_.clear(); + + policy::SchemaRegistry* registry = + policy::SchemaRegistryServiceFactory::GetForContext(profile); + ASSERT_TRUE(registry); + EXPECT_FALSE(registry->schema_map()->GetSchema(policy::PolicyNamespace( + policy::POLICY_DOMAIN_EXTENSIONS, kManagedStorageExtensionId))); + + MockSchemaRegistryObserver observer; + registry->AddObserver(&observer); + + // Install a managed extension. + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); + const Extension* extension = + LoadExtension(test_data_dir_.AppendASCII("settings/managed_storage")); + ASSERT_TRUE(extension); + Mock::VerifyAndClearExpectations(&observer); + registry->RemoveObserver(&observer); + + // Verify that its schema has been published, and verify its contents. + const policy::Schema* schema = + registry->schema_map()->GetSchema(policy::PolicyNamespace( + policy::POLICY_DOMAIN_EXTENSIONS, kManagedStorageExtensionId)); + ASSERT_TRUE(schema); + + ASSERT_TRUE(schema->valid()); + ASSERT_EQ(base::Value::TYPE_DICTIONARY, schema->type()); + ASSERT_TRUE(schema->GetKnownProperty("string-policy").valid()); + EXPECT_EQ(base::Value::TYPE_STRING, + schema->GetKnownProperty("string-policy").type()); + ASSERT_TRUE(schema->GetKnownProperty("int-policy").valid()); + EXPECT_EQ(base::Value::TYPE_INTEGER, + schema->GetKnownProperty("int-policy").type()); + ASSERT_TRUE(schema->GetKnownProperty("double-policy").valid()); + EXPECT_EQ(base::Value::TYPE_DOUBLE, + schema->GetKnownProperty("double-policy").type()); + ASSERT_TRUE(schema->GetKnownProperty("boolean-policy").valid()); + EXPECT_EQ(base::Value::TYPE_BOOLEAN, + schema->GetKnownProperty("boolean-policy").type()); + + policy::Schema list = schema->GetKnownProperty("list-policy"); + ASSERT_TRUE(list.valid()); + ASSERT_EQ(base::Value::TYPE_LIST, list.type()); + ASSERT_TRUE(list.GetItems().valid()); + EXPECT_EQ(base::Value::TYPE_STRING, list.GetItems().type()); + + policy::Schema dict = schema->GetKnownProperty("dict-policy"); + ASSERT_TRUE(dict.valid()); + ASSERT_EQ(base::Value::TYPE_DICTIONARY, dict.type()); + list = dict.GetKnownProperty("list"); + ASSERT_TRUE(list.valid()); + ASSERT_EQ(base::Value::TYPE_LIST, list.type()); + dict = list.GetItems(); + ASSERT_TRUE(dict.valid()); + ASSERT_EQ(base::Value::TYPE_DICTIONARY, dict.type()); + ASSERT_TRUE(dict.GetProperty("anything").valid()); + EXPECT_EQ(base::Value::TYPE_INTEGER, dict.GetProperty("anything").type()); } IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, ManagedStorage) { diff --git a/chrome/browser/policy/async_policy_loader.cc b/chrome/browser/policy/async_policy_loader.cc index 05c1bb0..ba88ce7 100644 --- a/chrome/browser/policy/async_policy_loader.cc +++ b/chrome/browser/policy/async_policy_loader.cc @@ -8,7 +8,6 @@ #include "base/location.h" #include "base/sequenced_task_runner.h" #include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" using base::Time; using base::TimeDelta; @@ -35,8 +34,8 @@ AsyncPolicyLoader::AsyncPolicyLoader( AsyncPolicyLoader::~AsyncPolicyLoader() {} -base::Time AsyncPolicyLoader::LastModificationTime() { - return base::Time(); +Time AsyncPolicyLoader::LastModificationTime() { + return Time(); } void AsyncPolicyLoader::Reload(bool force) { @@ -59,29 +58,23 @@ void AsyncPolicyLoader::Reload(bool force) { } // Filter out mismatching policies. - for (DescriptorMap::iterator it = descriptor_map_.begin(); - it != descriptor_map_.end(); ++it) { - it->second->FilterBundle(bundle.get()); - } + schema_map_->FilterBundle(bundle.get()); update_callback_.Run(bundle.Pass()); ScheduleNextReload(TimeDelta::FromSeconds(kReloadIntervalSeconds)); } -void AsyncPolicyLoader::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) { - if (descriptor->domain() != POLICY_DOMAIN_CHROME) { - descriptor_map_[descriptor->domain()] = descriptor; - Reload(true); - } -} - -scoped_ptr<PolicyBundle> AsyncPolicyLoader::InitialLoad() { +scoped_ptr<PolicyBundle> AsyncPolicyLoader::InitialLoad( + const scoped_refptr<SchemaMap>& schema_map) { // This is the first load, early during startup. Use this to record the // initial |last_modification_time_|, so that potential changes made before // installing the watches can be detected. last_modification_time_ = LastModificationTime(); - return Load(); + schema_map_ = schema_map; + scoped_ptr<PolicyBundle> bundle(Load()); + // Filter out mismatching policies. + schema_map_->FilterBundle(bundle.get()); + return bundle.Pass(); } void AsyncPolicyLoader::Init(const UpdateCallback& update_callback) { @@ -101,6 +94,12 @@ void AsyncPolicyLoader::Init(const UpdateCallback& update_callback) { ScheduleNextReload(TimeDelta::FromSeconds(kReloadIntervalSeconds)); } +void AsyncPolicyLoader::RefreshPolicies(scoped_refptr<SchemaMap> schema_map) { + DCHECK(task_runner_->RunsTasksOnCurrentThread()); + schema_map_ = schema_map; + Reload(true); +} + void AsyncPolicyLoader::ScheduleNextReload(TimeDelta delay) { DCHECK(task_runner_->RunsTasksOnCurrentThread()); weak_factory_.InvalidateWeakPtrs(); @@ -127,7 +126,7 @@ bool AsyncPolicyLoader::IsSafeToReload(const Time& now, TimeDelta* delay) { } // Check whether the settle interval has elapsed. - const base::TimeDelta age = now - last_modification_clock_; + const TimeDelta age = now - last_modification_clock_; if (age < kSettleInterval) { *delay = kSettleInterval - age; return false; diff --git a/chrome/browser/policy/async_policy_loader.h b/chrome/browser/policy/async_policy_loader.h index 57f346c..150ede6 100644 --- a/chrome/browser/policy/async_policy_loader.h +++ b/chrome/browser/policy/async_policy_loader.h @@ -5,14 +5,12 @@ #ifndef CHROME_BROWSER_POLICY_ASYNC_POLICY_LOADER_H_ #define CHROME_BROWSER_POLICY_ASYNC_POLICY_LOADER_H_ -#include <map> - #include "base/callback.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/time/time.h" -#include "components/policy/core/common/policy_namespace.h" +#include "chrome/browser/policy/schema_map.h" namespace base { class SequencedTaskRunner; @@ -21,7 +19,6 @@ class SequencedTaskRunner; namespace policy { class PolicyBundle; -class PolicyDomainDescriptor; // Base implementation for platform-specific policy loaders. Together with the // AsyncPolicyProvider, this base implementation takes care of the initial load, @@ -55,6 +52,11 @@ class AsyncPolicyLoader { // or base::Time() if it doesn't apply, which is the default. virtual base::Time LastModificationTime(); + // Used by the AsyncPolicyProvider to do the initial Load(). The first load + // is also used to initialize |last_modification_time_| and + // |schema_map_|. + scoped_ptr<PolicyBundle> InitialLoad(const scoped_refptr<SchemaMap>& schemas); + // Implementations should invoke Reload() when a change is detected. This // must be invoked from the background thread and will trigger a Load(), // and pass the returned bundle to the provider. @@ -66,19 +68,7 @@ class AsyncPolicyLoader { // makes sure the policies are reloaded if the update events aren't triggered. void Reload(bool force); - // Passes the current |descriptor| for a domain, which is used to determine - // which policy names are supported for each component. - void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor); - - protected: - typedef std::map<PolicyDomain, scoped_refptr<const PolicyDomainDescriptor> > - DescriptorMap; - - // Returns the current DescriptorMap. This can be used by implementations to - // determine the components registered for each domain, and to filter out - // unknonwn policies. - const DescriptorMap& descriptor_map() const { return descriptor_map_; } + const scoped_refptr<SchemaMap>& schema_map() const { return schema_map_; } private: // Allow AsyncPolicyProvider to call Init(). @@ -86,14 +76,13 @@ class AsyncPolicyLoader { typedef base::Callback<void(scoped_ptr<PolicyBundle>)> UpdateCallback; - // Used by the AsyncPolicyProvider to do the initial Load(). The first load - // is also used to initialize |last_modification_time_|. - scoped_ptr<PolicyBundle> InitialLoad(); - // Used by the AsyncPolicyProvider to install the |update_callback_|. // Invoked on the background thread. void Init(const UpdateCallback& update_callback); + // Used by the AsyncPolicyProvider to reload with an updated SchemaMap. + void RefreshPolicies(scoped_refptr<SchemaMap> schema_map); + // Cancels any pending periodic reload and posts one |delay| time units from // now. void ScheduleNextReload(base::TimeDelta delay); @@ -121,8 +110,8 @@ class AsyncPolicyLoader { // non-local filesystem involved. base::Time last_modification_clock_; - // A map of the currently registered domains and their descriptors. - DescriptorMap descriptor_map_; + // The current policy schemas that this provider should load. + scoped_refptr<SchemaMap> schema_map_; DISALLOW_COPY_AND_ASSIGN(AsyncPolicyLoader); }; diff --git a/chrome/browser/policy/async_policy_provider.cc b/chrome/browser/policy/async_policy_provider.cc index f564482..95a22e1 100644 --- a/chrome/browser/policy/async_policy_provider.cc +++ b/chrome/browser/policy/async_policy_provider.cc @@ -12,15 +12,17 @@ #include "base/sequenced_task_runner.h" #include "chrome/browser/policy/async_policy_loader.h" #include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" +#include "chrome/browser/policy/schema_registry.h" namespace policy { -AsyncPolicyProvider::AsyncPolicyProvider(scoped_ptr<AsyncPolicyLoader> loader) +AsyncPolicyProvider::AsyncPolicyProvider( + SchemaRegistry* registry, + scoped_ptr<AsyncPolicyLoader> loader) : loader_(loader.release()), weak_factory_(this) { // Make an immediate synchronous load on startup. - OnLoaderReloaded(loader_->InitialLoad()); + OnLoaderReloaded(loader_->InitialLoad(registry->schema_map())); } AsyncPolicyProvider::~AsyncPolicyProvider() { @@ -29,9 +31,9 @@ AsyncPolicyProvider::~AsyncPolicyProvider() { DCHECK(!loader_); } -void AsyncPolicyProvider::Init() { +void AsyncPolicyProvider::Init(SchemaRegistry* registry) { DCHECK(CalledOnValidThread()); - ConfigurationPolicyProvider::Init(); + ConfigurationPolicyProvider::Init(registry); if (!loader_) return; @@ -89,17 +91,6 @@ void AsyncPolicyProvider::RefreshPolicies() { refresh_callback_.callback()); } -void AsyncPolicyProvider::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) { - if (loader_) { - loader_->task_runner()->PostTask( - FROM_HERE, - base::Bind(&AsyncPolicyLoader::RegisterPolicyDomain, - base::Unretained(loader_), - descriptor)); - } -} - void AsyncPolicyProvider::ReloadAfterRefreshSync() { DCHECK(CalledOnValidThread()); // This task can only enter if it was posted from RefreshPolicies(), and it @@ -114,10 +105,11 @@ void AsyncPolicyProvider::ReloadAfterRefreshSync() { if (!loader_) return; - loader_->task_runner()->PostTask(FROM_HERE, - base::Bind(&AsyncPolicyLoader::Reload, - base::Unretained(loader_), - true /* force */)); + loader_->task_runner()->PostTask( + FROM_HERE, + base::Bind(&AsyncPolicyLoader::RefreshPolicies, + base::Unretained(loader_), + schema_map())); } void AsyncPolicyProvider::OnLoaderReloaded(scoped_ptr<PolicyBundle> bundle) { diff --git a/chrome/browser/policy/async_policy_provider.h b/chrome/browser/policy/async_policy_provider.h index 3302b96..572cecb 100644 --- a/chrome/browser/policy/async_policy_provider.h +++ b/chrome/browser/policy/async_policy_provider.h @@ -20,6 +20,7 @@ namespace policy { class AsyncPolicyLoader; class PolicyBundle; +class SchemaRegistry; // A policy provider that loads its policies asynchronously on a background // thread. Platform-specific providers are created by passing an implementation @@ -27,15 +28,17 @@ class PolicyBundle; class AsyncPolicyProvider : public ConfigurationPolicyProvider, public base::NonThreadSafe { public: - explicit AsyncPolicyProvider(scoped_ptr<AsyncPolicyLoader> loader); + // The AsyncPolicyProvider does a synchronous load in its constructor, and + // therefore it needs the |registry| at construction time. The same |registry| + // should be passed later to Init(). + AsyncPolicyProvider(SchemaRegistry* registry, + scoped_ptr<AsyncPolicyLoader> loader); virtual ~AsyncPolicyProvider(); // ConfigurationPolicyProvider implementation. - virtual void Init() OVERRIDE; + virtual void Init(SchemaRegistry* registry) OVERRIDE; virtual void Shutdown() OVERRIDE; virtual void RefreshPolicies() OVERRIDE; - virtual void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) OVERRIDE; private: // Helper for RefreshPolicies(). diff --git a/chrome/browser/policy/async_policy_provider_unittest.cc b/chrome/browser/policy/async_policy_provider_unittest.cc index 677dba8..c9c8ec6 100644 --- a/chrome/browser/policy/async_policy_provider_unittest.cc +++ b/chrome/browser/policy/async_policy_provider_unittest.cc @@ -13,6 +13,7 @@ #include "chrome/browser/policy/async_policy_loader.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/schema_registry.h" #include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -84,6 +85,7 @@ class AsyncPolicyProviderTest : public testing::Test { virtual void TearDown() OVERRIDE; base::MessageLoop loop_; + SchemaRegistry schema_registry_; PolicyBundle initial_bundle_; MockPolicyLoader* loader_; scoped_ptr<AsyncPolicyProvider> provider_; @@ -104,9 +106,9 @@ void AsyncPolicyProviderTest::SetUp() { EXPECT_CALL(*loader_, InitOnBackgroundThread()).Times(1); EXPECT_CALL(*loader_, MockLoad()).WillOnce(Return(&initial_bundle_)); - provider_.reset( - new AsyncPolicyProvider(scoped_ptr<AsyncPolicyLoader>(loader_))); - provider_->Init(); + provider_.reset(new AsyncPolicyProvider( + &schema_registry_, scoped_ptr<AsyncPolicyLoader>(loader_))); + provider_->Init(&schema_registry_); // Verify that the initial load is done synchronously: EXPECT_TRUE(provider_->policies().Equals(initial_bundle_)); diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc index 2a707eeb..20b6ea0 100644 --- a/chrome/browser/policy/browser_policy_connector.cc +++ b/chrome/browser/policy/browser_policy_connector.cc @@ -31,7 +31,6 @@ #include "chrome/browser/policy/cloud/cloud_policy_service.h" #include "chrome/browser/policy/cloud/device_management_service.h" #include "chrome/browser/policy/configuration_policy_provider.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_service_impl.h" #include "chrome/browser/policy/policy_statistics_collector.h" #include "chrome/common/chrome_paths.h" @@ -289,15 +288,18 @@ void BrowserPolicyConnector::Init( kServiceInitializationStartupDelay); if (g_testing_provider) - g_testing_provider->Init(); + g_testing_provider->Init(GetSchemaRegistry()); if (platform_provider_) - platform_provider_->Init(); + platform_provider_->Init(GetSchemaRegistry()); #if defined(OS_CHROMEOS) - global_user_cloud_policy_provider_.Init(); + global_user_cloud_policy_provider_.Init(GetSchemaRegistry()); if (device_cloud_policy_manager_) { - device_cloud_policy_manager_->Init(); + // For now the |device_cloud_policy_manager_| is using the global schema + // registry. Eventually it will have its own registry, once device cloud + // policy for extensions is introduced. + device_cloud_policy_manager_->Init(GetSchemaRegistry()); scoped_ptr<CloudPolicyClient::StatusProvider> status_provider( new DeviceStatusCollector( local_state_, @@ -439,12 +441,7 @@ scoped_ptr<PolicyService> BrowserPolicyConnector::CreatePolicyService( std::copy(additional_providers.begin(), additional_providers.end(), std::back_inserter(providers)); } - scoped_ptr<PolicyService> service(new PolicyServiceImpl(providers)); - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_CHROME); - descriptor->RegisterComponent("", Schema::Wrap(GetChromeSchemaData())); - service->RegisterPolicyDomain(descriptor); - return service.Pass(); + return scoped_ptr<PolicyService>(new PolicyServiceImpl(providers)); } const ConfigurationPolicyHandlerList* @@ -587,14 +584,13 @@ void BrowserPolicyConnector::SetTimezoneIfPolicyAvailable() { #endif } -// static ConfigurationPolicyProvider* BrowserPolicyConnector::CreatePlatformProvider() { #if defined(OS_WIN) const PolicyDefinitionList* policy_list = GetChromePolicyDefinitionList(); scoped_ptr<AsyncPolicyLoader> loader(PolicyLoaderWin::Create( BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE), policy_list)); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(GetSchemaRegistry(), loader.Pass()); #elif defined(OS_MACOSX) && !defined(OS_IOS) const PolicyDefinitionList* policy_list = GetChromePolicyDefinitionList(); scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderMac( @@ -602,7 +598,7 @@ ConfigurationPolicyProvider* BrowserPolicyConnector::CreatePlatformProvider() { policy_list, GetManagedPolicyPath(), new MacPreferences())); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(GetSchemaRegistry(), loader.Pass()); #elif defined(OS_POSIX) && !defined(OS_ANDROID) base::FilePath config_dir_path; if (PathService::Get(chrome::DIR_POLICY_FILES, &config_dir_path)) { @@ -610,7 +606,7 @@ ConfigurationPolicyProvider* BrowserPolicyConnector::CreatePlatformProvider() { BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE), config_dir_path, POLICY_SCOPE_MACHINE)); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(GetSchemaRegistry(), loader.Pass()); } else { return NULL; } diff --git a/chrome/browser/policy/browser_policy_connector.h b/chrome/browser/policy/browser_policy_connector.h index eb08147d..4fa0581 100644 --- a/chrome/browser/policy/browser_policy_connector.h +++ b/chrome/browser/policy/browser_policy_connector.h @@ -168,7 +168,7 @@ class BrowserPolicyConnector { // Set the timezone as soon as the policies are available. void SetTimezoneIfPolicyAvailable(); - static ConfigurationPolicyProvider* CreatePlatformProvider(); + ConfigurationPolicyProvider* CreatePlatformProvider(); // Whether Init() but not Shutdown() has been invoked. bool is_initialized_; diff --git a/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc b/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc index 872e01f..e705284 100644 --- a/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc @@ -16,6 +16,7 @@ #include "chrome/browser/policy/configuration_policy_provider_test.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/schema_registry.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -35,6 +36,7 @@ class TestHarness : public PolicyProviderTestHarness { virtual void SetUp() OVERRIDE; virtual ConfigurationPolicyProvider* CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) OVERRIDE; @@ -70,6 +72,7 @@ TestHarness::~TestHarness() {} void TestHarness::SetUp() {} ConfigurationPolicyProvider* TestHarness::CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) { // Create and initialize the store. @@ -176,7 +179,7 @@ class CloudPolicyManagerTest : public testing::Test { EXPECT_CALL(store_, Load()); manager_.reset(new TestCloudPolicyManager(&store_, loop_.message_loop_proxy())); - manager_->Init(); + manager_->Init(&schema_registry_); Mock::VerifyAndClearExpectations(&store_); manager_->AddObserver(&observer_); } @@ -196,6 +199,7 @@ class CloudPolicyManagerTest : public testing::Test { PolicyBundle expected_bundle_; // Policy infrastructure. + SchemaRegistry schema_registry_; MockConfigurationPolicyObserver observer_; MockCloudPolicyStore store_; scoped_ptr<TestCloudPolicyManager> manager_; diff --git a/chrome/browser/policy/cloud/component_cloud_policy_service.cc b/chrome/browser/policy/cloud/component_cloud_policy_service.cc index 3be5cd7..8e8cda3 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_service.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_service.cc @@ -4,6 +4,10 @@ #include "chrome/browser/policy/cloud/component_cloud_policy_service.h" +#include <map> +#include <string> +#include <utility> + #include "base/bind.h" #include "base/bind_helpers.h" #include "base/location.h" @@ -16,30 +20,28 @@ #include "chrome/browser/policy/cloud/component_cloud_policy_updater.h" #include "chrome/browser/policy/cloud/external_policy_data_fetcher.h" #include "chrome/browser/policy/cloud/resource_cache.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h" +#include "chrome/browser/policy/schema_map.h" +#include "components/policy/core/common/schema.h" #include "net/url_request/url_request_context_getter.h" namespace em = enterprise_management; namespace policy { +const char ComponentCloudPolicyService::kComponentNamespaceCache[] = + "component-namespace-cache"; + 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); - } +bool NotInSchemaMap(const scoped_refptr<SchemaMap> schema_map, + PolicyDomain domain, + const std::string& component_id) { + return schema_map->GetSchema(PolicyNamespace(domain, component_id)) == NULL; } } // namespace -const char ComponentCloudPolicyService::kComponentNamespaceCache[] = - "component-namespace-cache"; - ComponentCloudPolicyService::Delegate::~Delegate() {} // Owns the objects that live on the background thread, and posts back to the @@ -84,16 +86,12 @@ class ComponentCloudPolicyService::Backend // ComponentCloudPolicyStore::Delegate implementation: virtual void OnComponentCloudPolicyStoreUpdated() OVERRIDE; - // Passes the current descriptor of a domain, so that the disk cache - // can purge components that aren't being tracked anymore. - void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor); + // Passes the current SchemaMap so that the disk cache can purge components + // that aren't being tracked anymore. + void OnSchemasUpdated(scoped_refptr<SchemaMap> schema_map); private: - typedef std::map<PolicyDomain, scoped_refptr<const PolicyDomainDescriptor> > - DomainMap; - - scoped_ptr<ComponentMap> ReadCachedComponents(); + scoped_ptr<PolicyNamespaceKeys> ReadCachedComponents(); // The ComponentCloudPolicyService that owns |this|. Used to inform the // |service_| when policy changes. @@ -109,7 +107,7 @@ class ComponentCloudPolicyService::Backend scoped_ptr<ResourceCache> cache_; scoped_ptr<ComponentCloudPolicyStore> store_; scoped_ptr<ComponentCloudPolicyUpdater> updater_; - DomainMap domain_map_; + scoped_refptr<SchemaMap> schema_map_; DISALLOW_COPY_AND_ASSIGN(Backend); }; @@ -132,9 +130,8 @@ void ComponentCloudPolicyService::Backend::Init() { } void ComponentCloudPolicyService::Backend::FinalizeInit() { - // Read the components that were cached in the last RegisterPolicyDomain() - // calls for each domain. - scoped_ptr<ComponentMap> components = ReadCachedComponents(); + // Read the components that were cached in the last OnSchemasUpdated() call. + scoped_ptr<PolicyNamespaceKeys> components = ReadCachedComponents(); // Read the initial policy. store_->Load(); @@ -175,13 +172,15 @@ void ComponentCloudPolicyService::Backend::UpdateExternalPolicy( void ComponentCloudPolicyService::Backend:: OnComponentCloudPolicyStoreUpdated() { - scoped_ptr<PolicyBundle> bundle(new PolicyBundle); - bundle->CopyFrom(store_->policy()); - for (DomainMap::iterator it = domain_map_.begin(); - it != domain_map_.end(); ++it) { - it->second->FilterBundle(bundle.get()); + if (!schema_map_) { + // No schemas have been registered yet, so keep serving the initial policy + // obtained from FinalizeInit(). + return; } + scoped_ptr<PolicyBundle> bundle(new PolicyBundle); + bundle->CopyFrom(store_->policy()); + schema_map_->FilterBundle(bundle.get()); service_task_runner_->PostTask( FROM_HERE, base::Bind(&ComponentCloudPolicyService::OnPolicyUpdated, @@ -189,49 +188,61 @@ void ComponentCloudPolicyService::Backend:: base::Passed(&bundle))); } -void ComponentCloudPolicyService::Backend::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) { +void ComponentCloudPolicyService::Backend::OnSchemasUpdated( + scoped_refptr<SchemaMap> schema_map) { // Store the current list of components in the cache. - StringSet ids; - std::string policy_type; - if (ComponentCloudPolicyStore::GetPolicyType(descriptor->domain(), - &policy_type)) { - GetComponentIds(descriptor, &ids); + const DomainMap& domains = schema_map->GetDomains(); + for (DomainMap::const_iterator domain = domains.begin(); + domain != domains.end(); ++domain) { + std::string domain_policy_type; + if (!ComponentCloudPolicyStore::GetPolicyType(domain->first, + &domain_policy_type)) { + continue; + } Pickle pickle; - for (StringSet::const_iterator it = ids.begin(); it != ids.end(); ++it) - pickle.WriteString(*it); + const ComponentMap& components = domain->second; + for (ComponentMap::const_iterator comp = components.begin(); + comp != components.end(); ++comp) { + pickle.WriteString(comp->first); + } std::string data(reinterpret_cast<const char*>(pickle.data()), pickle.size()); - cache_->Store(kComponentNamespaceCache, policy_type, data); - } + cache_->Store(kComponentNamespaceCache, domain_policy_type, data); - domain_map_[descriptor->domain()] = descriptor; + // Purge any components that have been removed. + if (store_) { + store_->Purge(domain->first, + base::Bind(&NotInSchemaMap, schema_map, domain->first)); + } + } - // Purge any components that have been removed. - if (store_) - store_->Purge(descriptor->domain(), ids); + // Serve policies that may have updated while the backend was waiting for + // the schema_map. + const bool trigger_update = !schema_map_; + schema_map_ = schema_map; + if (trigger_update) + OnComponentCloudPolicyStoreUpdated(); } -scoped_ptr<ComponentCloudPolicyService::ComponentMap> +scoped_ptr<ComponentCloudPolicyService::PolicyNamespaceKeys> ComponentCloudPolicyService::Backend::ReadCachedComponents() { - scoped_ptr<ComponentMap> components(new ComponentMap); + scoped_ptr<PolicyNamespaceKeys> keys(new PolicyNamespaceKeys); 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; if (ComponentCloudPolicyStore::GetPolicyDomain(it->first, &domain)) { - StringSet& set = (*components)[domain]; const Pickle pickle(it->second.data(), it->second.size()); PickleIterator pickit(pickle); std::string id; while (pickit.ReadString(&id)) - set.insert(id); + keys->insert(std::make_pair(it->first, id)); } else { cache_->Delete(kComponentNamespaceCache, it->first); } } - return components.Pass(); + return keys.Pass(); } ComponentCloudPolicyService::ComponentCloudPolicyService( @@ -246,6 +257,7 @@ ComponentCloudPolicyService::ComponentCloudPolicyService( client_(NULL), store_(store), is_initialized_(false), + has_initial_keys_(false), weak_ptr_factory_(this) { store_->AddObserver(this); @@ -300,10 +312,7 @@ void ComponentCloudPolicyService::Disconnect() { DCHECK(CalledOnValidThread()); if (client_) { // Unregister all the current components. - for (ComponentMap::iterator it = registered_components_.begin(); - it != registered_components_.end(); ++it) { - RemoveNamespacesToFetch(it->first, it->second); - } + RemoveNamespacesToFetch(keys_); client_->RemoveObserver(this); client_ = NULL; @@ -316,29 +325,39 @@ void ComponentCloudPolicyService::Disconnect() { } } -void ComponentCloudPolicyService::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) { +void ComponentCloudPolicyService::OnSchemasUpdated( + const scoped_refptr<SchemaMap>& schema_map) { DCHECK(CalledOnValidThread()); - DCHECK(SupportsDomain(descriptor->domain())); - // Send the new descriptor to the backend, to purge the cache. + // Send the new schemas to the backend, to purge the cache. backend_task_runner_->PostTask(FROM_HERE, - base::Bind(&Backend::RegisterPolicyDomain, + base::Bind(&Backend::OnSchemasUpdated, base::Unretained(backend_.get()), - descriptor)); - - // Register the current list of components for |domain| at the |client_|. - StringSet current_ids; - GetComponentIds(descriptor, ¤t_ids); - StringSet& registered_ids = registered_components_[descriptor->domain()]; - if (client_ && is_initialized()) { - if (UpdateClientNamespaces( - descriptor->domain(), registered_ids, current_ids)) { - delegate_->OnComponentCloudPolicyRefreshNeeded(); + schema_map)); + + // Register the current list of components for supported domains at the + // |client_|. + PolicyNamespaceKeys new_keys; + const DomainMap& domains = schema_map->GetDomains(); + for (DomainMap::const_iterator domain = domains.begin(); + domain != domains.end(); ++domain) { + std::string domain_policy_type; + if (!ComponentCloudPolicyStore::GetPolicyType(domain->first, + &domain_policy_type)) { + continue; + } + const ComponentMap& components = domain->second; + for (ComponentMap::const_iterator comp = components.begin(); + comp != components.end(); ++comp) { + new_keys.insert(std::make_pair(domain_policy_type, comp->first)); } } - registered_ids = current_ids; + if (client_ && is_initialized() && UpdateClientNamespaces(keys_, new_keys)) + delegate_->OnComponentCloudPolicyRefreshNeeded(); + + keys_.swap(new_keys); + has_initial_keys_ = true; } void ComponentCloudPolicyService::OnPolicyFetched(CloudPolicyClient* client) { @@ -350,9 +369,7 @@ void ComponentCloudPolicyService::OnPolicyFetched(CloudPolicyClient* client) { for (CloudPolicyClient::ResponseMap::const_iterator it = responses.begin(); it != responses.end(); ++it) { const PolicyNamespaceKey& key(it->first); - PolicyDomain domain; - if (ComponentCloudPolicyStore::GetPolicyDomain(key.first, &domain) && - ContainsKey(registered_components_[domain], key.second)) { + if (ContainsKey(keys_, key)) { scoped_ptr<em::PolicyFetchResponse> response( new em::PolicyFetchResponse(*it->second)); backend_task_runner_->PostTask( @@ -409,7 +426,7 @@ void ComponentCloudPolicyService::InitializeBackend() { } void ComponentCloudPolicyService::OnBackendInitialized( - scoped_ptr<ComponentMap> cached_components, + scoped_ptr<PolicyNamespaceKeys> initial_keys, scoped_ptr<PolicyBundle> initial_policy) { DCHECK(CalledOnValidThread()); // InitializeBackend() may be called multiple times if the |store_| fires @@ -417,16 +434,10 @@ void ComponentCloudPolicyService::OnBackendInitialized( if (is_initialized()) return; - // RegisterPolicyDomain() may have been called while the backend was - // initializing; only update |registered_components_| from |cached_components| - // for domains that haven't registered yet. - for (ComponentMap::iterator it = cached_components->begin(); - it != cached_components->end(); ++it) { - // Lookup without inserting an empty set. - if (registered_components_.find(it->first) != registered_components_.end()) - continue; // Ignore the cached list if a more recent one was registered. - registered_components_[it->first].swap(it->second); - } + // OnSchemasUpdated() may have been called while the backend was initializing; + // in that case ignore the cached keys. + if (!has_initial_keys_) + keys_.swap(*initial_keys); // A client may have already connected while the backend was initializing. if (client_) @@ -440,16 +451,11 @@ void ComponentCloudPolicyService::OnBackendInitialized( void ComponentCloudPolicyService::InitializeClient() { DCHECK(CalledOnValidThread()); // Register all the current components. - bool added = false; - for (ComponentMap::iterator it = registered_components_.begin(); - it != registered_components_.end(); ++it) { - added |= !it->second.empty(); - AddNamespacesToFetch(it->first, it->second); - } + AddNamespacesToFetch(keys_); // The client may already have PolicyFetchResponses for registered components; // load them now. OnPolicyFetched(client_); - if (added && is_initialized()) + if (!keys_.empty() && is_initialized()) delegate_->OnComponentCloudPolicyRefreshNeeded(); } @@ -482,35 +488,33 @@ void ComponentCloudPolicyService::SetCredentialsAndReloadClient() { } bool ComponentCloudPolicyService::UpdateClientNamespaces( - PolicyDomain domain, - const StringSet& old_set, - const StringSet& new_set) { + const PolicyNamespaceKeys& old_keys, + const PolicyNamespaceKeys& new_keys) { DCHECK(CalledOnValidThread()); - StringSet added = base::STLSetDifference<StringSet>(new_set, old_set); - StringSet removed = base::STLSetDifference<StringSet>(old_set, new_set); - AddNamespacesToFetch(domain, added); - RemoveNamespacesToFetch(domain, removed); + PolicyNamespaceKeys added = + base::STLSetDifference<PolicyNamespaceKeys>(new_keys, old_keys); + PolicyNamespaceKeys removed = + base::STLSetDifference<PolicyNamespaceKeys>(old_keys, new_keys); + AddNamespacesToFetch(added); + RemoveNamespacesToFetch(removed); return !added.empty(); } -void ComponentCloudPolicyService::AddNamespacesToFetch(PolicyDomain domain, - const StringSet& set) { +void ComponentCloudPolicyService::AddNamespacesToFetch( + const PolicyNamespaceKeys& keys) { DCHECK(CalledOnValidThread()); - std::string policy_type; - if (ComponentCloudPolicyStore::GetPolicyType(domain, &policy_type)) { - for (StringSet::const_iterator it = set.begin(); it != set.end(); ++it) - client_->AddNamespaceToFetch(PolicyNamespaceKey(policy_type, *it)); + for (PolicyNamespaceKeys::const_iterator it = keys.begin(); + it != keys.end(); ++it) { + client_->AddNamespaceToFetch(*it); } } void ComponentCloudPolicyService::RemoveNamespacesToFetch( - PolicyDomain domain, - const StringSet& set) { + const PolicyNamespaceKeys& keys) { DCHECK(CalledOnValidThread()); - 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)); + for (PolicyNamespaceKeys::const_iterator it = keys.begin(); + it != keys.end(); ++it) { + client_->RemoveNamespaceToFetch(*it); } } diff --git a/chrome/browser/policy/cloud/component_cloud_policy_service.h b/chrome/browser/policy/cloud/component_cloud_policy_service.h index 876739c..774e93d 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_service.h +++ b/chrome/browser/policy/cloud/component_cloud_policy_service.h @@ -5,9 +5,7 @@ #ifndef CHROME_BROWSER_POLICY_CLOUD_COMPONENT_CLOUD_POLICY_SERVICE_H_ #define CHROME_BROWSER_POLICY_CLOUD_COMPONENT_CLOUD_POLICY_SERVICE_H_ -#include <map> #include <set> -#include <string> #include "base/basictypes.h" #include "base/compiler_specific.h" @@ -16,6 +14,7 @@ #include "base/memory/weak_ptr.h" #include "base/threading/non_thread_safe.h" #include "chrome/browser/policy/cloud/cloud_policy_client.h" +#include "chrome/browser/policy/cloud/cloud_policy_constants.h" #include "chrome/browser/policy/cloud/cloud_policy_store.h" #include "chrome/browser/policy/policy_bundle.h" #include "components/policy/core/common/policy_namespace.h" @@ -31,8 +30,8 @@ class URLRequestContextGetter; namespace policy { class ExternalPolicyDataFetcherBackend; -class PolicyDomainDescriptor; class ResourceCache; +class SchemaMap; // Manages cloud policy for components. // @@ -93,12 +92,12 @@ class ComponentCloudPolicyService : public CloudPolicyClient::Observer, // remote policy data. void Disconnect(); - // |descriptor| lists the complete set of components to track for its domain. + // |schema_map| contains the schemas for all the components that this + // service should load policy for. // This purges unused components from the cache, and starts updating the - // components listed in the descriptor. It's only valid to call this for - // domains that are supported, i.e. SupportsDomain(domain) is true. - void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor); + // components listed in the map. + // Unsupported domains in the map are ignored. + void OnSchemasUpdated(const scoped_refptr<SchemaMap>& schema_map); // CloudPolicyClient::Observer implementation: virtual void OnPolicyFetched(CloudPolicyClient* client) OVERRIDE; @@ -111,21 +110,19 @@ class ComponentCloudPolicyService : public CloudPolicyClient::Observer, private: class Backend; - typedef std::set<std::string> StringSet; - typedef std::map<PolicyDomain, StringSet> ComponentMap; + typedef std::set<PolicyNamespaceKey> PolicyNamespaceKeys; void InitializeBackend(); - void OnBackendInitialized(scoped_ptr<ComponentMap> components, + void OnBackendInitialized(scoped_ptr<PolicyNamespaceKeys> initial_keys, scoped_ptr<PolicyBundle> initial_policy); void InitializeClient(); void OnPolicyUpdated(scoped_ptr<PolicyBundle> policy); void SetCredentialsAndReloadClient(); - bool UpdateClientNamespaces(PolicyDomain domain, - const StringSet& old_set, - const StringSet& new_set); - void AddNamespacesToFetch(PolicyDomain domain, const StringSet& set); - void RemoveNamespacesToFetch(PolicyDomain domain, const StringSet& set); + bool UpdateClientNamespaces(const PolicyNamespaceKeys& old_keys, + const PolicyNamespaceKeys& new_keys); + void AddNamespacesToFetch(const PolicyNamespaceKeys& keys); + void RemoveNamespacesToFetch(const PolicyNamespaceKeys& keys); Delegate* delegate_; @@ -149,16 +146,14 @@ class ComponentCloudPolicyService : public CloudPolicyClient::Observer, CloudPolicyClient* client_; CloudPolicyStore* store_; - // The currently registered components for each policy domain. If a policy - // domain doesn't have an entry in this map then it hasn't registered its - // component yet. A domain in this map with an empty set of components means - // that the domain is registered, but has no components. - ComponentMap registered_components_; + // The currently registered components for each policy domain. + PolicyNamespaceKeys keys_; // Contains all the current policies for components. PolicyBundle policy_; bool is_initialized_; + bool has_initial_keys_; base::WeakPtrFactory<ComponentCloudPolicyService> weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(ComponentCloudPolicyService); diff --git a/chrome/browser/policy/cloud/component_cloud_policy_service_unittest.cc b/chrome/browser/policy/cloud/component_cloud_policy_service_unittest.cc index 9c6ae54..afb3662 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_service_unittest.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_service_unittest.cc @@ -19,11 +19,11 @@ #include "chrome/browser/policy/cloud/policy_builder.h" #include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/policy/external_data_fetcher.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/policy_types.h" #include "chrome/browser/policy/proto/cloud/chrome_extension_policy.pb.h" #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h" +#include "chrome/browser/policy/schema_registry.h" #include "components/policy/core/common/schema.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher_delegate.h" @@ -205,6 +205,7 @@ class ComponentCloudPolicyServiceTest : public testing::Test { ResourceCache* cache_; MockCloudPolicyClient client_; MockCloudPolicyStore store_; + SchemaRegistry registry_; scoped_ptr<ComponentCloudPolicyService> service_; ComponentPolicyBuilder builder_; PolicyMap expected_policy_; @@ -267,11 +268,13 @@ TEST_F(ComponentCloudPolicyServiceTest, InitializationWithCachedComponents) { TEST_F(ComponentCloudPolicyServiceTest, ConnectAfterRegister) { // Add some components. - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS); - descriptor->RegisterComponent(kTestExtension, CreateTestSchema()); - descriptor->RegisterComponent(kTestExtension2, CreateTestSchema()); - service_->RegisterPolicyDomain(descriptor); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension), + CreateTestSchema()); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension2), + CreateTestSchema()); + service_->OnSchemasUpdated(registry_.schema_map()); // Now connect the client. EXPECT_TRUE(client_.namespaces_to_fetch_.empty()); @@ -319,10 +322,10 @@ TEST_F(ComponentCloudPolicyServiceTest, StoreReadyAfterConnectAndRegister) { PopulateCache(); // Add some components. - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS); - descriptor->RegisterComponent(kTestExtension, CreateTestSchema()); - service_->RegisterPolicyDomain(descriptor); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension), + CreateTestSchema()); + service_->OnSchemasUpdated(registry_.schema_map()); // And connect the client. Make the client have some policies, with a new // download_url. @@ -358,10 +361,10 @@ TEST_F(ComponentCloudPolicyServiceTest, ConnectThenRegisterThenStoreReady) { // Now register the current components, before the backend has been // initialized. EXPECT_TRUE(client_.namespaces_to_fetch_.empty()); - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS); - descriptor->RegisterComponent(kTestExtension, CreateTestSchema()); - service_->RegisterPolicyDomain(descriptor); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension), + CreateTestSchema()); + service_->OnSchemasUpdated(registry_.schema_map()); EXPECT_TRUE(client_.namespaces_to_fetch_.empty()); // Now load the store. The client gets the namespaces. @@ -381,11 +384,11 @@ TEST_F(ComponentCloudPolicyServiceTest, FetchPolicy) { Mock::VerifyAndClearExpectations(&delegate_); // Register the components to fetch. - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS); - descriptor->RegisterComponent(kTestExtension, CreateTestSchema()); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension), + CreateTestSchema()); EXPECT_CALL(delegate_, OnComponentCloudPolicyRefreshNeeded()); - service_->RegisterPolicyDomain(descriptor); + service_->OnSchemasUpdated(registry_.schema_map()); Mock::VerifyAndClearExpectations(&delegate_); // Send back a fake policy fetch response. @@ -436,10 +439,10 @@ TEST_F(ComponentCloudPolicyServiceTest, LoadAndPurgeCache) { EXPECT_CALL(delegate_, OnComponentCloudPolicyUpdated()); // The service will start updating the components that are registered, which // starts by fetching policy for them. - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS); - descriptor->RegisterComponent(kTestExtension2, CreateTestSchema()); - service_->RegisterPolicyDomain(descriptor); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension2), + CreateTestSchema()); + service_->OnSchemasUpdated(registry_.schema_map()); RunUntilIdle(); Mock::VerifyAndClearExpectations(&delegate_); @@ -470,10 +473,10 @@ TEST_F(ComponentCloudPolicyServiceTest, UpdateCredentials) { // Connect the client and register an extension. service_->Connect(&client_, request_context_); EXPECT_CALL(delegate_, OnComponentCloudPolicyRefreshNeeded()); - scoped_refptr<PolicyDomainDescriptor> descriptor = new PolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS); - descriptor->RegisterComponent(kTestExtension, CreateTestSchema()); - service_->RegisterPolicyDomain(descriptor); + registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kTestExtension), + CreateTestSchema()); + service_->OnSchemasUpdated(registry_.schema_map()); Mock::VerifyAndClearExpectations(&delegate_); // Send the response to the service. The response data will be rejected, diff --git a/chrome/browser/policy/cloud/component_cloud_policy_store.cc b/chrome/browser/policy/cloud/component_cloud_policy_store.cc index e049c3b..879fd4b 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_store.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_store.cc @@ -12,7 +12,6 @@ #include "base/values.h" #include "chrome/browser/policy/cloud/cloud_policy_constants.h" #include "chrome/browser/policy/cloud/cloud_policy_validator.h" -#include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/proto/cloud/chrome_extension_policy.pb.h" @@ -188,22 +187,23 @@ void ComponentCloudPolicyStore::Delete(const PolicyNamespace& ns) { } } -void ComponentCloudPolicyStore::Purge(PolicyDomain domain, - const std::set<std::string>& keep) { +void ComponentCloudPolicyStore::Purge( + PolicyDomain domain, + const ResourceCache::SubkeyFilter& filter) { DCHECK(CalledOnValidThread()); const DomainConstants* constants = GetDomainConstants(domain); if (!constants) return; - cache_->PurgeOtherSubkeys(constants->proto_cache_key, keep); - cache_->PurgeOtherSubkeys(constants->data_cache_key, keep); + cache_->FilterSubkeys(constants->proto_cache_key, filter); + cache_->FilterSubkeys(constants->data_cache_key, filter); // Stop serving policies for purged namespaces. bool purged_current_policies = false; for (PolicyBundle::const_iterator it = policy_bundle_.begin(); it != policy_bundle_.end(); ++it) { if (it->first.domain == domain && - keep.find(it->first.component_id) == keep.end() && + filter.Run(it->first.component_id) && !policy_bundle_.Get(it->first).empty()) { policy_bundle_.Get(it->first).Clear(); purged_current_policies = true; @@ -214,8 +214,7 @@ void ComponentCloudPolicyStore::Purge(PolicyDomain domain, // policy state changes. std::map<PolicyNamespace, std::string>::iterator it = cached_hashes_.begin(); while (it != cached_hashes_.end()) { - if (it->first.domain == domain && - keep.find(it->first.component_id) == keep.end()) { + if (it->first.domain == domain && filter.Run(it->first.component_id)) { std::map<PolicyNamespace, std::string>::iterator prev = it; ++it; cached_hashes_.erase(prev); diff --git a/chrome/browser/policy/cloud/component_cloud_policy_store.h b/chrome/browser/policy/cloud/component_cloud_policy_store.h index 3e5285e..ad2ea0f 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_store.h +++ b/chrome/browser/policy/cloud/component_cloud_policy_store.h @@ -6,12 +6,12 @@ #define CHROME_BROWSER_POLICY_CLOUD_COMPONENT_CLOUD_POLICY_STORE_H_ #include <map> -#include <set> #include <string> #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" #include "base/threading/non_thread_safe.h" +#include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/policy/policy_bundle.h" #include "components/policy/core/common/policy_namespace.h" @@ -23,8 +23,6 @@ class PolicyFetchResponse; namespace policy { -class ResourceCache; - // Validates protobufs for external policy data, validates the data itself, and // caches both locally. class ComponentCloudPolicyStore : public base::NonThreadSafe { @@ -91,10 +89,10 @@ class ComponentCloudPolicyStore : public base::NonThreadSafe { // Deletes the storage of namespace |ns| and stops serving its policies. void Delete(const PolicyNamespace& ns); - // Deletes the storage of all components of |domain| that are not in - // |components_to_keep|, and stops serving their policies. + // Deletes the storage of all components of |domain| that pass then given + // |filter|, and stops serving their policies. void Purge(PolicyDomain domain, - const std::set<std::string>& components_to_keep); + const ResourceCache::SubkeyFilter& filter); // Validates |proto| and returns the corresponding policy namespace in |ns|, // and the parsed ExternalPolicyData in |payload|. diff --git a/chrome/browser/policy/cloud/component_cloud_policy_store_unittest.cc b/chrome/browser/policy/cloud/component_cloud_policy_store_unittest.cc index 66ac160..4e46371 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_store_unittest.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_store_unittest.cc @@ -5,10 +5,10 @@ #include "chrome/browser/policy/cloud/component_cloud_policy_store.h" #include <map> -#include <set> #include <string> #include "base/basictypes.h" +#include "base/bind.h" #include "base/callback.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/ref_counted.h" @@ -48,6 +48,14 @@ std::string TestPolicyHash() { return base::SHA1HashString(kTestPolicy); } +bool NotEqual(const std::string& expected, const std::string& key) { + return key != expected; +} + +bool True(const std::string& ignored) { + return true; +} + class MockComponentCloudPolicyStoreDelegate : public ComponentCloudPolicyStore::Delegate { public: @@ -282,10 +290,9 @@ TEST_F(ComponentCloudPolicyStoreTest, Purge) { EXPECT_FALSE(IsEmpty()); EXPECT_TRUE(store_->policy().Equals(expected_bundle_)); - // Purge other namespaces. - std::set<std::string> keep; - keep.insert(kTestExtension); - store_->Purge(POLICY_DOMAIN_EXTENSIONS, keep); + // Purge other components. + store_->Purge(POLICY_DOMAIN_EXTENSIONS, + base::Bind(&NotEqual, kTestExtension)); // The policy for |ns| is still served. EXPECT_TRUE(store_->policy().Equals(expected_bundle_)); @@ -301,8 +308,7 @@ TEST_F(ComponentCloudPolicyStoreTest, Purge) { // Now purge everything. EXPECT_CALL(store_delegate_, OnComponentCloudPolicyStoreUpdated()); - keep.clear(); - store_->Purge(POLICY_DOMAIN_EXTENSIONS, keep); + store_->Purge(POLICY_DOMAIN_EXTENSIONS, base::Bind(&True)); Mock::VerifyAndClearExpectations(&store_delegate_); // No policies are served anymore. diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc b/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc index 504dcbb..5e6248c 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc +++ b/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc @@ -10,6 +10,8 @@ #include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_store.h" +#include "chrome/browser/policy/schema_registry_service.h" +#include "chrome/browser/policy/schema_registry_service_factory.h" #include "components/browser_context_keyed_service/browser_context_dependency_manager.h" #include "content/public/browser/browser_context.h" @@ -49,7 +51,9 @@ UserCloudPolicyManagerFactory::RegisterForOffTheRecordBrowserContext( UserCloudPolicyManagerFactory::UserCloudPolicyManagerFactory() : BrowserContextKeyedBaseFactory( "UserCloudPolicyManager", - BrowserContextDependencyManager::GetInstance()) {} + BrowserContextDependencyManager::GetInstance()) { + DependsOn(SchemaRegistryServiceFactory::GetInstance()); +} UserCloudPolicyManagerFactory::~UserCloudPolicyManagerFactory() {} @@ -77,7 +81,7 @@ UserCloudPolicyManagerFactory::CreateManagerForOriginalBrowserContext( store.Pass(), scoped_ptr<CloudExternalDataManager>(), base::MessageLoopProxy::current())); - manager->Init(); + manager->Init(SchemaRegistryServiceFactory::GetForContext(context)); return manager.Pass(); } diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc b/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc index 8c4490e..be9e0cd 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc +++ b/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc @@ -12,6 +12,7 @@ #include "chrome/browser/policy/cloud/mock_user_cloud_policy_store.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/schema_registry.h" #include "net/url_request/url_request_context_getter.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -53,7 +54,7 @@ class UserCloudPolicyManagerTest : public testing::Test { scoped_ptr<UserCloudPolicyStore>(store_), scoped_ptr<CloudExternalDataManager>(), loop_.message_loop_proxy())); - manager_->Init(); + manager_->Init(&schema_registry_); manager_->AddObserver(&observer_); Mock::VerifyAndClearExpectations(store_); } @@ -66,6 +67,7 @@ class UserCloudPolicyManagerTest : public testing::Test { PolicyBundle expected_bundle_; // Policy infrastructure. + SchemaRegistry schema_registry_; MockConfigurationPolicyObserver observer_; MockUserCloudPolicyStore* store_; // Not owned. scoped_ptr<UserCloudPolicyManager> manager_; diff --git a/chrome/browser/policy/config_dir_policy_loader_unittest.cc b/chrome/browser/policy/config_dir_policy_loader_unittest.cc index 5ac6d65..0213a9d 100644 --- a/chrome/browser/policy/config_dir_policy_loader_unittest.cc +++ b/chrome/browser/policy/config_dir_policy_loader_unittest.cc @@ -33,6 +33,7 @@ class TestHarness : public PolicyProviderTestHarness { virtual void SetUp() OVERRIDE; virtual ConfigurationPolicyProvider* CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) OVERRIDE; @@ -82,11 +83,12 @@ void TestHarness::SetUp() { } ConfigurationPolicyProvider* TestHarness::CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) { scoped_ptr<AsyncPolicyLoader> loader(new ConfigDirPolicyLoader( task_runner, test_dir(), POLICY_SCOPE_MACHINE)); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(registry, loader.Pass()); } void TestHarness::InstallEmptyPolicy() { diff --git a/chrome/browser/policy/configuration_policy_provider.cc b/chrome/browser/policy/configuration_policy_provider.cc index 18f849c..2803451 100644 --- a/chrome/browser/policy/configuration_policy_provider.cc +++ b/chrome/browser/policy/configuration_policy_provider.cc @@ -6,7 +6,6 @@ #include "base/callback.h" #include "chrome/browser/policy/external_data_fetcher.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_map.h" #include "policy/policy_constants.h" @@ -68,16 +67,26 @@ void FixDeprecatedPolicies(PolicyMap* policies) { ConfigurationPolicyProvider::Observer::~Observer() {} ConfigurationPolicyProvider::ConfigurationPolicyProvider() - : did_shutdown_(false) {} + : did_shutdown_(false), + schema_registry_(NULL) {} ConfigurationPolicyProvider::~ConfigurationPolicyProvider() { DCHECK(did_shutdown_); } -void ConfigurationPolicyProvider::Init() {} +void ConfigurationPolicyProvider::Init(SchemaRegistry* registry) { + schema_registry_ = registry; + schema_registry_->AddObserver(this); +} void ConfigurationPolicyProvider::Shutdown() { did_shutdown_ = true; + if (schema_registry_) { + // Unit tests don't initialize the BrowserPolicyConnector but call + // shutdown; handle that. + schema_registry_->RemoveObserver(this); + schema_registry_ = NULL; + } } bool ConfigurationPolicyProvider::IsInitializationComplete( @@ -85,6 +94,12 @@ bool ConfigurationPolicyProvider::IsInitializationComplete( return true; } +void ConfigurationPolicyProvider::OnSchemaRegistryUpdated( + bool has_new_schemas) { + if (has_new_schemas) + RefreshPolicies(); +} + void ConfigurationPolicyProvider::UpdatePolicy( scoped_ptr<PolicyBundle> bundle) { if (bundle.get()) @@ -98,6 +113,11 @@ void ConfigurationPolicyProvider::UpdatePolicy( OnUpdatePolicy(this)); } +const scoped_refptr<SchemaMap>& +ConfigurationPolicyProvider::schema_map() const { + return schema_registry_->schema_map(); +} + void ConfigurationPolicyProvider::AddObserver(Observer* observer) { observer_list_.AddObserver(observer); } @@ -106,7 +126,4 @@ void ConfigurationPolicyProvider::RemoveObserver(Observer* observer) { observer_list_.RemoveObserver(observer); } -void ConfigurationPolicyProvider::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) {} - } // namespace policy diff --git a/chrome/browser/policy/configuration_policy_provider.h b/chrome/browser/policy/configuration_policy_provider.h index 3fef214..4fa4937 100644 --- a/chrome/browser/policy/configuration_policy_provider.h +++ b/chrome/browser/policy/configuration_policy_provider.h @@ -10,16 +10,15 @@ #include "base/memory/scoped_ptr.h" #include "base/observer_list.h" #include "chrome/browser/policy/policy_bundle.h" +#include "chrome/browser/policy/schema_registry.h" #include "components/policy/core/common/policy_namespace.h" namespace policy { -class PolicyDomainDescriptor; - // A mostly-abstract super class for platform-specific policy providers. // Platform-specific policy providers (Windows Group Policy, gconf, // etc.) should implement a subclass of this class. -class ConfigurationPolicyProvider { +class ConfigurationPolicyProvider : public SchemaRegistry::Observer { public: class Observer { public: @@ -39,7 +38,9 @@ class ConfigurationPolicyProvider { // are created early during startup to provide the initial policies; the // Init() call allows them to perform initialization tasks that require // running message loops. - virtual void Init(); + // The policy provider will load policy for the components registered in + // the |schema_registry| whose domain is supported by this provider. + virtual void Init(SchemaRegistry* registry); // Must be invoked before deleting the provider. Implementations can override // this method to do appropriate cleanup while threads are still running, and @@ -67,13 +68,10 @@ class ConfigurationPolicyProvider { virtual void AddObserver(Observer* observer); virtual void RemoveObserver(Observer* observer); - // Notifies the provider that there is interest in loading policy for the - // listed components in the given |descriptor|. The list is complete; all the - // components that matter for the domain are included, and components not - // included can be discarded. The provider can ignore this information or use - // it to selectively load the corresponding policy from its sources. - virtual void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor); + // SchemaRegistry::Observer: + // This base implementation calls RefreshPolicies if + // |has_new_schemas| is true. + virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE; protected: // Subclasses must invoke this to update the policies currently served by @@ -81,6 +79,8 @@ class ConfigurationPolicyProvider { // The observers are notified after the policies are updated. void UpdatePolicy(scoped_ptr<PolicyBundle> bundle); + const scoped_refptr<SchemaMap>& schema_map() const; + private: // The policies currently configured at this provider. PolicyBundle policy_bundle_; @@ -88,6 +88,8 @@ class ConfigurationPolicyProvider { // Whether Shutdown() has been invoked. bool did_shutdown_; + SchemaRegistry* schema_registry_; + ObserverList<Observer, true> observer_list_; DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProvider); diff --git a/chrome/browser/policy/configuration_policy_provider_test.cc b/chrome/browser/policy/configuration_policy_provider_test.cc index 55ef224..7f02a65 100644 --- a/chrome/browser/policy/configuration_policy_provider_test.cc +++ b/chrome/browser/policy/configuration_policy_provider_test.cc @@ -13,6 +13,7 @@ #include "chrome/browser/policy/mock_configuration_policy_provider.h" #include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" +#include "components/policy/core/common/policy_namespace.h" #include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" @@ -21,6 +22,96 @@ using ::testing::_; namespace policy { +const char kTestChromeSchema[] = + "{" + " \"type\": \"object\"," + " \"properties\": {" + " \"StringPolicy\": { \"type\": \"string\" }," + " \"BooleanPolicy\": { \"type\": \"boolean\" }," + " \"IntegerPolicy\": { \"type\": \"integer\" }," + " \"StringListPolicy\": {" + " \"type\": \"array\"," + " \"items\": { \"type\": \"string\" }" + " }," + " \"DictionaryPolicy\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"bool\": { \"type\": \"boolean\" }," + " \"double\": { \"type\": \"number\" }," + " \"int\": { \"type\": \"integer\" }," + " \"string\": { \"type\": \"string\" }," + " \"array\": {" + " \"type\": \"array\"," + " \"items\": { \"type\": \"string\" }" + " }," + " \"dictionary\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"sub\": { \"type\": \"string\" }," + " \"sublist\": {" + " \"type\": \"array\"," + " \"items\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"aaa\": { \"type\": \"integer\" }," + " \"bbb\": { \"type\": \"integer\" }," + " \"ccc\": { \"type\": \"string\" }," + " \"ddd\": { \"type\": \"string\" }" + " }" + " }" + " }" + " }" + " }," + " \"list\": {" + " \"type\": \"array\"," + " \"items\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"subdictindex\": { \"type\": \"integer\" }," + " \"subdict\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"bool\": { \"type\": \"boolean\" }," + " \"double\": { \"type\": \"number\" }," + " \"int\": { \"type\": \"integer\" }," + " \"string\": { \"type\": \"string\" }" + " }" + " }" + " }" + " }" + " }," + " \"dict\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"bool\": { \"type\": \"boolean\" }," + " \"double\": { \"type\": \"number\" }," + " \"int\": { \"type\": \"integer\" }," + " \"string\": { \"type\": \"string\" }," + " \"list\": {" + " \"type\": \"array\"," + " \"items\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"subdictindex\": { \"type\": \"integer\" }," + " \"subdict\": {" + " \"type\": \"object\"," + " \"properties\": {" + " \"bool\": { \"type\": \"boolean\" }," + " \"double\": { \"type\": \"number\" }," + " \"int\": { \"type\": \"integer\" }," + " \"string\": { \"type\": \"string\" }" + " }" + " }" + " }" + " }" + " }" + " }" + " }" + " }" + " }" + " }" + "}"; + namespace test_policy_definitions { const char kKeyString[] = "StringPolicy"; @@ -47,6 +138,14 @@ PolicyTestBase::PolicyTestBase() {} PolicyTestBase::~PolicyTestBase() {} +void PolicyTestBase::SetUp() { + std::string error; + chrome_schema_ = Schema::Parse(kTestChromeSchema, &error); + ASSERT_TRUE(chrome_schema_.valid()) << error; + schema_registry_.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_CHROME, ""), + chrome_schema_); +} + void PolicyTestBase::TearDown() { loop_.RunUntilIdle(); } @@ -80,9 +179,27 @@ void ConfigurationPolicyProviderTest::SetUp() { test_harness_.reset((*GetParam())()); test_harness_->SetUp(); + Schema extension_schema = + chrome_schema_.GetKnownProperty(test_policy_definitions::kKeyDictionary); + ASSERT_TRUE(extension_schema.valid()); + schema_registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), + extension_schema); + schema_registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"), + extension_schema); + schema_registry_.RegisterComponent( + PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, + "cccccccccccccccccccccccccccccccc"), + extension_schema); + provider_.reset(test_harness_->CreateProvider( - loop_.message_loop_proxy(), &test_policy_definitions::kList)); - provider_->Init(); + &schema_registry_, + loop_.message_loop_proxy(), + &test_policy_definitions::kList)); + provider_->Init(&schema_registry_); // Some providers do a reload on init. Make sure any notifications generated // are fired now. loop_.RunUntilIdle(); @@ -172,13 +289,14 @@ TEST_P(ConfigurationPolicyProviderTest, StringListValue) { TEST_P(ConfigurationPolicyProviderTest, DictionaryValue) { base::DictionaryValue expected_value; expected_value.SetBoolean("bool", true); + expected_value.SetDouble("double", 123.456); expected_value.SetInteger("int", 123); - expected_value.SetString("str", "omg"); + expected_value.SetString("string", "omg"); base::ListValue* list = new base::ListValue(); list->Set(0U, base::Value::CreateStringValue("first")); list->Set(1U, base::Value::CreateStringValue("second")); - expected_value.Set("list", list); + expected_value.Set("array", list); base::DictionaryValue* dict = new base::DictionaryValue(); dict->SetString("sub", "value"); @@ -192,7 +310,7 @@ TEST_P(ConfigurationPolicyProviderTest, DictionaryValue) { sub->SetString("ddd", "444"); list->Append(sub); dict->Set("sublist", list); - expected_value.Set("dict", dict); + expected_value.Set("dictionary", dict); CheckValue(test_policy_definitions::kKeyDictionary, expected_value, @@ -283,7 +401,7 @@ TEST_P(Configuration3rdPartyPolicyProviderTest, Load3rdParty) { policy_dict.SetBoolean("bool", true); policy_dict.SetDouble("double", 123.456); policy_dict.SetInteger("int", 789); - policy_dict.SetString("str", "string value"); + policy_dict.SetString("string", "string value"); base::ListValue* list = new base::ListValue(); for (int i = 0; i < 2; ++i) { diff --git a/chrome/browser/policy/configuration_policy_provider_test.h b/chrome/browser/policy/configuration_policy_provider_test.h index 8b1cd66..637a528 100644 --- a/chrome/browser/policy/configuration_policy_provider_test.h +++ b/chrome/browser/policy/configuration_policy_provider_test.h @@ -13,6 +13,8 @@ #include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "chrome/browser/policy/policy_types.h" +#include "chrome/browser/policy/schema_registry.h" +#include "components/policy/core/common/schema.h" #include "testing/gtest/include/gtest/gtest.h" namespace base { @@ -49,9 +51,13 @@ class PolicyTestBase : public testing::Test { virtual ~PolicyTestBase(); // testing::Test: + virtual void SetUp() OVERRIDE; virtual void TearDown() OVERRIDE; protected: + Schema chrome_schema_; + SchemaRegistry schema_registry_; + // Create an actual IO loop (needed by FilePathWatcher). base::MessageLoopForIO loop_; @@ -74,6 +80,7 @@ class PolicyProviderTestHarness { // Create a new policy provider. virtual ConfigurationPolicyProvider* CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) = 0; diff --git a/chrome/browser/policy/mock_configuration_policy_provider.h b/chrome/browser/policy/mock_configuration_policy_provider.h index ee3ddbf..eb7fd53 100644 --- a/chrome/browser/policy/mock_configuration_policy_provider.h +++ b/chrome/browser/policy/mock_configuration_policy_provider.h @@ -5,9 +5,10 @@ #ifndef CHROME_BROWSER_POLICY_MOCK_CONFIGURATION_POLICY_PROVIDER_H_ #define CHROME_BROWSER_POLICY_MOCK_CONFIGURATION_POLICY_PROVIDER_H_ +#include "base/basictypes.h" #include "chrome/browser/policy/configuration_policy_provider.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_map.h" +#include "chrome/browser/policy/schema_registry.h" #include "testing/gmock/include/gmock/gmock.h" namespace policy { @@ -25,15 +26,24 @@ class MockConfigurationPolicyProvider : public ConfigurationPolicyProvider { MOCK_CONST_METHOD1(IsInitializationComplete, bool(PolicyDomain domain)); MOCK_METHOD0(RefreshPolicies, void()); - MOCK_METHOD1(RegisterPolicyDomain, - void(scoped_refptr<const PolicyDomainDescriptor>)); - // Make public for tests. using ConfigurationPolicyProvider::UpdatePolicy; // Utility method that invokes UpdatePolicy() with a PolicyBundle that maps // the Chrome namespace to a copy of |policy|. void UpdateChromePolicy(const PolicyMap& policy); + + // Convenience method so that tests don't need to create a registry to create + // this mock. + using ConfigurationPolicyProvider::Init; + void Init() { + ConfigurationPolicyProvider::Init(®istry_); + } + + private: + SchemaRegistry registry_; + + DISALLOW_COPY_AND_ASSIGN(MockConfigurationPolicyProvider); }; class MockConfigurationPolicyObserver diff --git a/chrome/browser/policy/mock_policy_service.h b/chrome/browser/policy/mock_policy_service.h index 953ce2d..3c609d4 100644 --- a/chrome/browser/policy/mock_policy_service.h +++ b/chrome/browser/policy/mock_policy_service.h @@ -5,7 +5,6 @@ #ifndef CHROME_BROWSER_POLICY_MOCK_POLICY_SERVICE_H_ #define CHROME_BROWSER_POLICY_MOCK_POLICY_SERVICE_H_ -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_service.h" #include "testing/gmock/include/gmock/gmock.h" @@ -30,12 +29,7 @@ class MockPolicyService : public PolicyService { MOCK_METHOD2(AddObserver, void(PolicyDomain, Observer*)); MOCK_METHOD2(RemoveObserver, void(PolicyDomain, Observer*)); - MOCK_METHOD1(RegisterPolicyDomain, - void(scoped_refptr<const PolicyDomainDescriptor>)); - MOCK_CONST_METHOD1(GetPolicies, const PolicyMap&(const PolicyNamespace&)); - MOCK_CONST_METHOD1(GetPolicyDomainDescriptor, - scoped_refptr<const PolicyDomainDescriptor>(PolicyDomain)); MOCK_CONST_METHOD1(IsInitializationComplete, bool(PolicyDomain domain)); MOCK_METHOD1(RefreshPolicies, void(const base::Closure&)); }; diff --git a/chrome/browser/policy/policy_browsertest.cc b/chrome/browser/policy/policy_browsertest.cc index 7f94473e..5aaa078 100644 --- a/chrome/browser/policy/policy_browsertest.cc +++ b/chrome/browser/policy/policy_browsertest.cc @@ -153,7 +153,6 @@ using content::BrowserThread; using content::URLRequestMockHTTPJob; -using testing::AnyNumber; using testing::Mock; using testing::Return; using testing::_; @@ -591,7 +590,6 @@ class PolicyTest : public InProcessBrowserTest { CommandLine::ForCurrentProcess()->AppendSwitch("noerrdialogs"); EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); } diff --git a/chrome/browser/policy/policy_domain_descriptor.cc b/chrome/browser/policy/policy_domain_descriptor.cc deleted file mode 100644 index e2bee4a..0000000 --- a/chrome/browser/policy/policy_domain_descriptor.cc +++ /dev/null @@ -1,63 +0,0 @@ -// 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/policy_domain_descriptor.h" - -#include "base/logging.h" -#include "base/stl_util.h" -#include "base/values.h" -#include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_map.h" - -namespace policy { - -PolicyDomainDescriptor::PolicyDomainDescriptor(PolicyDomain domain) - : domain_(domain) {} - -void PolicyDomainDescriptor::RegisterComponent(const std::string& component_id, - Schema schema) { - schema_map_[component_id] = schema; -} - -void PolicyDomainDescriptor::FilterBundle(PolicyBundle* bundle) const { - // Chrome policies are not filtered, so that typos appear in about:policy. - DCHECK_NE(POLICY_DOMAIN_CHROME, domain_); - - for (PolicyBundle::iterator it_bundle = bundle->begin(); - it_bundle != bundle->end(); ++it_bundle) { - const PolicyNamespace& ns = it_bundle->first; - if (ns.domain != domain_) - continue; - - SchemaMap::const_iterator it_schema = schema_map_.find(ns.component_id); - if (it_schema == schema_map_.end()) { - // Component ID not found. - it_bundle->second->Clear(); - continue; - } - - // TODO(joaodasilva): if a component is registered but doesn't have a schema - // then its policies aren't filtered. This behavior is enabled for M29 to - // allow a graceful update of the Legacy Browser Support extension; it'll - // be removed for M32. http://crbug.com/240704 - Schema schema = it_schema->second; - if (!schema.valid()) - continue; - - PolicyMap* map = it_bundle->second; - for (PolicyMap::const_iterator it_map = map->begin(); - it_map != map->end();) { - const std::string& policy_name = it_map->first; - const base::Value* policy_value = it_map->second.value; - Schema policy_schema = schema.GetProperty(policy_name); - ++it_map; - if (!policy_value || !policy_schema.Validate(*policy_value)) - map->Erase(policy_name); - } - } -} - -PolicyDomainDescriptor::~PolicyDomainDescriptor() {} - -} // namespace policy diff --git a/chrome/browser/policy/policy_domain_descriptor.h b/chrome/browser/policy/policy_domain_descriptor.h deleted file mode 100644 index ae92a48..0000000 --- a/chrome/browser/policy/policy_domain_descriptor.h +++ /dev/null @@ -1,55 +0,0 @@ -// 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. - -#ifndef CHROME_BROWSER_POLICY_POLICY_DOMAIN_DESCRIPTOR_H_ -#define CHROME_BROWSER_POLICY_POLICY_DOMAIN_DESCRIPTOR_H_ - -#include <map> -#include <string> - -#include "base/basictypes.h" -#include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" -#include "components/policy/core/common/policy_namespace.h" -#include "components/policy/core/common/schema.h" - -namespace policy { - -class PolicyBundle; - -// For each policy domain, this class keeps the complete list of valid -// components for that domain, and the Schema for each component. -class PolicyDomainDescriptor - : public base::RefCountedThreadSafe<PolicyDomainDescriptor> { - public: - typedef std::map<std::string, Schema> SchemaMap; - - explicit PolicyDomainDescriptor(PolicyDomain domain); - - PolicyDomain domain() const { return domain_; } - const SchemaMap& components() const { return schema_map_; } - - // Registers the given |component_id| for this domain, and sets its current - // |schema|. This registration overrides any previously set schema for this - // component. - void RegisterComponent(const std::string& component_id, Schema schema); - - // Removes all the policies in |bundle| that don't match this descriptor. - // Policies of domains other than |domain_| are ignored. - void FilterBundle(PolicyBundle* bundle) const; - - private: - friend class base::RefCountedThreadSafe<PolicyDomainDescriptor>; - - ~PolicyDomainDescriptor(); - - PolicyDomain domain_; - SchemaMap schema_map_; - - DISALLOW_COPY_AND_ASSIGN(PolicyDomainDescriptor); -}; - -} // namespace policy - -#endif // CHROME_BROWSER_POLICY_POLICY_DOMAIN_DESCRIPTOR_H_ diff --git a/chrome/browser/policy/policy_domain_descriptor_unittest.cc b/chrome/browser/policy/policy_domain_descriptor_unittest.cc deleted file mode 100644 index 7841a8a..0000000 --- a/chrome/browser/policy/policy_domain_descriptor_unittest.cc +++ /dev/null @@ -1,218 +0,0 @@ -// 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/policy_domain_descriptor.h" - -#include <string> - -#include "base/callback.h" -#include "base/memory/weak_ptr.h" -#include "base/values.h" -#include "chrome/browser/policy/external_data_fetcher.h" -#include "chrome/browser/policy/external_data_manager.h" -#include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_map.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace policy { - -class PolicyDomainDescriptorTest : public testing::Test { - protected: - scoped_ptr<ExternalDataFetcher> CreateExternalDataFetcher() const; -}; - -scoped_ptr<ExternalDataFetcher> - PolicyDomainDescriptorTest::CreateExternalDataFetcher() const { - return make_scoped_ptr( - new ExternalDataFetcher(base::WeakPtr<ExternalDataManager>(), - std::string())); -} - -TEST_F(PolicyDomainDescriptorTest, FilterBundle) { - scoped_refptr<PolicyDomainDescriptor> descriptor = - new PolicyDomainDescriptor(POLICY_DOMAIN_EXTENSIONS); - EXPECT_EQ(POLICY_DOMAIN_EXTENSIONS, descriptor->domain()); - EXPECT_TRUE(descriptor->components().empty()); - - std::string error; - Schema schema = Schema::Parse( - "{" - " \"type\":\"object\"," - " \"properties\": {" - " \"Array\": {" - " \"type\": \"array\"," - " \"items\": { \"type\": \"string\" }" - " }," - " \"Boolean\": { \"type\": \"boolean\" }," - " \"Integer\": { \"type\": \"integer\" }," - " \"Null\": { \"type\": \"null\" }," - " \"Number\": { \"type\": \"number\" }," - " \"Object\": {" - " \"type\": \"object\"," - " \"properties\": {" - " \"a\": { \"type\": \"string\" }," - " \"b\": { \"type\": \"integer\" }" - " }" - " }," - " \"String\": { \"type\": \"string\" }" - " }" - "}", &error); - ASSERT_TRUE(schema.valid()) << error; - - descriptor->RegisterComponent("abc", schema); - - EXPECT_EQ(1u, descriptor->components().size()); - EXPECT_EQ(1u, descriptor->components().count("abc")); - - PolicyBundle bundle; - descriptor->FilterBundle(&bundle); - const PolicyBundle empty_bundle; - EXPECT_TRUE(bundle.Equals(empty_bundle)); - - // Other namespaces aren't filtered. - PolicyBundle expected_bundle; - PolicyNamespace chrome_ns(POLICY_DOMAIN_CHROME, ""); - expected_bundle.Get(chrome_ns).Set("ChromePolicy", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value"), - NULL); - bundle.CopyFrom(expected_bundle); - // Unknown components of the domain are filtered out. - PolicyNamespace another_extension_ns(POLICY_DOMAIN_EXTENSIONS, "xyz"); - bundle.Get(another_extension_ns).Set( - "AnotherExtensionPolicy", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value"), - NULL); - descriptor->FilterBundle(&bundle); - EXPECT_TRUE(bundle.Equals(expected_bundle)); - - PolicyNamespace extension_ns(POLICY_DOMAIN_EXTENSIONS, "abc"); - PolicyMap& map = expected_bundle.Get(extension_ns); - base::ListValue list; - list.AppendString("a"); - list.AppendString("b"); - map.Set("Array", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - list.DeepCopy(), NULL); - map.Set("Boolean", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateBooleanValue(true), NULL); - map.Set("Integer", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateIntegerValue(1), NULL); - map.Set("Null", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateNullValue(), NULL); - map.Set("Number", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateDoubleValue(1.2), NULL); - base::DictionaryValue dict; - dict.SetString("a", "b"); - dict.SetInteger("b", 2); - map.Set("Object", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - dict.DeepCopy(), NULL); - map.Set("String", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateStringValue("value"), NULL); - - bundle.MergeFrom(expected_bundle); - bundle.Get(extension_ns).Set("Unexpected", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("to-be-removed"), - NULL); - - descriptor->FilterBundle(&bundle); - EXPECT_TRUE(bundle.Equals(expected_bundle)); - - // Mismatched types are also removed. - bundle.Clear(); - PolicyMap& badmap = bundle.Get(extension_ns); - badmap.Set("Array", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateBooleanValue(false), NULL); - badmap.Set("Boolean", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateIntegerValue(0), NULL); - badmap.Set("Integer", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateBooleanValue(false), NULL); - badmap.Set("Null", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateBooleanValue(false), NULL); - badmap.Set("Number", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateBooleanValue(false), NULL); - badmap.Set("Object", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - base::Value::CreateBooleanValue(false), NULL); - badmap.Set("String", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - NULL, CreateExternalDataFetcher().release()); - - descriptor->FilterBundle(&bundle); - EXPECT_TRUE(bundle.Equals(empty_bundle)); -} - -TEST_F(PolicyDomainDescriptorTest, LegacyComponents) { - scoped_refptr<PolicyDomainDescriptor> descriptor = - new PolicyDomainDescriptor(POLICY_DOMAIN_EXTENSIONS); - EXPECT_EQ(POLICY_DOMAIN_EXTENSIONS, descriptor->domain()); - EXPECT_TRUE(descriptor->components().empty()); - - std::string error; - Schema schema = Schema::Parse( - "{" - " \"type\":\"object\"," - " \"properties\": {" - " \"String\": { \"type\": \"string\" }" - " }" - "}", &error); - ASSERT_TRUE(schema.valid()) << error; - - descriptor->RegisterComponent("with-schema", schema); - descriptor->RegisterComponent("without-schema", Schema()); - - EXPECT_EQ(2u, descriptor->components().size()); - - // |bundle| contains policies loaded by a policy provider. - PolicyBundle bundle; - - // Known components with schemas are filtered. - PolicyNamespace extension_ns(POLICY_DOMAIN_EXTENSIONS, "with-schema"); - bundle.Get(extension_ns).Set("String", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value 1"), - NULL); - - // Known components without a schema are not filtered. - PolicyNamespace without_schema_ns(POLICY_DOMAIN_EXTENSIONS, "without-schema"); - bundle.Get(without_schema_ns).Set("Schemaless", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value 2"), - NULL); - - // Other namespaces aren't filtered. - PolicyNamespace chrome_ns(POLICY_DOMAIN_CHROME, ""); - bundle.Get(chrome_ns).Set("ChromePolicy", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value 3"), - NULL); - - PolicyBundle expected_bundle; - expected_bundle.MergeFrom(bundle); - - // Unknown policies of known components with a schema are removed. - bundle.Get(extension_ns).Set("Surprise", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value 4"), - NULL); - - // Unknown components are removed. - PolicyNamespace unknown_ns(POLICY_DOMAIN_EXTENSIONS, "unknown"); - bundle.Get(unknown_ns).Set("Surprise", - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("value 5"), - NULL); - - descriptor->FilterBundle(&bundle); - EXPECT_TRUE(bundle.Equals(expected_bundle)); -} - -} // namespace policy diff --git a/chrome/browser/policy/policy_loader_mac.cc b/chrome/browser/policy/policy_loader_mac.cc index 2d8e220..50b0cf2 100644 --- a/chrome/browser/policy/policy_loader_mac.cc +++ b/chrome/browser/policy/policy_loader_mac.cc @@ -17,10 +17,10 @@ #include "base/values.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_load_status.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/preferences_mac.h" +#include "chrome/browser/policy/schema_map.h" #include "components/policy/core/common/schema.h" #include "policy/policy_constants.h" @@ -84,8 +84,8 @@ scoped_ptr<PolicyBundle> PolicyLoaderMac::Load() { scoped_ptr<PolicyBundle> bundle(new PolicyBundle()); // Load Chrome's policy. - // TODO(joaodasilva): use a schema for Chrome once it's generated and - // available from a PolicyDomainDescriptor. + // TODO(joaodasilva): Use the Chrome schema instead of the + // PolicyDefinitionList. PolicyMap& chrome_policy = bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); @@ -116,20 +116,7 @@ scoped_ptr<PolicyBundle> PolicyLoaderMac::Load() { status.Add(POLICY_LOAD_STATUS_NO_POLICY); // Load policy for the registered components. - static const struct { - PolicyDomain domain; - const char* domain_name; - } kSupportedDomains[] = { - { POLICY_DOMAIN_EXTENSIONS, "extensions" }, - }; - for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kSupportedDomains); ++i) { - DescriptorMap::const_iterator it = - descriptor_map().find(kSupportedDomains[i].domain); - if (it != descriptor_map().end()) { - LoadPolicyForDomain( - it->second, kSupportedDomains[i].domain_name, bundle.get()); - } - } + LoadPolicyForDomain(POLICY_DOMAIN_EXTENSIONS, "extensions", bundle.get()); return bundle.Pass(); } @@ -189,30 +176,30 @@ base::Value* PolicyLoaderMac::CreateValueFromProperty( } void PolicyLoaderMac::LoadPolicyForDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor, + PolicyDomain domain, const std::string& domain_name, PolicyBundle* bundle) { std::string id_prefix(base::mac::BaseBundleID()); id_prefix.append(".").append(domain_name).append("."); - for (PolicyDomainDescriptor::SchemaMap::const_iterator it_schema = - descriptor->components().begin(); - it_schema != descriptor->components().end(); ++it_schema) { + const ComponentMap* components = schema_map()->GetComponents(domain); + if (!components) + return; + + for (ComponentMap::const_iterator it = components->begin(); + it != components->end(); ++it) { PolicyMap policy; - LoadPolicyForComponent( - id_prefix + it_schema->first, it_schema->second, &policy); - if (!policy.empty()) { - bundle->Get(PolicyNamespace(descriptor->domain(), it_schema->first)) - .Swap(&policy); - } + LoadPolicyForComponent(id_prefix + it->first, it->second, &policy); + if (!policy.empty()) + bundle->Get(PolicyNamespace(domain, it->first)).Swap(&policy); } } void PolicyLoaderMac::LoadPolicyForComponent( const std::string& bundle_id_string, - Schema schema, + const Schema& schema, PolicyMap* policy) { - // TODO(joaodasilva): extensions may be registered in a PolicyDomainDescriptor + // TODO(joaodasilva): Extensions may be registered in a ComponentMap // without a schema, to allow a graceful update of the Legacy Browser Support // extension on Windows. Remove this check once that support is removed. if (!schema.valid()) diff --git a/chrome/browser/policy/policy_loader_mac.h b/chrome/browser/policy/policy_loader_mac.h index 68da9e4..b879892 100644 --- a/chrome/browser/policy/policy_loader_mac.h +++ b/chrome/browser/policy/policy_loader_mac.h @@ -13,6 +13,7 @@ #include "base/files/file_path_watcher.h" #include "base/memory/ref_counted.h" #include "chrome/browser/policy/async_policy_loader.h" +#include "components/policy/core/common/policy_namespace.h" class MacPreferences; @@ -23,7 +24,7 @@ class Value; namespace policy { -class PolicyDomainDescriptor; +class PolicyBundle; class PolicyMap; class Schema; struct PolicyDefinitionList; @@ -53,17 +54,17 @@ class PolicyLoaderMac : public AsyncPolicyLoader { // Callback for the FilePathWatcher. void OnFileUpdated(const base::FilePath& path, bool error); - // Loads policies for the components described in |descriptor|, which belong - // to the domain |domain_name|, and stores them in the |bundle|. + // Loads policies for the components described in the current schema_map() + // which belong to the domain |domain_name|, and stores them in the |bundle|. void LoadPolicyForDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor, + PolicyDomain domain, const std::string& domain_name, PolicyBundle* bundle); // Loads the policies described in |schema| from the bundle identified by // |bundle_id_string|, and stores them in |policy|. void LoadPolicyForComponent(const std::string& bundle_id_string, - Schema schema, + const Schema& schema, PolicyMap* policy); // List of recognized policies. diff --git a/chrome/browser/policy/policy_loader_mac_unittest.cc b/chrome/browser/policy/policy_loader_mac_unittest.cc index 76c2bc1..19695d2 100644 --- a/chrome/browser/policy/policy_loader_mac_unittest.cc +++ b/chrome/browser/policy/policy_loader_mac_unittest.cc @@ -127,6 +127,7 @@ class TestHarness : public PolicyProviderTestHarness { virtual void SetUp() OVERRIDE; virtual ConfigurationPolicyProvider* CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) OVERRIDE; @@ -160,12 +161,13 @@ TestHarness::~TestHarness() {} void TestHarness::SetUp() {} ConfigurationPolicyProvider* TestHarness::CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_definition_list) { prefs_ = new MockPreferences(); scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderMac( task_runner, policy_definition_list, base::FilePath(), prefs_)); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(registry, loader.Pass()); } void TestHarness::InstallEmptyPolicy() {} @@ -232,27 +234,27 @@ INSTANTIATE_TEST_CASE_P( class PolicyLoaderMacTest : public PolicyTestBase { protected: PolicyLoaderMacTest() - : prefs_(new MockPreferences()), - loader_(new PolicyLoaderMac(loop_.message_loop_proxy(), - &test_policy_definitions::kList, - base::FilePath(), - prefs_)), - provider_(scoped_ptr<AsyncPolicyLoader>(loader_)) {} + : prefs_(new MockPreferences()) {} virtual ~PolicyLoaderMacTest() {} virtual void SetUp() OVERRIDE { PolicyTestBase::SetUp(); - provider_.Init(); + scoped_ptr<AsyncPolicyLoader> loader( + new PolicyLoaderMac(loop_.message_loop_proxy(), + &test_policy_definitions::kList, + base::FilePath(), + prefs_)); + provider_.reset(new AsyncPolicyProvider(&schema_registry_, loader.Pass())); + provider_->Init(&schema_registry_); } virtual void TearDown() OVERRIDE { - provider_.Shutdown(); + provider_->Shutdown(); PolicyTestBase::TearDown(); } MockPreferences* prefs_; - PolicyLoaderMac* loader_; - AsyncPolicyProvider provider_; + scoped_ptr<AsyncPolicyProvider> provider_; }; TEST_F(PolicyLoaderMacTest, Invalid) { @@ -268,10 +270,10 @@ TEST_F(PolicyLoaderMacTest, Invalid) { prefs_->AddTestItem(name, invalid_data.get(), false); // Make the provider read the updated |prefs_|. - provider_.RefreshPolicies(); + provider_->RefreshPolicies(); loop_.RunUntilIdle(); const PolicyBundle kEmptyBundle; - EXPECT_TRUE(provider_.policies().Equals(kEmptyBundle)); + EXPECT_TRUE(provider_->policies().Equals(kEmptyBundle)); } TEST_F(PolicyLoaderMacTest, TestNonForcedValue) { @@ -283,7 +285,7 @@ TEST_F(PolicyLoaderMacTest, TestNonForcedValue) { prefs_->AddTestItem(name, test_value.get(), false); // Make the provider read the updated |prefs_|. - provider_.RefreshPolicies(); + provider_->RefreshPolicies(); loop_.RunUntilIdle(); PolicyBundle expected_bundle; expected_bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) @@ -292,7 +294,7 @@ TEST_F(PolicyLoaderMacTest, TestNonForcedValue) { POLICY_SCOPE_USER, base::Value::CreateStringValue("string value"), NULL); - EXPECT_TRUE(provider_.policies().Equals(expected_bundle)); + EXPECT_TRUE(provider_->policies().Equals(expected_bundle)); } TEST_F(PolicyLoaderMacTest, TestConversions) { diff --git a/chrome/browser/policy/policy_loader_win_unittest.cc b/chrome/browser/policy/policy_loader_win_unittest.cc index de822a1..9141cdc 100644 --- a/chrome/browser/policy/policy_loader_win_unittest.cc +++ b/chrome/browser/policy/policy_loader_win_unittest.cc @@ -34,6 +34,7 @@ #include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/preg_parser_win.h" +#include "chrome/browser/policy/schema_map.h" #include "components/json_schema/json_schema_constants.h" #include "policy/policy_constants.h" #include "testing/gtest/include/gtest/gtest.h" @@ -252,6 +253,7 @@ class RegistryTestHarness : public PolicyProviderTestHarness, virtual void SetUp() OVERRIDE; virtual ConfigurationPolicyProvider* CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_list) OVERRIDE; @@ -303,6 +305,7 @@ class PRegTestHarness : public PolicyProviderTestHarness, virtual void SetUp() OVERRIDE; virtual ConfigurationPolicyProvider* CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_list) OVERRIDE; @@ -420,11 +423,12 @@ RegistryTestHarness::~RegistryTestHarness() {} void RegistryTestHarness::SetUp() {} ConfigurationPolicyProvider* RegistryTestHarness::CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_list) { scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderWin( task_runner, policy_list, kRegistryChromePolicyKey, this)); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(registry, loader.Pass()); } void RegistryTestHarness::InstallEmptyPolicy() {} @@ -552,11 +556,12 @@ void PRegTestHarness::SetUp() { } ConfigurationPolicyProvider* PRegTestHarness::CreateProvider( + SchemaRegistry* registry, scoped_refptr<base::SequencedTaskRunner> task_runner, const PolicyDefinitionList* policy_list) { scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderWin( task_runner, policy_list, kRegistryChromePolicyKey, this)); - return new AsyncPolicyProvider(loader.Pass()); + return new AsyncPolicyProvider(registry, loader.Pass()); } void PRegTestHarness::InstallEmptyPolicy() {} @@ -789,12 +794,25 @@ class PolicyLoaderWinTest : public PolicyTestBase, virtual ~PolicyLoaderWinTest() {} virtual void SetUp() OVERRIDE { + PolicyTestBase::SetUp(); + ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &test_data_dir_)); test_data_dir_ = test_data_dir_.AppendASCII("chrome") .AppendASCII("test") .AppendASCII("data") .AppendASCII("policy") .AppendASCII("gpo"); + + // Unknown components will be filtered out. Register their names with an + // invalid schema to avoid that. + ComponentMap components; + components["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] = Schema(); + components["bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"] = Schema(); + components["int"] = Schema(); + components["merge"] = Schema(); + components["string"] = Schema(); + components["test"] = Schema(); + schema_registry_.RegisterComponents(POLICY_DOMAIN_EXTENSIONS, components); } // AppliedGPOListProvider: @@ -826,7 +844,8 @@ class PolicyLoaderWinTest : public PolicyTestBase, PolicyLoaderWin loader(loop_.message_loop_proxy(), &test_policy_definitions::kList, kTestPolicyKey, this); - scoped_ptr<PolicyBundle> loaded(loader.Load()); + scoped_ptr<PolicyBundle> loaded( + loader.InitialLoad(schema_registry_.schema_map())); return loaded->Equals(expected); } diff --git a/chrome/browser/policy/policy_prefs_browsertest.cc b/chrome/browser/policy/policy_prefs_browsertest.cc index 9d9abb4..1dd1dd0 100644 --- a/chrome/browser/policy/policy_prefs_browsertest.cc +++ b/chrome/browser/policy/policy_prefs_browsertest.cc @@ -37,7 +37,6 @@ #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" -using testing::AnyNumber; using testing::Return; using testing::_; @@ -389,7 +388,6 @@ class PolicyPrefsTest virtual void SetUpInProcessBrowserTestFixture() OVERRIDE { EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); } diff --git a/chrome/browser/policy/policy_service.h b/chrome/browser/policy/policy_service.h index b6c6c0e..58ce2c1 100644 --- a/chrome/browser/policy/policy_service.h +++ b/chrome/browser/policy/policy_service.h @@ -10,14 +10,11 @@ #include "base/basictypes.h" #include "base/callback.h" -#include "base/memory/ref_counted.h" #include "chrome/browser/policy/policy_map.h" #include "components/policy/core/common/policy_namespace.h" namespace policy { -class PolicyDomainDescriptor; - // The PolicyService merges policies from all available sources, taking into // account their priorities. Policy clients can retrieve policy for their domain // and register for notifications on policy updates. @@ -54,21 +51,8 @@ class PolicyService { virtual void RemoveObserver(PolicyDomain domain, Observer* observer) = 0; - // Registers the |descriptor| of a policy domain, indicated by - // |descriptor->domain()|. This overrides the descriptor previously set, if - // there was one. - // This registration signals that there is interest in receiving policy for - // the components listed in the descriptor. The policy providers will try to - // load policy for these components, and validate it against the descriptor. - // Cached data for components that aren't registered anymore may be dropped. - virtual void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) = 0; - virtual const PolicyMap& GetPolicies(const PolicyNamespace& ns) const = 0; - virtual scoped_refptr<const PolicyDomainDescriptor> GetPolicyDomainDescriptor( - PolicyDomain domain) const = 0; - // The PolicyService loads policy from several sources, and some require // asynchronous loads. IsInitializationComplete() returns true once all // sources have loaded their policies for the given |domain|. diff --git a/chrome/browser/policy/policy_service_impl.cc b/chrome/browser/policy/policy_service_impl.cc index 2cfdcbb..4604f75 100644 --- a/chrome/browser/policy/policy_service_impl.cc +++ b/chrome/browser/policy/policy_service_impl.cc @@ -9,7 +9,6 @@ #include "base/bind.h" #include "base/message_loop/message_loop.h" #include "base/stl_util.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" #include "chrome/browser/policy/policy_map.h" namespace policy { @@ -62,23 +61,11 @@ void PolicyServiceImpl::RemoveObserver(PolicyDomain domain, } } -void PolicyServiceImpl::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) { - domain_descriptors_[descriptor->domain()] = descriptor; - for (Iterator it = providers_.begin(); it != providers_.end(); ++it) - (*it)->RegisterPolicyDomain(descriptor); -} - const PolicyMap& PolicyServiceImpl::GetPolicies( const PolicyNamespace& ns) const { return policy_bundle_.Get(ns); } -scoped_refptr<const PolicyDomainDescriptor> -PolicyServiceImpl::GetPolicyDomainDescriptor(PolicyDomain domain) const { - return domain_descriptors_[domain]; -} - bool PolicyServiceImpl::IsInitializationComplete(PolicyDomain domain) const { DCHECK(domain >= 0 && domain < POLICY_DOMAIN_SIZE); return initialization_complete_[domain]; diff --git a/chrome/browser/policy/policy_service_impl.h b/chrome/browser/policy/policy_service_impl.h index 51716f2..6817574 100644 --- a/chrome/browser/policy/policy_service_impl.h +++ b/chrome/browser/policy/policy_service_impl.h @@ -38,12 +38,8 @@ class PolicyServiceImpl : public PolicyService, PolicyService::Observer* observer) OVERRIDE; virtual void RemoveObserver(PolicyDomain domain, PolicyService::Observer* observer) OVERRIDE; - virtual void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) OVERRIDE; virtual const PolicyMap& GetPolicies( const PolicyNamespace& ns) const OVERRIDE; - virtual scoped_refptr<const PolicyDomainDescriptor> GetPolicyDomainDescriptor( - PolicyDomain domain) const OVERRIDE; virtual bool IsInitializationComplete(PolicyDomain domain) const OVERRIDE; virtual void RefreshPolicies(const base::Closure& callback) OVERRIDE; @@ -77,10 +73,6 @@ class PolicyServiceImpl : public PolicyService, // Maps each policy namespace to its current policies. PolicyBundle policy_bundle_; - // Maps each policy domain to its current descriptor. - scoped_refptr<const PolicyDomainDescriptor> - domain_descriptors_[POLICY_DOMAIN_SIZE]; - // Maps each policy domain to its observer list. ObserverMap observers_; diff --git a/chrome/browser/policy/policy_service_impl_unittest.cc b/chrome/browser/policy/policy_service_impl_unittest.cc index a02f38f..365e5ff 100644 --- a/chrome/browser/policy/policy_service_impl_unittest.cc +++ b/chrome/browser/policy/policy_service_impl_unittest.cc @@ -7,15 +7,12 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/callback.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/run_loop.h" #include "base/values.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" #include "chrome/browser/policy/mock_policy_service.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" -#include "components/policy/core/common/schema.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -605,62 +602,4 @@ TEST_F(PolicyServiceTest, IsInitializationComplete) { policy_service_->RemoveObserver(POLICY_DOMAIN_EXTENSIONS, &observer); } -TEST_F(PolicyServiceTest, RegisterPolicyDomain) { - EXPECT_FALSE(policy_service_->GetPolicyDomainDescriptor(POLICY_DOMAIN_CHROME) - .get()); - EXPECT_FALSE(policy_service_->GetPolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS).get()); - - EXPECT_CALL(provider1_, RegisterPolicyDomain(_)).Times(AnyNumber()); - EXPECT_CALL(provider2_, RegisterPolicyDomain(_)).Times(AnyNumber()); - - scoped_refptr<const PolicyDomainDescriptor> chrome_descriptor = - new PolicyDomainDescriptor(POLICY_DOMAIN_CHROME); - EXPECT_CALL(provider0_, RegisterPolicyDomain(chrome_descriptor)); - policy_service_->RegisterPolicyDomain(chrome_descriptor); - Mock::VerifyAndClearExpectations(&provider0_); - - EXPECT_TRUE(policy_service_->GetPolicyDomainDescriptor(POLICY_DOMAIN_CHROME) - .get()); - EXPECT_FALSE(policy_service_->GetPolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS).get()); - - // Register another namespace. - std::string error; - Schema schema = Schema::Parse( - "{" - " \"type\":\"object\"," - " \"properties\": {" - " \"Boolean\": { \"type\": \"boolean\" }," - " \"Integer\": { \"type\": \"integer\" }," - " \"Null\": { \"type\": \"null\" }," - " \"Number\": { \"type\": \"number\" }," - " \"Object\": { \"type\": \"object\" }," - " \"String\": { \"type\": \"string\" }" - " }" - "}", &error); - ASSERT_TRUE(schema.valid()) << error; - scoped_refptr<PolicyDomainDescriptor> extensions_descriptor = - new PolicyDomainDescriptor(POLICY_DOMAIN_EXTENSIONS); - extensions_descriptor->RegisterComponent(kExtension, schema); - EXPECT_CALL(provider0_, RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor>(extensions_descriptor))); - policy_service_->RegisterPolicyDomain(extensions_descriptor); - Mock::VerifyAndClearExpectations(&provider0_); - - EXPECT_TRUE(policy_service_->GetPolicyDomainDescriptor(POLICY_DOMAIN_CHROME) - .get()); - EXPECT_TRUE(policy_service_->GetPolicyDomainDescriptor( - POLICY_DOMAIN_EXTENSIONS).get()); - - // Remove those components. - scoped_refptr<PolicyDomainDescriptor> empty_extensions_descriptor = - new PolicyDomainDescriptor(POLICY_DOMAIN_EXTENSIONS); - EXPECT_CALL(provider0_, RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor>( - empty_extensions_descriptor))); - policy_service_->RegisterPolicyDomain(empty_extensions_descriptor); - Mock::VerifyAndClearExpectations(&provider0_); -} - } // namespace policy diff --git a/chrome/browser/policy/policy_service_stub.cc b/chrome/browser/policy/policy_service_stub.cc index afbd8ea..c25b50f 100644 --- a/chrome/browser/policy/policy_service_stub.cc +++ b/chrome/browser/policy/policy_service_stub.cc @@ -5,7 +5,6 @@ #include "chrome/browser/policy/policy_service_stub.h" #include "base/message_loop/message_loop.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" namespace policy { @@ -19,19 +18,11 @@ void PolicyServiceStub::AddObserver(PolicyDomain domain, void PolicyServiceStub::RemoveObserver(PolicyDomain domain, Observer* observer) {} -void PolicyServiceStub::RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) {} - const PolicyMap& PolicyServiceStub::GetPolicies( const PolicyNamespace& ns) const { return kEmpty_; }; -scoped_refptr<const PolicyDomainDescriptor> -PolicyServiceStub::GetPolicyDomainDescriptor(PolicyDomain domain) const { - return NULL; -} - bool PolicyServiceStub::IsInitializationComplete(PolicyDomain domain) const { return true; } diff --git a/chrome/browser/policy/policy_service_stub.h b/chrome/browser/policy/policy_service_stub.h index 0b27cfc..c0849b3 100644 --- a/chrome/browser/policy/policy_service_stub.h +++ b/chrome/browser/policy/policy_service_stub.h @@ -24,15 +24,9 @@ class PolicyServiceStub : public PolicyService { virtual void RemoveObserver(PolicyDomain domain, Observer* observer) OVERRIDE; - virtual void RegisterPolicyDomain( - scoped_refptr<const PolicyDomainDescriptor> descriptor) OVERRIDE; - virtual const PolicyMap& GetPolicies( const PolicyNamespace& ns) const OVERRIDE; - virtual scoped_refptr<const PolicyDomainDescriptor> GetPolicyDomainDescriptor( - PolicyDomain domain) const OVERRIDE; - virtual bool IsInitializationComplete(PolicyDomain domain) const OVERRIDE; virtual void RefreshPolicies(const base::Closure& callback) OVERRIDE; diff --git a/chrome/browser/policy/profile_policy_connector.cc b/chrome/browser/policy/profile_policy_connector.cc index 96812cc..ba88c99 100644 --- a/chrome/browser/policy/profile_policy_connector.cc +++ b/chrome/browser/policy/profile_policy_connector.cc @@ -12,6 +12,8 @@ #include "chrome/browser/policy/cloud/cloud_policy_manager.h" #include "chrome/browser/policy/configuration_policy_provider.h" #include "chrome/browser/policy/policy_service.h" +#include "chrome/browser/policy/schema_registry_service.h" +#include "chrome/browser/policy/schema_registry_service_factory.h" #if defined(OS_CHROMEOS) #include "base/bind.h" @@ -56,7 +58,8 @@ void ProfilePolicyConnector::Init( // This case occurs for the signin profile. special_user_policy_provider_.reset( new LoginProfilePolicyProvider(connector->GetPolicyService())); - special_user_policy_provider_->Init(); + special_user_policy_provider_->Init( + SchemaRegistryServiceFactory::GetForContext(profile_)); } else { // |user| should never be NULL except for the signin profile. is_primary_user_ = user == chromeos::UserManager::Get()->GetPrimaryUser(); @@ -122,7 +125,8 @@ void ProfilePolicyConnector::InitializeDeviceLocalAccountPolicyProvider( return; special_user_policy_provider_.reset(new DeviceLocalAccountPolicyProvider( username, device_local_account_policy_service)); - special_user_policy_provider_->Init(); + special_user_policy_provider_->Init( + SchemaRegistryServiceFactory::GetForContext(profile_)); } #endif diff --git a/chrome/browser/policy/profile_policy_connector_factory.cc b/chrome/browser/policy/profile_policy_connector_factory.cc index 55369ac..52d8a1d 100644 --- a/chrome/browser/policy/profile_policy_connector_factory.cc +++ b/chrome/browser/policy/profile_policy_connector_factory.cc @@ -13,6 +13,7 @@ #include "components/user_prefs/pref_registry_syncable.h" #if defined(ENABLE_CONFIGURATION_POLICY) +#include "chrome/browser/policy/schema_registry_service_factory.h" #if defined(OS_CHROMEOS) #include "chrome/browser/chromeos/login/user.h" #include "chrome/browser/chromeos/login/user_manager.h" @@ -58,6 +59,7 @@ ProfilePolicyConnectorFactory::ProfilePolicyConnectorFactory() "ProfilePolicyConnector", BrowserContextDependencyManager::GetInstance()) { #if defined(ENABLE_CONFIGURATION_POLICY) + DependsOn(SchemaRegistryServiceFactory::GetInstance()); #if defined(OS_CHROMEOS) DependsOn(UserCloudPolicyManagerFactoryChromeOS::GetInstance()); #else diff --git a/chrome/browser/policy/schema_registry.cc b/chrome/browser/policy/schema_registry.cc index a456708..f005bd0 100644 --- a/chrome/browser/policy/schema_registry.cc +++ b/chrome/browser/policy/schema_registry.cc @@ -53,9 +53,8 @@ void SchemaRegistry::RemoveObserver(Observer* observer) { } void SchemaRegistry::Notify(bool has_new_schemas) { - FOR_EACH_OBSERVER(Observer, - observers_, - OnSchemaRegistryUpdated(schema_map_, has_new_schemas)); + FOR_EACH_OBSERVER( + Observer, observers_, OnSchemaRegistryUpdated(has_new_schemas)); } bool SchemaRegistry::HasObservers() const { @@ -108,9 +107,7 @@ void CombinedSchemaRegistry::UnregisterComponent(const PolicyNamespace& ns) { } } -void CombinedSchemaRegistry::OnSchemaRegistryUpdated( - const scoped_refptr<SchemaMap>& current_map, - bool has_new_schemas) { +void CombinedSchemaRegistry::OnSchemaRegistryUpdated(bool has_new_schemas) { Combine(has_new_schemas); } diff --git a/chrome/browser/policy/schema_registry.h b/chrome/browser/policy/schema_registry.h index 310f803..0769003 100644 --- a/chrome/browser/policy/schema_registry.h +++ b/chrome/browser/policy/schema_registry.h @@ -29,15 +29,11 @@ class SchemaRegistry : public base::NonThreadSafe { class Observer { public: // Invoked whenever schemas are registered or unregistered. - // |current_map| is a reference to the current map, also returned by - // schema_map(); // |has_new_schemas| is true if a new component has been registered since // the last update; this allows observers to ignore updates when // components are unregistered but still get a handle to the current map // (e.g. for periodic reloads). - virtual void OnSchemaRegistryUpdated( - const scoped_refptr<SchemaMap>& current_map, - bool has_new_schemas) = 0; + virtual void OnSchemaRegistryUpdated(bool has_new_schemas) = 0; protected: virtual ~Observer(); @@ -89,9 +85,7 @@ class CombinedSchemaRegistry : public SchemaRegistry, virtual void UnregisterComponent(const PolicyNamespace& ns) OVERRIDE; - virtual void OnSchemaRegistryUpdated( - const scoped_refptr<SchemaMap>& current_map, - bool has_new_schemas) OVERRIDE; + virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE; private: void Combine(bool has_new_schemas); diff --git a/chrome/browser/policy/schema_registry_unittest.cc b/chrome/browser/policy/schema_registry_unittest.cc index 7fd9afa..62bed9e 100644 --- a/chrome/browser/policy/schema_registry_unittest.cc +++ b/chrome/browser/policy/schema_registry_unittest.cc @@ -44,8 +44,7 @@ class MockSchemaRegistryObserver : public SchemaRegistry::Observer { MockSchemaRegistryObserver() {} virtual ~MockSchemaRegistryObserver() {} - MOCK_METHOD2(OnSchemaRegistryUpdated, - void(const scoped_refptr<SchemaMap>&, bool)); + MOCK_METHOD1(OnSchemaRegistryUpdated, void(bool)); }; } // namespace @@ -65,14 +64,14 @@ TEST(SchemaRegistryTest, Notifications) { EXPECT_FALSE(registry.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); registry.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"), schema); Mock::VerifyAndClearExpectations(&observer); // Re-register also triggers notifications, because the Schema might have // changed. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); registry.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"), schema); Mock::VerifyAndClearExpectations(&observer); @@ -80,7 +79,7 @@ TEST(SchemaRegistryTest, Notifications) { EXPECT_TRUE(registry.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, false)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(false)); registry.UnregisterComponent( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc")); Mock::VerifyAndClearExpectations(&observer); @@ -93,7 +92,7 @@ TEST(SchemaRegistryTest, Notifications) { components["abc"] = schema; components["def"] = schema; components["xyz"] = schema; - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); registry.RegisterComponents(POLICY_DOMAIN_EXTENSIONS, components); Mock::VerifyAndClearExpectations(&observer); @@ -112,38 +111,38 @@ TEST(SchemaRegistryTest, Combined) { CombinedSchemaRegistry combined; combined.AddObserver(&observer); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, _)).Times(0); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(_)).Times(0); registry1.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"), schema); Mock::VerifyAndClearExpectations(&observer); // Starting to track a registry issues notifications when it comes with new // schemas. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); combined.Track(®istry1); Mock::VerifyAndClearExpectations(&observer); // Adding a new empty registry does not trigger notifications. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, _)).Times(0); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(_)).Times(0); combined.Track(®istry2); Mock::VerifyAndClearExpectations(&observer); // Adding the same component to the combined registry itself triggers // notifications. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); combined.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"), schema); Mock::VerifyAndClearExpectations(&observer); // Adding components to the sub-registries triggers notifications. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); registry2.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "def"), schema); Mock::VerifyAndClearExpectations(&observer); // If the same component is published in 2 sub-registries then the combined // registry publishes one of them. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)); registry1.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "def"), schema); Mock::VerifyAndClearExpectations(&observer); @@ -160,7 +159,7 @@ TEST(SchemaRegistryTest, Combined) { EXPECT_FALSE(combined.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "xyz"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, false)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(false)); registry1.UnregisterComponent( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc")); Mock::VerifyAndClearExpectations(&observer); @@ -168,7 +167,7 @@ TEST(SchemaRegistryTest, Combined) { EXPECT_TRUE(combined.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, false)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(false)); combined.UnregisterComponent( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc")); Mock::VerifyAndClearExpectations(&observer); @@ -176,7 +175,7 @@ TEST(SchemaRegistryTest, Combined) { EXPECT_FALSE(combined.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "abc"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, false)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(false)); registry1.UnregisterComponent( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "def")); Mock::VerifyAndClearExpectations(&observer); @@ -184,7 +183,7 @@ TEST(SchemaRegistryTest, Combined) { EXPECT_TRUE(combined.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "def"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, false)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(false)); registry2.UnregisterComponent( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "def")); Mock::VerifyAndClearExpectations(&observer); @@ -192,7 +191,7 @@ TEST(SchemaRegistryTest, Combined) { EXPECT_FALSE(combined.schema_map()->GetSchema( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "def"))); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, true)).Times(2); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(true)).Times(2); registry1.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_CHROME, ""), schema); registry2.RegisterComponent(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "hij"), @@ -201,11 +200,11 @@ TEST(SchemaRegistryTest, Combined) { // Untracking |registry1| doesn't trigger an update nofitication, because it // doesn't contain any components. - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, _)).Times(0); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(_)).Times(0); combined.Untrack(®istry1); Mock::VerifyAndClearExpectations(&observer); - EXPECT_CALL(observer, OnSchemaRegistryUpdated(_, false)); + EXPECT_CALL(observer, OnSchemaRegistryUpdated(false)); combined.Untrack(®istry2); Mock::VerifyAndClearExpectations(&observer); diff --git a/chrome/browser/prefs/synced_pref_change_registrar_browsertest.cc b/chrome/browser/prefs/synced_pref_change_registrar_browsertest.cc index ecb4781..6a8d1e2 100644 --- a/chrome/browser/prefs/synced_pref_change_registrar_browsertest.cc +++ b/chrome/browser/prefs/synced_pref_change_registrar_browsertest.cc @@ -30,7 +30,6 @@ namespace { -using testing::AnyNumber; using testing::Return; using testing::_; @@ -104,7 +103,6 @@ class SyncedPrefChangeRegistrarTest : public InProcessBrowserTest { virtual void SetUpInProcessBrowserTestFixture() OVERRIDE { EXPECT_CALL(policy_provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(policy_provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting( &policy_provider_); } diff --git a/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc b/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc index cc23088..1cb889d 100644 --- a/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc +++ b/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc @@ -57,7 +57,6 @@ #include "testing/gmock/include/gmock/gmock.h" using testing::_; -using testing::AnyNumber; using testing::Return; #endif // defined(ENABLE_CONFIGURATION_POLICY) && !defined(OS_CHROMEOS) @@ -1038,7 +1037,6 @@ void StartupBrowserCreatorFirstRunTest::SetUpInProcessBrowserTestFixture() { EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); #endif // defined(ENABLE_CONFIGURATION_POLICY) } diff --git a/chrome/browser/ui/webui/options/certificate_manager_browsertest.cc b/chrome/browser/ui/webui/options/certificate_manager_browsertest.cc index 1a1b9b5..31255df 100644 --- a/chrome/browser/ui/webui/options/certificate_manager_browsertest.cc +++ b/chrome/browser/ui/webui/options/certificate_manager_browsertest.cc @@ -27,7 +27,6 @@ #include "crypto/nss_util.h" #endif -using testing::AnyNumber; using testing::Return; using testing::_; @@ -44,7 +43,6 @@ class CertificateManagerBrowserTest : public options::OptionsUIBrowserTest { // Setup the policy provider for injecting certs through ONC policy. EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); } diff --git a/chrome/browser/ui/webui/options/preferences_browsertest.cc b/chrome/browser/ui/webui/options/preferences_browsertest.cc index c1fe97a..8137b09 100644 --- a/chrome/browser/ui/webui/options/preferences_browsertest.cc +++ b/chrome/browser/ui/webui/options/preferences_browsertest.cc @@ -53,7 +53,6 @@ #endif using testing::AllOf; -using testing::AnyNumber; using testing::Mock; using testing::Property; using testing::Return; @@ -191,8 +190,6 @@ void PreferencesBrowserTest::SetUpInProcessBrowserTestFixture() { // Sets up a mock policy provider for user and device policies. EXPECT_CALL(policy_provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(policy_provider_, RegisterPolicyDomain(_)) - .Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting( &policy_provider_); }; diff --git a/chrome/browser/ui/webui/policy_ui.cc b/chrome/browser/ui/webui/policy_ui.cc index 28a665c..7494c91 100644 --- a/chrome/browser/ui/webui/policy_ui.cc +++ b/chrome/browser/ui/webui/policy_ui.cc @@ -34,6 +34,7 @@ #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/url_constants.h" +#include "components/policy/core/common/policy_namespace.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_service.h" @@ -62,7 +63,10 @@ #if !defined(OS_ANDROID) && !defined(OS_IOS) #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system.h" -#include "chrome/browser/policy/policy_domain_descriptor.h" +#include "chrome/browser/policy/schema_map.h" +#include "chrome/browser/policy/schema_registry.h" +#include "chrome/browser/policy/schema_registry_service.h" +#include "chrome/browser/policy/schema_registry_service_factory.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_set.h" #include "components/policy/core/common/schema.h" @@ -556,15 +560,17 @@ void PolicyUIHandler::SendPolicyNames() const { #if !defined(OS_ANDROID) && !defined(OS_IOS) // Add extension policy names. base::DictionaryValue* extension_policy_names = new base::DictionaryValue; + + Profile* profile = Profile::FromWebUI(web_ui()); extensions::ExtensionSystem* extension_system = - extensions::ExtensionSystem::Get(Profile::FromWebUI(web_ui())); + extensions::ExtensionSystem::Get(profile); const ExtensionSet* extensions = extension_system->extension_service()->extensions(); - scoped_refptr<const policy::PolicyDomainDescriptor> policy_domain_descriptor; - policy_domain_descriptor = GetPolicyService()-> - GetPolicyDomainDescriptor(policy::POLICY_DOMAIN_EXTENSIONS); - const policy::PolicyDomainDescriptor::SchemaMap& schema_map = - policy_domain_descriptor->components(); + + policy::SchemaRegistry* registry = + policy::SchemaRegistryServiceFactory::GetForContext( + profile->GetOriginalProfile()); + scoped_refptr<policy::SchemaMap> schema_map = registry->schema_map(); for (ExtensionSet::const_iterator it = extensions->begin(); it != extensions->end(); ++it) { @@ -575,17 +581,16 @@ void PolicyUIHandler::SendPolicyNames() const { continue; base::DictionaryValue* extension_value = new base::DictionaryValue; extension_value->SetString("name", extension->name()); - policy::PolicyDomainDescriptor::SchemaMap::const_iterator schema = - schema_map.find(extension->id()); + const policy::Schema* schema = + schema_map->GetSchema(policy::PolicyNamespace( + policy::POLICY_DOMAIN_EXTENSIONS, extension->id())); base::DictionaryValue* policy_names = new base::DictionaryValue; - if (schema != schema_map.end()) { + if (schema) { // Get policy names from the extension's policy schema. // Store in a map, not an array, for faster lookup on JS side. - policy::Schema policy_schema = schema->second; - for (policy::Schema::Iterator it_policies = - policy_schema.GetPropertiesIterator(); - !it_policies.IsAtEnd(); it_policies.Advance()) { - policy_names->SetBoolean(it_policies.key(), true); + for (policy::Schema::Iterator prop = schema->GetPropertiesIterator(); + !prop.IsAtEnd(); prop.Advance()) { + policy_names->SetBoolean(prop.key(), true); } } extension_value->Set("policyNames", policy_names); diff --git a/chrome/browser/ui/webui/policy_ui_browsertest.cc b/chrome/browser/ui/webui/policy_ui_browsertest.cc index feb7532..758543e 100644 --- a/chrome/browser/ui/webui/policy_ui_browsertest.cc +++ b/chrome/browser/ui/webui/policy_ui_browsertest.cc @@ -26,7 +26,6 @@ #include "ui/base/l10n/l10n_util.h" #include "url/gurl.h" -using testing::AnyNumber; using testing::Return; using testing::_; @@ -107,7 +106,6 @@ PolicyUITest::~PolicyUITest() { void PolicyUITest::SetUpInProcessBrowserTestFixture() { EXPECT_CALL(provider_, IsInitializationComplete(_)) .WillRepeatedly(Return(true)); - EXPECT_CALL(provider_, RegisterPolicyDomain(_)).Times(AnyNumber()); policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); } diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index f74203c..38ec5dd 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1544,8 +1544,6 @@ 'browser/policy/javascript_policy_handler.h', 'browser/policy/policy_bundle.cc', 'browser/policy/policy_bundle.h', - 'browser/policy/policy_domain_descriptor.cc', - 'browser/policy/policy_domain_descriptor.h', 'browser/policy/policy_error_map.cc', 'browser/policy/policy_error_map.h', 'browser/policy/policy_load_status.cc', @@ -2885,7 +2883,6 @@ ['include', 'browser/policy/external_data_fetcher.cc'], ['include', 'browser/policy/external_data_fetcher.h'], ['include', 'browser/policy/external_data_manager.h'], - ['include', 'browser/policy/policy_domain_descriptor.h'], ['include', 'browser/policy/policy_map.cc'], ['include', 'browser/policy/policy_map.h'], ['include', 'browser/policy/policy_service.cc'], diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index ee9e684..5bfac03 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1119,7 +1119,6 @@ 'browser/policy/mock_policy_service.cc', 'browser/policy/mock_policy_service.h', 'browser/policy/policy_bundle_unittest.cc', - 'browser/policy/policy_domain_descriptor_unittest.cc', 'browser/policy/policy_loader_mac_unittest.cc', 'browser/policy/policy_loader_win_unittest.cc', 'browser/policy/policy_map_unittest.cc', diff --git a/chrome/test/data/extensions/api_test/settings/managed_storage/manifest.json b/chrome/test/data/extensions/api_test/settings/managed_storage/manifest.json index 19c4c55..67151b4 100644 --- a/chrome/test/data/extensions/api_test/settings/managed_storage/manifest.json +++ b/chrome/test/data/extensions/api_test/settings/managed_storage/manifest.json @@ -7,5 +7,8 @@ "permissions": ["storage"], "background": { "scripts": ["background.js"] + }, + "storage": { + "managed_schema": "schema.json" } } diff --git a/chrome/test/data/extensions/api_test/settings/managed_storage/schema.json b/chrome/test/data/extensions/api_test/settings/managed_storage/schema.json new file mode 100644 index 0000000..4037d1f --- /dev/null +++ b/chrome/test/data/extensions/api_test/settings/managed_storage/schema.json @@ -0,0 +1,25 @@ +{ + "type": "object", + "properties": { + "string-policy": { "type": "string" }, + "int-policy": { "type": "integer" }, + "double-policy": { "type": "number" }, + "boolean-policy": { "type": "boolean" }, + "list-policy": { + "type": "array", + "items": { "type": "string" } + }, + "dict-policy": { + "type": "object", + "properties": { + "list": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { "type": "integer" } + } + } + } + } + } +} |