PART2: Make SignedSettings use proper Value types instead of string all around the place.

chrome/browser/chromeos/login/signed_settings_unittest.cc

 // Copyright (c) 2011 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/login/signed_settings.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stringprintf.h"
@@ skipping 59 matching lines @@
   virtual void compare_expected(T to_compare) = 0;
 };
 
 template <class T>
 class NormalDelegate : public DummyDelegate<T> {
  public:
   explicit NormalDelegate(T to_expect) : DummyDelegate<T>(to_expect) {}
   virtual ~NormalDelegate() {}
  protected:
   virtual void compare_expected(T to_compare) {
-    EXPECT_EQ(this->expected_, to_compare);  // without this-> this won't build.
+    // without this-> this won't build.
+    EXPECT_EQ(this->expected_, to_compare);
   }
 };
 
+// Specialize the template for base::Value objects because these compare
+// differently.

[Mattias Nissler (ping if slow), 2011/10/13 14:49:52]

Why not just make a subclass as is done below for ProtoDelegate? What you have
now is inconsistent.

[pastarmovj, 2011/10/26 15:44:59]

Done.

+template <>
+class NormalDelegate<const base::Value&>
+    : public DummyDelegate<const base::Value&> {
+ public:
+  explicit NormalDelegate(const base::Value& to_expect)
+      : DummyDelegate<const base::Value&>(to_expect) {}
+  virtual ~NormalDelegate() {}
+ protected:
+  virtual void compare_expected(const base::Value& to_compare) {
+    // without this-> this won't build.
+    EXPECT_TRUE(this->expected_.Equals(&to_compare));
+  }
+};
+
 class ProtoDelegate : public DummyDelegate<const em::PolicyFetchResponse&> {
  public:
   explicit ProtoDelegate(const em::PolicyFetchResponse& e)
       : DummyDelegate<const em::PolicyFetchResponse&>(e) {
   }
   virtual ~ProtoDelegate() {}
  protected:
   virtual void compare_expected(const em::PolicyFetchResponse& to_compare) {
     std::string ex_string, comp_string;
     EXPECT_TRUE(expected_.SerializeToString(&ex_string));
     EXPECT_TRUE(to_compare.SerializeToString(&comp_string));
     EXPECT_EQ(ex_string, comp_string);
   }
 };
 
 }  // anonymous namespace
 
 class SignedSettingsTest : public testing::Test {
  public:
   SignedSettingsTest()
       : fake_email_("fakey@example.com"),
         fake_domain_("*@example.com"),
         fake_prop_(kAccountsPrefAllowGuest),
-        fake_value_("false"),
+        fake_signature_("false"),
+        fake_value_(false),
+        fake_value_signature_(
+            fake_signature_.c_str(),
+            fake_signature_.c_str() + fake_signature_.length()),
         message_loop_(MessageLoop::TYPE_UI),
         ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE),
         mock_(new MockKeyUtils),
         injector_(mock_) /* injector_ takes ownership of mock_ */ {
   }
 
   virtual ~SignedSettingsTest() {}
 
   virtual void SetUp() {
@@ skipping 148 matching lines @@
     em::PolicyFetchResponse fake_policy;
     if (!data.empty())
       fake_policy.set_policy_data(data);
     if (!sig.empty())
       fake_policy.set_policy_data_signature(sig);
     EXPECT_TRUE(fake_policy.SerializeToString(out_serialized));
     return fake_policy;
   }
 
   void DoRetrieveProperty(const std::string& name,
-                          const std::string& value,
+                          const base::Value& value,
                           em::PolicyData* fake_pol) {
-    NormalDelegate<std::string> d(value);
+    NormalDelegate<const base::Value&> d(value);
     d.expect_success();
     scoped_refptr<SignedSettings> s(
         SignedSettings::CreateRetrievePropertyOp(name, &d));
     mock_service(s.get(), &m_);
     EXPECT_CALL(m_, GetStatus(_))
         .WillOnce(Return(OwnershipService::OWNERSHIP_TAKEN));
     EXPECT_CALL(m_, has_cached_policy())
         .WillOnce(Return(true));
 
     EXPECT_CALL(m_, cached_policy())
         .WillOnce(ReturnRef(*fake_pol));
 
     s->Execute();
     message_loop_.RunAllPending();
   }
 
   const std::string fake_email_;
   const std::string fake_domain_;
   const std::string fake_prop_;
-  const std::string fake_value_;
+  const std::string fake_signature_;
+  const base::FundamentalValue fake_value_;
+  const std::vector<uint8> fake_value_signature_;
   MockOwnershipService m_;
 
   ScopedTempDir tmpdir_;
   FilePath tmpfile_;
 
   MessageLoop message_loop_;
   BrowserThread ui_thread_;
   BrowserThread file_thread_;
 
   std::vector<uint8> fake_public_key_;
@@ skipping 171 matching lines @@
 TEST_F(SignedSettingsTest, StorePropertyNoKey) {
   FailingStorePropertyOp(OwnerManager::KEY_UNAVAILABLE);
 }
 
 TEST_F(SignedSettingsTest, StorePropertyFailed) {
   FailingStorePropertyOp(OwnerManager::OPERATION_FAILED);
 }
 
 TEST_F(SignedSettingsTest, RetrieveProperty) {
   em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>());
-  DoRetrieveProperty(fake_prop_, fake_value_, &fake_pol);
+  base::FundamentalValue fake_value(false);
+  DoRetrieveProperty(fake_prop_, fake_value, &fake_pol);
 }
 
 TEST_F(SignedSettingsTest, RetrieveOwnerProperty) {
   em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>());
   fake_pol.set_username(fake_email_);
-  DoRetrieveProperty(kDeviceOwner, fake_email_, &fake_pol);
+  base::StringValue fake_value(fake_email_);
+  DoRetrieveProperty(kDeviceOwner, fake_value, &fake_pol);
 }
 
 TEST_F(SignedSettingsTest, ExplicitlyAllowNewUsers) {
   em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>());
   SetAllowNewUsers(true, &fake_pol);
-  DoRetrieveProperty(kAccountsPrefAllowNewUser, "true", &fake_pol);
+  base::FundamentalValue fake_value(true);
+  DoRetrieveProperty(kAccountsPrefAllowNewUser, fake_value, &fake_pol);
 }
 
 TEST_F(SignedSettingsTest, ExplicitlyDisallowNewUsers) {
   std::vector<std::string> whitelist(1, fake_email_ + "m");
   em::PolicyData fake_pol = BuildPolicyData(whitelist);
   SetAllowNewUsers(false, &fake_pol);
-  DoRetrieveProperty(kAccountsPrefAllowNewUser, "false", &fake_pol);
+  base::FundamentalValue fake_value(false);
+  DoRetrieveProperty(kAccountsPrefAllowNewUser, fake_value, &fake_pol);
 }
 
 TEST_F(SignedSettingsTest, ImplicitlyDisallowNewUsers) {
   std::vector<std::string> whitelist(1, fake_email_ + "m");
   em::PolicyData fake_pol = BuildPolicyData(whitelist);
-  DoRetrieveProperty(kAccountsPrefAllowNewUser, "false", &fake_pol);
+  base::FundamentalValue fake_value(false);
+  DoRetrieveProperty(kAccountsPrefAllowNewUser, fake_value, &fake_pol);
 }
 
 TEST_F(SignedSettingsTest, AccidentallyDisallowNewUsers) {
   em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>());
   SetAllowNewUsers(false, &fake_pol);
-  DoRetrieveProperty(kAccountsPrefAllowNewUser, "true", &fake_pol);
+  base::FundamentalValue fake_value(true);
+  DoRetrieveProperty(kAccountsPrefAllowNewUser, fake_value, &fake_pol);
 }
 
 TEST_F(SignedSettingsTest, RetrievePropertyNotFound) {
-  NormalDelegate<std::string> d(fake_value_);
+  NormalDelegate<const base::Value&> d(fake_value_);
   d.expect_failure(SignedSettings::NOT_FOUND);
   scoped_refptr<SignedSettings> s(
       SignedSettings::CreateRetrievePropertyOp("unknown_prop", &d));
   mock_service(s.get(), &m_);
   EXPECT_CALL(m_, GetStatus(_))
       .WillOnce(Return(OwnershipService::OWNERSHIP_TAKEN));
   EXPECT_CALL(m_, has_cached_policy())
       .WillOnce(Return(true));
 
   em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>());
   EXPECT_CALL(m_, cached_policy())
       .WillOnce(ReturnRef(fake_pol));
 
   s->Execute();
   message_loop_.RunAllPending();
 }
 
 ACTION_P(Retrieve, s) { (*arg0)((void*)arg1, s.c_str(), s.length()); }
 ACTION_P(FinishKeyOp, s) { arg2->OnKeyOpComplete(OwnerManager::SUCCESS, s); }
 
 TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) {
-  NormalDelegate<std::string> d(fake_value_);
+  base::FundamentalValue fake_value(false);
+  NormalDelegate<const base::Value&> d(fake_value);
   d.expect_success();
   scoped_refptr<SignedSettings> s(
       SignedSettings::CreateRetrievePropertyOp(fake_prop_, &d));
 
   em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>());
   std::string data = fake_pol.SerializeAsString();
   std::string signed_serialized;
   em::PolicyFetchResponse signed_policy = BuildProto(data,
-                                                     fake_value_,
+                                                     fake_signature_,
                                                      &signed_serialized);
   MockLoginLibrary* lib = MockLoginLib();
   EXPECT_CALL(*lib, RequestRetrievePolicy(_, _))
       .WillOnce(Retrieve(signed_serialized))
       .RetiresOnSaturation();
 
   mock_service(s.get(), &m_);
 
   EXPECT_CALL(m_, GetStatus(_))
       .WillOnce(Return(OwnershipService::OWNERSHIP_TAKEN))
       .WillOnce(Return(OwnershipService::OWNERSHIP_TAKEN));
   EXPECT_CALL(m_, has_cached_policy())
       .WillOnce(Return(false))
       .WillOnce(Return(true));
   em::PolicyData out_pol;
   EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>()))
       .WillOnce(SaveArg<0>(&out_pol));
   EXPECT_CALL(m_, cached_policy())
       .WillOnce(ReturnRef(out_pol));
 
-  std::vector<uint8> fake_sig(fake_value_.c_str(),
-                              fake_value_.c_str() + fake_value_.length());
-  EXPECT_CALL(m_, StartVerifyAttempt(data, fake_sig, _))
-      .WillOnce(FinishKeyOp(fake_sig))
+  EXPECT_CALL(m_, StartVerifyAttempt(data, fake_value_signature_, _))
+      .WillOnce(FinishKeyOp(fake_value_signature_))
       .RetiresOnSaturation();
 
   s->Execute();
   message_loop_.RunAllPending();
   UnMockLoginLib();
 }
 
 TEST_F(SignedSettingsTest, SignAndStorePolicy) {
   NormalDelegate<bool> d(true);
   d.expect_success();
@@ skipping 14 matching lines @@
   EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>()))
       .WillOnce(SaveArg<0>(&out_pol));
 
   // Ask for signature over unsigned policy.
   s->Execute();
   message_loop_.RunAllPending();
 
   // Fake out a successful signing.
   std::string signed_serialized;
   em::PolicyFetchResponse signed_policy = BuildProto(data_serialized,
-                                                     fake_value_,
+                                                     fake_signature_,
                                                      &signed_serialized);
-  std::vector<uint8> fake_sig(fake_value_.c_str(),
-                              fake_value_.c_str() + fake_value_.length());
-
   MockLoginLibrary* lib = MockLoginLib();
   EXPECT_CALL(*lib, RequestStorePolicy(StrEq(signed_serialized), _, s.get()))
       .WillOnce(InvokeArgument<1>(static_cast<void*>(s.get()), true))
       .RetiresOnSaturation();
-  s->OnKeyOpComplete(OwnerManager::SUCCESS, fake_sig);
+  s->OnKeyOpComplete(OwnerManager::SUCCESS, fake_value_signature_);
   message_loop_.RunAllPending();
   UnMockLoginLib();
 }
 
 TEST_F(SignedSettingsTest, StoreSignedPolicy) {
   NormalDelegate<bool> d(true);
   d.expect_success();
 
   em::PolicyData in_pol = BuildPolicyData(std::vector<std::string>());
   std::string serialized = in_pol.SerializeAsString();
   std::string signed_serialized;
   em::PolicyFetchResponse signed_policy = BuildProto(serialized,
-                                                     fake_value_,
+                                                     fake_signature_,
                                                      &signed_serialized);
   scoped_refptr<SignedSettings> s(
       SignedSettings::CreateStorePolicyOp(&signed_policy, &d));
   MockLoginLibrary* lib = MockLoginLib();
   EXPECT_CALL(*lib, RequestStorePolicy(StrEq(signed_serialized), _, s.get()))
       .WillOnce(InvokeArgument<1>(static_cast<void*>(s.get()), true))
       .RetiresOnSaturation();
 
   mock_service(s.get(), &m_);
   em::PolicyData out_pol;
@@ skipping 26 matching lines @@
 
   s->Execute();
   message_loop_.RunAllPending();
 }
 
 TEST_F(SignedSettingsTest, RetrievePolicy) {
   em::PolicyData in_pol = BuildPolicyData(std::vector<std::string>());
   std::string serialized = in_pol.SerializeAsString();
   std::string signed_serialized;
   em::PolicyFetchResponse signed_policy = BuildProto(serialized,
-                                                     fake_value_,
+                                                     fake_signature_,
                                                      &signed_serialized);
   ProtoDelegate d(signed_policy);
   d.expect_success();
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
   MockLoginLibrary* lib = MockLoginLib();
   EXPECT_CALL(*lib, RequestRetrievePolicy(_, s.get()))
       .WillOnce(InvokeArgument<0>(static_cast<void*>(s.get()),
                                   signed_serialized.c_str(),
                                   signed_serialized.length()))
       .RetiresOnSaturation();
 
   mock_service(s.get(), &m_);
-  std::vector<uint8> fake_sig(fake_value_.c_str(),
-                              fake_value_.c_str() + fake_value_.length());
-  EXPECT_CALL(m_, StartVerifyAttempt(serialized, fake_sig, _))
+  EXPECT_CALL(m_, StartVerifyAttempt(serialized, fake_value_signature_, _))
       .Times(1);
   em::PolicyData out_pol;
   EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>()))
       .WillOnce(SaveArg<0>(&out_pol));
 
   s->Execute();
   message_loop_.RunAllPending();
   UnMockLoginLib();
 
   s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>());
@@ skipping 52 matching lines @@
       .RetiresOnSaturation();
 
   s->Execute();
   message_loop_.RunAllPending();
   UnMockLoginLib();
 }
 
 TEST_F(SignedSettingsTest, RetrieveMalsignedPolicy) {
   std::string signed_serialized;
   em::PolicyFetchResponse signed_policy = BuildProto(fake_prop_,
-                                                     fake_value_,
+                                                     fake_signature_,
                                                      &signed_serialized);
   ProtoDelegate d(signed_policy);
   d.expect_failure(SignedSettings::BAD_SIGNATURE);
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
   MockLoginLibrary* lib = MockLoginLib();
   EXPECT_CALL(*lib, RequestRetrievePolicy(_, s.get()))
       .WillOnce(InvokeArgument<0>(static_cast<void*>(s.get()),
                                   signed_serialized.c_str(),
                                   signed_serialized.length()))
       .RetiresOnSaturation();
 
   mock_service(s.get(), &m_);
-  std::vector<uint8> fake_sig(fake_value_.c_str(),
-                              fake_value_.c_str() + fake_value_.length());
-  EXPECT_CALL(m_, StartVerifyAttempt(fake_prop_, fake_sig, _))
+  EXPECT_CALL(m_, StartVerifyAttempt(fake_prop_, fake_value_signature_, _))
       .Times(1);
 
   s->Execute();
   message_loop_.RunAllPending();
   UnMockLoginLib();
 
   s->OnKeyOpComplete(OwnerManager::OPERATION_FAILED, std::vector<uint8>());
   message_loop_.RunAllPending();
 }
 
 }  // namespace chromeos