Add the AsyncPolicyProvider and AsyncPolicyLoader

chrome/browser/policy/async_policy_provider_unittest.cc

Index: chrome/browser/policy/async_policy_provider_unittest.cc
diff --git a/chrome/browser/policy/async_policy_provider_unittest.cc b/chrome/browser/policy/async_policy_provider_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..cf1bd11f311ea396e8ae76518ea4992e9e83871f
--- /dev/null
+++ b/chrome/browser/policy/async_policy_provider_unittest.cc
@@ -0,0 +1,311 @@
+// Copyright (c) 2012 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/async_policy_provider.h"
+
+#include "base/file_util.h"
+#include "base/message_loop.h"
+#include "base/scoped_temp_dir.h"
+#include "base/values.h"
+#include "chrome/browser/policy/async_policy_loader.h"
+#include "chrome/browser/policy/configuration_policy_provider.h"

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

already included in async_policy_provider.h and no explicit other usage here
AFAICS

[Joao da Silva, 2012/06/04 14:05:06]

Done.

+#include "chrome/browser/policy/mock_configuration_policy_provider.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/test/test_browser_thread.h"
+#include "policy/policy_constants.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::InvokeWithoutArgs;
+using testing::Mock;
+using testing::Return;
+using testing::ReturnNull;
+using testing::Sequence;
+
+namespace policy {
+
+namespace {
+
+class MockPolicyLoader : public AsyncPolicyLoader {
+ public:
+  MockPolicyLoader();
+  virtual ~MockPolicyLoader();
+
+  // Load() returns a scoped_ptr<PolicyBundle> but it can't be mocked because
+  // scoped_ptr is moveable but not copyable. This override forwards the
+  // call to MockLoad() which returns a PolicyBundle*, and returns a copy
+  // wrapped in a passed scoped_ptr.
+  virtual scoped_ptr<PolicyBundle> Load() OVERRIDE;
+
+  MOCK_METHOD0(MockLoad, const PolicyBundle*());
+  MOCK_METHOD0(InitOnFile, void());
+  MOCK_METHOD0(LastModificationTime, base::Time());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockPolicyLoader);
+};
+
+MockPolicyLoader::MockPolicyLoader() {}
+
+MockPolicyLoader::~MockPolicyLoader() {}
+
+scoped_ptr<PolicyBundle> MockPolicyLoader::Load() {
+  scoped_ptr<PolicyBundle> bundle;
+  const PolicyBundle* loaded = MockLoad();
+  if (loaded) {
+    bundle.reset(new PolicyBundle());
+    bundle->CopyFrom(*loaded);
+  }
+  return bundle.Pass();
+}
+
+}  // namespace
+
+class AsyncPolicyProviderTest : public testing::Test {
+ public:

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

you can use protected here if you like to.

[Joao da Silva, 2012/06/04 14:05:06]

Done.

+  AsyncPolicyProviderTest();
+  virtual ~AsyncPolicyProviderTest();
+
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+ protected:
+  PolicyBundle initial_bundle_;
+  MockPolicyLoader* loader_;
+  scoped_ptr<AsyncPolicyProvider> provider_;
+
+  // IO loop needed by the FilePathWatcher.
+  MessageLoopForIO loop_;
+
+ private:
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(AsyncPolicyProviderTest);
+};
+
+AsyncPolicyProviderTest::AsyncPolicyProviderTest()
+    : ui_thread_(content::BrowserThread::UI, &loop_),
+      file_thread_(content::BrowserThread::FILE, &loop_) {}
+
+AsyncPolicyProviderTest::~AsyncPolicyProviderTest() {}
+
+void AsyncPolicyProviderTest::SetUp() {
+  initial_bundle_.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateStringValue("initial"));
+  loader_ = new MockPolicyLoader();
+  EXPECT_CALL(*loader_, LastModificationTime())
+      .WillRepeatedly(Return(base::Time()));
+  EXPECT_CALL(*loader_, InitOnFile()).Times(1);
+  EXPECT_CALL(*loader_, MockLoad()).WillOnce(Return(&initial_bundle_));
+
+  provider_.reset(
+      new AsyncPolicyProvider(GetChromePolicyDefinitionList(), loader_));
+  // Verify that the initial load is done synchronously:
+  EXPECT_TRUE(provider_->policies().Equals(initial_bundle_));
+
+  loop_.RunAllPending();
+  Mock::VerifyAndClearExpectations(loader_);
+
+  EXPECT_CALL(*loader_, LastModificationTime())
+      .WillRepeatedly(Return(base::Time()));

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

you declared this expectation in line 99 already.

[Joao da Silva, 2012/06/04 14:05:06]

It is cleared in line 110.

+}
+
+void AsyncPolicyProviderTest::TearDown() {
+  // Tests that set file watchers may receive notifications during TearDown,
+  // and the |loop_| might have a Reload task already pending (before deletion
+  // of the |provider_| triggers deletion of the |loader_|. Make sure the
+  // loader isn't loading locals at this stage.
+  if (provider_.get())
+    EXPECT_CALL(*loader_, MockLoad()).WillRepeatedly(ReturnNull());
+
+  provider_.reset();
+  loop_.RunAllPending();
+}
+
+TEST_F(AsyncPolicyProviderTest, RefreshPolicies) {
+  PolicyBundle refreshed_bundle;
+  refreshed_bundle.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateStringValue("refreshed"));
+  EXPECT_CALL(*loader_, MockLoad()).WillOnce(Return(&refreshed_bundle));
+
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get(), &observer);
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(1);
+  provider_->RefreshPolicies();
+  loop_.RunAllPending();
+  // The refreshed policies are now provided.
+  EXPECT_TRUE(provider_->policies().Equals(refreshed_bundle));
+}
+
+TEST_F(AsyncPolicyProviderTest, RefreshPoliciesTwice) {
+  PolicyBundle refreshed_bundle;
+  refreshed_bundle.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateBooleanValue(true));
+  EXPECT_CALL(*loader_, MockLoad()).WillRepeatedly(Return(&refreshed_bundle));
+
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get(), &observer);
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  provider_->RefreshPolicies();
+  // Doesn't refresh before going through FILE.

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

thread

[Joao da Silva, 2012/06/04 14:05:06]

Done.

+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Doesn't refresh if another RefreshPolicies request is made.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  provider_->RefreshPolicies();
+  Mock::VerifyAndClearExpectations(&observer);
+
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(1);
+  loop_.RunAllPending();
+  // The refreshed policies are now provided.
+  EXPECT_TRUE(provider_->policies().Equals(refreshed_bundle));
+  Mock::VerifyAndClearExpectations(&observer);
+}
+
+TEST_F(AsyncPolicyProviderTest, RefreshPoliciesDuringReload) {
+  PolicyBundle reloaded_bundle;
+  reloaded_bundle.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateStringValue("reloaded"));
+  PolicyBundle refreshed_bundle;
+  refreshed_bundle.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateStringValue("refreshed"));
+
+  Sequence load_sequence;
+  // Reload.
+  EXPECT_CALL(*loader_, MockLoad()).InSequence(load_sequence)
+                                   .WillRepeatedly(Return(&reloaded_bundle));
+  // RefreshPolicies.
+  EXPECT_CALL(*loader_, MockLoad()).InSequence(load_sequence)
+                                   .WillRepeatedly(Return(&refreshed_bundle));

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

Configuring both of these as repeatedly seems ambiguous to me. Do you actually
mean WillOnce?

[Joao da Silva, 2012/06/04 14:05:06]

Yes, done.

+
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get(), &observer);
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+
+  // A Reload is triggered before RefreshPolicies, and it shouldn't trigger
+  // notifications.
+  loader_->Reload(true);
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Doesn't refresh before going through FILE.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  provider_->RefreshPolicies();
+  Mock::VerifyAndClearExpectations(&observer);
+
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(1);
+  loop_.RunAllPending();
+  // The refreshed policies are now provided, and the |reloaded_bundle| was
+  // dropped.
+  EXPECT_TRUE(provider_->policies().Equals(refreshed_bundle));
+  Mock::VerifyAndClearExpectations(&observer);
+}
+
+TEST_F(AsyncPolicyProviderTest, FileUpdated) {
+  PolicyBundle updated_bundle;
+  updated_bundle.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateStringValue("updated"));

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

It seems like a helper to initialize a policy bundle would be quite useful in a
number of place in the file.

[Joao da Silva, 2012/06/04 14:05:06]

Done.

+  EXPECT_CALL(*loader_, MockLoad()).WillRepeatedly(Return(&updated_bundle));
+
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get(), &observer);
+
+  // Watch a temporary file.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  ScopedTempDir tmp_dir;
+  ASSERT_TRUE(tmp_dir.CreateUniqueTempDir());
+  FilePath path(tmp_dir.path().AppendASCII("policy.json"));
+  loader_->WatchPath(path);
+  loop_.RunAllPending();
+  Mock::VerifyAndClearExpectations(&observer);
+  EXPECT_TRUE(provider_->policies().Equals(initial_bundle_));
+
+  // Update the file and trigger an update.
+  // OnUpdatePolicy() is expected multiple times instead of once because the
+  // FilePathWatcher generates more than one notification on some platforms.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get()))
+      .WillRepeatedly(InvokeWithoutArgs(&loop_, &MessageLoop::Quit));

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

Doing actual file watches in tests is very much discouraged, as the various
platform-level watcher implementations have a long history of flaking. Thus, if
you don't want people complaining about your test being flaky, you need to mock
out the watcher here.

[Joao da Silva, 2012/06/04 14:05:06]

Removed this tests, after the offline discussion. WatchPath is not part of the
AsyncPolicyLoader anymore either.

+  std::string content("{ \"ShowHomeButton\": true }");
+  ASSERT_EQ((int) content.size(),

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

use C++-style casts (also below).

[Joao da Silva, 2012/06/04 14:05:06]

Obsolete

+            file_util::WriteFile(path, content.c_str(), content.size()));
+  loop_.Run();
+  EXPECT_TRUE(provider_->policies().Equals(updated_bundle));
+}
+
+TEST_F(AsyncPolicyProviderTest, DirectoryUpdated) {
+  PolicyBundle updated_bundle;
+  updated_bundle.Get(POLICY_DOMAIN_CHROME, "")
+      .Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+           base::Value::CreateStringValue("updated"));
+  EXPECT_CALL(*loader_, MockLoad()).WillRepeatedly(Return(&updated_bundle));
+
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get(), &observer);
+
+  // Watch a temporary directory.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  ScopedTempDir tmp_dir;
+  ASSERT_TRUE(tmp_dir.CreateUniqueTempDir());
+  FilePath path(tmp_dir.path().AppendASCII("policy.json"));
+  // Initial content.
+  std::string content("{ \"ShowHomeButton\": false }");
+  ASSERT_EQ((int) content.size(),
+            file_util::WriteFile(path, content.c_str(), content.size()));
+  // Watch the directory.
+  loader_->WatchPath(tmp_dir.path());
+  loop_.RunAllPending();
+  Mock::VerifyAndClearExpectations(&observer);
+  EXPECT_TRUE(provider_->policies().Equals(initial_bundle_));
+
+  // Update the file and trigger an update.
+  // OnUpdatePolicy() is expected multiple times instead of once because the
+  // FilePathWatcher generates more than one notification on some platforms.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get()))
+      .WillRepeatedly(InvokeWithoutArgs(&loop_, &MessageLoop::Quit));
+  content = "{ \"ShowHomeButton\": true }";
+#if defined(OS_MACOSX)
+  // FilePathWatcher on the mac only notices directory changes, and not file

[Mattias Nissler (ping if slow), 2012/06/04 08:48:26]

s/mac/Mac/ according to our more picky Mac fanboys friends :)

[Joao da Silva, 2012/06/04 14:05:06]

iObsolete

+  // changes; create a new file on that platform instead.
+  // See the documentation of FilePathWatcher for the details.
+  path = FilePath(tmp_dir.path().AppendASCII("policy2.json"));
+#endif
+  ASSERT_EQ((int) content.size(),
+            file_util::WriteFile(path, content.c_str(), content.size()));
+
+  loop_.Run();
+  EXPECT_TRUE(provider_->policies().Equals(updated_bundle));
+}
+
+TEST_F(AsyncPolicyProviderTest, Shutdown) {
+  EXPECT_CALL(*loader_, MockLoad()).WillRepeatedly(Return(&initial_bundle_));
+
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get(), &observer);
+
+  // Though there is a pending Reload, the provider and the loader can be
+  // deleted at any time.
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  loader_->Reload(true);
+  Mock::VerifyAndClearExpectations(&observer);
+
+  EXPECT_CALL(observer, OnUpdatePolicy(provider_.get())).Times(0);
+  EXPECT_CALL(observer, OnProviderGoingAway(provider_.get()));
+  provider_.reset();
+  loop_.RunAllPending();
+  Mock::VerifyAndClearExpectations(&observer);
+}
+
+}  // namespace policy