diff options
38 files changed, 658 insertions, 123 deletions
diff --git a/chrome/app/policy/policy_templates.json b/chrome/app/policy/policy_templates.json index ffeb660..59aa304 100644 --- a/chrome/app/policy/policy_templates.json +++ b/chrome/app/policy/policy_templates.json @@ -101,9 +101,10 @@ # 'can_be_recommended' can be set to True to include that policy in the # recommended policies templates. This only affects the template generation; # all policies can be at the recommended level. The default is False. -# 'max_size' specifies the maximal size of the external data that a policy can -# reference, in bytes. This annotation is compulsory for policies of type -# 'external'. It is ignored for all other policy types. +# +# The 'max_size' key is used to specify the maximal size of the external data +# that a policy can reference, in bytes. This annotation is compulsory for +# policies of type 'external'. It is ignored for all other policy types. # # The 'future' key is used to indicate that a policy isn't yet ready for # usage. It defaults to False, and currently affects the generated policy diff --git a/chrome/browser/policy/cloud/cloud_external_data_manager_base.cc b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc index a030d23..9007067 100644 --- a/chrome/browser/policy/cloud/cloud_external_data_manager_base.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/policy/cloud/cloud_external_data_manager_base.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h" #include <map> #include <string> @@ -16,7 +16,7 @@ #include "base/sequenced_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/values.h" -#include "chrome/browser/policy/cloud/cloud_external_data_store.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_store.h" #include "chrome/browser/policy/cloud/cloud_policy_store.h" #include "chrome/browser/policy/cloud/external_policy_data_fetcher.h" #include "chrome/browser/policy/cloud/external_policy_data_updater.h" @@ -32,6 +32,10 @@ namespace { // Fetch data for at most two external data references at the same time. const int kMaxParallelFetches = 2; +// Allows policies to reference |max_external_data_size_for_testing| bytes of +// external data even if no |max_size| was specified in policy_templates.json. +int max_external_data_size_for_testing = 0; + } // namespace // Backend for the CloudExternalDataManagerBase that handles all data download, @@ -293,6 +297,9 @@ void CloudExternalDataManagerBase::Backend::FetchAll() { size_t CloudExternalDataManagerBase::Backend::GetMaxExternalDataSize( const std::string& policy) const { + if (max_external_data_size_for_testing) + 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 // policy_templates.json, allowing the maximum data size to be specified as @@ -430,6 +437,12 @@ void CloudExternalDataManagerBase::Fetch( &Backend::Fetch, base::Unretained(backend_.get()), policy, callback)); } +// static +void CloudExternalDataManagerBase::SetMaxExternalDataSizeForTesting( + int max_size) { + max_external_data_size_for_testing = max_size; +} + void CloudExternalDataManagerBase::FetchAll() { DCHECK(CalledOnValidThread()); backend_task_runner_->PostTask(FROM_HERE, base::Bind( diff --git a/chrome/browser/policy/cloud/cloud_external_data_manager_base.h b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.h index 244ec6b..6db8bc2 100644 --- a/chrome/browser/policy/cloud/cloud_external_data_manager_base.h +++ b/chrome/browser/chromeos/policy/cloud_external_data_manager_base.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_POLICY_CLOUD_CLOUD_EXTERNAL_DATA_MANAGER_BASE_H_ -#define CHROME_BROWSER_POLICY_CLOUD_CLOUD_EXTERNAL_DATA_MANAGER_BASE_H_ +#ifndef CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_MANAGER_BASE_H_ +#define CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_MANAGER_BASE_H_ #include "base/basictypes.h" #include "base/compiler_specific.h" @@ -56,6 +56,13 @@ class CloudExternalDataManagerBase : public CloudExternalDataManager, const std::string& policy, const ExternalDataFetcher::FetchCallback& callback) OVERRIDE; + // Allows policies to reference |max_size| bytes of external data even if no + // |max_size| was specified in policy_templates.json. + // TODO(bartfab): This override is only needed because there are no policies + // that reference external data and have a |max_size| yet. Once the first such + // policy is added, use that policy in tests and remove the override. + static void SetMaxExternalDataSizeForTesting(int max_size); + protected: friend class CouldExternalDataManagerBaseTest; @@ -87,4 +94,4 @@ class CloudExternalDataManagerBase : public CloudExternalDataManager, } // namespace policy -#endif // CHROME_BROWSER_POLICY_CLOUD_CLOUD_EXTERNAL_DATA_MANAGER_BASE_H_ +#endif // CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_MANAGER_BASE_H_ diff --git a/chrome/browser/policy/cloud/cloud_external_data_manager_base_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc index 7c8f029..fe1b247 100644 --- a/chrome/browser/policy/cloud/cloud_external_data_manager_base_unittest.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/policy/cloud/cloud_external_data_manager_base.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h" #include <map> #include <string> @@ -10,20 +10,20 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/files/scoped_temp_dir.h" +#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop_proxy.h" #include "base/run_loop.h" -#include "base/sequenced_task_runner.h" #include "base/sha1.h" #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" +#include "base/test/test_simple_task_runner.h" #include "base/values.h" -#include "chrome/browser/policy/cloud/cloud_external_data_store.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_store.h" #include "chrome/browser/policy/cloud/mock_cloud_policy_store.h" #include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/policy/policy_types.h" -#include "content/public/test/test_browser_thread_bundle.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" @@ -123,8 +123,7 @@ class CouldExternalDataManagerBaseTest : public testing::Test { void FetchAll(); - content::TestBrowserThreadBundle thread_bundle_; - + base::MessageLoop message_loop_; base::ScopedTempDir temp_dir_; scoped_ptr<ResourceCache> resource_cache_; MockCloudPolicyStore cloud_policy_store_; @@ -138,14 +137,13 @@ class CouldExternalDataManagerBaseTest : public testing::Test { DISALLOW_COPY_AND_ASSIGN(CouldExternalDataManagerBaseTest); }; -CouldExternalDataManagerBaseTest::CouldExternalDataManagerBaseTest() - : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) { +CouldExternalDataManagerBaseTest::CouldExternalDataManagerBaseTest() { } void CouldExternalDataManagerBaseTest::SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); resource_cache_.reset(new ResourceCache(temp_dir_.path(), - base::MessageLoopProxy::current())); + message_loop_.message_loop_proxy())); SetUpExternalDataManager(); // Set |kStringPolicy| to a string value. @@ -176,10 +174,12 @@ void CouldExternalDataManagerBaseTest::TearDown() { void CouldExternalDataManagerBaseTest::SetUpExternalDataManager() { external_data_manager_.reset(new CloudExternalDataManagerBase( &kPolicyDefinitionList, - base::MessageLoopProxy::current(), - base::MessageLoopProxy::current())); + message_loop_.message_loop_proxy(), + message_loop_.message_loop_proxy())); external_data_manager_->SetExternalDataStore(make_scoped_ptr( - new CloudExternalDataStore(kCacheKey, resource_cache_.get()))); + new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get()))); external_data_manager_->SetPolicyStore(&cloud_policy_store_); } @@ -314,7 +314,9 @@ TEST_F(CouldExternalDataManagerBaseTest, DownloadAndCache) { external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); std::string data; - EXPECT_TRUE(CloudExternalDataStore(kCacheKey, resource_cache_.get()).Load( + EXPECT_TRUE(CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get()).Load( k10BytePolicy, base::SHA1HashString(k10ByteData), 10, &data)); EXPECT_EQ(k10ByteData, data); } @@ -368,7 +370,9 @@ TEST_F(CouldExternalDataManagerBaseTest, DownloadAndCacheAll) { // Verify that the downloaded data is present in the cache. external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); - CloudExternalDataStore cache(kCacheKey, resource_cache_.get()); + CloudExternalDataStore cache(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get()); std::string data; EXPECT_TRUE(cache.Load(k10BytePolicy, base::SHA1HashString(k10ByteData), 10, &data)); @@ -486,7 +490,9 @@ TEST_F(CouldExternalDataManagerBaseTest, LoadFromCache) { // Store valid external data for |k10BytePolicy| in the cache. external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); - EXPECT_TRUE(CloudExternalDataStore(kCacheKey, resource_cache_.get()).Store( + EXPECT_TRUE(CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get()).Store( k10BytePolicy, base::SHA1HashString(k10ByteData), k10ByteData)); // Instantiate an external_data_manager_ that uses the primed cache. @@ -510,7 +516,9 @@ TEST_F(CouldExternalDataManagerBaseTest, PruneCacheOnStartup) { external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); scoped_ptr<CloudExternalDataStore> - cache(new CloudExternalDataStore(kCacheKey, resource_cache_.get())); + cache(new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get())); // Store valid external data for |k10BytePolicy| in the cache. EXPECT_TRUE(cache->Store(k10BytePolicy, base::SHA1HashString(k10ByteData), @@ -532,7 +540,9 @@ TEST_F(CouldExternalDataManagerBaseTest, PruneCacheOnStartup) { external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); - cache.reset(new CloudExternalDataStore(kCacheKey, resource_cache_.get())); + cache.reset(new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get())); std::string data; // Verify that the valid external data for |k10BytePolicy| is still in the // cache. @@ -554,7 +564,9 @@ TEST_F(CouldExternalDataManagerBaseTest, PruneCacheOnChange) { external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); scoped_ptr<CloudExternalDataStore> - cache(new CloudExternalDataStore(kCacheKey, resource_cache_.get())); + cache(new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get())); EXPECT_TRUE(cache->Store(k20BytePolicy, base::SHA1HashString(k20ByteData), k20ByteData)); @@ -574,7 +586,9 @@ TEST_F(CouldExternalDataManagerBaseTest, PruneCacheOnChange) { // the cache. external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); - cache.reset(new CloudExternalDataStore(kCacheKey, resource_cache_.get())); + cache.reset(new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get())); std::string data; EXPECT_FALSE(cache->Load(k20BytePolicy, base::SHA1HashString(k20ByteData), 20, &data)); @@ -585,7 +599,9 @@ TEST_F(CouldExternalDataManagerBaseTest, CacheCorruption) { external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); scoped_ptr<CloudExternalDataStore> - cache(new CloudExternalDataStore(kCacheKey, resource_cache_.get())); + cache(new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get())); // Store external data for |k10BytePolicy| that exceeds the maximal external // data size allowed for that policy. EXPECT_TRUE(cache->Store(k10BytePolicy, @@ -633,7 +649,9 @@ TEST_F(CouldExternalDataManagerBaseTest, CacheCorruption) { external_data_manager_.reset(); base::RunLoop().RunUntilIdle(); - cache.reset(new CloudExternalDataStore(kCacheKey, resource_cache_.get())); + cache.reset(new CloudExternalDataStore(kCacheKey, + message_loop_.message_loop_proxy(), + resource_cache_.get())); std::string data; // Verify that the invalid external data for |k10BytePolicy| has been pruned // from the cache. Load() will return |false| in two cases: diff --git a/chrome/browser/policy/cloud/cloud_external_data_store.cc b/chrome/browser/chromeos/policy/cloud_external_data_store.cc index 2478565..a00cd8a 100644 --- a/chrome/browser/policy/cloud/cloud_external_data_store.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_store.cc @@ -2,11 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/policy/cloud/cloud_external_data_store.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_store.h" #include <set> #include "base/logging.h" +#include "base/sequenced_task_runner.h" #include "base/sha1.h" #include "base/strings/string_number_conversions.h" #include "chrome/browser/policy/cloud/resource_cache.h" @@ -26,19 +27,22 @@ std::string GetSubkey(const std::string& policy, const std::string& hash) { } // namespace -CloudExternalDataStore::CloudExternalDataStore(const std::string& cache_key, - ResourceCache* cache) +CloudExternalDataStore::CloudExternalDataStore( + const std::string& cache_key, + scoped_refptr<base::SequencedTaskRunner> task_runner, + ResourceCache* cache) : cache_key_(cache_key), + task_runner_(task_runner), cache_(cache) { - DetachFromThread(); } CloudExternalDataStore::~CloudExternalDataStore() { - DCHECK(CalledOnValidThread()); + DCHECK(task_runner_->RunsTasksOnCurrentThread()); } void CloudExternalDataStore::Prune( const CloudExternalDataManager::Metadata& metadata) { + DCHECK(task_runner_->RunsTasksOnCurrentThread()); std::set<std::string> subkeys_to_keep; for (CloudExternalDataManager::Metadata::const_iterator it = metadata.begin(); it != metadata.end(); ++it) { @@ -50,7 +54,7 @@ void CloudExternalDataStore::Prune( bool CloudExternalDataStore::Store(const std::string& policy, const std::string& hash, const std::string& data) { - DCHECK(CalledOnValidThread()); + DCHECK(task_runner_->RunsTasksOnCurrentThread()); return cache_->Store(cache_key_, GetSubkey(policy, hash), data); } @@ -58,7 +62,7 @@ bool CloudExternalDataStore::Load(const std::string& policy, const std::string& hash, size_t max_size, std::string* data) { - DCHECK(CalledOnValidThread()); + DCHECK(task_runner_->RunsTasksOnCurrentThread()); const std::string subkey = GetSubkey(policy, hash); if (cache_->Load(cache_key_, subkey, data)) { if (data->size() <= max_size && base::SHA1HashString(*data) == hash) diff --git a/chrome/browser/policy/cloud/cloud_external_data_store.h b/chrome/browser/chromeos/policy/cloud_external_data_store.h index 937fd05..079a0db 100644 --- a/chrome/browser/policy/cloud/cloud_external_data_store.h +++ b/chrome/browser/chromeos/policy/cloud_external_data_store.h @@ -2,15 +2,19 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_POLICY_CLOUD_CLOUD_EXTERNAL_DATA_STORE_H_ -#define CHROME_BROWSER_POLICY_CLOUD_CLOUD_EXTERNAL_DATA_STORE_H_ +#ifndef CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_STORE_H_ +#define CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_STORE_H_ #include <string> #include "base/basictypes.h" -#include "base/threading/non_thread_safe.h" +#include "base/memory/ref_counted.h" #include "chrome/browser/policy/cloud/cloud_external_data_manager.h" +namespace base { +class SequencedTaskRunner; +} + namespace policy { class ResourceCache; @@ -21,14 +25,15 @@ class ResourceCache; // to be kept. Instances of this class may be created on any thread and may // share the same cache, however: // * After creation, the cache and all stores using it must always be accessed -// from the same thread (or, in the future, via the same SequencedWorkerPool -// with the same sequence identifier). +// via the same |task_runner| only. // * Stores sharing a cache must use different cache_keys to avoid namespace // overlaps. // * The cache must outlive all stores using it. -class CloudExternalDataStore : public base::NonThreadSafe { +class CloudExternalDataStore { public: - CloudExternalDataStore(const std::string& cache_key, ResourceCache* cache); + CloudExternalDataStore(const std::string& cache_key, + scoped_refptr<base::SequencedTaskRunner> task_runner, + ResourceCache* cache); ~CloudExternalDataStore(); // Removes all entries from the store whose (policy, hash) pair is not found @@ -51,6 +56,10 @@ class CloudExternalDataStore : public base::NonThreadSafe { private: std::string cache_key_; + + // Task runner that |this| runs on. + scoped_refptr<base::SequencedTaskRunner> task_runner_; + ResourceCache* cache_; // Not owned. DISALLOW_COPY_AND_ASSIGN(CloudExternalDataStore); @@ -58,4 +67,4 @@ class CloudExternalDataStore : public base::NonThreadSafe { } // namespace policy -#endif // CHROME_BROWSER_POLICY_CLOUD_CLOUD_EXTERNAL_DATA_STORE_H_ +#endif // CHROME_BROWSER_CHROMEOS_POLICY_CLOUD_EXTERNAL_DATA_STORE_H_ diff --git a/chrome/browser/policy/cloud/cloud_external_data_store_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_store_unittest.cc index 7d8838c..29bafee 100644 --- a/chrome/browser/policy/cloud/cloud_external_data_store_unittest.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_store_unittest.cc @@ -2,11 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/policy/cloud/cloud_external_data_store.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_store.h" #include "base/compiler_specific.h" #include "base/files/scoped_temp_dir.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/sha1.h" #include "base/test/test_simple_task_runner.h" @@ -38,6 +37,7 @@ class CouldExternalDataStoreTest : public testing::Test { const std::string kData1Hash; const std::string kData2Hash; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; base::ScopedTempDir temp_dir_; scoped_ptr<ResourceCache> resource_cache_; @@ -46,19 +46,18 @@ class CouldExternalDataStoreTest : public testing::Test { CouldExternalDataStoreTest::CouldExternalDataStoreTest() : kData1Hash(base::SHA1HashString(kData1)), - kData2Hash(base::SHA1HashString(kData2)) { + kData2Hash(base::SHA1HashString(kData2)), + task_runner_(new base::TestSimpleTaskRunner) { } void CouldExternalDataStoreTest::SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); - resource_cache_.reset(new ResourceCache( - temp_dir_.path(), - make_scoped_refptr(new base::TestSimpleTaskRunner))); + resource_cache_.reset(new ResourceCache(temp_dir_.path(), task_runner_)); } TEST_F(CouldExternalDataStoreTest, StoreAndLoad) { // Write an entry to a store. - CloudExternalDataStore store(kKey1, resource_cache_.get()); + CloudExternalDataStore store(kKey1, task_runner_, resource_cache_.get()); EXPECT_TRUE(store.Store(kPolicy1, kData1Hash, kData1)); // Check that loading and verifying the entry against an invalid hash fails. @@ -72,7 +71,7 @@ TEST_F(CouldExternalDataStoreTest, StoreAndLoad) { TEST_F(CouldExternalDataStoreTest, StoreTooLargeAndLoad) { // Write an entry to a store. - CloudExternalDataStore store(kKey1, resource_cache_.get()); + CloudExternalDataStore store(kKey1, task_runner_, resource_cache_.get()); EXPECT_TRUE(store.Store(kPolicy1, kData1Hash, kData2)); // Check that the entry has been written to the resource cache backing the @@ -95,7 +94,7 @@ TEST_F(CouldExternalDataStoreTest, StoreTooLargeAndLoad) { TEST_F(CouldExternalDataStoreTest, StoreInvalidAndLoad) { // Construct a store entry whose hash and contents do not match. - CloudExternalDataStore store(kKey1, resource_cache_.get()); + CloudExternalDataStore store(kKey1, task_runner_, resource_cache_.get()); EXPECT_TRUE(store.Store(kPolicy1, kData1Hash, kData2)); // Check that the entry has been written to the resource cache backing the @@ -117,7 +116,7 @@ TEST_F(CouldExternalDataStoreTest, StoreInvalidAndLoad) { TEST_F(CouldExternalDataStoreTest, Prune) { // Write two entries to a store. - CloudExternalDataStore store(kKey1, resource_cache_.get()); + CloudExternalDataStore store(kKey1, task_runner_, resource_cache_.get()); EXPECT_TRUE(store.Store(kPolicy1, kData1Hash, kData1)); EXPECT_TRUE(store.Store(kPolicy2, kData2Hash, kData2)); @@ -156,9 +155,9 @@ TEST_F(CouldExternalDataStoreTest, Prune) { TEST_F(CouldExternalDataStoreTest, SharedCache) { // Write entries to two stores for two different cache_keys sharing a cache. - CloudExternalDataStore store1(kKey1, resource_cache_.get()); + CloudExternalDataStore store1(kKey1, task_runner_, resource_cache_.get()); EXPECT_TRUE(store1.Store(kPolicy1, kData1Hash, kData1)); - CloudExternalDataStore store2(kKey2, resource_cache_.get()); + CloudExternalDataStore store2(kKey2, task_runner_, resource_cache_.get()); EXPECT_TRUE(store2.Store(kPolicy2, kData2Hash, kData2)); // Check that the entries have been assigned to the correct keys in the diff --git a/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc new file mode 100644 index 0000000..00481f3 --- /dev/null +++ b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc @@ -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. + +#include "chrome/browser/chromeos/policy/user_cloud_external_data_manager.h" + +#include "base/location.h" +#include "base/memory/scoped_ptr.h" +#include "base/sequenced_task_runner.h" +#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 { + +namespace { + +const char kCacheKey[] = "data"; + +} // namespace + +UserCloudExternalDataManager::UserCloudExternalDataManager( + const PolicyDefinitionList* policy_definitions, + 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, + backend_task_runner, + io_task_runner), + resource_cache_(new ResourceCache(cache_path, backend_task_runner)) { + SetPolicyStore(policy_store); + SetExternalDataStore(make_scoped_ptr(new CloudExternalDataStore( + kCacheKey, backend_task_runner, resource_cache_))); +} + +UserCloudExternalDataManager::~UserCloudExternalDataManager() { + SetExternalDataStore(scoped_ptr<CloudExternalDataStore>()); + backend_task_runner_->DeleteSoon(FROM_HERE, resource_cache_); +} + +} // namespace policy diff --git a/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h new file mode 100644 index 0000000..fa5a523 --- /dev/null +++ b/chrome/browser/chromeos/policy/user_cloud_external_data_manager.h @@ -0,0 +1,68 @@ +// 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_CHROMEOS_POLICY_USER_CLOUD_EXTERNAL_DATA_MANAGER_H_ +#define CHROME_BROWSER_CHROMEOS_POLICY_USER_CLOUD_EXTERNAL_DATA_MANAGER_H_ + +#include "base/basictypes.h" +#include "base/files/file_path.h" +#include "base/memory/ref_counted.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h" + +namespace base { +class SequencedTaskRunner; +} + +namespace policy { + +class CloudPolicyStore; +struct PolicyDefinitionList; +class ResourceCache; + +// Downloads, verifies, caches and retrieves external data referenced by +// policies. +// This is the implementation for regular users on Chrome OS. The code would +// work on desktop platforms as well but for now, is used on Chrome OS only +// 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 + // 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, + scoped_refptr<base::SequencedTaskRunner> backend_task_runner, + scoped_refptr<base::SequencedTaskRunner> io_task_runner, + const base::FilePath& cache_path, + CloudPolicyStore* policy_store); + virtual ~UserCloudExternalDataManager(); + + private: + // Cache used to store downloaded external data. The |resource_cache_| is + // owned by the manager but its destruction must be handled with care: + // * The manager owns a |backend_| which owns an |external_data_store_| which + // uses the |resource_cache_|. The |external_data_store_| must be destroyed + // before the |resource_cache_|. + // * After construction, |backend_|, |external_data_store_| and + // |resource_cache_| can only be accessed through the + // |backend_task_runner_|. + // + // It follows that in order to destroy |resource_cache_|, the manager must + // take the following steps: + // * Post a task to the |backend_task_runner_| that will tell the |backend_| + // to destroy the |external_data_store_|. + // * Post a task to the |backend_task_runner_| that will destroy the + // |resource_cache_|. + // Because of this destruction sequence, a scoped_ptr cannot be used. + ResourceCache* resource_cache_; + + DISALLOW_COPY_AND_ASSIGN(UserCloudExternalDataManager); +}; + +} // namespace policy + +#endif // CHROME_BROWSER_CHROMEOS_POLICY_USER_CLOUD_EXTERNAL_DATA_MANAGER_H_ diff --git a/chrome/browser/chromeos/policy/user_cloud_external_data_manager_browsertest.cc b/chrome/browser/chromeos/policy/user_cloud_external_data_manager_browsertest.cc new file mode 100644 index 0000000..1adae47 --- /dev/null +++ b/chrome/browser/chromeos/policy/user_cloud_external_data_manager_browsertest.cc @@ -0,0 +1,155 @@ +// 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 <string> + +#include "base/basictypes.h" +#include "base/bind.h" +#include "base/callback.h" +#include "base/file_util.h" +#include "base/files/file_path.h" +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "base/path_service.h" +#include "base/run_loop.h" +#include "base/sha1.h" +#include "base/strings/string_number_conversions.h" +#include "base/values.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base.h" +#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h" +#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" +#include "chrome/browser/policy/cloud/cloud_policy_core.h" +#include "chrome/browser/policy/cloud/cloud_policy_store.h" +#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_types.h" +#include "chrome/browser/policy/profile_policy_connector.h" +#include "chrome/browser/policy/profile_policy_connector_factory.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "content/public/test/test_utils.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "policy/policy_constants.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +namespace policy { + +namespace { + +const char kExternalDataPath[] = "policy/blank.html"; + +void ExternalDataFetchCallback(scoped_ptr<std::string>* destination, + const base::Closure& done_callback, + scoped_ptr<std::string> data) { + destination->reset(data.release()); + done_callback.Run(); +} + +} // namespace + +class UserCloudExternalDataManagerTest : public InProcessBrowserTest { + protected: + UserCloudExternalDataManagerTest(); + virtual ~UserCloudExternalDataManagerTest(); + + scoped_ptr<base::DictionaryValue> ConstructMetadata(const std::string& url, + const std::string& hash); + void SetExternalDataReference(const std::string& policy, + scoped_ptr<base::DictionaryValue> metadata); + + DISALLOW_COPY_AND_ASSIGN(UserCloudExternalDataManagerTest); +}; + +UserCloudExternalDataManagerTest::UserCloudExternalDataManagerTest() { +} + +UserCloudExternalDataManagerTest::~UserCloudExternalDataManagerTest() { +} + +scoped_ptr<base::DictionaryValue> + UserCloudExternalDataManagerTest::ConstructMetadata( + const std::string& url, + const std::string& hash) { + scoped_ptr<base::DictionaryValue> metadata(new base::DictionaryValue); + metadata->SetStringWithoutPathExpansion("url", url); + metadata->SetStringWithoutPathExpansion("hash", base::HexEncode(hash.c_str(), + hash.size())); + return metadata.Pass(); +} + +void UserCloudExternalDataManagerTest::SetExternalDataReference( + const std::string& policy, + scoped_ptr<base::DictionaryValue> metadata) { +#if defined(OS_CHROMEOS) + UserCloudPolicyManagerChromeOS* policy_manager = + UserCloudPolicyManagerFactoryChromeOS::GetForProfile( + browser()->profile()); +#else + UserCloudPolicyManager* policy_manager = + UserCloudPolicyManagerFactory::GetForProfile(browser()->profile()); +#endif + ASSERT_TRUE(policy_manager); + CloudPolicyStore* store = policy_manager->core()->store(); + ASSERT_TRUE(store); + PolicyMap policy_map; + policy_map.Set(policy, + POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + metadata.release(), + new ExternalDataFetcher(store->external_data_manager(), + policy)); + store->SetPolicyMapForTesting(policy_map); +} + +IN_PROC_BROWSER_TEST_F(UserCloudExternalDataManagerTest, FetchExternalData) { + CloudExternalDataManagerBase::SetMaxExternalDataSizeForTesting(1000); + + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + const GURL url = + embedded_test_server()->GetURL(std::string("/") + kExternalDataPath); + + base::FilePath test_dir; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir)); + std::string external_data; + ASSERT_TRUE(base::ReadFileToString(test_dir.AppendASCII(kExternalDataPath), + &external_data)); + ASSERT_FALSE(external_data.empty()); + + scoped_ptr<base::DictionaryValue> metadata = + ConstructMetadata(url.spec(), base::SHA1HashString(external_data)); + // TODO(bartfab): The test injects an ExternalDataFetcher for an arbitrary + // policy. This is only done because there are no policies that reference + // external data yet. Once the first such policy is added, switch the test to + // that policy and stop injecting a manually instantiated ExternalDataFetcher. + SetExternalDataReference(key::kHomepageLocation, + make_scoped_ptr(metadata->DeepCopy())); + content::RunAllPendingInMessageLoop(); + + ProfilePolicyConnector* policy_connector = + ProfilePolicyConnectorFactory::GetForProfile(browser()->profile()); + ASSERT_TRUE(policy_connector); + const PolicyMap& policies = policy_connector->policy_service()->GetPolicies( + PolicyNamespace(POLICY_DOMAIN_CHROME, std::string())); + const PolicyMap::Entry* policy_entry = policies.Get(key::kHomepageLocation); + ASSERT_TRUE(policy_entry); + EXPECT_TRUE(base::Value::Equals(metadata.get(), policy_entry->value)); + ASSERT_TRUE(policy_entry->external_data_fetcher); + + base::RunLoop run_loop; + scoped_ptr<std::string> fetched_external_data; + policy_entry->external_data_fetcher->Fetch(base::Bind( + &ExternalDataFetchCallback, + &fetched_external_data, + run_loop.QuitClosure())); + run_loop.Run(); + + ASSERT_TRUE(fetched_external_data); + EXPECT_EQ(external_data, *fetched_external_data); +} + +} // namespace policy diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc index 294ad25..c11889b 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc @@ -10,10 +10,12 @@ #include "base/message_loop/message_loop_proxy.h" #include "base/metrics/histogram.h" #include "base/metrics/sparse_histogram.h" +#include "base/sequenced_task_runner.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h" #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/cloud_policy_refresh_scheduler.h" #include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/policy/policy_bundle.h" @@ -50,6 +52,7 @@ const char kUMAInitialFetchOAuth2NetworkError[] = UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS( scoped_ptr<CloudPolicyStore> store, + scoped_ptr<CloudExternalDataManager> external_data_manager, const scoped_refptr<base::SequencedTaskRunner>& task_runner, scoped_ptr<ResourceCache> resource_cache, bool wait_for_policy_fetch, @@ -59,6 +62,7 @@ UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS( store.get(), task_runner), store_(store.Pass()), + external_data_manager_(external_data_manager.Pass()), wait_for_policy_fetch_(wait_for_policy_fetch), policy_fetch_timeout_(false, false) { time_init_started_ = base::Time::Now(); @@ -99,6 +103,8 @@ void UserCloudPolicyManagerChromeOS::Connect( core()->Connect(cloud_policy_client.Pass()); client()->AddObserver(this); + external_data_manager_->Connect(request_context); + if (component_policy_service_) component_policy_service_->Connect(client(), request_context); @@ -131,6 +137,7 @@ void UserCloudPolicyManagerChromeOS::Shutdown() { service()->RemoveObserver(this); token_fetcher_.reset(); component_policy_service_.reset(); + external_data_manager_->Disconnect(); CloudPolicyManager::Shutdown(); } diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h index ab73eef..80360e3 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h @@ -33,6 +33,7 @@ class URLRequestContextGetter; namespace policy { +class CloudExternalDataManager; class DeviceManagementService; class PolicyOAuth2TokenFetcher; class ResourceCache; @@ -51,6 +52,7 @@ class UserCloudPolicyManagerChromeOS // false as long as there hasn't been a successful policy fetch. UserCloudPolicyManagerChromeOS( scoped_ptr<CloudPolicyStore> store, + scoped_ptr<CloudExternalDataManager> external_data_manager, const scoped_refptr<base::SequencedTaskRunner>& task_runner, scoped_ptr<ResourceCache> resource_cache, bool wait_for_policy_fetch, @@ -124,6 +126,9 @@ class UserCloudPolicyManagerChromeOS // Owns the store, note that CloudPolicyManager just keeps a plain pointer. scoped_ptr<CloudPolicyStore> store_; + // Manages external data referenced by policies. + scoped_ptr<CloudExternalDataManager> external_data_manager_; + // Handles fetching and storing cloud policy for components. It uses the // |store_|, so destroy it first. scoped_ptr<ComponentCloudPolicyService> component_policy_service_; diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc index 477e3a1..f8be85b 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc @@ -4,24 +4,21 @@ #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h" -#include "base/basictypes.h" #include "base/bind.h" #include "base/bind_helpers.h" #include "base/callback.h" -#include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/testing_pref_service.h" #include "base/run_loop.h" +#include "base/sequenced_task_runner.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/test/test_simple_task_runner.h" -#include "base/time/time.h" #include "chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" -#include "chrome/browser/policy/cloud/cloud_policy_constants.h" -#include "chrome/browser/policy/cloud/cloud_policy_service.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" +#include "chrome/browser/policy/cloud/mock_cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/mock_cloud_policy_store.h" #include "chrome/browser/policy/cloud/mock_device_management_service.h" #include "chrome/browser/policy/cloud/resource_cache.h" @@ -81,6 +78,7 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { protected: UserCloudPolicyManagerChromeOSTest() : store_(NULL), + external_data_manager_(NULL), task_runner_(new base::TestSimpleTaskRunner()), profile_(NULL), signin_profile_(NULL) {} @@ -149,9 +147,12 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { void CreateManager(bool wait_for_fetch, int fetch_timeout) { store_ = new MockCloudPolicyStore(); + external_data_manager_ = new MockCloudExternalDataManager; + external_data_manager_->SetPolicyStore(store_); EXPECT_CALL(*store_, Load()); manager_.reset(new UserCloudPolicyManagerChromeOS( scoped_ptr<CloudPolicyStore>(store_), + scoped_ptr<CloudExternalDataManager>(external_data_manager_), task_runner_, scoped_ptr<ResourceCache>(), wait_for_fetch, @@ -291,7 +292,8 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { TestingPrefServiceSimple prefs_; MockConfigurationPolicyObserver observer_; MockDeviceManagementService device_management_service_; - MockCloudPolicyStore* store_; + MockCloudPolicyStore* store_; // Not owned. + MockCloudExternalDataManager* external_data_manager_; // Not owned. scoped_refptr<base::TestSimpleTaskRunner> task_runner_; scoped_ptr<UserCloudPolicyManagerChromeOS> manager_; scoped_ptr<UserCloudPolicyTokenForwarder> token_forwarder_; diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc index 9be111f..a208b9c 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 @@ -10,14 +10,18 @@ #include "base/memory/ref_counted.h" #include "base/message_loop/message_loop_proxy.h" #include "base/path_service.h" +#include "base/sequenced_task_runner.h" +#include "base/threading/sequenced_worker_pool.h" #include "base/time/time.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chromeos/login/user.h" #include "chrome/browser/chromeos/login/user_manager.h" +#include "chrome/browser/chromeos/policy/user_cloud_external_data_manager.h" #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h" #include "chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/policy/browser_policy_connector.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/device_management_service.h" #include "chrome/browser/policy/cloud/resource_cache.h" #include "chrome/browser/profiles/profile.h" @@ -28,6 +32,7 @@ #include "components/browser_context_keyed_service/browser_context_dependency_manager.h" #include "content/public/browser/browser_thread.h" #include "net/url_request/url_request_context_getter.h" +#include "policy/policy_constants.h" namespace policy { @@ -45,6 +50,10 @@ const base::FilePath::CharType kPolicy[] = FILE_PATH_LITERAL("Policy"); // Directory under kPolicy, in the user's profile dir, where external policy // resources are stored. const base::FilePath::CharType kResourceDir[] = FILE_PATH_LITERAL("Resources"); +// Directory in which to store external policy data. This is specified relative +// to kPolicy. +const base::FilePath::CharType kPolicyExternalDataDir[] = + FILE_PATH_LITERAL("External Data"); // Timeout in seconds after which to abandon the initial policy fetch and start // the session regardless. @@ -138,6 +147,8 @@ scoped_ptr<UserCloudPolicyManagerChromeOS> const base::FilePath token_cache_file = legacy_dir.Append(kToken); const base::FilePath resource_cache_dir = profile_dir.Append(kPolicy).Append(kResourceDir); + const base::FilePath external_data_dir = + profile_dir.Append(kPolicy).Append(kPolicyExternalDataDir); base::FilePath policy_key_dir; CHECK(PathService::Get(chromeos::DIR_USER_POLICY_KEYS, &policy_key_dir)); @@ -146,6 +157,19 @@ scoped_ptr<UserCloudPolicyManagerChromeOS> chromeos::DBusThreadManager::Get()->GetCryptohomeClient(), chromeos::DBusThreadManager::Get()->GetSessionManagerClient(), username, policy_key_dir, token_cache_file, policy_cache_file)); + + scoped_refptr<base::SequencedTaskRunner> backend_task_runner = + content::BrowserThread::GetBlockingPool()->GetSequencedTaskRunner( + content::BrowserThread::GetBlockingPool()->GetSequenceToken()); + scoped_refptr<base::SequencedTaskRunner> io_task_runner = + content::BrowserThread::GetMessageLoopProxyForThread( + content::BrowserThread::IO); + scoped_ptr<CloudExternalDataManager> external_data_manager( + new UserCloudExternalDataManager(GetChromePolicyDefinitionList(), + backend_task_runner, + io_task_runner, + external_data_dir, + store.get())); if (force_immediate_load) store->LoadImmediately(); @@ -160,6 +184,7 @@ scoped_ptr<UserCloudPolicyManagerChromeOS> scoped_ptr<UserCloudPolicyManagerChromeOS> manager( new UserCloudPolicyManagerChromeOS( store.PassAs<CloudPolicyStore>(), + external_data_manager.Pass(), base::MessageLoopProxy::current(), resource_cache.Pass(), wait_for_initial_policy, diff --git a/chrome/browser/policy/cloud/cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/cloud_policy_browsertest.cc index 61ca336..84ef567 100644 --- a/chrome/browser/policy/cloud/cloud_policy_browsertest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_browsertest.cc @@ -35,6 +35,7 @@ #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_source.h" #include "content/public/test/test_utils.h" +#include "net/url_request/url_request_context_getter.h" #include "policy/policy_constants.h" #include "policy/proto/chrome_settings.pb.h" #include "policy/proto/cloud_policy.pb.h" @@ -188,6 +189,7 @@ class CloudPolicyTest : public InProcessBrowserTest, UserCloudPolicyManagerFactory::GetForProfile(browser()->profile()); ASSERT_TRUE(policy_manager); policy_manager->Connect(g_browser_process->local_state(), + g_browser_process->system_request_context(), UserCloudPolicyManager::CreateCloudPolicyClient( connector->device_management_service()).Pass()); #endif // defined(OS_CHROMEOS) diff --git a/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc b/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc index e6e23d7d..8c4967c 100644 --- a/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc @@ -17,6 +17,7 @@ #include "chrome/common/chrome_switches.h" #include "chrome/test/base/in_process_browser_test.h" #include "net/base/net_errors.h" +#include "net/url_request/url_request_context_getter.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -73,6 +74,7 @@ class CloudPolicyManagerTest : public InProcessBrowserTest { ASSERT_TRUE(policy_manager()); policy_manager()->Connect( g_browser_process->local_state(), + g_browser_process->system_request_context(), UserCloudPolicyManager::CreateCloudPolicyClient( connector->device_management_service()).Pass()); #endif diff --git a/chrome/browser/policy/cloud/cloud_policy_store.cc b/chrome/browser/policy/cloud/cloud_policy_store.cc index 6ffed70..2c6dca7 100644 --- a/chrome/browser/policy/cloud/cloud_policy_store.cc +++ b/chrome/browser/policy/cloud/cloud_policy_store.cc @@ -17,7 +17,9 @@ CloudPolicyStore::CloudPolicyStore() invalidation_version_(0), is_initialized_(false) {} -CloudPolicyStore::~CloudPolicyStore() {} +CloudPolicyStore::~CloudPolicyStore() { + DCHECK(!external_data_manager_); +} void CloudPolicyStore::Store( const enterprise_management::PolicyFetchResponse& policy, @@ -58,4 +60,9 @@ void CloudPolicyStore::SetExternalDataManager( external_data_manager_->OnPolicyStoreLoaded(); } +void CloudPolicyStore::SetPolicyMapForTesting(const PolicyMap& policy_map) { + policy_map_.CopyFrom(policy_map); + NotifyStoreLoaded(); +} + } // namespace policy diff --git a/chrome/browser/policy/cloud/cloud_policy_store.h b/chrome/browser/policy/cloud/cloud_policy_store.h index 9e4899a..261cd8c 100644 --- a/chrome/browser/policy/cloud/cloud_policy_store.h +++ b/chrome/browser/policy/cloud/cloud_policy_store.h @@ -118,6 +118,14 @@ class CloudPolicyStore { void SetExternalDataManager( base::WeakPtr<CloudExternalDataManager> external_data_manager); + // Replaces |policy_map_| and calls the registered observers, simulating a + // successful load of |policy_map| from persistent storage. + // TODO(bartfab): This override is only needed because there are no policies + // that reference external data and therefore, no ExternalDataFetchers in the + // |policy_map_|. Once the first such policy is added, use that policy in + // tests and remove the override. + void SetPolicyMapForTesting(const PolicyMap& policy_map); + protected: // Invokes the corresponding callback on all registered observers. void NotifyStoreLoaded(); diff --git a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc index 8820638..be4e021 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc @@ -25,6 +25,7 @@ #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension.h" +#include "net/url_request/url_request_context_getter.h" #include "policy/proto/cloud_policy.pb.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -146,6 +147,7 @@ class ComponentCloudPolicyTest : public ExtensionBrowserTest { UserCloudPolicyManagerFactory::GetForProfile(browser()->profile()); ASSERT_TRUE(policy_manager); policy_manager->Connect(g_browser_process->local_state(), + g_browser_process->system_request_context(), UserCloudPolicyManager::CreateCloudPolicyClient( connector->device_management_service()).Pass()); #endif // defined(OS_CHROMEOS) diff --git a/chrome/browser/policy/cloud/mock_cloud_external_data_manager.cc b/chrome/browser/policy/cloud/mock_cloud_external_data_manager.cc new file mode 100644 index 0000000..7cea83f --- /dev/null +++ b/chrome/browser/policy/cloud/mock_cloud_external_data_manager.cc @@ -0,0 +1,18 @@ +// 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/cloud/mock_cloud_external_data_manager.h" + +#include "base/callback.h" +#include "net/url_request/url_request_context_getter.h" + +namespace policy { + +MockCloudExternalDataManager::MockCloudExternalDataManager() { +} + +MockCloudExternalDataManager::~MockCloudExternalDataManager() { +} + +} // namespace policy diff --git a/chrome/browser/policy/cloud/mock_cloud_external_data_manager.h b/chrome/browser/policy/cloud/mock_cloud_external_data_manager.h new file mode 100644 index 0000000..62bab52 --- /dev/null +++ b/chrome/browser/policy/cloud/mock_cloud_external_data_manager.h @@ -0,0 +1,39 @@ +// 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_CLOUD_MOCK_CLOUD_EXTERNAL_DATA_MANAGER_H_ +#define CHROME_BROWSER_POLICY_CLOUD_MOCK_CLOUD_EXTERNAL_DATA_MANAGER_H_ + +#include <string> + +#include "base/basictypes.h" +#include "base/memory/ref_counted.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" +#include "chrome/browser/policy/external_data_fetcher.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace net { +class URLRequestContextGetter; +} + +namespace policy { + +class MockCloudExternalDataManager : public CloudExternalDataManager { + public: + MockCloudExternalDataManager(); + virtual ~MockCloudExternalDataManager(); + + MOCK_METHOD0(OnPolicyStoreLoaded, void(void)); + MOCK_METHOD1(Connect, void(scoped_refptr<net::URLRequestContextGetter>)); + MOCK_METHOD0(Disconnect, void(void)); + MOCK_METHOD2(Fetch, void(const std::string&, + const ExternalDataFetcher::FetchCallback&)); + + private: + DISALLOW_COPY_AND_ASSIGN(MockCloudExternalDataManager); +}; + +} // namespace policy + +#endif // CHROME_BROWSER_POLICY_CLOUD_MOCK_CLOUD_EXTERNAL_DATA_MANAGER_H_ diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager.cc b/chrome/browser/policy/cloud/user_cloud_policy_manager.cc index c87f328..03a00da 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_manager.cc +++ b/chrome/browser/policy/cloud/user_cloud_policy_manager.cc @@ -6,12 +6,15 @@ #include "base/bind.h" #include "base/bind_helpers.h" +#include "base/sequenced_task_runner.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/cloud_policy_constants.h" #include "chrome/browser/policy/cloud/cloud_policy_service.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h" #include "chrome/browser/policy/cloud/user_cloud_policy_store.h" #include "chrome/browser/policy/policy_types.h" #include "chrome/common/pref_names.h" +#include "net/url_request/url_request_context_getter.h" namespace em = enterprise_management; @@ -20,13 +23,15 @@ namespace policy { UserCloudPolicyManager::UserCloudPolicyManager( Profile* profile, scoped_ptr<UserCloudPolicyStore> store, + scoped_ptr<CloudExternalDataManager> external_data_manager, const scoped_refptr<base::SequencedTaskRunner>& task_runner) : CloudPolicyManager( PolicyNamespaceKey(GetChromeUserPolicyType(), std::string()), store.get(), task_runner), profile_(profile), - store_(store.Pass()) { + store_(store.Pass()), + external_data_manager_(external_data_manager.Pass()) { UserCloudPolicyManagerFactory::GetInstance()->Register(profile_, this); } @@ -34,11 +39,22 @@ UserCloudPolicyManager::~UserCloudPolicyManager() { UserCloudPolicyManagerFactory::GetInstance()->Unregister(profile_, this); } +void UserCloudPolicyManager::Shutdown() { + if (external_data_manager_) + external_data_manager_->Disconnect(); + CloudPolicyManager::Shutdown(); + BrowserContextKeyedService::Shutdown(); +} + void UserCloudPolicyManager::Connect( - PrefService* local_state, scoped_ptr<CloudPolicyClient> client) { + PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, + scoped_ptr<CloudPolicyClient> client) { core()->Connect(client.Pass()); core()->StartRefreshScheduler(); core()->TrackRefreshDelayPref(local_state, prefs::kUserPolicyRefreshRate); + if (external_data_manager_) + external_data_manager_->Connect(request_context); } // static @@ -52,7 +68,12 @@ UserCloudPolicyManager::CreateCloudPolicyClient( } void UserCloudPolicyManager::DisconnectAndRemovePolicy() { + if (external_data_manager_) + external_data_manager_->Disconnect(); core()->Disconnect(); + // When the |store_| is cleared, it informs the |external_data_manager_| that + // all external data references have been removed, causing the + // |external_data_manager_| to clear its cache as well. store_->Clear(); } diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager.h b/chrome/browser/policy/cloud/user_cloud_policy_manager.h index 6ad2ecb..537163e 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_manager.h +++ b/chrome/browser/policy/cloud/user_cloud_policy_manager.h @@ -21,8 +21,13 @@ namespace base { class SequencedTaskRunner; } +namespace net { +class URLRequestContextGetter; +} + namespace policy { +class CloudExternalDataManager; class DeviceManagementService; class UserCloudPolicyStore; @@ -35,14 +40,19 @@ class UserCloudPolicyManager : public CloudPolicyManager, UserCloudPolicyManager( Profile* profile, scoped_ptr<UserCloudPolicyStore> store, + scoped_ptr<CloudExternalDataManager> external_data_manager, const scoped_refptr<base::SequencedTaskRunner>& task_runner); virtual ~UserCloudPolicyManager(); - // Initializes the cloud connection. |local_state| and - // |device_management_service| must stay valid until this object is deleted or - // DisconnectAndRemovePolicy() gets called. Virtual for mocking. - virtual void Connect(PrefService* local_state, - scoped_ptr<CloudPolicyClient> client); + virtual void Shutdown() OVERRIDE; + + // Initializes the cloud connection. |local_state| must stay valid until this + // object is deleted or DisconnectAndRemovePolicy() gets called. Virtual for + // mocking. + virtual void Connect( + PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, + scoped_ptr<CloudPolicyClient> client); // Shuts down the UserCloudPolicyManager (removes and stops refreshing the // cached cloud policy). This is typically called when a profile is being @@ -68,6 +78,9 @@ class UserCloudPolicyManager : public CloudPolicyManager, // CloudPolicyManager only keeps a plain CloudPolicyStore pointer. scoped_ptr<UserCloudPolicyStore> store_; + // Manages external data referenced by policies. + scoped_ptr<CloudExternalDataManager> external_data_manager_; + DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyManager); }; diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc b/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc index a0d721c..bad6ef8 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc +++ b/chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc @@ -7,6 +7,7 @@ #include "base/command_line.h" #include "base/logging.h" #include "base/message_loop/message_loop_proxy.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_store.h" #include "chrome/browser/profiles/profile.h" @@ -62,6 +63,7 @@ scoped_ptr<UserCloudPolicyManager> scoped_ptr<UserCloudPolicyManager> manager( new UserCloudPolicyManager(profile, store.Pass(), + scoped_ptr<CloudExternalDataManager>(), base::MessageLoopProxy::current())); manager->Init(); return manager.Pass(); @@ -73,10 +75,8 @@ void UserCloudPolicyManagerFactory::BrowserContextShutdown( if (profile->IsOffTheRecord()) return; UserCloudPolicyManager* manager = GetManagerForProfile(profile); - if (manager) { - manager->CloudPolicyManager::Shutdown(); - manager->BrowserContextKeyedService::Shutdown(); - } + if (manager) + manager->Shutdown(); } void UserCloudPolicyManagerFactory::SetEmptyTestingFactory( diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc b/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc index ac0a9ec..8c4490e 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc +++ b/chrome/browser/policy/cloud/user_cloud_policy_manager_unittest.cc @@ -4,13 +4,15 @@ #include "chrome/browser/policy/cloud/user_cloud_policy_manager.h" -#include "base/basictypes.h" #include "base/callback.h" -#include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" +#include "base/message_loop/message_loop_proxy.h" +#include "base/sequenced_task_runner.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/mock_user_cloud_policy_store.h" #include "chrome/browser/policy/external_data_fetcher.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "net/url_request/url_request_context_getter.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -26,8 +28,7 @@ namespace { class UserCloudPolicyManagerTest : public testing::Test { protected: - UserCloudPolicyManagerTest() - : store_(NULL) {} + UserCloudPolicyManagerTest() : store_(NULL) {} virtual void SetUp() OVERRIDE { // Set up a policy map for testing. @@ -40,18 +41,18 @@ class UserCloudPolicyManagerTest : public testing::Test { virtual void TearDown() OVERRIDE { if (manager_) { manager_->RemoveObserver(&observer_); - manager_->CloudPolicyManager::Shutdown(); - manager_->BrowserContextKeyedService::Shutdown(); + manager_->Shutdown(); } } void CreateManager() { store_ = new MockUserCloudPolicyStore(); EXPECT_CALL(*store_, Load()); - manager_.reset( - new UserCloudPolicyManager(NULL, - scoped_ptr<UserCloudPolicyStore>(store_), - loop_.message_loop_proxy())); + manager_.reset(new UserCloudPolicyManager( + NULL, + scoped_ptr<UserCloudPolicyStore>(store_), + scoped_ptr<CloudExternalDataManager>(), + loop_.message_loop_proxy())); manager_->Init(); manager_->AddObserver(&observer_); Mock::VerifyAndClearExpectations(store_); @@ -66,7 +67,7 @@ class UserCloudPolicyManagerTest : public testing::Test { // Policy infrastructure. MockConfigurationPolicyObserver observer_; - MockUserCloudPolicyStore* store_; + MockUserCloudPolicyStore* store_; // Not owned. scoped_ptr<UserCloudPolicyManager> manager_; private: diff --git a/chrome/browser/policy/cloud/user_cloud_policy_store_unittest.cc b/chrome/browser/policy/cloud/user_cloud_policy_store_unittest.cc index 38ec96a..e3f378f 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_store_unittest.cc +++ b/chrome/browser/policy/cloud/user_cloud_policy_store_unittest.cc @@ -9,6 +9,7 @@ #include "base/message_loop/message_loop.h" #include "base/prefs/pref_service.h" #include "base/run_loop.h" +#include "chrome/browser/policy/cloud/mock_cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/mock_cloud_policy_store.h" #include "chrome/browser/policy/cloud/policy_builder.h" #include "chrome/browser/signin/fake_signin_manager.h" @@ -17,13 +18,16 @@ #include "chrome/common/pref_names.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread.h" +#include "net/url_request/url_request_context_getter.h" #include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" using testing::AllOf; using testing::Eq; +using testing::Mock; using testing::Property; +using testing::Sequence; namespace policy { @@ -51,6 +55,8 @@ class UserCloudPolicyStoreTest : public testing::Test { PolicyBuilder::kFakeUsername); signin->Initialize(profile_.get(), NULL); store_.reset(new UserCloudPolicyStore(profile_.get(), policy_file())); + external_data_manager_.reset(new MockCloudExternalDataManager); + external_data_manager_->SetPolicyStore(store_.get()); store_->AddObserver(&observer_); policy_.payload().mutable_passwordmanagerenabled()->set_value(true); @@ -62,6 +68,7 @@ class UserCloudPolicyStoreTest : public testing::Test { virtual void TearDown() OVERRIDE { store_->RemoveObserver(&observer_); + external_data_manager_.reset(); store_.reset(); RunUntilIdle(); } @@ -91,6 +98,7 @@ class UserCloudPolicyStoreTest : public testing::Test { UserPolicyBuilder policy_; MockCloudPolicyStoreObserver observer_; scoped_ptr<UserCloudPolicyStore> store_; + scoped_ptr<MockCloudExternalDataManager> external_data_manager_; // CloudPolicyValidator() requires a FILE thread so declare one here. Both // |ui_thread_| and |file_thread_| share the same MessageLoop |loop_| so @@ -109,7 +117,9 @@ TEST_F(UserCloudPolicyStoreTest, LoadWithNoFile) { EXPECT_FALSE(store_->policy()); EXPECT_TRUE(store_->policy_map().empty()); - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s); store_->Load(); RunUntilIdle(); @@ -141,7 +151,9 @@ TEST_F(UserCloudPolicyStoreTest, LoadImmediatelyWithNoFile) { EXPECT_FALSE(store_->policy()); EXPECT_TRUE(store_->policy_map().empty()); - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s); store_->LoadImmediately(); // Should load without running the message loop. EXPECT_FALSE(store_->policy()); @@ -173,7 +185,9 @@ TEST_F(UserCloudPolicyStoreTest, Store) { // Store a simple policy and make sure it ends up as the currently active // policy. - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s); store_->Store(policy_.policy()); RunUntilIdle(); @@ -191,17 +205,24 @@ TEST_F(UserCloudPolicyStoreTest, StoreThenClear) { // Store a simple policy and make sure the file exists. // policy. - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s1; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s1); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s1); store_->Store(policy_.policy()); RunUntilIdle(); EXPECT_TRUE(store_->policy()); EXPECT_FALSE(store_->policy_map().empty()); + Mock::VerifyAndClearExpectations(external_data_manager_.get()); + Mock::VerifyAndClearExpectations(&observer_); + // Policy file should exist. ASSERT_TRUE(base::PathExists(policy_file())); - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s2; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s2); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s2); store_->Clear(); RunUntilIdle(); @@ -221,7 +242,9 @@ TEST_F(UserCloudPolicyStoreTest, StoreTwoTimes) { // Store a simple policy then store a second policy before the first one // finishes validating, and make sure the second policy ends up as the active // policy. - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).Times(2); + Sequence s1; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s1); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s1); UserPolicyBuilder first_policy; first_policy.payload().mutable_passwordmanagerenabled()->set_value(false); @@ -229,6 +252,13 @@ TEST_F(UserCloudPolicyStoreTest, StoreTwoTimes) { store_->Store(first_policy.policy()); RunUntilIdle(); + Mock::VerifyAndClearExpectations(external_data_manager_.get()); + Mock::VerifyAndClearExpectations(&observer_); + + Sequence s2; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s2); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s2); + store_->Store(policy_.policy()); RunUntilIdle(); @@ -243,7 +273,9 @@ TEST_F(UserCloudPolicyStoreTest, StoreTwoTimes) { TEST_F(UserCloudPolicyStoreTest, StoreThenLoad) { // Store a simple policy and make sure it can be read back in. // policy. - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s); store_->Store(policy_.policy()); RunUntilIdle(); @@ -266,7 +298,9 @@ TEST_F(UserCloudPolicyStoreTest, StoreThenLoad) { TEST_F(UserCloudPolicyStoreTest, StoreThenLoadImmediately) { // Store a simple policy and make sure it can be read back in. // policy. - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s); store_->Store(policy_.policy()); RunUntilIdle(); @@ -299,7 +333,9 @@ TEST_F(UserCloudPolicyStoreTest, StoreValidationError) { TEST_F(UserCloudPolicyStoreTest, LoadValidationError) { // Force a validation error by changing the username after policy is stored. - EXPECT_CALL(observer_, OnStoreLoaded(store_.get())); + Sequence s; + EXPECT_CALL(*external_data_manager_, OnPolicyStoreLoaded()).InSequence(s); + EXPECT_CALL(observer_, OnStoreLoaded(store_.get())).InSequence(s); store_->Store(policy_.policy()); RunUntilIdle(); diff --git a/chrome/browser/policy/cloud/user_policy_signin_service.cc b/chrome/browser/policy/cloud/user_policy_signin_service.cc index baef750..3ab2fba 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service.cc @@ -21,16 +21,19 @@ #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" #include "google_apis/gaia/gaia_constants.h" +#include "net/url_request/url_request_context_getter.h" namespace policy { UserPolicySigninService::UserPolicySigninService( Profile* profile, PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, DeviceManagementService* device_management_service, ProfileOAuth2TokenService* token_service) : UserPolicySigninServiceBase(profile, local_state, + request_context, device_management_service), oauth2_token_service_(token_service) { if (profile->GetPrefs()->GetBoolean(prefs::kDisableCloudPolicyOnSignin)) diff --git a/chrome/browser/policy/cloud/user_policy_signin_service.h b/chrome/browser/policy/cloud/user_policy_signin_service.h index fccb980..7def3e3 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service.h @@ -9,12 +9,17 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "chrome/browser/policy/cloud/user_policy_signin_service_base.h" #include "chrome/browser/signin/profile_oauth2_token_service.h" class Profile; +namespace net { +class URLRequestContextGetter; +} + namespace policy { class CloudPolicyClientRegistrationHelper; @@ -25,10 +30,12 @@ class UserPolicySigninService : public UserPolicySigninServiceBase, public OAuth2TokenService::Observer { public: // Creates a UserPolicySigninService associated with the passed |profile|. - UserPolicySigninService(Profile* profile, - PrefService* local_state, - DeviceManagementService* device_management_service, - ProfileOAuth2TokenService* oauth2_token_service); + UserPolicySigninService( + Profile* profile, + PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, + DeviceManagementService* device_management_service, + ProfileOAuth2TokenService* oauth2_token_service); virtual ~UserPolicySigninService(); // Registers a CloudPolicyClient for fetching policy for a user. The diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_android.cc b/chrome/browser/policy/cloud/user_policy_signin_service_android.cc index c630659..6029c8f 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_android.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_android.cc @@ -21,6 +21,7 @@ #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "net/base/network_change_notifier.h" +#include "net/url_request/url_request_context_getter.h" namespace policy { @@ -38,10 +39,12 @@ enterprise_management::DeviceRegisterRequest::Type GetRegistrationType() { UserPolicySigninService::UserPolicySigninService( Profile* profile, PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, DeviceManagementService* device_management_service, AndroidProfileOAuth2TokenService* token_service) : UserPolicySigninServiceBase(profile, local_state, + request_context, device_management_service), weak_factory_(this), oauth2_token_service_(token_service) {} diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_android.h b/chrome/browser/policy/cloud/user_policy_signin_service_android.h index e990580..4ae9716 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_android.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service_android.h @@ -9,6 +9,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/policy/cloud/user_policy_signin_service_base.h" @@ -16,6 +17,10 @@ class AndroidProfileOAuth2TokenService; class Profile; +namespace net { +class URLRequestContextGetter; +} + namespace policy { class CloudPolicyClientRegistrationHelper; @@ -24,10 +29,12 @@ class CloudPolicyClientRegistrationHelper; class UserPolicySigninService : public UserPolicySigninServiceBase { public: // Creates a UserPolicySigninService associated with the passed |profile|. - UserPolicySigninService(Profile* profile, - PrefService* local_state, - DeviceManagementService* device_management_service, - AndroidProfileOAuth2TokenService* token_service); + UserPolicySigninService( + Profile* profile, + PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, + DeviceManagementService* device_management_service, + AndroidProfileOAuth2TokenService* token_service); virtual ~UserPolicySigninService(); // Registers a CloudPolicyClient for fetching policy for |username|. diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_base.cc b/chrome/browser/policy/cloud/user_policy_signin_service_base.cc index 1baf659..324d5c1 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_base.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_base.cc @@ -19,15 +19,18 @@ #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "content/public/browser/notification_source.h" +#include "net/url_request/url_request_context_getter.h" namespace policy { UserPolicySigninServiceBase::UserPolicySigninServiceBase( Profile* profile, PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, DeviceManagementService* device_management_service) : profile_(profile), local_state_(local_state), + request_context_(request_context), device_management_service_(device_management_service), weak_factory_(this) { if (profile_->GetPrefs()->GetBoolean(prefs::kDisableCloudPolicyOnSignin)) @@ -222,7 +225,7 @@ void UserPolicySigninServiceBase::InitializeUserCloudPolicyManager( scoped_ptr<CloudPolicyClient> client) { UserCloudPolicyManager* manager = GetManager(); DCHECK(!manager->core()->client()); - manager->Connect(local_state_, client.Pass()); + manager->Connect(local_state_, request_context_, client.Pass()); DCHECK(manager->core()->service()); // Observe the client to detect errors fetching policy. diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_base.h b/chrome/browser/policy/cloud/user_policy_signin_service_base.h index a551c35..947ed38 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_base.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service_base.h @@ -10,6 +10,7 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/policy/cloud/cloud_policy_client.h" @@ -22,6 +23,10 @@ class PrefService; class Profile; class SigninManager; +namespace net { +class URLRequestContextGetter; +} + namespace policy { class DeviceManagementService; @@ -57,6 +62,7 @@ class UserPolicySigninServiceBase : public BrowserContextKeyedService, UserPolicySigninServiceBase( Profile* profile, PrefService* local_state, + scoped_refptr<net::URLRequestContextGetter> request_context, DeviceManagementService* device_management_service); virtual ~UserPolicySigninServiceBase(); @@ -139,6 +145,7 @@ class UserPolicySigninServiceBase : public BrowserContextKeyedService, content::NotificationRegistrar registrar_; PrefService* local_state_; + scoped_refptr<net::URLRequestContextGetter> request_context_; DeviceManagementService* device_management_service_; base::WeakPtrFactory<UserPolicySigninServiceBase> weak_factory_; diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc b/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc index fbeafd1..06b0150 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc @@ -4,6 +4,7 @@ #include "chrome/browser/policy/cloud/user_policy_signin_service_factory.h" +#include "base/memory/ref_counted.h" #include "base/prefs/pref_service.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/policy/browser_policy_connector.h" @@ -14,6 +15,7 @@ #include "chrome/common/pref_names.h" #include "components/browser_context_keyed_service/browser_context_dependency_manager.h" #include "components/user_prefs/pref_registry_syncable.h" +#include "net/url_request/url_request_context_getter.h" #if defined(OS_ANDROID) #include "chrome/browser/policy/cloud/user_policy_signin_service_android.h" @@ -73,6 +75,7 @@ UserPolicySigninServiceFactory::BuildServiceInstanceFor( return new UserPolicySigninService( profile, g_browser_process->local_state(), + g_browser_process->system_request_context(), device_management_service, ProfileOAuth2TokenServiceFactory::GetForProfile(profile)); } diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc b/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc index db4643b..8416b06 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop_proxy.h" #include "base/prefs/pref_service.h" @@ -12,6 +10,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/policy/browser_policy_connector.h" +#include "chrome/browser/policy/cloud/cloud_external_data_manager.h" #include "chrome/browser/policy/cloud/cloud_policy_constants.h" #include "chrome/browser/policy/cloud/mock_device_management_service.h" #include "chrome/browser/policy/cloud/mock_user_cloud_policy_store.h" @@ -146,7 +145,8 @@ class FakeAndroidProfileOAuth2TokenService class UserPolicySigninServiceTest : public testing::Test { public: UserPolicySigninServiceTest() - : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), + : mock_store_(NULL), + thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), register_completed_(false) {} MOCK_METHOD1(OnPolicyRefresh, void(bool)); @@ -205,6 +205,7 @@ class UserPolicySigninServiceTest : public testing::Test { manager_.reset(new UserCloudPolicyManager( profile_.get(), scoped_ptr<UserCloudPolicyStore>(mock_store_), + scoped_ptr<CloudExternalDataManager>(), base::MessageLoopProxy::current())); Mock::VerifyAndClearExpectations(mock_store_); @@ -352,9 +353,7 @@ class UserPolicySigninServiceTest : public testing::Test { } scoped_ptr<TestingProfile> profile_; - // Weak pointer to a MockUserCloudPolicyStore - lifetime is managed by the - // UserCloudPolicyManager. - MockUserCloudPolicyStore* mock_store_; + MockUserCloudPolicyStore* mock_store_; // Not owned. scoped_ptr<UserCloudPolicyManager> manager_; // BrowserPolicyConnector and UrlFetcherFactory want to initialize and free diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index cbd7009..ec87d2c 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1401,10 +1401,6 @@ 'browser/policy/browser_policy_connector.h', 'browser/policy/cloud/cloud_external_data_manager.cc', 'browser/policy/cloud/cloud_external_data_manager.h', - 'browser/policy/cloud/cloud_external_data_manager_base.cc', - 'browser/policy/cloud/cloud_external_data_manager_base.h', - 'browser/policy/cloud/cloud_external_data_store.cc', - 'browser/policy/cloud/cloud_external_data_store.h', 'browser/policy/cloud/cloud_policy_client.cc', 'browser/policy/cloud/cloud_policy_client.h', 'browser/policy/cloud/cloud_policy_client_registration_helper.cc', @@ -3106,10 +3102,6 @@ 'browser/policy/async_policy_loader.h', 'browser/policy/async_policy_provider.cc', 'browser/policy/async_policy_provider.h', - 'browser/policy/cloud/cloud_external_data_manager_base.cc', - 'browser/policy/cloud/cloud_external_data_manager_base.h', - 'browser/policy/cloud/cloud_external_data_store.cc', - 'browser/policy/cloud/cloud_external_data_store.h', 'browser/policy/cloud/component_cloud_policy_service.cc', 'browser/policy/cloud/component_cloud_policy_service.h', 'browser/policy/cloud/component_cloud_policy_store.cc', @@ -3186,7 +3178,6 @@ 'browser/parsers/metadata_parser_jpeg.cc', 'browser/parsers/metadata_parser_jpeg_factory.cc', 'browser/parsers/metadata_parser_manager.cc', - 'browser/policy/cloud/cloud_external_data_manager.cc', 'browser/policy/policy_load_status.cc', 'browser/policy/proto/cloud/chrome_extension_policy.proto', 'browser/process_singleton.cc', diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi index 3776b6c..9592392 100644 --- a/chrome/chrome_browser_chromeos.gypi +++ b/chrome/chrome_browser_chromeos.gypi @@ -639,6 +639,10 @@ 'browser/chromeos/policy/app_pack_updater.h', 'browser/chromeos/policy/auto_enrollment_client.cc', 'browser/chromeos/policy/auto_enrollment_client.h', + 'browser/chromeos/policy/cloud_external_data_manager_base.cc', + 'browser/chromeos/policy/cloud_external_data_manager_base.h', + 'browser/chromeos/policy/cloud_external_data_store.cc', + 'browser/chromeos/policy/cloud_external_data_store.h', 'browser/chromeos/policy/configuration_policy_handler_chromeos.cc', 'browser/chromeos/policy/configuration_policy_handler_chromeos.h', 'browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc', @@ -681,6 +685,8 @@ 'browser/chromeos/policy/recommendation_restorer.h', 'browser/chromeos/policy/recommendation_restorer_factory.cc', 'browser/chromeos/policy/recommendation_restorer_factory.h', + 'browser/chromeos/policy/user_cloud_external_data_manager.cc', + 'browser/chromeos/policy/user_cloud_external_data_manager.h', 'browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc', 'browser/chromeos/policy/user_cloud_policy_manager_chromeos.h', 'browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 813c64e..9b110fb 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1049,6 +1049,7 @@ 'browser/chromeos/policy/login_screen_default_policy_browsertest.cc', 'browser/chromeos/policy/policy_cert_verifier_browsertest.cc', 'browser/chromeos/policy/power_policy_browsertest.cc', + 'browser/chromeos/policy/user_cloud_external_data_manager_browsertest.cc', 'browser/chromeos/policy/variations_service_policy_browsertest.cc', 'browser/chromeos/power/peripheral_battery_observer_browsertest.cc', 'browser/chromeos/profiles/profile_helper_browsertest.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 9e4682c..aa7b463 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -146,6 +146,8 @@ 'browser/password_manager/password_form_data.h', 'browser/password_manager/test_password_store.cc', 'browser/password_manager/test_password_store.h', + 'browser/policy/cloud/mock_cloud_external_data_manager.cc', + 'browser/policy/cloud/mock_cloud_external_data_manager.h', 'browser/policy/cloud/mock_cloud_policy_client.cc', 'browser/policy/cloud/mock_cloud_policy_client.h', 'browser/policy/cloud/mock_cloud_policy_store.cc', @@ -689,6 +691,8 @@ 'browser/chromeos/offline/offline_load_page_unittest.cc', 'browser/chromeos/options/network_property_ui_data_unittest.cc', 'browser/chromeos/policy/auto_enrollment_client_unittest.cc', + 'browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc', + 'browser/chromeos/policy/cloud_external_data_store_unittest.cc', 'browser/chromeos/policy/configuration_policy_handler_chromeos_unittest.cc', 'browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc', 'browser/chromeos/policy/device_cloud_policy_store_chromeos_unittest.cc', @@ -1042,8 +1046,6 @@ 'browser/plugins/plugin_prefs_unittest.cc', 'browser/policy/async_policy_provider_unittest.cc', 'browser/policy/browser_policy_connector_unittest.cc', - 'browser/policy/cloud/cloud_external_data_manager_base_unittest.cc', - 'browser/policy/cloud/cloud_external_data_store_unittest.cc', 'browser/policy/cloud/cloud_policy_client_unittest.cc', 'browser/policy/cloud/cloud_policy_core_unittest.cc', 'browser/policy/cloud/cloud_policy_invalidator_unittest.cc', @@ -2484,8 +2486,6 @@ 'browser/storage_monitor/media_storage_util_unittest.cc', 'browser/net/gaia/gaia_oauth_fetcher_unittest.cc', 'browser/policy/async_policy_provider_unittest.cc', - 'browser/policy/cloud/cloud_external_data_manager_base_unittest.cc', - 'browser/policy/cloud/cloud_external_data_store_unittest.cc', 'browser/policy/cloud/component_cloud_policy_service_unittest.cc', 'browser/policy/cloud/component_cloud_policy_store_unittest.cc', 'browser/policy/cloud/component_cloud_policy_updater_unittest.cc', |