Removing FakePolicyWatcher and introducing FakeAsyncPolicyLoader.

remoting/host/policy_hack/policy_watcher_unittest.cc

 // 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 "base/basictypes.h"
 #include "base/bind.h"
+#include "base/json/json_writer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
+#include "components/policy/core/common/fake_async_policy_loader.h"
 #include "policy/policy_constants.h"
 #include "remoting/host/dns_blackhole_checker.h"
-#include "remoting/host/policy_hack/fake_policy_watcher.h"
 #include "remoting/host/policy_hack/mock_policy_callback.h"
+#include "remoting/host/policy_hack/policy_service_watcher.h"
 #include "remoting/host/policy_hack/policy_watcher.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace remoting {
 namespace policy_hack {
 
 class PolicyWatcherTest : public testing::Test {
  public:
   PolicyWatcherTest() : message_loop_(base::MessageLoop::TYPE_IO) {}
 
   void SetUp() override {
     message_loop_proxy_ = base::MessageLoopProxy::current();
     policy_updated_callback_ = base::Bind(
         &MockPolicyCallback::OnPolicyUpdate,
         base::Unretained(&mock_policy_callback_));
     policy_error_callback_ = base::Bind(
         &MockPolicyCallback::OnPolicyError,
         base::Unretained(&mock_policy_callback_));
-    policy_watcher_.reset(new FakePolicyWatcher(message_loop_proxy_));
+
+    scoped_ptr<policy::FakeAsyncPolicyLoader> policy_loader(
+        new policy::FakeAsyncPolicyLoader(message_loop_proxy_));
+    // retaining a pointer to keep control over policy contents.

[Sergey Ulanov, 2015/01/23 18:29:11]

s/retaining/Retaining/

[Łukasz Anforowicz, 2015/01/23 19:24:45]

Done.

+    policy_loader_ = policy_loader.get();

[Sergey Ulanov, 2015/01/23 18:29:11]

nit: this code would be shorter without policy_loader variable:

policy_loader_ = new policy::FakeAsyncPolicyLoader(message_loop_proxy_);
policy_watcher_ = PolicyServiceWatcher::CreateFromPolicyLoader(
    message_loop_proxy_, make_scoped_ptr(policy_loader));

[Łukasz Anforowicz, 2015/01/23 19:24:45]

Good point.  I'll make the change.

+    policy_watcher_ = PolicyServiceWatcher::CreateFromPolicyLoader(
+        message_loop_proxy_, policy_loader.Pass());
+
     nat_true_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
     nat_false_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal,
                           false);
     nat_one_.SetInteger(policy::key::kRemoteAccessHostFirewallTraversal, 1);
     domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
                             std::string());
     domain_full_.SetString(policy::key::kRemoteAccessHostDomain, kHostDomain);
     SetDefaults(nat_true_others_default_);
     nat_true_others_default_.SetBoolean(
         policy::key::kRemoteAccessHostFirewallTraversal, true);
@@ skipping 72 matching lines @@
     base::RunLoop().RunUntilIdle();
   }
 
   void StopWatching() {
     EXPECT_CALL(*this, PostPolicyWatcherShutdown()).Times(1);
     policy_watcher_->StopWatching(base::Bind(
         &PolicyWatcherTest::PostPolicyWatcherShutdown, base::Unretained(this)));
     base::RunLoop().RunUntilIdle();
   }
 
+  void SetPolicies(const base::DictionaryValue& dict) {
+    // Copy |dict| into |policy_bundle|.
+    policy::PolicyNamespace policy_namespace =
+        policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string());
+    policy::PolicyBundle policy_bundle;
+    policy::PolicyMap& policy_map = policy_bundle.Get(policy_namespace);
+    policy_map.LoadFrom(&dict, policy::POLICY_LEVEL_MANDATORY,
+                        policy::POLICY_SCOPE_MACHINE);
+
+    // Simulate a policy file/registry/preference update.
+    policy_loader_->SetPolicies(policy_bundle);
+    policy_loader_->PostReloadOnBackgroundThread(true /* force reload asap */);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void SignalTransientErrorForTest() {
+    policy_watcher_->SignalTransientPolicyError();
+  }
+
   MOCK_METHOD0(PostPolicyWatcherShutdown, void());
 
   static const char* kHostDomain;
   static const char* kPortRange;
   base::MessageLoop message_loop_;
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
   MockPolicyCallback mock_policy_callback_;
   PolicyWatcher::PolicyUpdatedCallback policy_updated_callback_;
   PolicyWatcher::PolicyErrorCallback policy_error_callback_;
-  scoped_ptr<FakePolicyWatcher> policy_watcher_;
+  policy::FakeAsyncPolicyLoader* policy_loader_;

[Sergey Ulanov, 2015/01/23 18:29:11]

add a comment that this is owned by the |policy_watcher_|

+  scoped_ptr<PolicyWatcher> policy_watcher_;
   base::DictionaryValue empty_;
   base::DictionaryValue nat_true_;
   base::DictionaryValue nat_false_;
   base::DictionaryValue nat_one_;
   base::DictionaryValue domain_empty_;
   base::DictionaryValue domain_full_;
   base::DictionaryValue nat_true_others_default_;
   base::DictionaryValue nat_false_others_default_;
   base::DictionaryValue domain_empty_others_default_;
   base::DictionaryValue domain_full_others_default_;
@@ skipping 36 matching lines @@
 #if !defined(NDEBUG)
     dict.SetString(policy::key::kRemoteAccessHostDebugOverridePolicies, "");
 #endif
   }
 };
 
 const char* PolicyWatcherTest::kHostDomain = "google.com";
 const char* PolicyWatcherTest::kPortRange = "12400-12409";
 
 MATCHER_P(IsPolicies, dict, "") {
-  return arg->Equals(dict);
+  bool equal = arg->Equals(dict);
+  if (!equal) {
+    std::string actual_value;
+    base::JSONWriter::WriteWithOptions(
+        arg, base::JSONWriter::OPTIONS_PRETTY_PRINT, &actual_value);
+
+    std::string expected_value;
+    base::JSONWriter::WriteWithOptions(
+        dict, base::JSONWriter::OPTIONS_PRETTY_PRINT, &expected_value);
+
+    *result_listener << "Policies are not equal. ";
+    *result_listener << "Expected policy: " << expected_value << ". ";
+    *result_listener << "Actual policy: " << actual_value << ".";
+  }
+  return equal;
 }
 
 TEST_F(PolicyWatcherTest, None) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatTrue) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 
+  SetPolicies(nat_true_);
   StartWatching();
-  policy_watcher_->SetPolicies(&nat_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatFalse) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_false_others_default_)));
 
+  SetPolicies(nat_false_);
   StartWatching();
-  policy_watcher_->SetPolicies(&nat_false_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatOne) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_false_others_default_)));
 
+  SetPolicies(nat_one_);
   StartWatching();
-  policy_watcher_->SetPolicies(&nat_one_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, DomainEmpty) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&domain_empty_others_default_)));
 
+  SetPolicies(domain_empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&domain_empty_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, DomainFull) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&domain_full_others_default_)));
 
+  SetPolicies(domain_full_);
   StartWatching();
-  policy_watcher_->SetPolicies(&domain_full_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenTrue) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_true_);
+  SetPolicies(nat_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrue) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_true_);
-  policy_watcher_->SetPolicies(&nat_true_);
+  SetPolicies(nat_true_);
+  SetPolicies(nat_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrueThenFalse) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_true_);
-  policy_watcher_->SetPolicies(&nat_true_);
-  policy_watcher_->SetPolicies(&nat_false_);
+  SetPolicies(nat_true_);
+  SetPolicies(nat_true_);
+  SetPolicies(nat_false_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenFalse) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_false_);
+  SetPolicies(nat_false_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenFalseThenTrue) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_false_);
-  policy_watcher_->SetPolicies(&nat_true_);
+  SetPolicies(nat_false_);
+  SetPolicies(nat_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, ChangeOneRepeatedlyThenTwo) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(
                   &nat_true_domain_empty_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&domain_full_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&domain_empty_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_domain_full_)));
 
+  SetPolicies(nat_true_domain_empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&nat_true_domain_empty_);
-  policy_watcher_->SetPolicies(&nat_true_domain_full_);
-  policy_watcher_->SetPolicies(&nat_false_domain_full_);
-  policy_watcher_->SetPolicies(&nat_false_domain_empty_);
-  policy_watcher_->SetPolicies(&nat_true_domain_full_);
+  SetPolicies(nat_true_domain_full_);
+  SetPolicies(nat_false_domain_full_);
+  SetPolicies(nat_false_domain_empty_);
+  SetPolicies(nat_true_domain_full_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, FilterUnknownPolicies) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&unknown_policies_);
-  policy_watcher_->SetPolicies(&empty_);
+  SetPolicies(unknown_policies_);
+  SetPolicies(empty_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, DebugOverrideNatPolicy) {
 #if !defined(NDEBUG)
-  EXPECT_CALL(mock_policy_callback_,
+  EXPECT_CALL(
+      mock_policy_callback_,
       OnPolicyUpdatePtr(IsPolicies(&nat_false_overridden_others_default_)));
 #else
   EXPECT_CALL(mock_policy_callback_,
-      OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
+              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 #endif
 
+  SetPolicies(nat_true_and_overridden_);
   StartWatching();
-  policy_watcher_->SetPolicies(&nat_true_and_overridden_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, PairingFalseThenTrue) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&pairing_false_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&pairing_true_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&pairing_false_);
-  policy_watcher_->SetPolicies(&pairing_true_);
+  SetPolicies(pairing_false_);
+  SetPolicies(pairing_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, GnubbyAuth) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&gnubby_auth_false_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&gnubby_auth_true_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&gnubby_auth_false_);
-  policy_watcher_->SetPolicies(&gnubby_auth_true_);
+  SetPolicies(gnubby_auth_false_);
+  SetPolicies(gnubby_auth_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, Relay) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&relay_false_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&relay_true_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&relay_false_);
-  policy_watcher_->SetPolicies(&relay_true_);
+  SetPolicies(relay_false_);
+  SetPolicies(relay_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, UdpPortRange) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&port_range_full_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&port_range_empty_)));
 
+  SetPolicies(empty_);
   StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&port_range_full_);
-  policy_watcher_->SetPolicies(&port_range_empty_);
+  SetPolicies(port_range_full_);
+  SetPolicies(port_range_empty_);
   StopWatching();
 }
 
 const int kMaxTransientErrorRetries = 5;
 
 TEST_F(PolicyWatcherTest, SingleTransientErrorDoesntTriggerErrorCallback) {
   EXPECT_CALL(mock_policy_callback_, OnPolicyErrorPtr()).Times(0);
 
   StartWatching();
-  policy_watcher_->SignalTransientErrorForTest();
+  SignalTransientErrorForTest();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, MultipleTransientErrorsTriggerErrorCallback) {
   EXPECT_CALL(mock_policy_callback_, OnPolicyErrorPtr());
 
   StartWatching();
   for (int i = 0; i < kMaxTransientErrorRetries; i++) {
-    policy_watcher_->SignalTransientErrorForTest();
+    SignalTransientErrorForTest();
   }
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, PolicyUpdateResetsTransientErrorsCounter) {
   testing::InSequence s;
-  EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_, OnPolicyUpdatePtr(testing::_));
   EXPECT_CALL(mock_policy_callback_, OnPolicyErrorPtr()).Times(0);
 
   StartWatching();
   for (int i = 0; i < (kMaxTransientErrorRetries - 1); i++) {
-    policy_watcher_->SignalTransientErrorForTest();
+    SignalTransientErrorForTest();
   }
-  policy_watcher_->SetPolicies(&nat_true_);
+  SetPolicies(nat_true_);
   for (int i = 0; i < (kMaxTransientErrorRetries - 1); i++) {
-    policy_watcher_->SignalTransientErrorForTest();
+    SignalTransientErrorForTest();
   }
   StopWatching();
 }
 
 // Unit tests cannot instantiate PolicyWatcher on ChromeOS
 // (as this requires running inside a browser process).
 #ifndef OS_CHROMEOS
 
 namespace {
 
@@ skipping 53 matching lines @@
     run_loop.RunUntilIdle();
   }
 
   // Today, the only verification offered by this test is:
   // - Manual verification of policy values dumped by OnPolicyUpdatedDumpPolicy
   // - Automated verification that nothing crashed
 }
 
 #endif
 
-// TODO(lukasza): We should consider adding a test against a
-// MockConfigurationPolicyProvider.
-
 }  // namespace policy_hack
 }  // namespace remoting