diff options
| author | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-12 14:52:22 +0000 |
|---|---|---|
| committer | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-12 14:52:22 +0000 |
| commit | cb7dc11a3b170650c4a0e054be38c7739caa8f88 (patch) | |
| tree | e2eb0cc9048065d5e1ef75ee31b077832fd50171 | |
| parent | 8d01f640208ff07115140f592ad1cf6de360cf0c (diff) | |
| download | chromium_src-cb7dc11a3b170650c4a0e054be38c7739caa8f88.zip chromium_src-cb7dc11a3b170650c4a0e054be38c7739caa8f88.tar.gz chromium_src-cb7dc11a3b170650c4a0e054be38c7739caa8f88.tar.bz2 | |
Removed the PolicyDefinitionList.
This list was generated at compile time and included policy information
extracted from the policy templates file that was useful at runtime, such as
the policy names, types, deprecation status, etc.
Most of this information is now available via the Chrome policy schema, so
this list was largely redundant.
The chrome-specific data in this list is now available in a list of
PolicyDetails, also generated at compile time and kept in sync with the Chrome
schema data.
Refactored all the remaining users of PolicyDefinitionList to use the schema
and/or PolicyDetails now. Also introduced a test helper to mock out
PolicyDetails in tests.
Cleanup: moved policy_test_utils.h to policy/test/
BUG=270667
R=bartfab@chromium.org, dubroy@chromium.org
Review URL: https://codereview.chromium.org/58313002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@234515 0039d316-1c4b-4281-b951-d872f2087c98
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_ |