Commit a3880656 authored by Roman Sorokin's avatar Roman Sorokin Committed by Commit Bot

Chrome OS: Get rid of delay in FakeSessionManagerClient

Follow-up to CL:1564085

Bug: 934224
Change-Id: I8291ed461528d6f31ee5167289740296fea29cbe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1583779Reviewed-by: default avatarPavol Marko <pmarko@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659057}
parent 73473b62
...@@ -46,6 +46,8 @@ constexpr int kInitialEnrollmentModulusPowerLimit = 6; ...@@ -46,6 +46,8 @@ constexpr int kInitialEnrollmentModulusPowerLimit = 6;
// (https://crbug.com/846645). // (https://crbug.com/846645).
const int kInitialEnrollmentModulusPowerOutdatedServer = 14; const int kInitialEnrollmentModulusPowerOutdatedServer = 14;
const int kMaxRequestStateKeysTries = 10;
// Maximum time to wait for the auto-enrollment check to reach a decision. // Maximum time to wait for the auto-enrollment check to reach a decision.
// Note that this encompasses all steps |AutoEnrollmentController| performs in // Note that this encompasses all steps |AutoEnrollmentController| performs in
// order to determine if the device should be auto-enrolled. // order to determine if the device should be auto-enrolled.
...@@ -427,6 +429,7 @@ void AutoEnrollmentController::Start() { ...@@ -427,6 +429,7 @@ void AutoEnrollmentController::Start() {
safeguard_timer_.Start(FROM_HERE, kSafeguardTimeout, safeguard_timer_.Start(FROM_HERE, kSafeguardTimeout,
base::BindRepeating(&AutoEnrollmentController::Timeout, base::BindRepeating(&AutoEnrollmentController::Timeout,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
request_state_keys_tries_ = 0;
// The system clock sync state is not known yet, and this // The system clock sync state is not known yet, and this
// |AutoEnrollmentController| could wait for it if requested. // |AutoEnrollmentController| could wait for it if requested.
...@@ -481,11 +484,6 @@ void AutoEnrollmentController::SetAutoEnrollmentClientFactoryForTesting( ...@@ -481,11 +484,6 @@ void AutoEnrollmentController::SetAutoEnrollmentClientFactoryForTesting(
testing_auto_enrollment_client_factory_ = auto_enrollment_client_factory; testing_auto_enrollment_client_factory_ = auto_enrollment_client_factory;
} }
void AutoEnrollmentController::FireSafeguardTimerForTesting() {
DCHECK(safeguard_timer_.IsRunning());
safeguard_timer_.FireNow();
}
AutoEnrollmentController::InitialEnrollmentRequirement AutoEnrollmentController::InitialEnrollmentRequirement
AutoEnrollmentController::GetInitialEnrollmentRequirement() { AutoEnrollmentController::GetInitialEnrollmentRequirement() {
system::StatisticsProvider* provider = system::StatisticsProvider* provider =
...@@ -639,6 +637,7 @@ void AutoEnrollmentController::OnOwnershipStatusCheckDone( ...@@ -639,6 +637,7 @@ void AutoEnrollmentController::OnOwnershipStatusCheckDone(
case DeviceSettingsService::OWNERSHIP_NONE: case DeviceSettingsService::OWNERSHIP_NONE:
switch (auto_enrollment_check_type_) { switch (auto_enrollment_check_type_) {
case AutoEnrollmentCheckType::kFRE: case AutoEnrollmentCheckType::kFRE:
++request_state_keys_tries_;
// For FRE, request state keys first. // For FRE, request state keys first.
g_browser_process->platform_part() g_browser_process->platform_part()
->browser_policy_connector_chromeos() ->browser_policy_connector_chromeos()
...@@ -675,6 +674,13 @@ void AutoEnrollmentController::StartClientForFRE( ...@@ -675,6 +674,13 @@ void AutoEnrollmentController::StartClientForFRE(
if (state_keys.empty()) { if (state_keys.empty()) {
LOG(ERROR) << "No state keys available"; LOG(ERROR) << "No state keys available";
if (fre_requirement_ == FRERequirement::kExplicitlyRequired) { if (fre_requirement_ == FRERequirement::kExplicitlyRequired) {
if (request_state_keys_tries_ >= kMaxRequestStateKeysTries) {
if (safeguard_timer_.IsRunning())
safeguard_timer_.Stop();
Timeout();
return;
}
++request_state_keys_tries_;
// Retry to fetch the state keys. For devices where FRE is required to be // Retry to fetch the state keys. For devices where FRE is required to be
// checked, we can't proceed with empty state keys. // checked, we can't proceed with empty state keys.
g_browser_process->platform_part() g_browser_process->platform_part()
......
...@@ -143,8 +143,6 @@ class AutoEnrollmentController { ...@@ -143,8 +143,6 @@ class AutoEnrollmentController {
void SetAutoEnrollmentClientFactoryForTesting( void SetAutoEnrollmentClientFactoryForTesting(
policy::AutoEnrollmentClient::Factory* auto_enrollment_client_factory); policy::AutoEnrollmentClient::Factory* auto_enrollment_client_factory);
void FireSafeguardTimerForTesting();
private: private:
class SystemClockSyncWaiter; class SystemClockSyncWaiter;
...@@ -286,6 +284,9 @@ class AutoEnrollmentController { ...@@ -286,6 +284,9 @@ class AutoEnrollmentController {
// it's never set back to |false|. // it's never set back to |false|.
bool system_clock_sync_wait_requested_ = false; bool system_clock_sync_wait_requested_ = false;
// Keeps track of number of tries to request state keys.
int request_state_keys_tries_ = 0;
// TODO(igorcov): Merge the two weak_ptr factories in one. // TODO(igorcov): Merge the two weak_ptr factories in one.
base::WeakPtrFactory<AutoEnrollmentController> client_start_weak_factory_{ base::WeakPtrFactory<AutoEnrollmentController> client_start_weak_factory_{
this}; this};
......
...@@ -137,12 +137,6 @@ class AutoEnrollmentLocalPolicyServer : public EnrollmentLocalPolicyServerBase { ...@@ -137,12 +137,6 @@ class AutoEnrollmentLocalPolicyServer : public EnrollmentLocalPolicyServerBase {
->GetStateKeysBroker(); ->GetStateKeysBroker();
} }
void FireSafeguardTimer() {
WizardController::default_controller()
->GetAutoEnrollmentController()
->FireSafeguardTimerForTesting();
}
private: private:
DISALLOW_COPY_AND_ASSIGN(AutoEnrollmentLocalPolicyServer); DISALLOW_COPY_AND_ASSIGN(AutoEnrollmentLocalPolicyServer);
}; };
...@@ -534,9 +528,6 @@ IN_PROC_BROWSER_TEST_F(AutoEnrollmentNoStateKeys, FREExplicitlyRequired) { ...@@ -534,9 +528,6 @@ IN_PROC_BROWSER_TEST_F(AutoEnrollmentNoStateKeys, FREExplicitlyRequired) {
host()->StartWizard(AutoEnrollmentCheckScreenView::kScreenId); host()->StartWizard(AutoEnrollmentCheckScreenView::kScreenId);
OobeScreenWaiter(AutoEnrollmentCheckScreenView::kScreenId).Wait(); OobeScreenWaiter(AutoEnrollmentCheckScreenView::kScreenId).Wait();
// Chrome waits for state keys to be available, force a timeout.
FireSafeguardTimer();
OobeScreenWaiter(ErrorScreenView::kScreenId).Wait(); OobeScreenWaiter(ErrorScreenView::kScreenId).Wait();
test::OobeJS().ExpectHasNoClass("allow-guest-signin", {"error-message"}); test::OobeJS().ExpectHasNoClass("allow-guest-signin", {"error-message"});
} }
......
...@@ -545,12 +545,7 @@ void FakeSessionManagerClient::SetFlagsForUser( ...@@ -545,12 +545,7 @@ void FakeSessionManagerClient::SetFlagsForUser(
void FakeSessionManagerClient::GetServerBackedStateKeys( void FakeSessionManagerClient::GetServerBackedStateKeys(
StateKeysCallback callback) { StateKeysCallback callback) {
if (force_state_keys_missing_) { if (force_state_keys_missing_) {
// Need delay to prevent excessive warning output in tests caused by being PostReply(FROM_HERE, std::move(callback), std::vector<std::string>());
// retried in a tight loop.
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(std::move(callback), std::vector<std::string>()),
base::TimeDelta::FromSeconds(1));
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment