Index: chrome/browser/extensions/extension_install_checker_unittest.cc |
diff --git a/chrome/browser/extensions/extension_install_checker_unittest.cc b/chrome/browser/extensions/extension_install_checker_unittest.cc |
index f70d9ddce34242be4c5b38bee2b6ca13b7b94b3f..26c0c044985ca6996425e45161d44b25ad1b7fc0 100644 |
--- a/chrome/browser/extensions/extension_install_checker_unittest.cc |
+++ b/chrome/browser/extensions/extension_install_checker_unittest.cc |
@@ -18,10 +18,6 @@ const BlacklistState kBlacklistStateError = BLACKLISTED_MALWARE; |
const char kDummyRequirementsError[] = "Requirements error"; |
const char kDummyPolicyError[] = "Cannot install extension"; |
-const char kDummyPolicyError2[] = "Another policy error"; |
-const char kDummyRequirementsError2[] = "Another requirements error"; |
-const BlacklistState kBlacklistState2 = BLACKLISTED_SECURITY_VULNERABILITY; |
- |
} // namespace |
// Stubs most of the checks since we are interested in validating the logic in |
@@ -30,7 +26,7 @@ const BlacklistState kBlacklistState2 = BLACKLISTED_SECURITY_VULNERABILITY; |
class ExtensionInstallCheckerForTest : public ExtensionInstallChecker { |
public: |
ExtensionInstallCheckerForTest() |
- : ExtensionInstallChecker(NULL), |
+ : ExtensionInstallChecker(nullptr, nullptr), |
requirements_check_called_(false), |
blacklist_check_called_(false), |
policy_check_called_(false), |
@@ -50,21 +46,21 @@ class ExtensionInstallCheckerForTest : public ExtensionInstallChecker { |
bool blacklist_check_called() const { return blacklist_check_called_; } |
bool policy_check_called() const { return policy_check_called_; } |
- void MockCheckRequirements(int sequence_number) { |
+ void MockCheckRequirements() { |
std::vector<std::string> errors; |
if (!requirements_error_.empty()) |
errors.push_back(requirements_error_); |
- OnRequirementsCheckDone(sequence_number, errors); |
+ OnRequirementsCheckDone(errors); |
} |
- void MockCheckBlacklistState(int sequence_number) { |
- OnBlacklistStateCheckDone(sequence_number, blacklist_state_); |
+ void MockCheckBlacklistState() { |
+ OnBlacklistStateCheckDone(blacklist_state_); |
} |
protected: |
void CheckRequirements() override { |
requirements_check_called_ = true; |
- MockCheckRequirements(current_sequence_number()); |
+ MockCheckRequirements(); |
} |
void CheckManagementPolicy() override { |
@@ -75,15 +71,7 @@ class ExtensionInstallCheckerForTest : public ExtensionInstallChecker { |
void CheckBlacklistState() override { |
blacklist_check_called_ = true; |
- MockCheckBlacklistState(current_sequence_number()); |
- } |
- |
- void ResetResults() override { |
- ExtensionInstallChecker::ResetResults(); |
- |
- requirements_check_called_ = false; |
- blacklist_check_called_ = false; |
- policy_check_called_ = false; |
+ MockCheckBlacklistState(); |
} |
bool requirements_check_called_; |
@@ -106,7 +94,7 @@ class ExtensionInstallCheckerAsync : public ExtensionInstallCheckerForTest { |
base::ThreadTaskRunnerHandle::Get()->PostTask( |
FROM_HERE, |
base::Bind(&ExtensionInstallCheckerForTest::MockCheckRequirements, |
- base::Unretained(this), current_sequence_number())); |
+ base::Unretained(this))); |
} |
void CheckBlacklistState() override { |
@@ -115,7 +103,7 @@ class ExtensionInstallCheckerAsync : public ExtensionInstallCheckerForTest { |
base::ThreadTaskRunnerHandle::Get()->PostTask( |
FROM_HERE, |
base::Bind(&ExtensionInstallCheckerForTest::MockCheckBlacklistState, |
- base::Unretained(this), current_sequence_number())); |
+ base::Unretained(this))); |
} |
}; |
@@ -148,40 +136,6 @@ class ExtensionInstallCheckerTest : public testing::Test { |
ExtensionInstallCheckerTest() {} |
~ExtensionInstallCheckerTest() override {} |
- void RunSecondInvocation(ExtensionInstallCheckerForTest* checker, |
- int checks_failed) { |
- EXPECT_GT(checks_failed, 0); |
- EXPECT_FALSE(checker->is_running()); |
- ValidateExpectedCalls(ExtensionInstallChecker::CHECK_ALL, *checker); |
- |
- // Set up different return values. |
- checker->set_blacklist_state(kBlacklistState2); |
- checker->set_policy_check_error(kDummyPolicyError2); |
- checker->set_requirements_error(kDummyRequirementsError2); |
- |
- // Run the install checker again and ensure the second set of return values |
- // is received. |
- checker->Start( |
- ExtensionInstallChecker::CHECK_ALL, |
- false /* fail fast */, |
- base::Bind(&ExtensionInstallCheckerTest::ValidateSecondInvocation, |
- base::Unretained(this), |
- checker)); |
- } |
- |
- void ValidateSecondInvocation(ExtensionInstallCheckerForTest* checker, |
- int checks_failed) { |
- EXPECT_FALSE(checker->is_running()); |
- EXPECT_EQ(ExtensionInstallChecker::CHECK_REQUIREMENTS | |
- ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY, |
- checks_failed); |
- ValidateExpectedCalls(ExtensionInstallChecker::CHECK_ALL, *checker); |
- |
- EXPECT_EQ(kBlacklistState2, checker->blacklist_state()); |
- ExpectPolicyError(kDummyPolicyError2, *checker); |
- ExpectRequirementsError(kDummyRequirementsError2, *checker); |
- } |
- |
protected: |
void SetAllErrors(ExtensionInstallCheckerForTest* checker) { |
checker->set_blacklist_state(kBlacklistStateError); |
@@ -284,35 +238,32 @@ class ExtensionInstallCheckerTest : public testing::Test { |
ExpectBlacklistError(*checker); |
} |
- void DoRunSubsetOfChecks(ExtensionInstallCheckerForTest* checker) { |
- // Test check set 1. |
- int tests_to_run = ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY | |
- ExtensionInstallChecker::CHECK_REQUIREMENTS; |
- SetAllErrors(checker); |
- RunChecker(checker, false, tests_to_run, tests_to_run, tests_to_run); |
- |
- ExpectRequirementsError(*checker); |
- ExpectPolicyError(*checker); |
- ExpectBlacklistPass(*checker); |
- |
- // Test check set 2. |
- tests_to_run = ExtensionInstallChecker::CHECK_BLACKLIST | |
- ExtensionInstallChecker::CHECK_REQUIREMENTS; |
- SetAllErrors(checker); |
- RunChecker(checker, false, tests_to_run, tests_to_run, tests_to_run); |
- |
- ExpectRequirementsError(*checker); |
- ExpectPolicyPass(*checker); |
- ExpectBlacklistError(*checker); |
- |
- // Test a single check. |
- tests_to_run = ExtensionInstallChecker::CHECK_BLACKLIST; |
- SetAllErrors(checker); |
- RunChecker(checker, false, tests_to_run, tests_to_run, tests_to_run); |
- |
- ExpectRequirementsPass(*checker); |
- ExpectPolicyPass(*checker); |
- ExpectBlacklistError(*checker); |
+ void DoRunSubsetOfChecks(int checks_to_run) { |
+ ExtensionInstallCheckerForTest sync_checker; |
+ ExtensionInstallCheckerAsync async_checker; |
+ ExtensionInstallCheckerForTest* checkers[] = { |
+ &sync_checker, &async_checker, |
+ }; |
+ |
+ for (ExtensionInstallCheckerForTest* checker : checkers) { |
Devlin
2017/03/16 19:29:41
optional nit: just inline checkers
michaelpg
2017/03/17 04:25:23
IMO the polymorphism is a bit more obvious being e
|
+ SetAllErrors(checker); |
+ RunChecker(checker, false, checks_to_run, checks_to_run, checks_to_run); |
+ |
+ if (checks_to_run & ExtensionInstallChecker::CHECK_REQUIREMENTS) |
+ ExpectRequirementsError(*checker); |
+ else |
+ ExpectRequirementsPass(*checker); |
+ |
+ if (checks_to_run & ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY) |
+ ExpectPolicyError(*checker); |
+ else |
+ ExpectPolicyPass(*checker); |
+ |
+ if (checks_to_run & ExtensionInstallChecker::CHECK_BLACKLIST) |
+ ExpectBlacklistError(*checker); |
+ else |
+ ExpectBlacklistPass(*checker); |
+ } |
} |
private: |
@@ -320,53 +271,6 @@ class ExtensionInstallCheckerTest : public testing::Test { |
base::MessageLoop message_loop; |
}; |
-class ExtensionInstallCheckerMultipleInvocationTest |
- : public ExtensionInstallCheckerTest { |
- public: |
- ExtensionInstallCheckerMultipleInvocationTest() : callback_count_(0) {} |
- ~ExtensionInstallCheckerMultipleInvocationTest() override {} |
- |
- void RunSecondInvocation(ExtensionInstallCheckerForTest* checker, |
- int checks_failed) { |
- ASSERT_EQ(0, callback_count_); |
- ++callback_count_; |
- EXPECT_FALSE(checker->is_running()); |
- EXPECT_GT(checks_failed, 0); |
- ValidateExpectedCalls(ExtensionInstallChecker::CHECK_ALL, *checker); |
- |
- // Set up different return values. |
- checker->set_blacklist_state(kBlacklistState2); |
- checker->set_policy_check_error(kDummyPolicyError2); |
- checker->set_requirements_error(kDummyRequirementsError2); |
- |
- // Run the install checker again and ensure the second set of return values |
- // is received. |
- checker->Start(ExtensionInstallChecker::CHECK_ALL, |
- false /* fail fast */, |
- base::Bind(&ExtensionInstallCheckerMultipleInvocationTest:: |
- ValidateSecondInvocation, |
- base::Unretained(this), |
- checker)); |
- } |
- |
- void ValidateSecondInvocation(ExtensionInstallCheckerForTest* checker, |
- int checks_failed) { |
- ASSERT_EQ(1, callback_count_); |
- EXPECT_FALSE(checker->is_running()); |
- EXPECT_EQ(ExtensionInstallChecker::CHECK_REQUIREMENTS | |
- ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY, |
- checks_failed); |
- ValidateExpectedCalls(ExtensionInstallChecker::CHECK_ALL, *checker); |
- |
- EXPECT_EQ(kBlacklistState2, checker->blacklist_state()); |
- ExpectPolicyError(kDummyPolicyError2, *checker); |
- ExpectRequirementsError(kDummyRequirementsError2, *checker); |
- } |
- |
- private: |
- int callback_count_; |
-}; |
- |
// Test the case where all tests pass. |
TEST_F(ExtensionInstallCheckerTest, AllSucceeded) { |
ExtensionInstallCheckerForTest sync_checker; |
@@ -387,41 +291,43 @@ TEST_F(ExtensionInstallCheckerTest, AllFailed) { |
// Test running only a subset of tests. |
TEST_F(ExtensionInstallCheckerTest, RunSubsetOfChecks) { |
- ExtensionInstallCheckerForTest sync_checker; |
- ExtensionInstallCheckerAsync async_checker; |
- DoRunSubsetOfChecks(&sync_checker); |
- DoRunSubsetOfChecks(&async_checker); |
+ DoRunSubsetOfChecks(ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY | |
+ ExtensionInstallChecker::CHECK_REQUIREMENTS); |
+ DoRunSubsetOfChecks(ExtensionInstallChecker::CHECK_BLACKLIST | |
+ ExtensionInstallChecker::CHECK_REQUIREMENTS); |
+ DoRunSubsetOfChecks(ExtensionInstallChecker::CHECK_BLACKLIST); |
} |
// Test fail fast with synchronous callbacks. |
TEST_F(ExtensionInstallCheckerTest, FailFastSync) { |
// This test assumes some internal knowledge of the implementation - that |
// the policy check runs first. |
- ExtensionInstallCheckerForTest checker; |
- SetAllErrors(&checker); |
- RunChecker(&checker, |
- true /* fail fast */, |
- ExtensionInstallChecker::CHECK_ALL, |
- ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY, |
- ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY); |
- |
- ExpectRequirementsPass(checker); |
- ExpectPolicyError(checker); |
- ExpectBlacklistPass(checker); |
+ { |
+ ExtensionInstallCheckerForTest checker; |
+ SetAllErrors(&checker); |
+ RunChecker(&checker, true /* fail fast */, |
+ ExtensionInstallChecker::CHECK_ALL, |
+ ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY, |
+ ExtensionInstallChecker::CHECK_MANAGEMENT_POLICY); |
- // This test assumes some internal knowledge of the implementation - that |
- // the requirements check runs before the blacklist check. |
- SetAllErrors(&checker); |
- RunChecker(&checker, |
- true /* fail fast */, |
- ExtensionInstallChecker::CHECK_REQUIREMENTS | |
- ExtensionInstallChecker::CHECK_BLACKLIST, |
- ExtensionInstallChecker::CHECK_REQUIREMENTS, |
- ExtensionInstallChecker::CHECK_REQUIREMENTS); |
+ ExpectRequirementsPass(checker); |
+ ExpectPolicyError(checker); |
+ ExpectBlacklistPass(checker); |
+ } |
- ExpectRequirementsError(checker); |
- ExpectPolicyPass(checker); |
- ExpectBlacklistPass(checker); |
+ { |
+ ExtensionInstallCheckerForTest checker; |
+ SetAllErrors(&checker); |
+ RunChecker(&checker, true /* fail fast */, |
+ ExtensionInstallChecker::CHECK_REQUIREMENTS | |
+ ExtensionInstallChecker::CHECK_BLACKLIST, |
+ ExtensionInstallChecker::CHECK_REQUIREMENTS, |
+ ExtensionInstallChecker::CHECK_REQUIREMENTS); |
+ |
+ ExpectRequirementsError(checker); |
+ ExpectPolicyPass(checker); |
+ ExpectBlacklistPass(checker); |
+ } |
} |
// Test fail fast with asynchronous callbacks. |
@@ -448,41 +354,4 @@ TEST_F(ExtensionInstallCheckerTest, FailFastAsync) { |
ExpectBlacklistPass(checker); |
} |
-// Test multiple invocations of the install checker. Wait for all checks to |
-// complete. |
-TEST_F(ExtensionInstallCheckerMultipleInvocationTest, CompleteAll) { |
- ExtensionInstallCheckerAsync checker; |
- SetAllErrors(&checker); |
- |
- // Start the second check as soon as the callback of the first run is invoked. |
- checker.Start( |
- ExtensionInstallChecker::CHECK_ALL, |
- false /* fail fast */, |
- base::Bind( |
- &ExtensionInstallCheckerMultipleInvocationTest::RunSecondInvocation, |
- base::Unretained(this), |
- &checker)); |
- base::RunLoop().RunUntilIdle(); |
-} |
- |
-// Test multiple invocations of the install checker and fail fast. |
-TEST_F(ExtensionInstallCheckerMultipleInvocationTest, FailFast) { |
- ExtensionInstallCheckerAsync checker; |
- SetAllErrors(&checker); |
- |
- // The policy check is synchronous and needs to pass for the other tests to |
- // run. |
- checker.set_policy_check_error(std::string()); |
- |
- // Start the second check as soon as the callback of the first run is invoked. |
- checker.Start( |
- ExtensionInstallChecker::CHECK_ALL, |
- true /* fail fast */, |
- base::Bind( |
- &ExtensionInstallCheckerMultipleInvocationTest::RunSecondInvocation, |
- base::Unretained(this), |
- &checker)); |
- base::RunLoop().RunUntilIdle(); |
-} |
- |
} // namespace extensions |