Removing FakePolicyWatcher and introducing FakeAsyncPolicyLoader.

remoting/host/policy_hack/policy_watcher_unittest.cc

Index: remoting/host/policy_hack/policy_watcher_unittest.cc
diff --git a/remoting/host/policy_hack/policy_watcher_unittest.cc b/remoting/host/policy_hack/policy_watcher_unittest.cc
index e4b855427a41b6bf88c10fb872c1c8f19d7b411d..5f05cf5dcb12d6246178aa5e72601273cb024078 100644
--- a/remoting/host/policy_hack/policy_watcher_unittest.cc
+++ b/remoting/host/policy_hack/policy_watcher_unittest.cc
@@ -7,10 +7,11 @@
 #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"
@@ -30,7 +31,14 @@ class PolicyWatcherTest : public testing::Test {
     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.
+    policy_loader_ = policy_loader.get();
+    policy_watcher_ = PolicyServiceWatcher::CreateFromPolicyLoader(
+        message_loop_proxy_, policy_loader.Pass());

[Łukasz Anforowicz, 2015/01/22 00:12:28]

I am not happy with handing over a scoped_ptr<FakeAsyncPolicyLoader> to product
code, while keeping a pointer to the same FakeAsyncPolicyLoader in the test
code.  OTOH, I don't see how to make it better without adding ref-counting.

[Mattias Nissler (ping if slow), 2015/01/22 16:05:08]

I think what you have is fair. If you really feel strong about it, I'd suggest
to change PolicyServiceWatcher to not own the policy loader, but document that
the passed pointer must stay valid for the life time of the watcher. It'd then
be up to the embedding code to manage life time.

[Łukasz Anforowicz, 2015/01/22 18:28:06]

Thanks.  I'll leave it as-is.  Moving the ownership of AsyncPolicyLoader would
result in a leaky abstraction/encapsulation for whoever needs to create a
PolicyServiceWatcher (right now it is also kind of leaky but only from
test-code-perspective - for product-code everything is nice looking).

+
     nat_true_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
     nat_false_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal,
                           false);
@@ -123,6 +131,19 @@ class PolicyWatcherTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
+  void SetPolicies(const base::DictionaryValue& dict) {
+    policy_loader_->ClearPolicies();
+    policy_loader_->AddPolicies(
+        policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string()),
+        dict, policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_MACHINE);
+    policy_loader_->PostReloadOnBackgroundThread(true /* force reload asap */);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void SignalTransientErrorForTest() {
+    policy_watcher_->SignalTransientPolicyError();
+  }
+
   MOCK_METHOD0(PostPolicyWatcherShutdown, void());
 
   static const char* kHostDomain;
@@ -132,7 +153,8 @@ class PolicyWatcherTest : public testing::Test {
   MockPolicyCallback mock_policy_callback_;
   PolicyWatcher::PolicyUpdatedCallback policy_updated_callback_;
   PolicyWatcher::PolicyErrorCallback policy_error_callback_;
-  scoped_ptr<FakePolicyWatcher> policy_watcher_;
+  policy::FakeAsyncPolicyLoader* policy_loader_;
+  scoped_ptr<PolicyWatcher> policy_watcher_;
   base::DictionaryValue empty_;
   base::DictionaryValue nat_true_;
   base::DictionaryValue nat_false_;
@@ -188,239 +210,354 @@ class PolicyWatcherTest : public testing::Test {
 const char* PolicyWatcherTest::kHostDomain = "google.com";
 const char* PolicyWatcherTest::kPortRange = "12400-12409";
 
-MATCHER_P(IsPolicies, dict, "") {
-  return arg->Equals(dict);
+using ::testing::Matcher;
+using ::testing::MatcherInterface;
+using ::testing::MatchResultListener;
+
+class DictionaryValueEqMatcher

[Łukasz Anforowicz, 2015/01/22 00:12:28]

I've added a quick-and-dirty DictionaryValueEqMatcher to have the test tell me
why the 2 dictionary are not equal (the old IsPolicies matcher didn't give any
details).  I hope this is ok:
1. It retains the old behavior (verified by the CHECK at the end of
MatchAndExplain method)
2. While far from perfect (i.e. silly descriptions returned from DescribeTo
method) I think it is worth preserving (especially since it is hidden inside
policy_watcher_unittest.cc / not exposed to rest of Chromium code).

[Mattias Nissler (ping if slow), 2015/01/22 16:05:08]

I'm not sure there's justification enough to add all this code just for
debugging. Perhaps it's sufficient to just call Equals() and just JSON-serialize
the two dictionaries and print them if they are different? That's probably good
enough to quickly spot the reason for failure.

[Łukasz Anforowicz, 2015/01/22 18:28:06]

Doh... yes - this is much easier.  Thanks.

+    : public MatcherInterface<const base::DictionaryValue*> {
+ public:
+  explicit DictionaryValueEqMatcher(const base::DictionaryValue& expected_dict)
+      : expected_dict_(expected_dict.DeepCopy()) {}
+
+  virtual bool MatchAndExplain(const base::DictionaryValue* actual_dict,
+                               MatchResultListener* listener) const {
+    if (actual_dict == nullptr) {
+      *listener << "actual dictionary is null";
+      return false;
+    }
+
+    for (base::DictionaryValue::Iterator i(*actual_dict); !i.IsAtEnd();
+         i.Advance()) {
+      if (!MatchAndExplainPair(i, *expected_dict_, "actual", "expected",
+                               listener)) {
+        return false;
+      }
+    }
+
+    for (base::DictionaryValue::Iterator i(*expected_dict_); !i.IsAtEnd();
+         i.Advance()) {
+      if (!MatchAndExplainPair(i, *actual_dict, "expected", "actual",
+                               listener)) {
+        return false;
+      }
+    }
+
+    // Double-check the code above correctly calculates dictionary equality.
+    CHECK(actual_dict->Equals(expected_dict_.get()));
+
+    return true;
+  }
+
+  virtual void DescribeTo(::std::ostream* os) const {
+    *os << "dictionary equality check";
+  }
+
+  virtual void DescribeNegationTo(::std::ostream* os) const {
+    *os << "dictionary inequality check";
+  }
+
+ private:
+  virtual bool MatchAndExplainPair(const base::DictionaryValue::Iterator& iter,
+                                   const base::DictionaryValue& other_dict,
+                                   const std::string& iter_name,
+                                   const std::string& other_name,
+                                   MatchResultListener* listener) const {
+    const std::string& key = iter.key();
+    if (!other_dict.HasKey(key)) {
+      *listener << "key '" << key << "' is present in " << iter_name
+                << ", but not in " << other_name << " dictionary.";
+      return false;
+    }
+
+    const base::Value* other_value;
+    CHECK(other_dict.Get(key, &other_value));
+
+    if (other_value->GetType() != iter.value().GetType()) {
+      *listener << "value of '" << key << "' key "
+                << "is a " << other_value->GetType() << " in " << other_name
+                << " but is a " << iter.value().GetType() << " in " << iter_name
+                << " dictionary.";
+      return false;
+    }
+
+    if (!other_value->Equals(&iter.value())) {
+      switch (iter.value().GetType()) {
+        case base::Value::Type::TYPE_BOOLEAN: {
+          bool other_bool;
+          bool iter_bool;
+          CHECK(iter.value().GetAsBoolean(&iter_bool));
+          CHECK(other_value->GetAsBoolean(&other_bool));
+          *listener << "value of '" << key << "' key "
+                    << "equals " << other_bool << " in " << other_name
+                    << " but equals " << iter_bool << " in " << iter_name
+                    << " dictionary";
+          break;
+        }
+        case base::Value::Type::TYPE_STRING: {
+          std::string other_string;
+          std::string iter_string;
+          CHECK(iter.value().GetAsString(&iter_string));
+          CHECK(other_value->GetAsString(&other_string));
+          *listener << "value of '" << key << "' key "
+                    << "equals '" << other_string << "' in " << other_name
+                    << " but equals '" << iter_string << "' in " << iter_name
+                    << " dictionary";
+          break;
+        }
+        default: {
+          *listener << "value of '" << key << "' key "
+                    << "is different in " << other_name << " and in "
+                    << iter_name << " dictionary";
+          break;
+        }
+      }
+      return false;
+    }
+
+    return true;
+  }
+
+  scoped_ptr<base::DictionaryValue> expected_dict_;
+};
+
+inline Matcher<const base::DictionaryValue*> DictionaryValueEq(
+    const base::DictionaryValue& expected_dict) {
+  return MakeMatcher(new DictionaryValueEqMatcher(expected_dict));
 }
 
 TEST_F(PolicyWatcherTest, None) {
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+  EXPECT_CALL(
+      mock_policy_callback_,
+      OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+  EXPECT_CALL(
+      mock_policy_callback_,
+      OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(
+                  DictionaryValueEq(nat_true_domain_empty_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&domain_full_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(domain_full_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&domain_empty_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(domain_empty_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_domain_full_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_,
-      OnPolicyUpdatePtr(IsPolicies(&nat_false_overridden_others_default_)));
+              OnPolicyUpdatePtr(
+                  DictionaryValueEq(nat_false_overridden_others_default_)));
 #else
   EXPECT_CALL(mock_policy_callback_,
-      OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&pairing_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(pairing_false_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&pairing_true_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&gnubby_auth_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(gnubby_auth_false_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&gnubby_auth_true_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&relay_false_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(relay_false_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&relay_true_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&port_range_full_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(port_range_full_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&port_range_empty_)));
+              OnPolicyUpdatePtr(DictionaryValueEq(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();
 }
 
@@ -430,7 +567,7 @@ TEST_F(PolicyWatcherTest, SingleTransientErrorDoesntTriggerErrorCallback) {
   EXPECT_CALL(mock_policy_callback_, OnPolicyErrorPtr()).Times(0);
 
   StartWatching();
-  policy_watcher_->SignalTransientErrorForTest();
+  SignalTransientErrorForTest();
   StopWatching();
 }
 
@@ -439,24 +576,23 @@ TEST_F(PolicyWatcherTest, MultipleTransientErrorsTriggerErrorCallback) {
 
   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();
 }
@@ -530,8 +666,5 @@ TEST_F(PolicyWatcherTest, TestRealChromotingPolicy) {
 
 #endif
 
-// TODO(lukasza): We should consider adding a test against a
-// MockConfigurationPolicyProvider.
-
 }  // namespace policy_hack
 }  // namespace remoting