diff options
37 files changed, 531 insertions, 332 deletions
diff --git a/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc index 9007067..2b0cac2 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc @@ -10,6 +10,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" +#include "base/callback.h" #include "base/location.h" #include "base/logging.h" #include "base/message_loop/message_loop_proxy.h" @@ -23,7 +24,6 @@ #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/policy_map.h" #include "net/url_request/url_request_context_getter.h" -#include "policy/policy_constants.h" namespace policy { @@ -42,11 +42,11 @@ int max_external_data_size_for_testing = 0; // verification, caching and retrieval. class CloudExternalDataManagerBase::Backend { public: - // The |policy_definitions| are used to determine the maximum size that the + // |get_policy_details| is used to determine the maximum size that the // data referenced by each policy can have. This class can be instantiated on // any thread but from then on, may be accessed via the |task_runner_| only. // All FetchCallbacks will be invoked via |callback_task_runner|. - Backend(const PolicyDefinitionList* policy_definitions, + Backend(const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> task_runner, scoped_refptr<base::SequencedTaskRunner> callback_task_runner); @@ -101,8 +101,7 @@ class CloudExternalDataManagerBase::Backend { // Map from policy names to the lists of callbacks defined above. typedef std::map<std::string, FetchCallbackList> FetchCallbackMap; - // Looks up the maximum size that the data referenced by |policy| can have in - // |policy_definitions_|. + // Looks up the maximum size that the data referenced by |policy| can have. size_t GetMaxExternalDataSize(const std::string& policy) const; // Invokes |callback| via the |callback_task_runner_|, passing |data| as a @@ -116,7 +115,7 @@ class CloudExternalDataManagerBase::Backend { // Used to determine the maximum size that the data referenced by each policy // can have. - const PolicyDefinitionList* policy_definitions_; + GetChromePolicyDetailsCallback get_policy_details_; scoped_refptr<base::SequencedTaskRunner> task_runner_; scoped_refptr<base::SequencedTaskRunner> callback_task_runner_; @@ -147,10 +146,10 @@ class CloudExternalDataManagerBase::Backend { }; CloudExternalDataManagerBase::Backend::Backend( - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> task_runner, scoped_refptr<base::SequencedTaskRunner> callback_task_runner) - : policy_definitions_(policy_definitions), + : get_policy_details_(get_policy_details), task_runner_(task_runner), callback_task_runner_(callback_task_runner), metadata_set_(false) { @@ -301,14 +300,12 @@ size_t CloudExternalDataManagerBase::Backend::GetMaxExternalDataSize( return max_external_data_size_for_testing; // Look up the maximum size that the data referenced by |policy| can have in - // policy_definitions_, which is constructed from the information in + // get_policy_details, which is constructed from the information in // policy_templates.json, allowing the maximum data size to be specified as // part of the policy definition. - for (const PolicyDefinitionList::Entry* entry = policy_definitions_->begin; - entry != policy_definitions_->end; ++entry) { - if (entry->name == policy) - return entry->max_external_data_size; - } + const PolicyDetails* details = get_policy_details_.Run(policy); + if (details) + return details->max_external_data_size; NOTREACHED(); return 0; } @@ -339,12 +336,12 @@ void CloudExternalDataManagerBase::Backend::StartDownload( } CloudExternalDataManagerBase::CloudExternalDataManagerBase( - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> backend_task_runner, scoped_refptr<base::SequencedTaskRunner> io_task_runner) : backend_task_runner_(backend_task_runner), io_task_runner_(io_task_runner), - backend_(new Backend(policy_definitions, + backend_(new Backend(get_policy_details, backend_task_runner_, base::MessageLoopProxy::current())) { } diff --git a/chrome/browser/chromeos/policy/cloud_external_data_manager_base.h b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.h index f07299f..96c80da 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_manager_base.h +++ b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.h @@ -11,6 +11,7 @@ #include "base/memory/scoped_ptr.h" #include "base/threading/non_thread_safe.h" #include "chrome/browser/policy/cloud/cloud_external_data_manager.h" +#include "components/policy/core/common/policy_details.h" namespace base { class SequencedTaskRunner; @@ -20,7 +21,6 @@ namespace policy { class CloudExternalDataStore; class ExternalPolicyDataFetcherBackend; -struct PolicyDefinitionList; // Downloads, verifies, caches and retrieves external data referenced by // policies. @@ -29,12 +29,12 @@ struct PolicyDefinitionList; class CloudExternalDataManagerBase : public CloudExternalDataManager, public base::NonThreadSafe { public: - // The |policy_definitions| are used to determine the maximum size that the + // |get_policy_details| is used to determine the maximum size that the // data referenced by each policy can have. Download scheduling, verification, // caching and retrieval tasks are done via the |backend_task_runner|, which // must support file I/O. Network I/O is done via the |io_task_runner|. CloudExternalDataManagerBase( - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> backend_task_runner, scoped_refptr<base::SequencedTaskRunner> io_task_runner); virtual ~CloudExternalDataManagerBase(); diff --git a/chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc index 0ef3531..1bcffc4 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc @@ -24,13 +24,13 @@ #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/policy_types.h" +#include "chrome/browser/policy/test/policy_test_utils.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher.h" #include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_request_status.h" #include "net/url_request/url_request_test_util.h" -#include "policy/policy_constants.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -53,15 +53,11 @@ const char k20BytePolicyURL[] = "http://localhost/20_bytes"; const char k10ByteData[] = "10 bytes.."; const char k20ByteData[] = "20 bytes............"; -const PolicyDefinitionList::Entry kPolicyDefinitionListEntries[] = { - { kStringPolicy, base::Value::TYPE_STRING, false, 1, 0 }, - { k10BytePolicy, base::Value::TYPE_DICTIONARY, false, 2, 10 }, - { k20BytePolicy, base::Value::TYPE_DICTIONARY, false, 3, 20 }, -}; - -const PolicyDefinitionList kPolicyDefinitionList = { - kPolicyDefinitionListEntries, - kPolicyDefinitionListEntries + arraysize(kPolicyDefinitionListEntries), +const PolicyDetails kPolicyDetails[] = { +// is_deprecated is_device_policy id max_external_data_size + { false, false, 1, 0 }, + { false, false, 2, 10 }, + { false, false, 3, 20 }, }; const char kCacheKey[] = "data"; @@ -140,6 +136,7 @@ class CloudExternalDataManagerBaseTest : public testing::Test { scoped_ptr<CloudExternalDataManagerBase> external_data_manager_; std::map<int, std::string*> callback_data_; + PolicyDetailsMap policy_details_; DISALLOW_COPY_AND_ASSIGN(CloudExternalDataManagerBaseTest); }; @@ -171,6 +168,10 @@ void CloudExternalDataManagerBaseTest::SetUp() { request_content_getter_ = new net::TestURLRequestContextGetter( base::MessageLoopProxy::current()); + + policy_details_.SetDetails(kStringPolicy, &kPolicyDetails[0]); + policy_details_.SetDetails(k10BytePolicy, &kPolicyDetails[1]); + policy_details_.SetDetails(k20BytePolicy, &kPolicyDetails[2]); } void CloudExternalDataManagerBaseTest::TearDown() { @@ -181,7 +182,7 @@ void CloudExternalDataManagerBaseTest::TearDown() { void CloudExternalDataManagerBaseTest::SetUpExternalDataManager() { external_data_manager_.reset(new CloudExternalDataManagerBase( - &kPolicyDefinitionList, + policy_details_.GetCallback(), message_loop_.message_loop_proxy(), message_loop_.message_loop_proxy())); external_data_manager_->SetExternalDataStore(make_scoped_ptr( diff --git a/chrome/browser/chromeos/policy/device_local_account_external_data_manager.cc b/chrome/browser/chromeos/policy/device_local_account_external_data_manager.cc index 6a38a38..8d43bc7 100644 --- a/chrome/browser/chromeos/policy/device_local_account_external_data_manager.cc +++ b/chrome/browser/chromeos/policy/device_local_account_external_data_manager.cc @@ -15,11 +15,11 @@ namespace policy { DeviceLocalAccountExternalDataManager::DeviceLocalAccountExternalDataManager( const std::string& account_id, - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> backend_task_runner, scoped_refptr<base::SequencedTaskRunner> io_task_runner, ResourceCache* resource_cache) - : CloudExternalDataManagerBase(policy_definitions, + : CloudExternalDataManagerBase(get_policy_details, backend_task_runner, io_task_runner) { SetExternalDataStore(make_scoped_ptr(new CloudExternalDataStore( diff --git a/chrome/browser/chromeos/policy/device_local_account_external_data_manager.h b/chrome/browser/chromeos/policy/device_local_account_external_data_manager.h index 68edd01..473b937 100644 --- a/chrome/browser/chromeos/policy/device_local_account_external_data_manager.h +++ b/chrome/browser/chromeos/policy/device_local_account_external_data_manager.h @@ -11,6 +11,7 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h" +#include "components/policy/core/common/policy_details.h" namespace base { class SequencedTaskRunner; @@ -18,7 +19,6 @@ class SequencedTaskRunner; namespace policy { -struct PolicyDefinitionList; class ResourceCache; // Downloads, verifies, caches and retrieves external data referenced by @@ -39,7 +39,7 @@ class DeviceLocalAccountExternalDataManager friend class DeviceLocalAccountExternalDataService; friend class base::RefCounted<DeviceLocalAccountExternalDataManager>; - // The |policy_definitions| are used to determine the maximum size that the + // |get_policy_details| is used to determine the maximum size that the // data referenced by each policy can have. Download scheduling, verification, // caching and retrieval tasks are done via the |backend_task_runner|, which // must support file I/O. Network I/O is done via the |io_task_runner|. The @@ -50,7 +50,7 @@ class DeviceLocalAccountExternalDataManager // cache, all its users must access the cache via |backend_task_runner| only. DeviceLocalAccountExternalDataManager( const std::string& account_id, - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> backend_task_runner, scoped_refptr<base::SequencedTaskRunner> io_task_runner, ResourceCache* resource_cache); diff --git a/chrome/browser/chromeos/policy/device_local_account_external_data_service.cc b/chrome/browser/chromeos/policy/device_local_account_external_data_service.cc index e751a4d..6162540 100644 --- a/chrome/browser/chromeos/policy/device_local_account_external_data_service.cc +++ b/chrome/browser/chromeos/policy/device_local_account_external_data_service.cc @@ -76,7 +76,7 @@ scoped_refptr<DeviceLocalAccountExternalDataManager> if (!external_data_manager) { external_data_manager = new DeviceLocalAccountExternalDataManager( account_id, - GetChromePolicyDefinitionList(), + base::Bind(&GetChromePolicyDetails), backend_task_runner_, io_task_runner_, resource_cache_.get()); diff --git a/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc index 00481f3..39f859c 100644 --- a/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc +++ b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc @@ -10,7 +10,6 @@ #include "chrome/browser/chromeos/policy/cloud_external_data_store.h" #include "chrome/browser/policy/cloud/cloud_policy_store.h" #include "chrome/browser/policy/cloud/resource_cache.h" -#include "policy/policy_constants.h" namespace policy { @@ -21,12 +20,12 @@ const char kCacheKey[] = "data"; } // namespace UserCloudExternalDataManager::UserCloudExternalDataManager( - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> backend_task_runner, scoped_refptr<base::SequencedTaskRunner> io_task_runner, const base::FilePath& cache_path, CloudPolicyStore* policy_store) - : CloudExternalDataManagerBase(policy_definitions, + : CloudExternalDataManagerBase(get_policy_details, backend_task_runner, io_task_runner), resource_cache_(new ResourceCache(cache_path, backend_task_runner)) { diff --git a/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h index fa5a523..4e1910d 100644 --- a/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h +++ b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h @@ -9,6 +9,7 @@ #include "base/files/file_path.h" #include "base/memory/ref_counted.h" #include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h" +#include "components/policy/core/common/policy_details.h" namespace base { class SequencedTaskRunner; @@ -17,7 +18,6 @@ class SequencedTaskRunner; namespace policy { class CloudPolicyStore; -struct PolicyDefinitionList; class ResourceCache; // Downloads, verifies, caches and retrieves external data referenced by @@ -27,14 +27,14 @@ class ResourceCache; // because no other platform has policies referencing external data. class UserCloudExternalDataManager : public CloudExternalDataManagerBase { public: - // The |policy_definitions| are used to determine the maximum size that the + // |get_policy_details| is used to determine the maximum size that the // data referenced by each policy can have. Download scheduling, verification, // caching and retrieval tasks are done via the |backend_task_runner|, which // must support file I/O. Network I/O is done via the |io_task_runner|. The // manager is responsible for external data references by policies in // |policy_store|. UserCloudExternalDataManager( - const PolicyDefinitionList* policy_definitions, + const GetChromePolicyDetailsCallback& get_policy_details, scoped_refptr<base::SequencedTaskRunner> backend_task_runner, scoped_refptr<base::SequencedTaskRunner> io_task_runner, const base::FilePath& cache_path, 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 4f43f89..6b31eab 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 @@ -4,6 +4,7 @@ #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h" +#include "base/bind.h" #include "base/command_line.h" #include "base/files/file_path.h" #include "base/logging.h" @@ -173,7 +174,7 @@ scoped_ptr<UserCloudPolicyManagerChromeOS> content::BrowserThread::GetMessageLoopProxyForThread( content::BrowserThread::IO); scoped_ptr<CloudExternalDataManager> external_data_manager( - new UserCloudExternalDataManager(GetChromePolicyDefinitionList(), + new UserCloudExternalDataManager(base::Bind(&GetChromePolicyDetails), backend_task_runner, io_task_runner, external_data_dir, diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc index 20b6ea0..7154182 100644 --- a/chrome/browser/policy/browser_policy_connector.cc +++ b/chrome/browser/policy/browser_policy_connector.cc @@ -335,6 +335,8 @@ void BrowserPolicyConnector::Init( policy_statistics_collector_.reset( new policy::PolicyStatisticsCollector( + base::Bind(&GetChromePolicyDetails), + GetChromeSchema(), GetPolicyService(), local_state_, base::MessageLoop::current()->message_loop_proxy())); @@ -586,16 +588,12 @@ void BrowserPolicyConnector::SetTimezoneIfPolicyAvailable() { ConfigurationPolicyProvider* BrowserPolicyConnector::CreatePlatformProvider() { #if defined(OS_WIN) - const PolicyDefinitionList* policy_list = GetChromePolicyDefinitionList(); scoped_ptr<AsyncPolicyLoader> loader(PolicyLoaderWin::Create( - BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE), - policy_list)); + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE))); return new AsyncPolicyProvider(GetSchemaRegistry(), loader.Pass()); #elif defined(OS_MACOSX) && !defined(OS_IOS) - const PolicyDefinitionList* policy_list = GetChromePolicyDefinitionList(); scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderMac( BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE), - policy_list, GetManagedPolicyPath(), new MacPreferences())); return new AsyncPolicyProvider(GetSchemaRegistry(), loader.Pass()); diff --git a/chrome/browser/policy/cloud/cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/cloud_policy_browsertest.cc index a7ecddf..fae2417 100644 --- a/chrome/browser/policy/cloud/cloud_policy_browsertest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_browsertest.cc @@ -23,10 +23,10 @@ #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/policy_service.h" -#include "chrome/browser/policy/policy_test_utils.h" #include "chrome/browser/policy/profile_policy_connector.h" #include "chrome/browser/policy/profile_policy_connector_factory.h" #include "chrome/browser/policy/test/local_policy_test_server.h" +#include "chrome/browser/policy/test/policy_test_utils.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/chrome_switches.h" diff --git a/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc b/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc index c83e680..e9a20ae 100644 --- a/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc @@ -10,8 +10,8 @@ #include "chrome/browser/policy/cloud/cloud_policy_client.h" #include "chrome/browser/policy/cloud/mock_cloud_policy_client.h" #include "chrome/browser/policy/cloud/test_request_interceptor.h" -#include "chrome/browser/policy/policy_test_utils.h" #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h" +#include "chrome/browser/policy/test/policy_test_utils.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/chrome_switches.h" diff --git a/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc b/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc index e705284..06adfc3 100644 --- a/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_manager_unittest.cc @@ -37,8 +37,7 @@ class TestHarness : public PolicyProviderTestHarness { virtual ConfigurationPolicyProvider* CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_definition_list) OVERRIDE; + scoped_refptr<base::SequencedTaskRunner> task_runner) OVERRIDE; virtual void InstallEmptyPolicy() OVERRIDE; virtual void InstallStringPolicy(const std::string& policy_name, @@ -73,8 +72,7 @@ void TestHarness::SetUp() {} ConfigurationPolicyProvider* TestHarness::CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_definition_list) { + scoped_refptr<base::SequencedTaskRunner> task_runner) { // Create and initialize the store. store_.NotifyStoreLoaded(); ConfigurationPolicyProvider* provider = new CloudPolicyManager( diff --git a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc index eeb534a..53bda5b 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc @@ -16,11 +16,11 @@ #include "chrome/browser/policy/cloud/cloud_policy_constants.h" #include "chrome/browser/policy/cloud/mock_cloud_policy_client.h" #include "chrome/browser/policy/policy_service.h" -#include "chrome/browser/policy/policy_test_utils.h" #include "chrome/browser/policy/profile_policy_connector.h" #include "chrome/browser/policy/profile_policy_connector_factory.h" #include "chrome/browser/policy/proto/cloud/chrome_extension_policy.pb.h" #include "chrome/browser/policy/test/local_policy_test_server.h" +#include "chrome/browser/policy/test/policy_test_utils.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" diff --git a/chrome/browser/policy/config_dir_policy_loader_unittest.cc b/chrome/browser/policy/config_dir_policy_loader_unittest.cc index 0213a9d..7c8813b 100644 --- a/chrome/browser/policy/config_dir_policy_loader_unittest.cc +++ b/chrome/browser/policy/config_dir_policy_loader_unittest.cc @@ -34,8 +34,7 @@ class TestHarness : public PolicyProviderTestHarness { virtual ConfigurationPolicyProvider* CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_definition_list) OVERRIDE; + scoped_refptr<base::SequencedTaskRunner> task_runner) OVERRIDE; virtual void InstallEmptyPolicy() OVERRIDE; virtual void InstallStringPolicy(const std::string& policy_name, @@ -84,8 +83,7 @@ void TestHarness::SetUp() { ConfigurationPolicyProvider* TestHarness::CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_definition_list) { + scoped_refptr<base::SequencedTaskRunner> task_runner) { scoped_ptr<AsyncPolicyLoader> loader(new ConfigDirPolicyLoader( task_runner, test_dir(), POLICY_SCOPE_MACHINE)); return new AsyncPolicyProvider(registry, loader.Pass()); diff --git a/chrome/browser/policy/configuration_policy_handler_list.cc b/chrome/browser/policy/configuration_policy_handler_list.cc index ad3d0ba..31995fd 100644 --- a/chrome/browser/policy/configuration_policy_handler_list.cc +++ b/chrome/browser/policy/configuration_policy_handler_list.cc @@ -25,6 +25,7 @@ #include "chrome/browser/sessions/restore_on_startup_policy_handler.h" #include "chrome/browser/sync/sync_policy_handler.h" #include "chrome/common/pref_names.h" +#include "components/policy/core/common/policy_details.h" #include "components/policy/core/common/policy_pref_names.h" #include "extensions/common/manifest.h" #include "grit/generated_resources.h" @@ -491,7 +492,8 @@ void ConfigurationPolicyHandlerList::ApplyPolicySettings( for (PolicyMap::const_iterator it = policies.begin(); it != policies.end(); ++it) { - if (IsDeprecatedPolicy(it->first)) + const PolicyDetails* details = GetChromePolicyDetails(it->first); + if (details && details->is_deprecated) errors->AddError(it->first, IDS_POLICY_DEPRECATED); } } diff --git a/chrome/browser/policy/configuration_policy_provider_test.cc b/chrome/browser/policy/configuration_policy_provider_test.cc index 7f02a65..2dd3f2b 100644 --- a/chrome/browser/policy/configuration_policy_provider_test.cc +++ b/chrome/browser/policy/configuration_policy_provider_test.cc @@ -112,7 +112,7 @@ const char kTestChromeSchema[] = " }" "}"; -namespace test_policy_definitions { +namespace test_keys { const char kKeyString[] = "StringPolicy"; const char kKeyBoolean[] = "BooleanPolicy"; @@ -120,19 +120,7 @@ const char kKeyInteger[] = "IntegerPolicy"; const char kKeyStringList[] = "StringListPolicy"; const char kKeyDictionary[] = "DictionaryPolicy"; -static const PolicyDefinitionList::Entry kEntries[] = { - { kKeyString, base::Value::TYPE_STRING }, - { kKeyBoolean, base::Value::TYPE_BOOLEAN }, - { kKeyInteger, base::Value::TYPE_INTEGER }, - { kKeyStringList, base::Value::TYPE_LIST }, - { kKeyDictionary, base::Value::TYPE_DICTIONARY }, -}; - -const PolicyDefinitionList kList = { - kEntries, kEntries + arraysize(kEntries) -}; - -} // namespace test_policy_definitions +} // namespace test_keys PolicyTestBase::PolicyTestBase() {} @@ -180,7 +168,7 @@ void ConfigurationPolicyProviderTest::SetUp() { test_harness_->SetUp(); Schema extension_schema = - chrome_schema_.GetKnownProperty(test_policy_definitions::kKeyDictionary); + chrome_schema_.GetKnownProperty(test_keys::kKeyDictionary); ASSERT_TRUE(extension_schema.valid()); schema_registry_.RegisterComponent( PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, @@ -195,10 +183,8 @@ void ConfigurationPolicyProviderTest::SetUp() { "cccccccccccccccccccccccccccccccc"), extension_schema); - provider_.reset(test_harness_->CreateProvider( - &schema_registry_, - loop_.message_loop_proxy(), - &test_policy_definitions::kList)); + provider_.reset(test_harness_->CreateProvider(&schema_registry_, + loop_.message_loop_proxy())); provider_->Init(&schema_registry_); // Some providers do a reload on init. Make sure any notifications generated // are fired now. @@ -246,31 +232,31 @@ TEST_P(ConfigurationPolicyProviderTest, Empty) { TEST_P(ConfigurationPolicyProviderTest, StringValue) { const char kTestString[] = "string_value"; base::StringValue expected_value(kTestString); - CheckValue(test_policy_definitions::kKeyString, + CheckValue(test_keys::kKeyString, expected_value, base::Bind(&PolicyProviderTestHarness::InstallStringPolicy, base::Unretained(test_harness_.get()), - test_policy_definitions::kKeyString, + test_keys::kKeyString, kTestString)); } TEST_P(ConfigurationPolicyProviderTest, BooleanValue) { base::FundamentalValue expected_value(true); - CheckValue(test_policy_definitions::kKeyBoolean, + CheckValue(test_keys::kKeyBoolean, expected_value, base::Bind(&PolicyProviderTestHarness::InstallBooleanPolicy, base::Unretained(test_harness_.get()), - test_policy_definitions::kKeyBoolean, + test_keys::kKeyBoolean, true)); } TEST_P(ConfigurationPolicyProviderTest, IntegerValue) { base::FundamentalValue expected_value(42); - CheckValue(test_policy_definitions::kKeyInteger, + CheckValue(test_keys::kKeyInteger, expected_value, base::Bind(&PolicyProviderTestHarness::InstallIntegerPolicy, base::Unretained(test_harness_.get()), - test_policy_definitions::kKeyInteger, + test_keys::kKeyInteger, 42)); } @@ -278,11 +264,11 @@ TEST_P(ConfigurationPolicyProviderTest, StringListValue) { base::ListValue expected_value; expected_value.Set(0U, base::Value::CreateStringValue("first")); expected_value.Set(1U, base::Value::CreateStringValue("second")); - CheckValue(test_policy_definitions::kKeyStringList, + CheckValue(test_keys::kKeyStringList, expected_value, base::Bind(&PolicyProviderTestHarness::InstallStringListPolicy, base::Unretained(test_harness_.get()), - test_policy_definitions::kKeyStringList, + test_keys::kKeyStringList, &expected_value)); } @@ -312,11 +298,11 @@ TEST_P(ConfigurationPolicyProviderTest, DictionaryValue) { dict->Set("sublist", list); expected_value.Set("dictionary", dict); - CheckValue(test_policy_definitions::kKeyDictionary, + CheckValue(test_keys::kKeyDictionary, expected_value, base::Bind(&PolicyProviderTestHarness::InstallDictionaryPolicy, base::Unretained(test_harness_.get()), - test_policy_definitions::kKeyDictionary, + test_keys::kKeyDictionary, &expected_value)); } @@ -335,15 +321,14 @@ TEST_P(ConfigurationPolicyProviderTest, RefreshPolicies) { EXPECT_TRUE(provider_->policies().Equals(bundle)); // OnUpdatePolicy is called when there are changes. - test_harness_->InstallStringPolicy(test_policy_definitions::kKeyString, - "value"); + test_harness_->InstallStringPolicy(test_keys::kKeyString, "value"); EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(1); provider_->RefreshPolicies(); loop_.RunUntilIdle(); Mock::VerifyAndClearExpectations(&observer); bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) - .Set(test_policy_definitions::kKeyString, + .Set(test_keys::kKeyString, test_harness_->policy_level(), test_harness_->policy_scope(), base::Value::CreateStringValue("value"), @@ -414,8 +399,8 @@ TEST_P(Configuration3rdPartyPolicyProviderTest, Load3rdParty) { policy_dict.Set("dict", policy_dict.DeepCopy()); // Install these policies as a Chrome policy. - test_harness_->InstallDictionaryPolicy( - test_policy_definitions::kKeyDictionary, &policy_dict); + test_harness_->InstallDictionaryPolicy(test_keys::kKeyDictionary, + &policy_dict); // Install them as 3rd party policies too. base::DictionaryValue policy_3rdparty; policy_3rdparty.Set("extensions.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -433,7 +418,7 @@ TEST_P(Configuration3rdPartyPolicyProviderTest, Load3rdParty) { loop_.RunUntilIdle(); PolicyMap expected_policy; - expected_policy.Set(test_policy_definitions::kKeyDictionary, + expected_policy.Set(test_keys::kKeyDictionary, test_harness_->policy_level(), test_harness_->policy_scope(), policy_dict.DeepCopy(), diff --git a/chrome/browser/policy/configuration_policy_provider_test.h b/chrome/browser/policy/configuration_policy_provider_test.h index 637a528..a8dfcdf 100644 --- a/chrome/browser/policy/configuration_policy_provider_test.h +++ b/chrome/browser/policy/configuration_policy_provider_test.h @@ -27,23 +27,16 @@ class Value; namespace policy { class ConfigurationPolicyProvider; -struct PolicyDefinitionList; -// A stripped-down policy definition list that contains entries for the -// different policy setting types supported. -namespace test_policy_definitions { +namespace test_keys { -// String policy keys. extern const char kKeyString[]; extern const char kKeyBoolean[]; extern const char kKeyInteger[]; extern const char kKeyStringList[]; extern const char kKeyDictionary[]; -// Policy definition list that contains entries for the keys above. -extern const PolicyDefinitionList kList; - -} // namespace test_policy_definitions +} // namespace test_keys class PolicyTestBase : public testing::Test { public: @@ -81,8 +74,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; + scoped_refptr<base::SequencedTaskRunner> task_runner) = 0; // Returns the policy level and scope set by the policy provider. PolicyLevel policy_level() const; diff --git a/chrome/browser/policy/generate_policy_source_unittest.cc b/chrome/browser/policy/generate_policy_source_unittest.cc index c823410..e0e5ee4 100644 --- a/chrome/browser/policy/generate_policy_source_unittest.cc +++ b/chrome/browser/policy/generate_policy_source_unittest.cc @@ -6,6 +6,8 @@ #include "base/memory/scoped_ptr.h" #include "base/values.h" +#include "build/build_config.h" +#include "components/policy/core/common/policy_details.h" #include "components/policy/core/common/schema.h" #include "policy/policy_constants.h" #include "testing/gtest/include/gtest/gtest.h" @@ -55,6 +57,14 @@ TEST(GeneratePolicySource, ChromeSchemaData) { ASSERT_TRUE(subschema.GetProperty(key::kProxyPacUrl).valid()); ASSERT_TRUE(subschema.GetProperty(key::kProxyBypassList).valid()); + // Verify that all the Chrome policies are there. + for (Schema::Iterator it = schema.GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { + EXPECT_TRUE(it.key()); + EXPECT_FALSE(std::string(it.key()).empty()); + EXPECT_TRUE(GetChromePolicyDetails(it.key())); + } + // The properties are iterated in order. const char* kExpectedProperties[] = { key::kProxyBypassList, @@ -75,4 +85,39 @@ TEST(GeneratePolicySource, ChromeSchemaData) { EXPECT_TRUE(*next == NULL); } +TEST(GeneratePolicySource, PolicyDetails) { + EXPECT_FALSE(GetChromePolicyDetails("")); + EXPECT_FALSE(GetChromePolicyDetails("no such policy")); + EXPECT_FALSE(GetChromePolicyDetails("AlternateErrorPagesEnable")); + EXPECT_FALSE(GetChromePolicyDetails("alternateErrorPagesEnabled")); + EXPECT_FALSE(GetChromePolicyDetails("AAlternateErrorPagesEnabled")); + + const PolicyDetails* details = + GetChromePolicyDetails(key::kAlternateErrorPagesEnabled); + ASSERT_TRUE(details); + EXPECT_FALSE(details->is_deprecated); + EXPECT_FALSE(details->is_device_policy); + EXPECT_EQ(5, details->id); + EXPECT_EQ(0u, details->max_external_data_size); + + details = GetChromePolicyDetails(key::kJavascriptEnabled); + ASSERT_TRUE(details); + EXPECT_TRUE(details->is_deprecated); + EXPECT_FALSE(details->is_device_policy); + EXPECT_EQ(9, details->id); + EXPECT_EQ(0u, details->max_external_data_size); + +#if defined(OS_CHROMEOS) + details = GetChromePolicyDetails(key::kDevicePolicyRefreshRate); + ASSERT_TRUE(details); + EXPECT_FALSE(details->is_deprecated); + EXPECT_TRUE(details->is_device_policy); + EXPECT_EQ(90, details->id); + EXPECT_EQ(0u, details->max_external_data_size); +#endif + + // TODO(bartfab): add a test that verifies a max_external_data_size larger + // than 0, once a type 'external' policy is added. +} + } // namespace policy diff --git a/chrome/browser/policy/policy_loader_mac.cc b/chrome/browser/policy/policy_loader_mac.cc index 50b0cf2..63c3132 100644 --- a/chrome/browser/policy/policy_loader_mac.cc +++ b/chrome/browser/policy/policy_loader_mac.cc @@ -61,11 +61,9 @@ void ArrayEntryToValue(const void* value, void* context) { PolicyLoaderMac::PolicyLoaderMac( scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list, const base::FilePath& managed_policy_path, MacPreferences* preferences) : AsyncPolicyLoader(task_runner), - policy_list_(policy_list), preferences_(preferences), managed_policy_path_(managed_policy_path) {} @@ -84,17 +82,17 @@ scoped_ptr<PolicyBundle> PolicyLoaderMac::Load() { scoped_ptr<PolicyBundle> bundle(new PolicyBundle()); // Load Chrome's policy. - // TODO(joaodasilva): Use the Chrome schema instead of the - // PolicyDefinitionList. PolicyMap& chrome_policy = bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); PolicyLoadStatusSample status; bool policy_present = false; - const PolicyDefinitionList::Entry* current; - for (current = policy_list_->begin; current != policy_list_->end; ++current) { + const Schema* schema = + schema_map()->GetSchema(PolicyNamespace(POLICY_DOMAIN_CHROME, "")); + for (Schema::Iterator it = schema->GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { base::ScopedCFTypeRef<CFStringRef> name( - base::SysUTF8ToCFStringRef(current->name)); + base::SysUTF8ToCFStringRef(it.key())); base::ScopedCFTypeRef<CFPropertyListRef> value( preferences_->CopyAppValue(name, kCFPreferencesCurrentApplication)); if (!value.get()) @@ -107,7 +105,7 @@ scoped_ptr<PolicyBundle> PolicyLoaderMac::Load() { // TODO(joaodasilva): figure the policy scope. base::Value* policy = CreateValueFromProperty(value); if (policy) - chrome_policy.Set(current->name, level, POLICY_SCOPE_USER, policy, NULL); + chrome_policy.Set(it.key(), level, POLICY_SCOPE_USER, policy, NULL); else status.Add(POLICY_LOAD_STATUS_PARSE_ERROR); } diff --git a/chrome/browser/policy/policy_loader_mac.h b/chrome/browser/policy/policy_loader_mac.h index b879892..e863bfa 100644 --- a/chrome/browser/policy/policy_loader_mac.h +++ b/chrome/browser/policy/policy_loader_mac.h @@ -27,14 +27,12 @@ namespace policy { class PolicyBundle; class PolicyMap; class Schema; -struct PolicyDefinitionList; // A policy loader that loads policies from the Mac preferences system, and // watches the managed preferences files for updates. class PolicyLoaderMac : public AsyncPolicyLoader { public: PolicyLoaderMac(scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list, const base::FilePath& managed_policy_path, MacPreferences* preferences); virtual ~PolicyLoaderMac(); @@ -67,9 +65,6 @@ class PolicyLoaderMac : public AsyncPolicyLoader { const Schema& schema, PolicyMap* policy); - // List of recognized policies. - const PolicyDefinitionList* policy_list_; - scoped_ptr<MacPreferences> preferences_; // Path to the managed preferences file for the current user, if it could diff --git a/chrome/browser/policy/policy_loader_mac_unittest.cc b/chrome/browser/policy/policy_loader_mac_unittest.cc index 19695d2..38f7284 100644 --- a/chrome/browser/policy/policy_loader_mac_unittest.cc +++ b/chrome/browser/policy/policy_loader_mac_unittest.cc @@ -17,7 +17,6 @@ #include "chrome/browser/policy/policy_loader_mac.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/preferences_mock_mac.h" -#include "policy/policy_constants.h" #include "testing/gtest/include/gtest/gtest.h" using base::ScopedCFTypeRef; @@ -128,8 +127,7 @@ class TestHarness : public PolicyProviderTestHarness { virtual ConfigurationPolicyProvider* CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_definition_list) OVERRIDE; + scoped_refptr<base::SequencedTaskRunner> task_runner) OVERRIDE; virtual void InstallEmptyPolicy() OVERRIDE; virtual void InstallStringPolicy(const std::string& policy_name, @@ -162,11 +160,10 @@ void TestHarness::SetUp() {} ConfigurationPolicyProvider* TestHarness::CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_definition_list) { + scoped_refptr<base::SequencedTaskRunner> task_runner) { prefs_ = new MockPreferences(); - scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderMac( - task_runner, policy_definition_list, base::FilePath(), prefs_)); + scoped_ptr<AsyncPolicyLoader> loader( + new PolicyLoaderMac(task_runner, base::FilePath(), prefs_)); return new AsyncPolicyProvider(registry, loader.Pass()); } @@ -239,11 +236,8 @@ class PolicyLoaderMacTest : public PolicyTestBase { virtual void SetUp() OVERRIDE { PolicyTestBase::SetUp(); - scoped_ptr<AsyncPolicyLoader> loader( - new PolicyLoaderMac(loop_.message_loop_proxy(), - &test_policy_definitions::kList, - base::FilePath(), - prefs_)); + scoped_ptr<AsyncPolicyLoader> loader(new PolicyLoaderMac( + loop_.message_loop_proxy(), base::FilePath(), prefs_)); provider_.reset(new AsyncPolicyProvider(&schema_registry_, loader.Pass())); provider_->Init(&schema_registry_); } @@ -259,7 +253,7 @@ class PolicyLoaderMacTest : public PolicyTestBase { TEST_F(PolicyLoaderMacTest, Invalid) { ScopedCFTypeRef<CFStringRef> name( - base::SysUTF8ToCFStringRef(test_policy_definitions::kKeyString)); + base::SysUTF8ToCFStringRef(test_keys::kKeyString)); const char buffer[] = "binary \xde\xad\xbe\xef data"; ScopedCFTypeRef<CFDataRef> invalid_data( CFDataCreate(kCFAllocatorDefault, @@ -278,7 +272,7 @@ TEST_F(PolicyLoaderMacTest, Invalid) { TEST_F(PolicyLoaderMacTest, TestNonForcedValue) { ScopedCFTypeRef<CFStringRef> name( - base::SysUTF8ToCFStringRef(test_policy_definitions::kKeyString)); + base::SysUTF8ToCFStringRef(test_keys::kKeyString)); ScopedCFTypeRef<CFPropertyListRef> test_value( base::SysUTF8ToCFStringRef("string value")); ASSERT_TRUE(test_value.get()); @@ -289,7 +283,7 @@ TEST_F(PolicyLoaderMacTest, TestNonForcedValue) { loop_.RunUntilIdle(); PolicyBundle expected_bundle; expected_bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) - .Set(test_policy_definitions::kKeyString, + .Set(test_keys::kKeyString, POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER, base::Value::CreateStringValue("string value"), diff --git a/chrome/browser/policy/policy_loader_win.cc b/chrome/browser/policy/policy_loader_win.cc index 57bea21..4f708a6 100644 --- a/chrome/browser/policy/policy_loader_win.cc +++ b/chrome/browser/policy/policy_loader_win.cc @@ -33,6 +33,8 @@ #include "chrome/browser/policy/preg_parser_win.h" #include "chrome/browser/policy/registry_dict_win.h" #include "components/json_schema/json_schema_constants.h" +#include "components/policy/core/common/policy_namespace.h" +#include "components/policy/core/common/schema.h" #include "policy/policy_constants.h" namespace schema = json_schema_constants; @@ -210,12 +212,10 @@ const base::FilePath::CharType PolicyLoaderWin::kPRegFileName[] = PolicyLoaderWin::PolicyLoaderWin( scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list, const string16& chrome_policy_key, AppliedGPOListProvider* gpo_provider) : AsyncPolicyLoader(task_runner), is_initialized_(false), - policy_list_(policy_list), chrome_policy_key_(chrome_policy_key), gpo_provider_(gpo_provider), user_policy_changed_event_(false, false), @@ -245,10 +245,10 @@ PolicyLoaderWin::~PolicyLoaderWin() { // static scoped_ptr<PolicyLoaderWin> PolicyLoaderWin::Create( - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list) { + scoped_refptr<base::SequencedTaskRunner> task_runner) { return make_scoped_ptr( - new PolicyLoaderWin(task_runner, policy_list, kRegistryChromePolicyKey, + new PolicyLoaderWin(task_runner, + kRegistryChromePolicyKey, g_win_gpo_list_provider.Pointer())); } @@ -327,15 +327,20 @@ scoped_ptr<PolicyBundle> PolicyLoaderWin::Load() { } void PolicyLoaderWin::BuildChromePolicySchema() { + // TODO(joaodasilva): use the Schema directly instead of building this + // DictionaryValue. scoped_ptr<base::DictionaryValue> properties(new base::DictionaryValue()); - for (const PolicyDefinitionList::Entry* e = policy_list_->begin; - e != policy_list_->end; ++e) { - const std::string schema_type = GetSchemaTypeForValueType(e->value_type); + const Schema* chrome_schema = + schema_map()->GetSchema(PolicyNamespace(POLICY_DOMAIN_CHROME, "")); + for (Schema::Iterator it = chrome_schema->GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { + const std::string schema_type = + GetSchemaTypeForValueType(it.schema().type()); scoped_ptr<base::DictionaryValue> entry_schema(new base::DictionaryValue()); entry_schema->SetStringWithoutPathExpansion(json_schema_constants::kType, schema_type); - if (e->value_type == base::Value::TYPE_LIST) { + if (it.schema().type() == base::Value::TYPE_LIST) { scoped_ptr<base::DictionaryValue> items_schema( new base::DictionaryValue()); items_schema->SetStringWithoutPathExpansion( @@ -343,7 +348,7 @@ void PolicyLoaderWin::BuildChromePolicySchema() { entry_schema->SetWithoutPathExpansion(json_schema_constants::kItems, items_schema.release()); } - properties->SetWithoutPathExpansion(e->name, entry_schema.release()); + properties->SetWithoutPathExpansion(it.key(), entry_schema.release()); } chrome_policy_schema_.SetStringWithoutPathExpansion( json_schema_constants::kType, json_schema_constants::kObject); diff --git a/chrome/browser/policy/policy_loader_win.h b/chrome/browser/policy/policy_loader_win.h index 3b6f13a..d6453c1 100644 --- a/chrome/browser/policy/policy_loader_win.h +++ b/chrome/browser/policy/policy_loader_win.h @@ -28,7 +28,6 @@ class AppliedGPOListProvider; class PolicyLoadStatusSample; class PolicyMap; class RegistryDict; -struct PolicyDefinitionList; // Interface for mocking out GPO enumeration in tests. class AppliedGPOListProvider { @@ -51,15 +50,13 @@ class PolicyLoaderWin : public AsyncPolicyLoader, static const base::FilePath::CharType kPRegFileName[]; PolicyLoaderWin(scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list, const string16& chrome_policy_key, AppliedGPOListProvider* gpo_provider); virtual ~PolicyLoaderWin(); // Creates a policy loader that uses the Win API to access GPO. static scoped_ptr<PolicyLoaderWin> Create( - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list); + scoped_refptr<base::SequencedTaskRunner> task_runner); // AsyncPolicyLoader implementation. virtual void InitOnBackgroundThread() OVERRIDE; @@ -110,7 +107,6 @@ class PolicyLoaderWin : public AsyncPolicyLoader, virtual void OnObjectSignaled(HANDLE object) OVERRIDE; bool is_initialized_; - const PolicyDefinitionList* policy_list_; const string16 chrome_policy_key_; class AppliedGPOListProvider* gpo_provider_; base::DictionaryValue chrome_policy_schema_; diff --git a/chrome/browser/policy/policy_loader_win_unittest.cc b/chrome/browser/policy/policy_loader_win_unittest.cc index 9141cdc..c6b7976 100644 --- a/chrome/browser/policy/policy_loader_win_unittest.cc +++ b/chrome/browser/policy/policy_loader_win_unittest.cc @@ -254,8 +254,7 @@ class RegistryTestHarness : public PolicyProviderTestHarness, virtual ConfigurationPolicyProvider* CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list) OVERRIDE; + scoped_refptr<base::SequencedTaskRunner> task_runner) OVERRIDE; virtual void InstallEmptyPolicy() OVERRIDE; virtual void InstallStringPolicy(const std::string& policy_name, @@ -306,8 +305,7 @@ class PRegTestHarness : public PolicyProviderTestHarness, virtual ConfigurationPolicyProvider* CreateProvider( SchemaRegistry* registry, - scoped_refptr<base::SequencedTaskRunner> task_runner, - const PolicyDefinitionList* policy_list) OVERRIDE; + scoped_refptr<base::SequencedTaskRunner> task_runner) OVERRIDE; virtual void InstallEmptyPolicy() OVERRIDE; virtual void InstallStringPolicy(const std::string& policy_name, @@ -424,10 +422,9 @@ 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)); + scoped_refptr<base::SequencedTaskRunner> task_runner) { + scoped_ptr<AsyncPolicyLoader> loader( + new PolicyLoaderWin(task_runner, kRegistryChromePolicyKey, this)); return new AsyncPolicyProvider(registry, loader.Pass()); } @@ -557,10 +554,9 @@ 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)); + scoped_refptr<base::SequencedTaskRunner> task_runner) { + scoped_ptr<AsyncPolicyLoader> loader( + new PolicyLoaderWin(task_runner, kRegistryChromePolicyKey, this)); return new AsyncPolicyProvider(registry, loader.Pass()); } @@ -841,9 +837,7 @@ class PolicyLoaderWinTest : public PolicyTestBase, } bool Matches(const PolicyBundle& expected) { - PolicyLoaderWin loader(loop_.message_loop_proxy(), - &test_policy_definitions::kList, kTestPolicyKey, - this); + PolicyLoaderWin loader(loop_.message_loop_proxy(), kTestPolicyKey, this); scoped_ptr<PolicyBundle> loaded( loader.InitialLoad(schema_registry_.schema_map())); return loaded->Equals(expected); @@ -853,13 +847,13 @@ class PolicyLoaderWinTest : public PolicyTestBase, RegKey hklm_key(HKEY_CURRENT_USER, kTestPolicyKey, KEY_ALL_ACCESS); ASSERT_TRUE(hklm_key.Valid()); hklm_key.WriteValue( - UTF8ToUTF16(test_policy_definitions::kKeyString).c_str(), + UTF8ToUTF16(test_keys::kKeyString).c_str(), UTF8ToUTF16("registry").c_str()); } bool MatchesRegistrySentinel() { base::DictionaryValue expected_policy; - expected_policy.SetString(test_policy_definitions::kKeyString, "registry"); + expected_policy.SetString(test_keys::kKeyString, "registry"); PolicyBundle expected; expected.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) .LoadFrom(&expected_policy, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER); @@ -868,14 +862,13 @@ class PolicyLoaderWinTest : public PolicyTestBase, bool MatchesTestBundle() { base::DictionaryValue expected_policy; - expected_policy.SetBoolean(test_policy_definitions::kKeyBoolean, true); - expected_policy.SetString(test_policy_definitions::kKeyString, "GPO"); - expected_policy.SetInteger(test_policy_definitions::kKeyInteger, 42); + expected_policy.SetBoolean(test_keys::kKeyBoolean, true); + expected_policy.SetString(test_keys::kKeyString, "GPO"); + expected_policy.SetInteger(test_keys::kKeyInteger, 42); scoped_ptr<base::ListValue> list(new base::ListValue()); list->AppendString("GPO 1"); list->AppendString("GPO 2"); - expected_policy.Set(test_policy_definitions::kKeyStringList, - list.release()); + expected_policy.Set(test_keys::kKeyStringList, list.release()); PolicyBundle expected; expected.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) .LoadFrom(&expected_policy, POLICY_LEVEL_MANDATORY, @@ -895,16 +888,16 @@ const char16 PolicyLoaderWinTest::kTestPolicyKey[] = TEST_F(PolicyLoaderWinTest, HKLMOverHKCU) { RegKey hklm_key(HKEY_LOCAL_MACHINE, kTestPolicyKey, KEY_ALL_ACCESS); ASSERT_TRUE(hklm_key.Valid()); - hklm_key.WriteValue(UTF8ToUTF16(test_policy_definitions::kKeyString).c_str(), + hklm_key.WriteValue(UTF8ToUTF16(test_keys::kKeyString).c_str(), UTF8ToUTF16("hklm").c_str()); RegKey hkcu_key(HKEY_CURRENT_USER, kTestPolicyKey, KEY_ALL_ACCESS); ASSERT_TRUE(hkcu_key.Valid()); - hkcu_key.WriteValue(UTF8ToUTF16(test_policy_definitions::kKeyString).c_str(), + hkcu_key.WriteValue(UTF8ToUTF16(test_keys::kKeyString).c_str(), UTF8ToUTF16("hkcu").c_str()); PolicyBundle expected; expected.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) - .Set(test_policy_definitions::kKeyString, + .Set(test_keys::kKeyString, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE, base::Value::CreateStringValue("hklm"), NULL); diff --git a/chrome/browser/policy/policy_prefs_browsertest.cc b/chrome/browser/policy/policy_prefs_browsertest.cc index 1dd1dd0..65dfcf1 100644 --- a/chrome/browser/policy/policy_prefs_browsertest.cc +++ b/chrome/browser/policy/policy_prefs_browsertest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include <algorithm> +#include <iterator> #include <map> #include <sstream> #include <string> @@ -12,6 +13,7 @@ #include "base/file_util.h" #include "base/files/file_path.h" #include "base/json/json_reader.h" +#include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "base/prefs/pref_service.h" @@ -30,6 +32,7 @@ #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/ui_test_utils.h" +#include "components/policy/core/common/schema.h" #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test_utils.h" #include "policy/policy_constants.h" @@ -209,12 +212,16 @@ class PolicyTestCases { ADD_FAILURE() << "Error parsing policy_test_cases.json: " << error_string; return; } - const PolicyDefinitionList* list = GetChromePolicyDefinitionList(); - for (const PolicyDefinitionList::Entry* policy = list->begin; - policy != list->end; ++policy) { - PolicyTestCase* policy_test_case = GetPolicyTestCase(dict, policy->name); + Schema chrome_schema = Schema::Wrap(GetChromeSchemaData()); + if (!chrome_schema.valid()) { + ADD_FAILURE(); + return; + } + for (Schema::Iterator it = chrome_schema.GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { + PolicyTestCase* policy_test_case = GetPolicyTestCase(dict, it.key()); if (policy_test_case) - (*policy_test_cases_)[policy->name] = policy_test_case; + (*policy_test_cases_)[it.key()] = policy_test_case; } } @@ -379,11 +386,90 @@ void VerifyControlledSettingIndicators(Browser* browser, } // namespace +// A forward iterator that iterates over the Chrome policy names, using the +// Chrome schema data. +class TestCaseIterator + : public std::iterator<std::forward_iterator_tag, const char*> { + public: + static TestCaseIterator GetBegin() { + Schema chrome_schema = Schema::Wrap(GetChromeSchemaData()); + CHECK(chrome_schema.valid()); + return TestCaseIterator(chrome_schema.GetPropertiesIterator()); + } + + static TestCaseIterator GetEnd() { + return TestCaseIterator(); + } + + TestCaseIterator() {} + + explicit TestCaseIterator(const Schema::Iterator& it) + : it_(it.IsAtEnd() ? NULL : new Schema::Iterator(it)) {} + + TestCaseIterator(const TestCaseIterator& other) + : it_(other.it_ ? new Schema::Iterator(*other.it_) : NULL) {} + + ~TestCaseIterator() {} + + TestCaseIterator& operator=(const TestCaseIterator& other) { + it_.reset(other.it_ ? new Schema::Iterator(*other.it_) : NULL); + return *this; + } + + bool Equals(const TestCaseIterator& other) const { + // Assume that both iterators are working with the same Schema; therefore + // the key()s returned are the same. + if (!it_ || !other.it_) + return !it_ && !other.it_; + return it_->key() == other.it_->key(); + } + + bool operator==(const TestCaseIterator& other) const { + return Equals(other); + } + + bool operator !=(const TestCaseIterator& other) const { + return !Equals(other); + } + + const char* operator*() const { + if (!it_) { + NOTREACHED(); + return NULL; + } + return it_->key(); + } + + TestCaseIterator& Advance() { + if (it_) { + it_->Advance(); + if (it_->IsAtEnd()) + it_.reset(); + } else { + NOTREACHED(); + } + return *this; + } + + TestCaseIterator& operator++() { + return Advance(); + } + + TestCaseIterator operator++(int) { + TestCaseIterator current = *this; + Advance(); + return current; + } + + private: + scoped_ptr<Schema::Iterator> it_; +}; + // Base class for tests that change policy and are parameterized with a policy // definition. class PolicyPrefsTest : public InProcessBrowserTest, - public testing::WithParamInterface<PolicyDefinitionList::Entry> { + public testing::WithParamInterface<const char*> { protected: virtual void SetUpInProcessBrowserTestFixture() OVERRIDE { EXPECT_CALL(provider_, IsInitializationComplete(_)) @@ -411,20 +497,22 @@ TEST(PolicyPrefsTestCoverageTest, AllPoliciesHaveATestCase) { // This test fails when a policy is added to // chrome/app/policy/policy_templates.json but a test case is not added to // chrome/test/data/policy/policy_test_cases.json. + Schema chrome_schema = Schema::Wrap(GetChromeSchemaData()); + ASSERT_TRUE(chrome_schema.valid()); + PolicyTestCases policy_test_cases; - const PolicyDefinitionList* list = GetChromePolicyDefinitionList(); - for (const PolicyDefinitionList::Entry* policy = list->begin; - policy != list->end; ++policy) { - EXPECT_TRUE(ContainsKey(policy_test_cases.map(), policy->name)) - << "Missing policy test case for: " << policy->name; + for (Schema::Iterator it = chrome_schema.GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { + EXPECT_TRUE(ContainsKey(policy_test_cases.map(), it.key())) + << "Missing policy test case for: " << it.key(); } } IN_PROC_BROWSER_TEST_P(PolicyPrefsTest, PolicyToPrefsMapping) { // Verifies that policies make their corresponding preferences become managed, // and that the user can't override that setting. - const PolicyTestCase* test_case = policy_test_cases_.Get(GetParam().name); - ASSERT_TRUE(test_case) << "PolicyTestCase not found for " << GetParam().name; + const PolicyTestCase* test_case = policy_test_cases_.Get(GetParam()); + ASSERT_TRUE(test_case) << "PolicyTestCase not found for " << GetParam(); const ScopedVector<PrefMapping>& pref_mappings = test_case->pref_mappings(); if (!test_case->IsSupported() || pref_mappings.empty()) return; @@ -469,9 +557,9 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefsTest, CheckPolicyIndicators) { // Verifies that controlled setting indicators correctly show whether a pref's // value is recommended or enforced by a corresponding policy. const PolicyTestCase* policy_test_case = - policy_test_cases_.Get(GetParam().name); + policy_test_cases_.Get(GetParam()); ASSERT_TRUE(policy_test_case) << "PolicyTestCase not found for " - << GetParam().name; + << GetParam(); const ScopedVector<PrefMapping>& pref_mappings = policy_test_case->pref_mappings(); if (!policy_test_case->IsSupported() || pref_mappings.empty()) @@ -564,10 +652,9 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefsTest, CheckPolicyIndicators) { } } -INSTANTIATE_TEST_CASE_P( - PolicyPrefsTestInstance, - PolicyPrefsTest, - testing::ValuesIn(GetChromePolicyDefinitionList()->begin, - GetChromePolicyDefinitionList()->end)); +INSTANTIATE_TEST_CASE_P(PolicyPrefsTestInstance, + PolicyPrefsTest, + testing::ValuesIn(TestCaseIterator::GetBegin(), + TestCaseIterator::GetEnd())); } // namespace policy diff --git a/chrome/browser/policy/policy_statistics_collector.cc b/chrome/browser/policy/policy_statistics_collector.cc index e2f59b9..51fe04c 100644 --- a/chrome/browser/policy/policy_statistics_collector.cc +++ b/chrome/browser/policy/policy_statistics_collector.cc @@ -8,14 +8,15 @@ #include <string> #include "base/bind.h" +#include "base/callback.h" #include "base/location.h" +#include "base/logging.h" #include "base/metrics/sparse_histogram.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" #include "base/task_runner.h" #include "chrome/browser/policy/policy_service.h" #include "components/policy/core/common/policy_pref_names.h" -#include "policy/policy_constants.h" namespace policy { @@ -23,10 +24,14 @@ const int PolicyStatisticsCollector::kStatisticsUpdateRate = 24 * 60 * 60 * 1000; // 24 hours. PolicyStatisticsCollector::PolicyStatisticsCollector( + const GetChromePolicyDetailsCallback& get_details, + const Schema& chrome_schema, PolicyService* policy_service, PrefService* prefs, const scoped_refptr<base::TaskRunner>& task_runner) - : policy_service_(policy_service), + : get_details_(get_details), + chrome_schema_(chrome_schema), + policy_service_(policy_service), prefs_(prefs), task_runner_(task_runner) { } @@ -58,16 +63,19 @@ void PolicyStatisticsCollector::RecordPolicyUse(int id) { } void PolicyStatisticsCollector::CollectStatistics() { - const policy::PolicyDefinitionList* policy_list = - policy::GetChromePolicyDefinitionList(); const PolicyMap& policies = policy_service_->GetPolicies( PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); // Collect statistics. - for (const policy::PolicyDefinitionList::Entry* policy = policy_list->begin; - policy != policy_list->end; ++policy) { - if (policies.Get(policy->name)) - RecordPolicyUse(policy->id); + for (Schema::Iterator it(chrome_schema_.GetPropertiesIterator()); + !it.IsAtEnd(); it.Advance()) { + if (policies.Get(it.key())) { + const PolicyDetails* details = get_details_.Run(it.key()); + if (details) + RecordPolicyUse(details->id); + else + NOTREACHED(); + } } // Take care of next update. diff --git a/chrome/browser/policy/policy_statistics_collector.h b/chrome/browser/policy/policy_statistics_collector.h index 326d7d9..e0444cc 100644 --- a/chrome/browser/policy/policy_statistics_collector.h +++ b/chrome/browser/policy/policy_statistics_collector.h @@ -9,6 +9,8 @@ #include "base/cancelable_callback.h" #include "base/memory/ref_counted.h" #include "base/time/time.h" +#include "components/policy/core/common/policy_details.h" +#include "components/policy/core/common/schema.h" class PrefService; class PrefRegistrySimple; @@ -29,7 +31,9 @@ class PolicyStatisticsCollector { // Neither |policy_service| nor |prefs| can be NULL and must stay valid // throughout the lifetime of PolicyStatisticsCollector. - PolicyStatisticsCollector(PolicyService* policy_service, + PolicyStatisticsCollector(const GetChromePolicyDetailsCallback& get_details, + const Schema& chrome_schema, + PolicyService* policy_service, PrefService* prefs, const scoped_refptr<base::TaskRunner>& task_runner); virtual ~PolicyStatisticsCollector(); @@ -47,6 +51,8 @@ class PolicyStatisticsCollector { void CollectStatistics(); void ScheduleUpdate(base::TimeDelta delay); + GetChromePolicyDetailsCallback get_details_; + Schema chrome_schema_; PolicyService* policy_service_; PrefService* prefs_; diff --git a/chrome/browser/policy/policy_statistics_collector_unittest.cc b/chrome/browser/policy/policy_statistics_collector_unittest.cc index e55d67a..68b7223 100644 --- a/chrome/browser/policy/policy_statistics_collector_unittest.cc +++ b/chrome/browser/policy/policy_statistics_collector_unittest.cc @@ -7,20 +7,18 @@ #include "base/callback.h" #include "base/compiler_specific.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/testing_pref_service.h" #include "base/test/test_simple_task_runner.h" -#include "base/time/time.h" #include "base/values.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_policy_service.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/policy_statistics_collector.h" #include "chrome/browser/policy/policy_types.h" +#include "chrome/browser/policy/test/policy_test_utils.h" #include "components/policy/core/common/policy_pref_names.h" -#include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -28,23 +26,43 @@ namespace policy { namespace { -using testing::_; -using testing::Lt; -using testing::Return; using testing::ReturnRef; // Arbitrary policy names used for testing. -const char* const kTestPolicy1 = key::kAlternateErrorPagesEnabled; -const char* const kTestPolicy2 = key::kSearchSuggestEnabled; +const char kTestPolicy1[] = "Test Policy 1"; +const char kTestPolicy2[] = "Test Policy 2"; + +const int kTestPolicy1Id = 42; +const int kTestPolicy2Id = 123; + +const char kTestChromeSchema[] = + "{" + " \"type\": \"object\"," + " \"properties\": {" + " \"Test Policy 1\": { \"type\": \"string\" }," + " \"Test Policy 2\": { \"type\": \"string\" }" + " }" + "}"; + +const PolicyDetails kTestPolicyDetails[] = { + // is_deprecated is_device_policy id max_external_data_size + { false, false, kTestPolicy1Id, 0 }, + { false, false, kTestPolicy2Id, 0 }, +}; class TestPolicyStatisticsCollector : public PolicyStatisticsCollector { public: TestPolicyStatisticsCollector( + const GetChromePolicyDetailsCallback& get_details, + const Schema& chrome_schema, PolicyService* policy_service, PrefService* prefs, const scoped_refptr<base::TaskRunner>& task_runner) - : PolicyStatisticsCollector(policy_service, prefs, task_runner) { - } + : PolicyStatisticsCollector(get_details, + chrome_schema, + policy_service, + prefs, + task_runner) {} MOCK_METHOD1(RecordPolicyUse, void(int)); }; @@ -56,28 +74,20 @@ class PolicyStatisticsCollectorTest : public testing::Test { PolicyStatisticsCollectorTest() : update_delay_(base::TimeDelta::FromMilliseconds( PolicyStatisticsCollector::kStatisticsUpdateRate)), - test_policy_id1_(-1), - test_policy_id2_(-1), task_runner_(new base::TestSimpleTaskRunner()) { } virtual void SetUp() OVERRIDE { + std::string error; + chrome_schema_ = Schema::Parse(kTestChromeSchema, &error); + ASSERT_TRUE(chrome_schema_.valid()) << error; + + policy_details_.SetDetails(kTestPolicy1, &kTestPolicyDetails[0]); + policy_details_.SetDetails(kTestPolicy2, &kTestPolicyDetails[1]); + prefs_.registry()->RegisterInt64Pref( policy_prefs::kLastPolicyStatisticsUpdate, 0); - // Find ids for kTestPolicy1 and kTestPolicy2. - const policy::PolicyDefinitionList* policy_list = - policy::GetChromePolicyDefinitionList(); - for (const policy::PolicyDefinitionList::Entry* policy = policy_list->begin; - policy != policy_list->end; ++policy) { - if (strcmp(policy->name, kTestPolicy1) == 0) - test_policy_id1_ = policy->id; - else if (strcmp(policy->name, kTestPolicy2) == 0) - test_policy_id2_ = policy->id; - } - ASSERT_TRUE(test_policy_id1_ != -1); - ASSERT_TRUE(test_policy_id2_ != -1); - // Set up default function behaviour. EXPECT_CALL(policy_service_, GetPolicies(PolicyNamespace(POLICY_DOMAIN_CHROME, @@ -88,6 +98,8 @@ class PolicyStatisticsCollectorTest : public testing::Test { last_delay_ = base::TimeDelta::FromDays(-1); policy_map_.Clear(); policy_statistics_collector_.reset(new TestPolicyStatisticsCollector( + policy_details_.GetCallback(), + chrome_schema_, &policy_service_, &prefs_, task_runner_)); @@ -108,11 +120,10 @@ class PolicyStatisticsCollectorTest : public testing::Test { const base::TimeDelta update_delay_; - int test_policy_id1_; - int test_policy_id2_; - base::TimeDelta last_delay_; + PolicyDetailsMap policy_details_; + Schema chrome_schema_; TestingPrefServiceSimple prefs_; MockPolicyService policy_service_; PolicyMap policy_map_; @@ -128,7 +139,7 @@ TEST_F(PolicyStatisticsCollectorTest, CollectPending) { (base::Time::Now() - update_delay_).ToInternalValue()); EXPECT_CALL(*policy_statistics_collector_.get(), - RecordPolicyUse(test_policy_id1_)); + RecordPolicyUse(kTestPolicy1Id)); policy_statistics_collector_->Initialize(); EXPECT_EQ(1u, task_runner_->GetPendingTasks().size()); @@ -143,7 +154,7 @@ TEST_F(PolicyStatisticsCollectorTest, CollectPendingVeryOld) { base::Time::FromDoubleT(1.0).ToInternalValue()); EXPECT_CALL(*policy_statistics_collector_.get(), - RecordPolicyUse(test_policy_id1_)); + RecordPolicyUse(kTestPolicy1Id)); policy_statistics_collector_->Initialize(); EXPECT_EQ(1u, task_runner_->GetPendingTasks().size()); @@ -169,9 +180,9 @@ TEST_F(PolicyStatisticsCollectorTest, MultiplePolicies) { (base::Time::Now() - update_delay_).ToInternalValue()); EXPECT_CALL(*policy_statistics_collector_.get(), - RecordPolicyUse(test_policy_id1_)); + RecordPolicyUse(kTestPolicy1Id)); EXPECT_CALL(*policy_statistics_collector_.get(), - RecordPolicyUse(test_policy_id2_)); + RecordPolicyUse(kTestPolicy2Id)); policy_statistics_collector_->Initialize(); EXPECT_EQ(1u, task_runner_->GetPendingTasks().size()); diff --git a/chrome/browser/policy/policy_test_utils.cc b/chrome/browser/policy/test/policy_test_utils.cc index 245a2dc..cf0b015 100644 --- a/chrome/browser/policy/policy_test_utils.cc +++ b/chrome/browser/policy/test/policy_test_utils.cc @@ -2,10 +2,13 @@ // 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_test_utils.h" +#include "chrome/browser/policy/test/policy_test_utils.h" #include <string> +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/callback.h" #include "base/json/json_writer.h" #include "base/logging.h" #include "base/values.h" @@ -13,6 +16,24 @@ namespace policy { +PolicyDetailsMap::PolicyDetailsMap() {} + +PolicyDetailsMap::~PolicyDetailsMap() {} + +GetChromePolicyDetailsCallback PolicyDetailsMap::GetCallback() const { + return base::Bind(&PolicyDetailsMap::Lookup, base::Unretained(this)); +} + +void PolicyDetailsMap::SetDetails(const std::string& policy, + const PolicyDetails* details) { + map_[policy] = details; +} + +const PolicyDetails* PolicyDetailsMap::Lookup(const std::string& policy) const { + PolicyDetailsMapping::const_iterator it = map_.find(policy); + return it == map_.end() ? NULL : it->second; +} + bool PolicyServiceIsEmpty(const PolicyService* service) { const PolicyMap& map = service->GetPolicies( PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); diff --git a/chrome/browser/policy/policy_test_utils.h b/chrome/browser/policy/test/policy_test_utils.h index b412e9f..d59b662 100644 --- a/chrome/browser/policy/policy_test_utils.h +++ b/chrome/browser/policy/test/policy_test_utils.h @@ -2,20 +2,47 @@ // 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_TEST_UTILS_H_ -#define CHROME_BROWSER_POLICY_POLICY_TEST_UTILS_H_ +#ifndef CHROME_BROWSER_POLICY_TEST_POLICY_TEST_UTILS_H_ +#define CHROME_BROWSER_POLICY_TEST_POLICY_TEST_UTILS_H_ +#include <map> #include <ostream> +#include <string> +#include "base/basictypes.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/policy_service.h" #include "chrome/browser/policy/policy_types.h" +#include "policy/policy_constants.h" namespace policy { class PolicyBundle; struct PolicyNamespace; +// A mapping of policy names to PolicyDetails that can be used to set the +// PolicyDetails for test policies. +class PolicyDetailsMap { + public: + PolicyDetailsMap(); + ~PolicyDetailsMap(); + + // The returned callback's lifetime is tied to |this| object. + GetChromePolicyDetailsCallback GetCallback() const; + + // Does not take ownership of |details|. + void SetDetails(const std::string& policy, const PolicyDetails* details); + + private: + typedef std::map<std::string, const PolicyDetails*> PolicyDetailsMapping; + + const PolicyDetails* Lookup(const std::string& policy) const; + + PolicyDetailsMapping map_; + + DISALLOW_COPY_AND_ASSIGN(PolicyDetailsMap); +}; + // Returns true if |service| is not serving any policies. Otherwise logs the // current policies and returns false. bool PolicyServiceIsEmpty(const PolicyService* service); @@ -30,4 +57,4 @@ std::ostream& operator<<(std::ostream& os, const policy::PolicyMap& policies); std::ostream& operator<<(std::ostream& os, const policy::PolicyMap::Entry& e); std::ostream& operator<<(std::ostream& os, const policy::PolicyNamespace& ns); -#endif // CHROME_BROWSER_POLICY_POLICY_TEST_UTILS_H_ +#endif // CHROME_BROWSER_POLICY_TEST_POLICY_TEST_UTILS_H_ diff --git a/chrome/browser/ui/webui/policy_ui.cc b/chrome/browser/ui/webui/policy_ui.cc index 7494c91..d343f15 100644 --- a/chrome/browser/ui/webui/policy_ui.cc +++ b/chrome/browser/ui/webui/policy_ui.cc @@ -32,9 +32,14 @@ #include "chrome/browser/policy/profile_policy_connector.h" #include "chrome/browser/policy/profile_policy_connector_factory.h" #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.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/browser/profiles/profile.h" #include "chrome/common/url_constants.h" #include "components/policy/core/common/policy_namespace.h" +#include "components/policy/core/common/schema.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_service.h" @@ -63,13 +68,8 @@ #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/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" #include "extensions/common/manifest.h" #include "extensions/common/manifest_constants.h" #endif @@ -547,13 +547,19 @@ void PolicyUIHandler::OnPolicyUpdated(const policy::PolicyNamespace& ns, void PolicyUIHandler::SendPolicyNames() const { base::DictionaryValue names; + Profile* profile = Profile::FromWebUI(web_ui()); + policy::SchemaRegistry* registry = + policy::SchemaRegistryServiceFactory::GetForContext( + profile->GetOriginalProfile()); + scoped_refptr<policy::SchemaMap> schema_map = registry->schema_map(); + // Add Chrome policy names. base::DictionaryValue* chrome_policy_names = new base::DictionaryValue; - const policy::PolicyDefinitionList* list = - policy::GetChromePolicyDefinitionList(); - for (const policy::PolicyDefinitionList::Entry* entry = list->begin; - entry != list->end; ++entry) { - chrome_policy_names->SetBoolean(entry->name, true); + policy::PolicyNamespace chrome_ns(policy::POLICY_DOMAIN_CHROME, ""); + const policy::Schema* chrome_schema = schema_map->GetSchema(chrome_ns); + for (policy::Schema::Iterator it = chrome_schema->GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { + chrome_policy_names->SetBoolean(it.key(), true); } names.Set("chromePolicyNames", chrome_policy_names); @@ -561,17 +567,11 @@ void PolicyUIHandler::SendPolicyNames() const { // 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); const ExtensionSet* extensions = extension_system->extension_service()->extensions(); - 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) { const extensions::Extension* extension = it->get(); diff --git a/chrome/browser/ui/webui/policy_ui_browsertest.cc b/chrome/browser/ui/webui/policy_ui_browsertest.cc index 758543e..1dcc97e 100644 --- a/chrome/browser/ui/webui/policy_ui_browsertest.cc +++ b/chrome/browser/ui/webui/policy_ui_browsertest.cc @@ -17,6 +17,7 @@ #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/ui_test_utils.h" +#include "components/policy/core/common/schema.h" #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test_utils.h" #include "grit/generated_resources.h" @@ -173,12 +174,13 @@ IN_PROC_BROWSER_TEST_F(PolicyUITest, SendPolicyNames) { // Expect that the policy table contains all known policies in alphabetical // order and none of the policies have a set value. std::vector<std::vector<std::string> > expected_policies; - const policy::PolicyDefinitionList* policies = - policy::GetChromePolicyDefinitionList(); - for (const policy::PolicyDefinitionList::Entry* policy = policies->begin; - policy != policies->end; ++policy) { + policy::Schema chrome_schema = + policy::Schema::Wrap(policy::GetChromeSchemaData()); + ASSERT_TRUE(chrome_schema.valid()); + for (policy::Schema::Iterator it = chrome_schema.GetPropertiesIterator(); + !it.IsAtEnd(); it.Advance()) { expected_policies.push_back( - PopulateExpectedPolicy(policy->name, std::string(), NULL, false)); + PopulateExpectedPolicy(it.key(), std::string(), NULL, false)); } // Retrieve the contents of the policy table from the UI and verify that it @@ -239,19 +241,20 @@ IN_PROC_BROWSER_TEST_F(PolicyUITest, SendPolicyValues) { // * All known policies whose value has not been set, in alphabetical order. std::vector<std::vector<std::string> > expected_policies; size_t first_unset_position = 0; - const policy::PolicyDefinitionList* policies = - policy::GetChromePolicyDefinitionList(); - for (const policy::PolicyDefinitionList::Entry* policy = policies->begin; - policy != policies->end; ++policy) { + policy::Schema chrome_schema = + policy::Schema::Wrap(policy::GetChromeSchemaData()); + ASSERT_TRUE(chrome_schema.valid()); + for (policy::Schema::Iterator props = chrome_schema.GetPropertiesIterator(); + !props.IsAtEnd(); props.Advance()) { std::map<std::string, std::string>::const_iterator it = - expected_values.find(policy->name); + expected_values.find(props.key()); const std::string value = it == expected_values.end() ? std::string() : it->second; - const policy::PolicyMap::Entry* metadata = values.Get(policy->name); + const policy::PolicyMap::Entry* metadata = values.Get(props.key()); expected_policies.insert( metadata ? expected_policies.begin() + first_unset_position++ : expected_policies.end(), - PopulateExpectedPolicy(policy->name, value, metadata, false)); + PopulateExpectedPolicy(props.key(), value, metadata, false)); } expected_policies.insert( expected_policies.begin() + first_unset_position++, diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 5bfac03..f98b8db 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -168,10 +168,10 @@ 'browser/policy/mock_configuration_policy_provider.h', 'browser/policy/preferences_mock_mac.cc', 'browser/policy/preferences_mock_mac.h', - 'browser/policy/policy_test_utils.cc', - 'browser/policy/policy_test_utils.h', 'browser/policy/test/local_policy_test_server.cc', 'browser/policy/test/local_policy_test_server.h', + 'browser/policy/test/policy_test_utils.cc', + 'browser/policy/test/policy_test_utils.h', 'browser/prefs/pref_service_mock_builder.cc', 'browser/prefs/pref_service_mock_builder.h', 'browser/profile_resetter/profile_resetter_test_base.cc', diff --git a/chrome/tools/build/generate_policy_source.py b/chrome/tools/build/generate_policy_source.py index da620e7..a5f1989 100755 --- a/chrome/tools/build/generate_policy_source.py +++ b/chrome/tools/build/generate_policy_source.py @@ -212,6 +212,7 @@ def _WritePolicyConstantHeader(policies, os, f): '\n' '#include "base/basictypes.h"\n' '#include "base/values.h"\n' + '#include "components/policy/core/common/policy_details.h"\n' '\n' 'namespace policy {\n' '\n' @@ -224,27 +225,10 @@ def _WritePolicyConstantHeader(policies, os, f): 'configuration resides.\n' 'extern const wchar_t kRegistryChromePolicyKey[];\n') - f.write('// Lists metadata such as name, expected type and id for all\n' - '// policies. Used to initialize ConfigurationPolicyProviders and\n' - '// CloudExternalDataManagers.\n' - 'struct PolicyDefinitionList {\n' - ' struct Entry {\n' - ' const char* name;\n' - ' base::Value::Type value_type;\n' - ' bool device_policy;\n' - ' int id;\n' - ' size_t max_external_data_size;\n' - ' };\n' - '\n' - ' const Entry* begin;\n' - ' const Entry* end;\n' - '};\n' - '\n' - '// Returns true if the given policy is deprecated.\n' - 'bool IsDeprecatedPolicy(const std::string& policy);\n' - '\n' - '// Returns the default policy definition list for Chrome.\n' - 'const PolicyDefinitionList* GetChromePolicyDefinitionList();\n' + f.write('// Returns the PolicyDetails for |policy| if |policy| is a known\n' + '// Chrome policy, otherwise returns NULL.\n' + 'const PolicyDetails* GetChromePolicyDetails(' + 'const std::string& policy);\n' '\n' '// Returns the schema data of the Chrome policy schema.\n' 'const internal::SchemaData* GetChromeSchemaData();\n' @@ -264,10 +248,6 @@ def _WritePolicyConstantHeader(policies, os, f): #------------------ policy constants source ------------------------# -def _GetValueType(policy_type): - return policy_type if policy_type != 'TYPE_EXTERNAL' else 'TYPE_DICTIONARY' - - # A mapping of the simple schema types to base::Value::Types. SIMPLE_SCHEMA_NAME_MAP = { 'boolean': 'TYPE_BOOLEAN', @@ -363,6 +343,9 @@ class SchemaNodesGenerator: begin = len(self.property_nodes) self.property_nodes += properties end = len(self.property_nodes) + if index == 0: + self.root_properties_begin = begin + self.root_properties_end = end extra = len(self.properties_nodes) self.properties_nodes.append((begin, end, additionalProperties, name)) @@ -405,10 +388,12 @@ class SchemaNodesGenerator: def _WritePolicyConstantSource(policies, os, f): - f.write('#include "base/basictypes.h"\n' + f.write('#include "policy/policy_constants.h"\n' + '\n' + '#include <algorithm>\n' + '\n' '#include "base/logging.h"\n' '#include "components/policy/core/common/schema_internal.h"\n' - '#include "policy/policy_constants.h"\n' '\n' 'namespace policy {\n' '\n' @@ -426,35 +411,30 @@ def _WritePolicyConstantSource(policies, os, f): if policy.is_supported: chrome_schema['properties'][policy.name] = policy.schema - f.write('const PolicyDefinitionList::Entry kEntries[] = {\n') + # Note: this list must be kept in sync with the known property list of the + # Chrome schema, so that binary seaching in the PropertyNode array gets the + # right index on this array as well. See the implementation of + # GetChromePolicyDetails() below. + f.write('const PolicyDetails kChromePolicyDetails[] = {\n' + '// is_deprecated is_device_policy id max_external_data_size\n') for policy in policies: if policy.is_supported: - f.write(' { key::k%s, base::Value::%s, %s, %s, %s },\n' % - (policy.name, _GetValueType(policy.policy_type), - 'true' if policy.is_device_only else 'false', policy.id, - policy.max_size)) + f.write(' { %-14s %-16s %3s, %24s },\n' % ( + 'true,' if policy.is_deprecated else 'false,', + 'true,' if policy.is_device_only else 'false,', + policy.id, + policy.max_size)) f.write('};\n\n') - f.write('const PolicyDefinitionList kChromePolicyList = {\n' - ' kEntries,\n' - ' kEntries + arraysize(kEntries),\n' - '};\n\n') - - has_deprecated_policies = any( - [p.is_supported and p.is_deprecated for p in policies]) - - if has_deprecated_policies: - f.write('// List of deprecated policies.\n' - 'const char* kDeprecatedPolicyList[] = {\n') - for policy in policies: - if policy.is_supported and policy.is_deprecated: - f.write(' key::k%s,\n' % policy.name) - f.write('};\n\n') - schema_generator = SchemaNodesGenerator(shared_strings) schema_generator.Generate(chrome_schema, 'root node') schema_generator.Write(f) + f.write('bool CompareKeys(const internal::PropertyNode& node,\n' + ' const std::string& key) {\n' + ' return node.key < key;\n' + '}\n\n') + f.write('} // namespace\n\n') if os == 'win': @@ -466,25 +446,40 @@ def _WritePolicyConstantSource(policies, os, f): 'L"' + CHROMIUM_POLICY_KEY + '";\n' '#endif\n\n') - f.write('bool IsDeprecatedPolicy(const std::string& policy) {\n') - if has_deprecated_policies: - # arraysize() doesn't work with empty arrays. - f.write(' for (size_t i = 0; i < arraysize(kDeprecatedPolicyList);' - ' ++i) {\n' - ' if (policy == kDeprecatedPolicyList[i])\n' - ' return true;\n' - ' }\n') - f.write(' return false;\n' - '}\n\n') - - f.write('const PolicyDefinitionList* GetChromePolicyDefinitionList() {\n' - ' return &kChromePolicyList;\n' - '}\n\n') - f.write('const internal::SchemaData* GetChromeSchemaData() {\n' ' return &kChromeSchemaData;\n' '}\n\n') + f.write('const PolicyDetails* GetChromePolicyDetails(' + 'const std::string& policy) {\n' + ' // First index in kPropertyNodes of the Chrome policies.\n' + ' static const int begin_index = %s;\n' + ' // One-past-the-end of the Chrome policies in kPropertyNodes.\n' + ' static const int end_index = %s;\n' % + (schema_generator.root_properties_begin, + schema_generator.root_properties_end)) + f.write(' const internal::PropertyNode* begin =\n' + ' kPropertyNodes + begin_index;\n' + ' const internal::PropertyNode* end = kPropertyNodes + end_index;\n' + ' const internal::PropertyNode* it =\n' + ' std::lower_bound(begin, end, policy, CompareKeys);\n' + ' if (it == end || it->key != policy)\n' + ' return NULL;\n' + ' // This relies on kPropertyNodes from begin_index to end_index\n' + ' // having exactly the same policies (and in the same order) as\n' + ' // kChromePolicyDetails, so that binary searching on the first\n' + ' // gets the same results as a binary search on the second would.\n' + ' // However, kPropertyNodes has the policy names and\n' + ' // kChromePolicyDetails doesn\'t, so we obtain the index into\n' + ' // the second array by searching the first to avoid duplicating\n' + ' // the policy name pointers.\n' + ' // Offsetting |it| from |begin| here obtains the index we\'re\n' + ' // looking for.\n' + ' size_t index = it - begin;\n' + ' CHECK_LT(index, arraysize(kChromePolicyDetails));\n' + ' return kChromePolicyDetails + index;\n' + '}\n\n') + f.write('namespace key {\n\n') for policy in policies: # TODO(joaodasilva): Include only supported policies in diff --git a/components/policy.gypi b/components/policy.gypi index 32aed7e..cadd308 100644 --- a/components/policy.gypi +++ b/components/policy.gypi @@ -20,6 +20,7 @@ 'conditions': [ ['configuration_policy==1', { 'sources': [ + 'policy/core/common/policy_details.h', 'policy/core/common/policy_namespace.cc', 'policy/core/common/policy_namespace.h', 'policy/core/common/policy_pref_names.cc', diff --git a/components/policy/core/common/policy_details.h b/components/policy/core/common/policy_details.h new file mode 100644 index 0000000..8320336 --- /dev/null +++ b/components/policy/core/common/policy_details.h @@ -0,0 +1,43 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_POLICY_CORE_COMMON_POLICY_DETAILS_H_ +#define COMPONENTS_POLICY_CORE_COMMON_POLICY_DETAILS_H_ + +#include <string> + +#include "base/basictypes.h" +#include "base/callback_forward.h" +#include "components/policy/policy_export.h" + +namespace policy { + +// Contains read-only metadata about a Chrome policy. +struct POLICY_EXPORT PolicyDetails { + // True if this policy has been deprecated. + bool is_deprecated; + + // True if this policy is a Chrome OS device policy. + bool is_device_policy; + + // The id of the protobuf field that contains this policy, + // in the cloud policy protobuf. + int id; + + // If this policy references external data then this is the maximum size + // allowed for that data. + // Otherwise this field is 0 and doesn't have any meaning. + size_t max_external_data_size; +}; + +// A typedef for functions that match the signature of +// GetChromePolicyDetails(). This can be used to inject that +// function into objects, so that it can be easily mocked for +// tests. +typedef base::Callback<const PolicyDetails*(const std::string&)> + GetChromePolicyDetailsCallback; + +} // namespace policy + +#endif // COMPONENTS_POLICY_CORE_COMMON_POLICY_DETAILS_H_ |