chrome/browser/chromeos/login/signed_settings_unittest.cc - Issue 8091002: PART2: Make SignedSettings use proper Value types instead of string all around the place.

Unified Diff: chrome/browser/chromeos/login/signed_settings_unittest.cc

Issue 8091002: PART2: Make SignedSettings use proper Value types instead of string all around the place. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Addressed comments and rebased on a the current PART1 version. Created 9 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« chrome/browser/chromeos/login/signed_settings_temp_storage.cc ('K') | « chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc ('k') | chrome/browser/chromeos/login/wizard_controller.cc » ('j') | chrome/browser/chromeos/user_cros_settings_provider.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/chromeos/login/signed_settings_unittest.cc

diff --git a/chrome/browser/chromeos/login/signed_settings_unittest.cc b/chrome/browser/chromeos/login/signed_settings_unittest.cc

index 495b1f5ada9aa51638a93818361075fb8a8f8e0b..dea6c5803aa1c479898118935092bdd0ed72eb7c 100644

--- a/chrome/browser/chromeos/login/signed_settings_unittest.cc

+++ b/chrome/browser/chromeos/login/signed_settings_unittest.cc

@@ -77,7 +77,24 @@ class NormalDelegate : public DummyDelegate<T> {

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

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));

}

};

@@ -104,7 +121,11 @@ class SignedSettingsTest : public testing::Test {

: 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),

@@ -273,9 +294,9 @@ class SignedSettingsTest : public testing::Test {

}

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));

@@ -295,7 +316,9 @@ class SignedSettingsTest : public testing::Test {

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_;

@@ -487,42 +510,48 @@ TEST_F(SignedSettingsTest, StorePropertyFailed) {

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));

@@ -544,7 +573,8 @@ 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));

@@ -553,7 +583,7 @@ TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) {

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(_, _))

@@ -574,10 +604,8 @@ TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) {

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();

@@ -612,16 +640,13 @@ TEST_F(SignedSettingsTest, SignAndStorePolicy) {

// 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();

}

@@ -634,7 +659,7 @@ TEST_F(SignedSettingsTest, StoreSignedPolicy) {

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));

@@ -681,7 +706,7 @@ TEST_F(SignedSettingsTest, RetrievePolicy) {

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();

@@ -695,9 +720,7 @@ TEST_F(SignedSettingsTest, RetrievePolicy) {

.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&>()))

@@ -770,7 +793,7 @@ TEST_F(SignedSettingsTest, RetrieveUnsignedPolicy) {

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);

@@ -784,9 +807,7 @@ TEST_F(SignedSettingsTest, RetrieveMalsignedPolicy) {

.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();