[High5] Add initial tests for PIN-only OOBE flow
Add a fixture to test the PIN-only OOBE flow, and a couple of tests to
ensure that the proper strings are shown when the PIN setup screen is
being offered as a main/auxiliary factor.
Bug: b/365059362, b/348326316
Change-Id: I3827dd55e2df682af5a5a0036c1ce8496c62ebe6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5878358
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Hardik Goyal <hardikgoyal@chromium.org>
Commit-Queue: Hardik Goyal <hardikgoyal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1358332}
diff --git a/chrome/browser/ash/login/screens/pin_setup_screen.cc b/chrome/browser/ash/login/screens/pin_setup_screen.cc
index d9724d81..c4a0c9c5 100644
--- a/chrome/browser/ash/login/screens/pin_setup_screen.cc
+++ b/chrome/browser/ash/login/screens/pin_setup_screen.cc
@@ -163,7 +163,7 @@
if (view_) {
view_->Show(token, is_child_account, has_login_support_.value_or(false),
- using_pin_as_main_factor_);
+ using_pin_as_main_factor_.value_or(false));
}
}
@@ -203,6 +203,15 @@
if (!is_hidden() && view_) {
view_->SetLoginSupportAvailable(has_login_support_.value());
}
+
+ // When hardware support is available, PIN will be offered as a main factor.
+ if (ash::switches::IsOobePinOnlyPrototypeEnabled()) {
+ // The first value is only set once, based on hardware capability.
+ if (using_pin_as_main_factor_.has_value()) {
+ return;
+ }
+ using_pin_as_main_factor_ = login_available;
+ }
}
void PinSetupScreen::OnTokenTimedOut() {
diff --git a/chrome/browser/ash/login/screens/pin_setup_screen.h b/chrome/browser/ash/login/screens/pin_setup_screen.h
index b29738e..cc8f30e8 100644
--- a/chrome/browser/ash/login/screens/pin_setup_screen.h
+++ b/chrome/browser/ash/login/screens/pin_setup_screen.h
@@ -78,8 +78,8 @@
// immediately.
std::optional<bool> has_login_support_;
- // Wether PIN is being offered as the main sign-in factor (PIN-only OOBE).
- bool using_pin_as_main_factor_ = false;
+ // Whether PIN is being offered as the main sign-in factor (PIN-only OOBE).
+ std::optional<bool> using_pin_as_main_factor_;
base::WeakPtr<PinSetupScreenView> view_;
ScreenExitCallback exit_callback_;
diff --git a/chrome/browser/ash/login/screens/pin_setup_screen_browsertest.cc b/chrome/browser/ash/login/screens/pin_setup_screen_browsertest.cc
index 4ffab65..a0f5717 100644
--- a/chrome/browser/ash/login/screens/pin_setup_screen_browsertest.cc
+++ b/chrome/browser/ash/login/screens/pin_setup_screen_browsertest.cc
@@ -10,12 +10,15 @@
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
+#include "ash/constants/ash_switches.h"
#include "ash/public/cpp/test/shell_test_api.h"
#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/metrics/histogram_base.h"
#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_command_line.h"
+#include "base/test/scoped_feature_list.h"
#include "chrome/browser/ash/login/screen_manager.h"
#include "chrome/browser/ash/login/test/cryptohome_mixin.h"
#include "chrome/browser/ash/login/test/js_checker.h"
@@ -27,19 +30,22 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/ash/login/login_display_host.h"
#include "chrome/browser/ui/webui/ash/login/pin_setup_screen_handler.h"
+#include "chrome/grit/generated_resources.h"
#include "chromeos/ash/components/cryptohome/constants.h"
#include "chromeos/ash/components/dbus/userdataauth/fake_userdataauth_client.h"
#include "chromeos/ash/components/osauth/public/auth_session_storage.h"
#include "components/user_manager/user_type.h"
#include "content/public/test/browser_test.h"
#include "testing/gmock/include/gmock/gmock.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/chromeos/devicetype_utils.h"
namespace ash {
namespace {
using ::testing::ElementsAre;
-constexpr char kPinSetupScreen[] = "pin-setup";
+constexpr auto* kPinSetupScreen = PinSetupScreenView::kScreenId.name;
constexpr char kPinSetupScreenCompletionTime[] =
"OOBE.StepCompletionTime.Pin-setup";
constexpr char kPinSetupScreenCompletionTimeByExitReason[] =
@@ -51,9 +57,13 @@
const test::UIPath kBackButton = {kPinSetupScreen, "backButton"};
const test::UIPath kNextButton = {kPinSetupScreen, "nextButton"};
const test::UIPath kSkipButton = {kPinSetupScreen, "setupSkipButton"};
+const test::UIPath kSkipButtonCore = {kPinSetupScreen, "setupSkipButton",
+ "button"};
const test::UIPath kDoneButton = {kPinSetupScreen, "doneButton"};
const test::UIPath kPinKeyboardInput = {kPinSetupScreen, "pinKeyboard",
"pinKeyboard", "pinInput"};
+const test::UIPath kSetupTitle = {kPinSetupScreen, "setupTitle"};
+const test::UIPath kSetupSubtitle = {kPinSetupScreen, "setupSubtitle"};
enum class PinPolicy {
kUnlock,
@@ -227,6 +237,24 @@
}
}
+ void WaitForSetupTitleAndSubtitle(int title_msg_id,
+ int subtitle_msg_id,
+ bool subtitle_has_device_name = false) {
+ auto expected_title = l10n_util::GetStringUTF8(title_msg_id);
+ auto expected_subtitle =
+ subtitle_has_device_name
+ ? l10n_util::GetStringFUTF8(subtitle_msg_id,
+ ui::GetChromeOSDeviceName())
+ : l10n_util::GetStringUTF8(subtitle_msg_id);
+
+ test::OobeJS()
+ .CreateElementTextContentWaiter(expected_title, kSetupTitle)
+ ->Wait();
+ test::OobeJS()
+ .CreateElementTextContentWaiter(expected_subtitle, kSetupSubtitle)
+ ->Wait();
+ }
+
std::optional<PinSetupScreen::Result> screen_result_;
base::HistogramTester histogram_tester_;
bool screen_exited_ = false;
@@ -374,6 +402,18 @@
ExpectUserActionMetric(PinSetupScreen::UserAction::kDoneButtonClicked);
}
+// Ensures the correct strings when PIN is being offered not as the main factor.
+IN_PROC_BROWSER_TEST_F(PinSetupScreenTest,
+ CorrectStringsWhenPinIsNotTheMainFactor) {
+ ShowPinSetupScreen();
+ WaitForScreenShown();
+
+ WaitForSetupTitleAndSubtitle(IDS_DISCOVER_PIN_SETUP_TITLE1,
+ IDS_DISCOVER_PIN_SETUP_SUBTITLE1);
+ test::OobeJS().ExpectElementText(
+ l10n_util::GetStringUTF8(IDS_DISCOVER_PIN_SETUP_SKIP), kSkipButtonCore);
+}
+
// Fixture to pretend that hardware support for login is not available.
class PinSetupScreenTestWithoutLoginSupport : public PinSetupScreenTest {
public:
@@ -416,4 +456,39 @@
ExpectExitResultAndMetric(PinSetupScreen::Result::kUserSkip);
}
+class PinSetupScreenTestAsMainFactor : public PinSetupScreenTest {
+ public:
+ PinSetupScreenTestAsMainFactor() {
+ SetHardwareSupport(true);
+ scoped_feature_list_.InitWithFeatures(
+ /*enabled_features=*/{ash::features::kAllowPasswordlessSetup},
+ /*disabled_features=*/{});
+ scoped_command_line_.GetProcessCommandLine()->AppendSwitch(
+ ash::switches::kOobeEnablePinOnlyPrototype);
+ }
+
+ ~PinSetupScreenTestAsMainFactor() override = default;
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+ base::test::ScopedCommandLine scoped_command_line_;
+};
+
+// Tests that the strings are correct when setting up PIN as the main factor.
+IN_PROC_BROWSER_TEST_F(PinSetupScreenTestAsMainFactor,
+ TitleAndSubtitleStrings) {
+ ShowPinSetupScreen();
+ WaitForScreenShown();
+
+ WaitForSetupTitleAndSubtitle(
+ IDS_DISCOVER_PIN_SETUP_PIN_AS_MAIN_FACTOR_TITLE,
+ IDS_DISCOVER_PIN_SETUP_PIN_AS_MAIN_FACTOR_SUBTITLE,
+ /*subtitle_has_device_name=*/true);
+
+ // Check that the 'Skip' button shows 'Use password instead'
+ test::OobeJS().ExpectElementText(
+ l10n_util::GetStringUTF8(IDS_DISCOVER_PIN_SETUP_PIN_AS_MAIN_FACTOR_SKIP),
+ kSkipButtonCore);
+}
+
} // namespace ash
diff --git a/chrome/browser/resources/chromeos/login/screens/osauth/pin_setup.html b/chrome/browser/resources/chromeos/login/screens/osauth/pin_setup.html
index a29105ec..ccfddd55 100644
--- a/chrome/browser/resources/chromeos/login/screens/osauth/pin_setup.html
+++ b/chrome/browser/resources/chromeos/login/screens/osauth/pin_setup.html
@@ -13,7 +13,7 @@
<oobe-adaptive-dialog id="setup" role="dialog" for-step="start,confirm">
<iron-icon slot="icon" icon="oobe-32:lock"></iron-icon>
- <h1 slot="title" for-step="start" aria-live="polite">
+ <h1 id="setupTitle" slot="title" for-step="start" aria-live="polite">
<div hidden="[[!usingPinAsMainSignInFactor]]">
[[i18nDynamic(locale, 'discoverPinSetupPinAsMainFactorTitle')]]
</div>
@@ -34,7 +34,7 @@
[[i18nDynamic(locale, 'discoverPinSetupTitle2ForChild')]]
</div>
</h1>
- <div slot="subtitle" for-step="start">
+ <div id="setupSubtitle" slot="subtitle" for-step="start">
<div hidden="[[!usingPinAsMainSignInFactor]]">
[[i18nDynamic(locale, 'discoverPinSetupPinAsMainFactorSubtitle')]]
</div>