Add Back button into security token PIN UI

Add the "BACK" button into the PIN keypad displayed on a user pod that
use the challenge-response authentication (a.k.a. smart card login).

Clicking this button allows to exit the PIN keypad and abort the
challenge-response authentication.

Before this CL, there was no way to abort the authentication besides
waiting some internal timeout (e.g., a few minutes for D-Bus calls) is
hit.

Bug: 983103
Test: start existing challenge-response user sign-in, check that the PIN keypad with the BACK button is shown, click BACK, check that the PIN keypad disappears
Change-Id: Ideb451820b40578f06e98db97fb259473118b047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1784616
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693686}

ash/ash_strings.grd

@@ -1575,6 +1575,9 @@ This file contains the strings for ash.
       <message name="IDS_ASH_LOGIN_START_SMART_CARD_AUTH_BUTTON_ACCESSIBLE_NAME" desc="Text to be spoken when the focus is set to the button that starts the smart card based user authentication.">
         Sign in with smart card
       </message>
+      <message name="IDS_ASH_PIN_KEYBOARD_BACK_BUTTON" desc="Text shown on the action button of the PIN keyboard that cancels the request and goes back to the login screen UI with no PIN keyboard. NOTE: The text must be short, in order to fit into the button boundaries.">
+        BACK
+      </message>
       <message name="IDS_ASH_PIN_KEYBOARD_DELETE_ACCESSIBLE_NAME" desc="Text to be spoken when the focus is set to the delete button of the PIN keyboard.">
         Delete
       </message>

ash/ash_strings_grd/IDS_ASH_PIN_KEYBOARD_BACK_BUTTON.png.sha1

+62a945d68e3c09d40a2e72fcc956613aeb55eaea
\ No newline at end of file

ash/login/ui/login_auth_user_view.cc

@@ -754,7 +754,9 @@ LoginAuthUserView::LoginAuthUserView(const LoginUserInfo& user,
       base::BindRepeating(&LoginPasswordView::InsertNumber,
                           base::Unretained(password_view.get())),
       base::BindRepeating(&LoginPasswordView::Backspace,
-                          base::Unretained(password_view.get())));
+                          base::Unretained(password_view.get())),
+      base::BindRepeating(&LoginAuthUserView::OnPinBack,
+                          base::Unretained(this)));
   pin_view_ = pin_view.get();
   DCHECK(pin_view_->layer());
 
@@ -953,6 +955,8 @@ void LoginAuthUserView::SetAuthMethods(uint32_t auth_methods,
   // keyboard button, causing unexpected accessibility effects.
   pin_view_->SetVisible(has_pin);
 
+  pin_view_->SetBackButtonVisible(security_token_pin_request_.has_value());
+
   password_view_->SetEnabled(has_password);
   password_view_->SetEnabledOnEmptyPassword(has_tap);
   password_view_->SetFocusEnabledForChildViews(has_password);
@@ -1338,6 +1342,14 @@ void LoginAuthUserView::OnOnlineSignInMessageTap() {
       true /*can_close*/, current_user().basic_user_info.account_id);
 }
 
+void LoginAuthUserView::OnPinBack() {
+  // Exiting from the PIN keyboard during a security token PIN request should
+  // abort it. Note that the back button isn't shown when the PIN view is used
+  // in other contexts.
+  DCHECK(security_token_pin_request_);
+  ClearSecurityTokenPinRequest();
+}
+
 bool LoginAuthUserView::HasAuthMethod(AuthMethods auth_method) const {
   return (auth_methods_ & auth_method) != 0;
 }

ash/login/ui/login_auth_user_view.h

@@ -184,6 +184,9 @@ class ASH_EXPORT LoginAuthUserView
   // Called when the online sign-in message is tapped. It opens the Gaia screen.
   void OnOnlineSignInMessageTap();
 
+  // Called when the user presses the back button of the PIN keyboard.
+  void OnPinBack();
+
   // Helper method to check if an auth method is enable. Use it like this:
   // bool has_tap = HasAuthMethod(AUTH_TAP).
   bool HasAuthMethod(AuthMethods auth_method) const;

ash/login/ui/login_pin_view.cc

@@ -28,6 +28,7 @@
 #include "ui/views/animation/ink_drop_mask.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/layout/box_layout.h"
+#include "ui/views/layout/fill_layout.h"
 #include "ui/views/painter.h"
 
 namespace ash {
@@ -370,6 +371,31 @@ class LoginPinView::BackspacePinButton : public BasePinButton {
   DISALLOW_COPY_AND_ASSIGN(BackspacePinButton);
 };
 
+// A PIN button with the label "back".
+class LoginPinView::BackButton : public BasePinButton {
+ public:
+  BackButton(const gfx::Size& size, const base::RepeatingClosure& on_press)
+      : BasePinButton(size,
+                      l10n_util::GetStringUTF16(
+                          IDS_ASH_LOGIN_BACK_BUTTON_ACCESSIBLE_NAME),
+                      on_press) {
+    const gfx::FontList& base_font_list = views::Label::GetDefaultFontList();
+    views::Label* label = AddChildView(std::make_unique<views::Label>(
+        l10n_util::GetStringUTF16(IDS_ASH_PIN_KEYBOARD_BACK_BUTTON),
+        views::style::CONTEXT_BUTTON, views::style::STYLE_PRIMARY));
+    label->SetEnabledColor(login_constants::kButtonEnabledColor);
+    label->SetAutoColorReadabilityEnabled(false);
+    label->SetSubpixelRenderingEnabled(false);
+    label->SetFontList(base_font_list.Derive(-3, gfx::Font::FontStyle::NORMAL,
+                                             gfx::Font::Weight::MEDIUM));
+  }
+
+  ~BackButton() override = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BackButton);
+};
+
 // static
 gfx::Size LoginPinView::TestApi::GetButtonSize(Style style) {
   return gfx::Size(kButtonWidthDp, style == Style::kNumeric
@@ -389,6 +415,10 @@ views::View* LoginPinView::TestApi::GetBackspaceButton() const {
   return view_->backspace_;
 }
 
+views::View* LoginPinView::TestApi::GetBackButton() const {
+  return view_->back_button_;
+}
+
 void LoginPinView::TestApi::SetBackspaceTimers(
     std::unique_ptr<base::OneShotTimer> delay_timer,
     std::unique_ptr<base::RepeatingTimer> repeat_timer) {
@@ -398,10 +428,12 @@ void LoginPinView::TestApi::SetBackspaceTimers(
 
 LoginPinView::LoginPinView(Style keyboard_style,
                            const OnPinKey& on_key,
-                           const OnPinBackspace& on_backspace)
+                           const OnPinBackspace& on_backspace,
+                           const OnPinBack& on_back)
     : NonAccessibleView(kLoginPinViewClassName),
       on_key_(on_key),
-      on_backspace_(on_backspace) {
+      on_backspace_(on_backspace),
+      on_back_(on_back) {
   DCHECK(on_key_);
   DCHECK(on_backspace_);
 
@@ -430,6 +462,15 @@ LoginPinView::LoginPinView(Style keyboard_style,
         new DigitPinButton(value, show_letters, button_size, on_key_));
   };
 
+  // Wrap the back button view with a container having the fill layout, so that
+  // it consumes the same amount of space even when the button is hidden.
+  auto back_button_container = std::make_unique<NonAccessibleView>();
+  back_button_container->SetLayoutManager(
+      std::make_unique<views::FillLayout>());
+  back_button_ = back_button_container->AddChildView(
+      std::make_unique<BackButton>(button_size, on_back_));
+  back_button_->SetVisible(false);
+
   // 1-2-3
   auto* row = build_and_add_row();
   add_digit_button(row, 1);
@@ -448,11 +489,9 @@ LoginPinView::LoginPinView(Style keyboard_style,
   add_digit_button(row, 8);
   add_digit_button(row, 9);
 
-  // 0-backspace
+  // back-0-backspace
   row = build_and_add_row();
-  auto* spacer = new NonAccessibleView();
-  spacer->SetPreferredSize(button_size);
-  row->AddChildView(spacer);
+  row->AddChildView(std::move(back_button_container));
   add_digit_button(row, 0);
   backspace_ = new BackspacePinButton(button_size, on_backspace_);
   row->AddChildView(backspace_);
@@ -460,6 +499,10 @@ LoginPinView::LoginPinView(Style keyboard_style,
 
 LoginPinView::~LoginPinView() = default;
 
+void LoginPinView::SetBackButtonVisible(bool visible) {
+  back_button_->SetVisible(visible);
+}
+
 void LoginPinView::OnPasswordTextChanged(bool is_empty) {
   // Disabling the backspace button will make it lose focus. The previous
   // focusable view is a button in PIN keyboard, which is slightly more expected

ash/login/ui/login_pin_view.h

@@ -39,10 +39,12 @@ namespace ash {
 //   |   7   |  |   8   |  |   9   |
 //   |P Q R S|  | T U V |  |W X Y Z|
 //    -------    -------    -------
-//               _______    _______
-//              |   0   |  |  <-   |
-//              |   +   |  |       |
-//               -------    -------
+//    _______    _______    _______
+//   |  BACK |  |   0   |  |  <-   |
+//   |       |  |   +   |  |       |
+//    -------    -------    -------
+//
+// The "BACK" button is hidden by default.
 //
 class ASH_EXPORT LoginPinView : public NonAccessibleView {
  public:
@@ -66,6 +68,7 @@ class ASH_EXPORT LoginPinView : public NonAccessibleView {
 
     views::View* GetButton(int number) const;
     views::View* GetBackspaceButton() const;
+    views::View* GetBackButton() const;
 
     // Sets the timers that are used for backspace auto-submit. |delay_timer| is
     // the initial delay before an auto-submit, and |repeat_timer| fires
@@ -79,25 +82,35 @@ class ASH_EXPORT LoginPinView : public NonAccessibleView {
 
   using OnPinKey = base::RepeatingCallback<void(int value)>;
   using OnPinBackspace = base::RepeatingClosure;
+  using OnPinBack = base::RepeatingClosure;
 
   // Creates PIN view with the specified |keyboard_style|.
-  // |on_key| is called whenever the user taps one of the pin buttons.
+  // |on_key| is called whenever the user taps one of the pin buttons; must be
+  // non-null.
   // |on_backspace| is called when the user wants to erase the most recently
-  // tapped key. Neither callback can be null.
-  explicit LoginPinView(Style keyboard_style,
-                        const OnPinKey& on_key,
-                        const OnPinBackspace& on_backspace);
+  // tapped key; must be non-null.
+  // |on_back| is called when the user taps the back button; must be non-null
+  // if the back button is shown.
+  LoginPinView(Style keyboard_style,
+               const OnPinKey& on_key,
+               const OnPinBackspace& on_backspace,
+               const OnPinBack& on_back);
   ~LoginPinView() override;
 
+  void SetBackButtonVisible(bool visible);
+
   // Called when the password field text changed.
   void OnPasswordTextChanged(bool is_empty);
 
  private:
+  class BackButton;
   class BackspacePinButton;
 
+  BackButton* back_button_;
   BackspacePinButton* backspace_;
   OnPinKey on_key_;
   OnPinBackspace on_backspace_;
+  OnPinBack on_back_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginPinView);
 };

ash/login/ui/login_pin_view_unittest.cc

@@ -33,6 +33,8 @@ class LoginPinViewTest : public LoginTestBase {
                          base::BindRepeating(&LoginPinViewTest::OnPinKey,
                                              base::Unretained(this)),
                          base::BindRepeating(&LoginPinViewTest::OnPinBackspace,
+                                             base::Unretained(this)),
+                         base::BindRepeating(&LoginPinViewTest::OnPinBack,
                                              base::Unretained(this)));
 
     SetWidget(CreateWidgetWithContent(view_));
@@ -41,11 +43,14 @@ class LoginPinViewTest : public LoginTestBase {
   // Called when a password is submitted.
   void OnPinKey(int value) { value_ = value; }
   void OnPinBackspace() { ++backspace_; }
+  void OnPinBack() { ++back_; }
 
   LoginPinView* view_ = nullptr;  // Owned by test widget view hierarchy.
   base::Optional<int> value_;
   // Number of times the backspace event has been fired.
   int backspace_ = 0;
+  // Number of times the back event has been fired.
+  int back_ = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LoginPinViewTest);
@@ -74,6 +79,8 @@ TEST_F(LoginPinViewTest, ButtonsFireEvents) {
   test_api.GetBackspaceButton()->RequestFocus();
   generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
   EXPECT_EQ(1, backspace_);
+
+  EXPECT_EQ(0, back_);
 }
 
 // Validates buttons have the correct spacing for alphanumeric PIN keyboard
@@ -216,4 +223,36 @@ TEST_F(LoginPinViewTest, BackspaceAutoSubmitsAndRepeats) {
   EXPECT_EQ(0, backspace_);
 }
 
+// Verifies that pressing Enter on the "back" button fires the corresponding
+// event.
+TEST_F(LoginPinViewTest, BackButtonEnter) {
+  CreateLoginPinViewWithStyle(LoginPinView::Style::kAlphanumeric);
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  LoginPinView::TestApi test_api(view_);
+
+  EXPECT_FALSE(test_api.GetBackButton()->GetVisible());
+
+  view_->SetBackButtonVisible(true);
+  test_api.GetBackButton()->RequestFocus();
+  EXPECT_EQ(0, back_);
+  generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
+  EXPECT_EQ(1, back_);
+}
+
+// Verifies that clicking on the "back" button fires the corresponding event.
+TEST_F(LoginPinViewTest, BackButtonClick) {
+  CreateLoginPinViewWithStyle(LoginPinView::Style::kAlphanumeric);
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  LoginPinView::TestApi test_api(view_);
+
+  EXPECT_FALSE(test_api.GetBackButton()->GetVisible());
+
+  view_->SetBackButtonVisible(true);
+  generator->MoveMouseTo(
+      test_api.GetBackButton()->GetBoundsInScreen().CenterPoint());
+  EXPECT_EQ(0, back_);
+  generator->PressLeftButton();
+  EXPECT_EQ(1, back_);
+}
+
 }  // namespace ash

ash/login/ui/parent_access_view.cc

@@ -623,12 +623,13 @@ ParentAccessView::ParentAccessView(const AccountId& account_id,
   add_spacer(kAccessCodeToPinKeyboardDistanceDp);
 
   // Pin keyboard.
-  pin_keyboard_view_ = new LoginPinView(
-      LoginPinView::Style::kNumeric,
-      base::BindRepeating(&AccessCodeInput::InsertDigit,
-                          base::Unretained(access_code_view_)),
-      base::BindRepeating(&AccessCodeInput::Backspace,
-                          base::Unretained(access_code_view_)));
+  pin_keyboard_view_ =
+      new LoginPinView(LoginPinView::Style::kNumeric,
+                       base::BindRepeating(&AccessCodeInput::InsertDigit,
+                                           base::Unretained(access_code_view_)),
+                       base::BindRepeating(&AccessCodeInput::Backspace,
+                                           base::Unretained(access_code_view_)),
+                       LoginPinView::OnPinBack());
   // Backspace key is always enabled and |access_code_| field handles it.
   pin_keyboard_view_->OnPasswordTextChanged(false);
   AddChildView(pin_keyboard_view_);