Commit 26b7042c authored by Nina Satragno's avatar Nina Satragno Committed by Commit Bot

[webauthn] Deflake inline enrollment UI test

Deflake AuthenticatorDialogTest.InvokeUi_inline_bio_enrollment. If the
timer fired between the dialog being destroyed and the test being
destroyed, we had a use-after-free of the raw pointer to the model.
Solve this by using a WeakPtr.

Additionally, if the current step changed by e.g. a user running the
test interactively, we hit a DCHECK on OnSampleCollected since the
current step was no longer inline bio enrollment. Fix this by checking
the current step before running the callback.

This fix is okay since the main point of this test is to run
interactively to try the UI and as a basic sanity check.

Fixed: 1092314
Change-Id: I5cec125de3a921df8b7e7a44d5cb1feedf634f4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2235979
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Auto-Submit: Nina Satragno <nsatragno@chromium.org>
Reviewed-by: default avatarMartin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#776095}
parent 48cac1c1
...@@ -90,12 +90,18 @@ class AuthenticatorDialogTest : public DialogBrowserTest { ...@@ -90,12 +90,18 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
model->CollectPIN(8, base::BindOnce([](std::string pin) {})); model->CollectPIN(8, base::BindOnce([](std::string pin) {}));
} else if (name == "inline_bio_enrollment") { } else if (name == "inline_bio_enrollment") {
model->StartInlineBioEnrollment(base::DoNothing()); model->StartInlineBioEnrollment(base::DoNothing());
timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(2), timer_.Start(
base::BindLambdaForTesting([&, weak_model = model.get()] { FROM_HERE, base::TimeDelta::FromSeconds(2),
weak_model->OnSampleCollected(--bio_samples_remaining_); base::BindLambdaForTesting([&, weak_model = model->GetWeakPtr()] {
if (bio_samples_remaining_ <= 0) if (!weak_model || weak_model->current_step() !=
timer_.Stop(); AuthenticatorRequestDialogModel::Step::
})); kInlineBioEnrollment) {
return;
}
weak_model->OnSampleCollected(--bio_samples_remaining_);
if (bio_samples_remaining_ <= 0)
timer_.Stop();
}));
} else if (name == "retry_uv") { } else if (name == "retry_uv") {
model->OnRetryUserVerification(5); model->OnRetryUserVerification(5);
} else if (name == "retry_uv_two_tries_remaining") { } else if (name == "retry_uv_two_tries_remaining") {
......
...@@ -569,3 +569,8 @@ void AuthenticatorRequestDialogModel::set_cable_transport_info( ...@@ -569,3 +569,8 @@ void AuthenticatorRequestDialogModel::set_cable_transport_info(
have_paired_phones_ = have_paired_phones; have_paired_phones_ = have_paired_phones;
qr_generator_key_ = std::move(qr_generator_key); qr_generator_key_ = std::move(qr_generator_key);
} }
base::WeakPtr<AuthenticatorRequestDialogModel>
AuthenticatorRequestDialogModel::GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}
...@@ -416,6 +416,8 @@ class AuthenticatorRequestDialogModel { ...@@ -416,6 +416,8 @@ class AuthenticatorRequestDialogModel {
bool offer_try_again_in_ui() const { return offer_try_again_in_ui_; } bool offer_try_again_in_ui() const { return offer_try_again_in_ui_; }
base::WeakPtr<AuthenticatorRequestDialogModel> GetWeakPtr();
private: private:
// Contains the state that will be reset when calling StartOver(). StartOver() // Contains the state that will be reset when calling StartOver(). StartOver()
// might be called at an arbitrary point of execution. // might be called at an arbitrary point of execution.
......
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