From 3352840b3bf284efed94bfd09657156e24c83e14 Mon Sep 17 00:00:00 2001 From: "joaodasilva@chromium.org" Date: Mon, 16 Dec 2013 18:38:32 +0000 Subject: Cleanup the policy code after the recent moves into the component. - policy_transformations.cc is not needed anymore, since PolicyServiceImpl can now refer to the generated policies by name. - moved generate_policy_source_unittest.cc into the component. - removed stale .gitignore entries. BUG=271392 TBR=joaodasilva@chromium.org Review URL: https://codereview.chromium.org/113813003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240940 0039d316-1c4b-4281-b951-d872f2087c98 --- .gitignore | 6 - ...cloud_external_data_policy_observer_unittest.cc | 10 +- .../network_configuration_updater_unittest.cc | 3 +- .../browser/net/proxy_policy_handler_unittest.cc | 5 +- chrome/browser/policy/browser_policy_connector.cc | 4 +- .../policy/configuration_policy_pref_store_test.cc | 3 +- .../policy/generate_policy_source_unittest.cc | 123 --------------------- chrome/browser/policy/policy_transformations.cc | 71 ------------ chrome/browser/policy/policy_transformations.h | 18 --- .../policy/policy_transformations_unittest.cc | 76 ------------- chrome/browser/policy/profile_policy_connector.cc | 4 +- chrome/browser/prefs/proxy_policy_unittest.cc | 4 +- chrome/chrome_browser.gypi | 2 - chrome/chrome_tests_unit.gypi | 2 - chrome/test/base/testing_profile.cc | 3 +- components/components_tests.gyp | 1 + .../core/common/generate_policy_source_unittest.cc | 123 +++++++++++++++++++++ components/policy/core/common/policy_pref_names.h | 4 - .../policy/core/common/policy_service_impl.cc | 64 +++++++++-- .../policy/core/common/policy_service_impl.h | 8 +- .../core/common/policy_service_impl_unittest.cc | 95 ++++++++-------- 21 files changed, 240 insertions(+), 389 deletions(-) delete mode 100644 chrome/browser/policy/generate_policy_source_unittest.cc delete mode 100644 chrome/browser/policy/policy_transformations.cc delete mode 100644 chrome/browser/policy/policy_transformations.h delete mode 100644 chrome/browser/policy/policy_transformations_unittest.cc create mode 100644 components/policy/core/common/generate_policy_source_unittest.cc diff --git a/.gitignore b/.gitignore index 02892b0..dac93a5 100644 --- a/.gitignore +++ b/.gitignore @@ -59,12 +59,6 @@ v8.log # The Chrome OS build creates a /c symlink due to http://crbug.com/54866. /c /ceee/internal/ -/chrome/app/policy/chrome_settings_proto_compile.xml -/chrome/app/policy/chrome_settings_proto_generated_compile.xml -/chrome/app/policy/cloud_policy_backend_header_compile.xml -/chrome/app/policy/cloud_policy_proto_compile.xml -/chrome/app/policy/cloud_policy_proto_generated_compile.xml -/chrome/app/policy/policy_proto_compile.xml /chrome/app/theme/default_100_percent/google_chrome /chrome/app/theme/default_200_percent/google_chrome /chrome/app/theme/google_chrome diff --git a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc index 5057fe9..8f60889 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc @@ -323,9 +323,8 @@ void CloudExternalDataPolicyObserverTest::LogInAsDeviceLocalAccount( PolicyServiceImpl::Providers providers; providers.push_back(device_local_account_policy_provider_.get()); TestingProfile::Builder builder; - builder.SetPolicyService(scoped_ptr(new PolicyServiceImpl( - providers, - PolicyServiceImpl::PreprocessCallback()))); + builder.SetPolicyService( + scoped_ptr(new PolicyServiceImpl(providers))); profile_ = builder.Build(); profile_->set_profile_name(user_id); @@ -356,9 +355,8 @@ void CloudExternalDataPolicyObserverTest::LogInAsRegularUser() { PolicyServiceImpl::Providers providers; providers.push_back(&user_policy_provider_); TestingProfile::Builder builder; - builder.SetPolicyService(scoped_ptr(new PolicyServiceImpl( - providers, - PolicyServiceImpl::PreprocessCallback()))); + builder.SetPolicyService( + scoped_ptr(new PolicyServiceImpl(providers))); profile_ = builder.Build(); profile_->set_profile_name(kRegularUserID); diff --git a/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc b/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc index 48d3172..4571072 100644 --- a/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc +++ b/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc @@ -136,8 +136,7 @@ class NetworkConfigurationUpdaterTest : public testing::Test { provider_.Init(); PolicyServiceImpl::Providers providers; providers.push_back(&provider_); - policy_service_.reset(new PolicyServiceImpl( - providers, PolicyServiceImpl::PreprocessCallback())); + policy_service_.reset(new PolicyServiceImpl(providers)); scoped_ptr fake_toplevel_onc = chromeos::onc::ReadDictionaryFromJson(kFakeONC); diff --git a/chrome/browser/net/proxy_policy_handler_unittest.cc b/chrome/browser/net/proxy_policy_handler_unittest.cc index 023eb26..6112f12 100644 --- a/chrome/browser/net/proxy_policy_handler_unittest.cc +++ b/chrome/browser/net/proxy_policy_handler_unittest.cc @@ -4,12 +4,10 @@ #include -#include "base/bind.h" #include "base/memory/scoped_ptr.h" #include "base/values.h" #include "chrome/browser/net/proxy_policy_handler.h" #include "chrome/browser/policy/configuration_policy_pref_store_test.h" -#include "chrome/browser/policy/policy_transformations.h" #include "chrome/browser/prefs/proxy_config_dictionary.h" #include "chrome/browser/prefs/proxy_prefs.h" #include "chrome/common/pref_names.h" @@ -32,8 +30,7 @@ class ProxyPolicyHandlerTest // preprocessor. The previous store must be nulled out first so that it // removes itself from the service's observer list. store_ = NULL; - policy_service_.reset( - new PolicyServiceImpl(providers_, base::Bind(&FixDeprecatedPolicies))); + policy_service_.reset(new PolicyServiceImpl(providers_)); store_ = new ConfigurationPolicyPrefStore( policy_service_.get(), &handler_list_, POLICY_LEVEL_MANDATORY); } diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc index df48847..0a8cdcd 100644 --- a/chrome/browser/policy/browser_policy_connector.cc +++ b/chrome/browser/policy/browser_policy_connector.cc @@ -26,7 +26,6 @@ #include "base/threading/sequenced_worker_pool.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/policy/configuration_policy_handler_list_factory.h" -#include "chrome/browser/policy/policy_transformations.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_version_info.h" @@ -398,8 +397,7 @@ PolicyService* BrowserPolicyConnector::GetPolicyService() { providers.push_back(&global_user_cloud_policy_provider_); #endif } - policy_service_.reset(new PolicyServiceImpl( - providers, base::Bind(&policy::FixDeprecatedPolicies))); + policy_service_.reset(new PolicyServiceImpl(providers)); } return policy_service_.get(); } diff --git a/chrome/browser/policy/configuration_policy_pref_store_test.cc b/chrome/browser/policy/configuration_policy_pref_store_test.cc index 5a0ca23..ec9b479 100644 --- a/chrome/browser/policy/configuration_policy_pref_store_test.cc +++ b/chrome/browser/policy/configuration_policy_pref_store_test.cc @@ -24,8 +24,7 @@ ConfigurationPolicyPrefStoreTest::ConfigurationPolicyPrefStoreTest() .WillRepeatedly(Return(false)); provider_.Init(); providers_.push_back(&provider_); - policy_service_.reset(new PolicyServiceImpl( - providers_, PolicyServiceImpl::PreprocessCallback())); + policy_service_.reset(new PolicyServiceImpl(providers_)); store_ = new ConfigurationPolicyPrefStore( policy_service_.get(), &handler_list_, POLICY_LEVEL_MANDATORY); } diff --git a/chrome/browser/policy/generate_policy_source_unittest.cc b/chrome/browser/policy/generate_policy_source_unittest.cc deleted file mode 100644 index e0e5ee4..0000000 --- a/chrome/browser/policy/generate_policy_source_unittest.cc +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright (c) 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include - -#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" - -// This unittest tests the code generated by -// chrome/tools/build/generate_policy_source.py. - -namespace policy { - -TEST(GeneratePolicySource, ChromeSchemaData) { - Schema schema = Schema::Wrap(GetChromeSchemaData()); - ASSERT_TRUE(schema.valid()); - EXPECT_EQ(base::Value::TYPE_DICTIONARY, schema.type()); - - Schema subschema = schema.GetAdditionalProperties(); - EXPECT_FALSE(subschema.valid()); - - subschema = schema.GetProperty("no such policy exists"); - EXPECT_FALSE(subschema.valid()); - - subschema = schema.GetProperty(key::kAlternateErrorPagesEnabled); - ASSERT_TRUE(subschema.valid()); - EXPECT_EQ(base::Value::TYPE_BOOLEAN, subschema.type()); - - subschema = schema.GetProperty(key::kIncognitoModeAvailability); - ASSERT_TRUE(subschema.valid()); - EXPECT_EQ(base::Value::TYPE_INTEGER, subschema.type()); - - subschema = schema.GetProperty(key::kProxyMode); - ASSERT_TRUE(subschema.valid()); - EXPECT_EQ(base::Value::TYPE_STRING, subschema.type()); - - subschema = schema.GetProperty(key::kCookiesAllowedForUrls); - ASSERT_TRUE(subschema.valid()); - EXPECT_EQ(base::Value::TYPE_LIST, subschema.type()); - ASSERT_TRUE(subschema.GetItems().valid()); - EXPECT_EQ(base::Value::TYPE_STRING, subschema.GetItems().type()); - - subschema = schema.GetProperty(key::kProxySettings); - ASSERT_TRUE(subschema.valid()); - EXPECT_EQ(base::Value::TYPE_DICTIONARY, subschema.type()); - EXPECT_FALSE(subschema.GetAdditionalProperties().valid()); - EXPECT_FALSE(subschema.GetProperty("no such proxy key exists").valid()); - ASSERT_TRUE(subschema.GetProperty(key::kProxyMode).valid()); - ASSERT_TRUE(subschema.GetProperty(key::kProxyServer).valid()); - ASSERT_TRUE(subschema.GetProperty(key::kProxyServerMode).valid()); - 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, - key::kProxyMode, - key::kProxyPacUrl, - key::kProxyServer, - key::kProxyServerMode, - NULL, - }; - const char** next = kExpectedProperties; - for (Schema::Iterator it(subschema.GetPropertiesIterator()); - !it.IsAtEnd(); it.Advance(), ++next) { - ASSERT_TRUE(*next != NULL); - EXPECT_STREQ(*next, it.key()); - ASSERT_TRUE(it.schema().valid()); - EXPECT_EQ(base::Value::TYPE_STRING, it.schema().type()); - } - 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_transformations.cc b/chrome/browser/policy/policy_transformations.cc deleted file mode 100644 index a678dc6..0000000 --- a/chrome/browser/policy/policy_transformations.cc +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/policy/policy_transformations.h" - -#include "base/memory/scoped_ptr.h" -#include "base/values.h" -#include "components/policy/core/common/policy_bundle.h" -#include "components/policy/core/common/policy_map.h" -#include "policy/policy_constants.h" - -namespace policy { - -namespace { - -const char* kProxyPolicies[] = { - key::kProxyMode, - key::kProxyServerMode, - key::kProxyServer, - key::kProxyPacUrl, - key::kProxyBypassList, -}; - -void FixDeprecatedPolicies(PolicyMap* policies) { - // Proxy settings have been configured by 5 policies that didn't mix well - // together, and maps of policies had to take this into account when merging - // policy sources. The proxy settings will eventually be configured by a - // single Dictionary policy when all providers have support for that. For - // now, the individual policies are mapped here to a single Dictionary policy - // that the rest of the policy machinery uses. - - // The highest (level, scope) pair for an existing proxy policy is determined - // first, and then only policies with those exact attributes are merged. - PolicyMap::Entry current_priority; // Defaults to the lowest priority. - scoped_ptr proxy_settings(new base::DictionaryValue); - for (size_t i = 0; i < arraysize(kProxyPolicies); ++i) { - const PolicyMap::Entry* entry = policies->Get(kProxyPolicies[i]); - if (entry) { - if (entry->has_higher_priority_than(current_priority)) { - proxy_settings->Clear(); - current_priority = *entry; - } - if (!entry->has_higher_priority_than(current_priority) && - !current_priority.has_higher_priority_than(*entry)) { - proxy_settings->Set(kProxyPolicies[i], entry->value->DeepCopy()); - } - policies->Erase(kProxyPolicies[i]); - } - } - // Sets the new |proxy_settings| if kProxySettings isn't set yet, or if the - // new priority is higher. - const PolicyMap::Entry* existing = policies->Get(key::kProxySettings); - if (!proxy_settings->empty() && - (!existing || current_priority.has_higher_priority_than(*existing))) { - policies->Set(key::kProxySettings, - current_priority.level, - current_priority.scope, - proxy_settings.release(), - NULL); - } -} - -} // namespace - -void FixDeprecatedPolicies(PolicyBundle* bundle) { - FixDeprecatedPolicies( - &bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))); -} - -} // namespace policy diff --git a/chrome/browser/policy/policy_transformations.h b/chrome/browser/policy/policy_transformations.h deleted file mode 100644 index 12f4c9f..0000000 --- a/chrome/browser/policy/policy_transformations.h +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_POLICY_POLICY_TRANSFORMATIONS_H_ -#define CHROME_BROWSER_POLICY_POLICY_TRANSFORMATIONS_H_ - -namespace policy { - -class PolicyBundle; - -// Helper that converts deprecated chrome policies into their corresponding -// actual policies. -void FixDeprecatedPolicies(PolicyBundle* bundle); - -} // namespace policy - -#endif // CHROME_BROWSER_POLICY_POLICY_TRANSFORMATIONS_H_ diff --git a/chrome/browser/policy/policy_transformations_unittest.cc b/chrome/browser/policy/policy_transformations_unittest.cc deleted file mode 100644 index 5ca7c0e..0000000 --- a/chrome/browser/policy/policy_transformations_unittest.cc +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/policy/policy_transformations.h" - -#include "base/memory/scoped_ptr.h" -#include "base/values.h" -#include "components/policy/core/common/policy_bundle.h" -#include "components/policy/core/common/policy_map.h" -#include "policy/policy_constants.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace policy { - -TEST(PolicyTransformationsTest, FixDeprecatedPolicies) { - PolicyBundle policy_bundle; - PolicyMap& policy_map = - policy_bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); - - // Individual proxy policy values in the Chrome namespace should be collected - // into a dictionary. - policy_map.Set(key::kProxyServerMode, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateIntegerValue(3), - NULL); - - // Both these policies should be ignored, since there's a higher priority - // policy available. - policy_map.Set(key::kProxyMode, - POLICY_LEVEL_RECOMMENDED, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("pac_script"), - NULL); - policy_map.Set(key::kProxyPacUrl, - POLICY_LEVEL_RECOMMENDED, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("http://example.com/wpad.dat"), - NULL); - - // Add a value to a non-Chrome namespace. - policy_bundle.Get(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, std::string())) - .Set(key::kProxyServerMode, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateIntegerValue(3), - NULL); - - PolicyBundle actual_bundle; - actual_bundle.CopyFrom(policy_bundle); - FixDeprecatedPolicies(&actual_bundle); - - PolicyBundle expected_bundle; - // The resulting Chrome namespace map should have the collected policy. - PolicyMap& expected_map = - expected_bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); - scoped_ptr expected_value(new base::DictionaryValue); - expected_value->SetInteger(key::kProxyServerMode, 3); - expected_map.Set(key::kProxySettings, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - expected_value.release(), - NULL); - // The resulting Extensions namespace map shouldn't have been modified. - expected_bundle.Get(PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, std::string())) - .Set(key::kProxyServerMode, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateIntegerValue(3), - NULL); - - EXPECT_TRUE(expected_bundle.Equals(actual_bundle)); -} - -} // namespace policy diff --git a/chrome/browser/policy/profile_policy_connector.cc b/chrome/browser/policy/profile_policy_connector.cc index 0b8c02e..9dda4da 100644 --- a/chrome/browser/policy/profile_policy_connector.cc +++ b/chrome/browser/policy/profile_policy_connector.cc @@ -10,7 +10,6 @@ #include "base/logging.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/policy/browser_policy_connector.h" -#include "chrome/browser/policy/policy_transformations.h" #include "components/policy/core/common/cloud/cloud_policy_manager.h" #include "components/policy/core/common/configuration_policy_provider.h" #include "components/policy/core/common/forwarding_policy_provider.h" @@ -88,8 +87,7 @@ void ProfilePolicyConnector::Init( providers.push_back(special_user_policy_provider_.get()); #endif - policy_service_.reset(new PolicyServiceImpl( - providers, base::Bind(&policy::FixDeprecatedPolicies))); + policy_service_.reset(new PolicyServiceImpl(providers)); #if defined(OS_CHROMEOS) if (is_primary_user_) { diff --git a/chrome/browser/prefs/proxy_policy_unittest.cc b/chrome/browser/prefs/proxy_policy_unittest.cc index 39824cf..07c645a 100644 --- a/chrome/browser/prefs/proxy_policy_unittest.cc +++ b/chrome/browser/prefs/proxy_policy_unittest.cc @@ -7,7 +7,6 @@ #include "base/command_line.h" #include "base/memory/ref_counted.h" #include "base/message_loop/message_loop.h" -#include "chrome/browser/policy/policy_transformations.h" #include "chrome/browser/prefs/browser_prefs.h" #include "chrome/browser/prefs/pref_service_mock_factory.h" #include "chrome/browser/prefs/pref_service_syncable.h" @@ -91,8 +90,7 @@ class ProxyPolicyTest : public testing::Test { PolicyServiceImpl::Providers providers; providers.push_back(&provider_); - policy_service_.reset( - new PolicyServiceImpl(providers, base::Bind(&FixDeprecatedPolicies))); + policy_service_.reset(new PolicyServiceImpl(providers)); provider_.Init(); } diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index bdda04b..1d4756d 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1489,8 +1489,6 @@ 'browser/policy/file_selection_dialogs_policy_handler.h', 'browser/policy/javascript_policy_handler.cc', 'browser/policy/javascript_policy_handler.h', - 'browser/policy/policy_transformations.cc', - 'browser/policy/policy_transformations.h', 'browser/policy/profile_policy_connector.cc', 'browser/policy/profile_policy_connector_stub.cc', 'browser/policy/profile_policy_connector.h', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 366ea13..4a13145 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1091,10 +1091,8 @@ 'browser/policy/configuration_policy_pref_store_test.h', 'browser/policy/configuration_policy_pref_store_unittest.cc', 'browser/policy/file_selection_dialogs_policy_handler_unittest.cc', - 'browser/policy/generate_policy_source_unittest.cc', 'browser/policy/javascript_policy_handler_unittest.cc', 'browser/policy/policy_path_parser_unittest.cc', - 'browser/policy/policy_transformations_unittest.cc', 'browser/policy/url_blacklist_manager_unittest.cc', 'browser/policy/url_blacklist_policy_handler_unittest.cc', 'browser/predictors/autocomplete_action_predictor_table_unittest.cc', diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc index 816e6bc..54aff83 100644 --- a/chrome/test/base/testing_profile.cc +++ b/chrome/test/base/testing_profile.cc @@ -655,8 +655,7 @@ void TestingProfile::CreateProfilePolicyConnector() { if (!policy_service_) { #if defined(ENABLE_CONFIGURATION_POLICY) std::vector providers; - policy_service_.reset(new policy::PolicyServiceImpl( - providers, policy::PolicyServiceImpl::PreprocessCallback())); + policy_service_.reset(new policy::PolicyServiceImpl(providers)); #else policy_service_.reset(new policy::PolicyServiceStub()); #endif diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 29055103..6faa5fb 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -227,6 +227,7 @@ 'policy/core/common/cloud/user_info_fetcher_unittest.cc', 'policy/core/common/config_dir_policy_loader_unittest.cc', 'policy/core/common/forwarding_policy_provider_unittest.cc', + 'policy/core/common/generate_policy_source_unittest.cc', 'policy/core/common/policy_bundle_unittest.cc', 'policy/core/common/policy_loader_mac_unittest.cc', 'policy/core/common/policy_loader_win_unittest.cc', diff --git a/components/policy/core/common/generate_policy_source_unittest.cc b/components/policy/core/common/generate_policy_source_unittest.cc new file mode 100644 index 0000000..1eca86e --- /dev/null +++ b/components/policy/core/common/generate_policy_source_unittest.cc @@ -0,0 +1,123 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#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" + +// This unittest tests the code generated by +// chrome/tools/build/generate_policy_source.py. + +namespace policy { + +TEST(GeneratePolicySource, ChromeSchemaData) { + Schema schema = Schema::Wrap(GetChromeSchemaData()); + ASSERT_TRUE(schema.valid()); + EXPECT_EQ(base::Value::TYPE_DICTIONARY, schema.type()); + + Schema subschema = schema.GetAdditionalProperties(); + EXPECT_FALSE(subschema.valid()); + + subschema = schema.GetProperty("no such policy exists"); + EXPECT_FALSE(subschema.valid()); + + subschema = schema.GetProperty(key::kAlternateErrorPagesEnabled); + ASSERT_TRUE(subschema.valid()); + EXPECT_EQ(base::Value::TYPE_BOOLEAN, subschema.type()); + + subschema = schema.GetProperty(key::kIncognitoModeAvailability); + ASSERT_TRUE(subschema.valid()); + EXPECT_EQ(base::Value::TYPE_INTEGER, subschema.type()); + + subschema = schema.GetProperty(key::kProxyMode); + ASSERT_TRUE(subschema.valid()); + EXPECT_EQ(base::Value::TYPE_STRING, subschema.type()); + + subschema = schema.GetProperty(key::kCookiesAllowedForUrls); + ASSERT_TRUE(subschema.valid()); + EXPECT_EQ(base::Value::TYPE_LIST, subschema.type()); + ASSERT_TRUE(subschema.GetItems().valid()); + EXPECT_EQ(base::Value::TYPE_STRING, subschema.GetItems().type()); + + subschema = schema.GetProperty(key::kProxySettings); + ASSERT_TRUE(subschema.valid()); + EXPECT_EQ(base::Value::TYPE_DICTIONARY, subschema.type()); + EXPECT_FALSE(subschema.GetAdditionalProperties().valid()); + EXPECT_FALSE(subschema.GetProperty("no such proxy key exists").valid()); + ASSERT_TRUE(subschema.GetProperty(key::kProxyMode).valid()); + ASSERT_TRUE(subschema.GetProperty(key::kProxyServer).valid()); + ASSERT_TRUE(subschema.GetProperty(key::kProxyServerMode).valid()); + 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, + key::kProxyMode, + key::kProxyPacUrl, + key::kProxyServer, + key::kProxyServerMode, + NULL, + }; + const char** next = kExpectedProperties; + for (Schema::Iterator it(subschema.GetPropertiesIterator()); + !it.IsAtEnd(); it.Advance(), ++next) { + ASSERT_TRUE(*next != NULL); + EXPECT_STREQ(*next, it.key()); + ASSERT_TRUE(it.schema().valid()); + EXPECT_EQ(base::Value::TYPE_STRING, it.schema().type()); + } + 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/components/policy/core/common/policy_pref_names.h b/components/policy/core/common/policy_pref_names.h index 5ebcef6..9b76919 100644 --- a/components/policy/core/common/policy_pref_names.h +++ b/components/policy/core/common/policy_pref_names.h @@ -10,10 +10,6 @@ namespace policy { namespace policy_prefs { -// Constants for the names of policy-related preferences. -// TODO(dconnelly): remove POLICY_EXPORT once the policy code moves to the -// policy component (crbug.com/271392). - POLICY_EXPORT extern const char kLastPolicyStatisticsUpdate[]; POLICY_EXPORT extern const char kUserPolicyRefreshRate[]; diff --git a/components/policy/core/common/policy_service_impl.cc b/components/policy/core/common/policy_service_impl.cc index 8fdd265..d515d19 100644 --- a/components/policy/core/common/policy_service_impl.cc +++ b/components/policy/core/common/policy_service_impl.cc @@ -9,18 +9,68 @@ #include "base/bind.h" #include "base/message_loop/message_loop.h" #include "base/stl_util.h" +#include "base/values.h" #include "components/policy/core/common/policy_bundle.h" #include "components/policy/core/common/policy_map.h" +#include "policy/policy_constants.h" namespace policy { typedef PolicyServiceImpl::Providers::const_iterator Iterator; -PolicyServiceImpl::PolicyServiceImpl( - const Providers& providers, - const PreprocessCallback& preprocess_callback) - : preprocess_callback_(preprocess_callback), - update_task_ptr_factory_(this) { +namespace { + +const char* kProxyPolicies[] = { + key::kProxyMode, + key::kProxyServerMode, + key::kProxyServer, + key::kProxyPacUrl, + key::kProxyBypassList, +}; + +void FixDeprecatedPolicies(PolicyMap* policies) { + // Proxy settings have been configured by 5 policies that didn't mix well + // together, and maps of policies had to take this into account when merging + // policy sources. The proxy settings will eventually be configured by a + // single Dictionary policy when all providers have support for that. For + // now, the individual policies are mapped here to a single Dictionary policy + // that the rest of the policy machinery uses. + + // The highest (level, scope) pair for an existing proxy policy is determined + // first, and then only policies with those exact attributes are merged. + PolicyMap::Entry current_priority; // Defaults to the lowest priority. + scoped_ptr proxy_settings(new base::DictionaryValue); + for (size_t i = 0; i < arraysize(kProxyPolicies); ++i) { + const PolicyMap::Entry* entry = policies->Get(kProxyPolicies[i]); + if (entry) { + if (entry->has_higher_priority_than(current_priority)) { + proxy_settings->Clear(); + current_priority = *entry; + } + if (!entry->has_higher_priority_than(current_priority) && + !current_priority.has_higher_priority_than(*entry)) { + proxy_settings->Set(kProxyPolicies[i], entry->value->DeepCopy()); + } + policies->Erase(kProxyPolicies[i]); + } + } + // Sets the new |proxy_settings| if kProxySettings isn't set yet, or if the + // new priority is higher. + const PolicyMap::Entry* existing = policies->Get(key::kProxySettings); + if (!proxy_settings->empty() && + (!existing || current_priority.has_higher_priority_than(*existing))) { + policies->Set(key::kProxySettings, + current_priority.level, + current_priority.scope, + proxy_settings.release(), + NULL); + } +} + +} // namespace + +PolicyServiceImpl::PolicyServiceImpl(const Providers& providers) + : update_task_ptr_factory_(this) { for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain) initialization_complete_[domain] = true; providers_ = providers; @@ -130,12 +180,12 @@ void PolicyServiceImpl::NotifyNamespaceUpdated( void PolicyServiceImpl::MergeAndTriggerUpdates() { // Merge from each provider in their order of priority. + const PolicyNamespace chrome_namespace(POLICY_DOMAIN_CHROME, std::string()); PolicyBundle bundle; for (Iterator it = providers_.begin(); it != providers_.end(); ++it) { PolicyBundle provided_bundle; provided_bundle.CopyFrom((*it)->policies()); - if (!preprocess_callback_.is_null()) - preprocess_callback_.Run(&provided_bundle); + FixDeprecatedPolicies(&provided_bundle.Get(chrome_namespace)); bundle.MergeFrom(provided_bundle); } diff --git a/components/policy/core/common/policy_service_impl.h b/components/policy/core/common/policy_service_impl.h index a10e881..1a67a41 100644 --- a/components/policy/core/common/policy_service_impl.h +++ b/components/policy/core/common/policy_service_impl.h @@ -28,15 +28,12 @@ class POLICY_EXPORT PolicyServiceImpl public ConfigurationPolicyProvider::Observer { public: typedef std::vector Providers; - typedef base::Callback PreprocessCallback; // The PolicyServiceImpl will merge policies from |providers|. |providers| // must be sorted in decreasing order of priority; the first provider will // have the highest priority. The PolicyServiceImpl does not take ownership of // the providers, and they must outlive the PolicyServiceImpl. - // |preprocess_callback| will be applied every PolicyBundle before merginng. - PolicyServiceImpl(const Providers& providers, - const PreprocessCallback& preprocess_callback); + explicit PolicyServiceImpl(const Providers& providers); virtual ~PolicyServiceImpl(); @@ -90,9 +87,6 @@ class POLICY_EXPORT PolicyServiceImpl // call to RefreshPolicies(). std::set refresh_pending_; - // Callback invoked to manipulate a PolicyBundle before it is merged. - PreprocessCallback preprocess_callback_; - // List of callbacks to invoke once all providers refresh after a // RefreshPolicies() call. std::vector refresh_callbacks_; diff --git a/components/policy/core/common/policy_service_impl_unittest.cc b/components/policy/core/common/policy_service_impl_unittest.cc index e95a1fb..d8350b8 100644 --- a/components/policy/core/common/policy_service_impl_unittest.cc +++ b/components/policy/core/common/policy_service_impl_unittest.cc @@ -13,6 +13,7 @@ #include "components/policy/core/common/external_data_fetcher.h" #include "components/policy/core/common/mock_configuration_policy_provider.h" #include "components/policy/core/common/mock_policy_service.h" +#include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -29,17 +30,6 @@ const char kExtension[] = "extension-id"; const char kSameLevelPolicy[] = "policy-same-level-and-scope"; const char kDiffLevelPolicy[] = "chrome-diff-level-and-scope"; -void SetPolicyMapValue(const std::string& key, - const std::string& value, - PolicyBundle* bundle) { - bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())) - .Set(key, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - new base::StringValue(value), - NULL); -} - // Helper to compare the arguments to an EXPECT_CALL of OnPolicyUpdated() with // their expected values. MATCHER_P(PolicyEquals, expected, "") { @@ -122,8 +112,7 @@ class PolicyServiceTest : public testing::Test { providers.push_back(&provider0_); providers.push_back(&provider1_); providers.push_back(&provider2_); - policy_service_.reset(new PolicyServiceImpl( - providers, PolicyServiceImpl::PreprocessCallback())); + policy_service_.reset(new PolicyServiceImpl(providers)); } virtual void TearDown() OVERRIDE { @@ -534,39 +523,6 @@ TEST_F(PolicyServiceTest, NamespaceMerge) { PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, kExtension)).Equals(expected)); } -TEST_F(PolicyServiceTest, PolicyPreprocessing) { - // Reset the PolicyServiceImpl to one that has the preprocessor. - PolicyServiceImpl::Providers providers; - providers.push_back(&provider0_); - policy_service_.reset(new PolicyServiceImpl( - providers, base::Bind(&SetPolicyMapValue, kSameLevelPolicy, "bar"))); - - // Set the policy value to "foo". - scoped_ptr bundle(new PolicyBundle()); - PolicyMap& map = - bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); - map.Set(kSameLevelPolicy, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("foo"), - NULL); - - // Push the update through the provider. - provider0_.UpdatePolicy(bundle.Pass()); - RunUntilIdle(); - - // The value should have been changed from "foo" to "bar". - const PolicyMap& actual = policy_service_->GetPolicies( - PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); - PolicyMap expected; - expected.Set(kSameLevelPolicy, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - base::Value::CreateStringValue("bar"), - NULL); - EXPECT_TRUE(actual.Equals(expected)); -} - TEST_F(PolicyServiceTest, IsInitializationComplete) { // |provider0| has all domains initialized. Mock::VerifyAndClearExpectations(&provider1_); @@ -579,8 +535,7 @@ TEST_F(PolicyServiceTest, IsInitializationComplete) { providers.push_back(&provider0_); providers.push_back(&provider1_); providers.push_back(&provider2_); - policy_service_.reset(new PolicyServiceImpl( - providers, PolicyServiceImpl::PreprocessCallback())); + policy_service_.reset(new PolicyServiceImpl(providers)); EXPECT_FALSE(policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME)); EXPECT_FALSE( policy_service_->IsInitializationComplete(POLICY_DOMAIN_EXTENSIONS)); @@ -648,4 +603,48 @@ TEST_F(PolicyServiceTest, IsInitializationComplete) { policy_service_->RemoveObserver(POLICY_DOMAIN_EXTENSIONS, &observer); } +TEST_F(PolicyServiceTest, FixDeprecatedPolicies) { + const PolicyNamespace chrome_namespace(POLICY_DOMAIN_CHROME, std::string()); + const PolicyNamespace extension_namespace(POLICY_DOMAIN_EXTENSIONS, "xyz"); + + scoped_ptr policy_bundle(new PolicyBundle()); + PolicyMap& policy_map = policy_bundle->Get(chrome_namespace); + // Individual proxy policy values in the Chrome namespace should be collected + // into a dictionary. + policy_map.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, base::Value::CreateIntegerValue(3), NULL); + + // Both these policies should be ignored, since there's a higher priority + // policy available. + policy_map.Set(key::kProxyMode, POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER, + base::Value::CreateStringValue("pac_script"), NULL); + policy_map.Set(key::kProxyPacUrl, POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER, + base::Value::CreateStringValue("http://example.com/wpad.dat"), + NULL); + + // Add a value to a non-Chrome namespace. + policy_bundle->Get(extension_namespace) + .Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateIntegerValue(3), NULL); + + // The resulting Chrome namespace map should have the collected policy. + PolicyMap expected_chrome; + scoped_ptr expected_value(new base::DictionaryValue); + expected_value->SetInteger(key::kProxyServerMode, 3); + expected_chrome.Set(key::kProxySettings, POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, expected_value.release(), NULL); + + // The resulting Extensions namespace map shouldn't have been modified. + PolicyMap expected_extension; + expected_extension.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, base::Value::CreateIntegerValue(3), + NULL); + + provider0_.UpdatePolicy(policy_bundle.Pass()); + RunUntilIdle(); + + EXPECT_TRUE(VerifyPolicies(chrome_namespace, expected_chrome)); + EXPECT_TRUE(VerifyPolicies(extension_namespace, expected_extension)); +} + } // namespace policy -- cgit v1.1