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/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.
+    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);
     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) {
+    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;
   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_;
+  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 35 matching lines @@
     dict.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth, true);
 #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);
+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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+
+  SetPolicies(empty_);
+  StartWatching();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatTrue) {
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&nat_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+
+  SetPolicies(nat_true_);
+  StartWatching();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatFalse) {
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_false_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&nat_false_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_others_default_)));
+
+  SetPolicies(nat_false_);
+  StartWatching();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatOne) {
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_false_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&nat_one_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_others_default_)));
+
+  SetPolicies(nat_one_);
+  StartWatching();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, DomainEmpty) {
-  EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&domain_empty_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&domain_empty_);
+  EXPECT_CALL(
+      mock_policy_callback_,
+      OnPolicyUpdatePtr(DictionaryValueEq(domain_empty_others_default_)));
+
+  SetPolicies(domain_empty_);
+  StartWatching();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, DomainFull) {
-  EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&domain_full_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&domain_full_);
+  EXPECT_CALL(
+      mock_policy_callback_,
+      OnPolicyUpdatePtr(DictionaryValueEq(domain_full_others_default_)));
+
+  SetPolicies(domain_full_);
+  StartWatching();
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenTrue) {
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  SetPolicies(nat_true_);
   StopWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrue) {
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_true_);
-  policy_watcher_->SetPolicies(&nat_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_true_);
-  policy_watcher_->SetPolicies(&nat_true_);
-  policy_watcher_->SetPolicies(&nat_false_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_false_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&nat_false_);
-  policy_watcher_->SetPolicies(&nat_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  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_);
+              OnPolicyUpdatePtr(
+                  DictionaryValueEq(nat_true_domain_empty_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(domain_full_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_false_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(domain_empty_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_domain_full_)));
+
+  SetPolicies(nat_true_domain_empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&unknown_policies_);
-  policy_watcher_->SetPolicies(&empty_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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
 
-  StartWatching();
-  policy_watcher_->SetPolicies(&nat_true_and_overridden_);
+  SetPolicies(nat_true_and_overridden_);
+  StartWatching();
   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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&pairing_false_);
-  policy_watcher_->SetPolicies(&pairing_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(pairing_false_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(pairing_true_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&gnubby_auth_false_);
-  policy_watcher_->SetPolicies(&gnubby_auth_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(gnubby_auth_false_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(gnubby_auth_true_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&relay_false_);
-  policy_watcher_->SetPolicies(&relay_true_);
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(relay_false_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(relay_true_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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_)));
-
-  StartWatching();
-  policy_watcher_->SetPolicies(&empty_);
-  policy_watcher_->SetPolicies(&port_range_full_);
-  policy_watcher_->SetPolicies(&port_range_empty_);
-  StopWatching();
-}
-
+              OnPolicyUpdatePtr(DictionaryValueEq(nat_true_others_default_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(port_range_full_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(DictionaryValueEq(port_range_empty_)));
+
+  SetPolicies(empty_);
+  StartWatching();
+  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