Chrome OS - Minor clean up in LoginAuthUserView

LoginAuthUserView::SetAuthMethods has been misusing its AuthMethods
enum to control the visibility of the pin pad on the lock/login
screen.

Change the code so that it properly uses AuthMethods to indicate if
PIN authentication is possible and use an extra parameter to control
the visibility of the pin pad.

Bug: 1075994
Change-Id: I8f1a2eea009dc0d922f02bc9ced3f7005f773b83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328860Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Commit-Queue: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/heads/master@{#793606}

ash/login/ui/lock_contents_view.cc

@@ -1773,6 +1773,7 @@ void LockContentsView::LayoutAuth(LoginBigUserView* to_update,
       UserState* state = FindStateForUser(
           view->auth_user()->current_user().basic_user_info.account_id);
       uint32_t to_update_auth;
+      bool show_pinpad = false;
       if (state->force_online_sign_in) {
         to_update_auth = LoginAuthUserView::AUTH_ONLINE_SIGN_IN;
       } else if (state->disable_auth) {
@@ -1789,16 +1790,16 @@ void LockContentsView::LayoutAuth(LoginBigUserView* to_update,
         // not interfere with PIN entry.
         const bool is_keyboard_visible_for_view =
             GetKeyboardControllerForView() ? keyboard_shown_ : false;
-        if ((state->show_pin || state->show_pin_pad_for_password) &&
-            !is_keyboard_visible_for_view) {
+        show_pinpad = !is_keyboard_visible_for_view &&
+                      (state->show_pin || state->show_pin_pad_for_password);
+        if (state->show_pin)
           to_update_auth |= LoginAuthUserView::AUTH_PIN;
-        }
         if (state->enable_tap_auth)
           to_update_auth |= LoginAuthUserView::AUTH_TAP;
         if (state->fingerprint_state != FingerprintState::UNAVAILABLE)
           to_update_auth |= LoginAuthUserView::AUTH_FINGERPRINT;
       }
-      view->auth_user()->SetAuthMethods(to_update_auth, state->show_pin);
+      view->auth_user()->SetAuthMethods(to_update_auth, show_pinpad);
     } else if (view->public_account()) {
       view->public_account()->SetAuthEnabled(true /*enabled*/, animate);
     }
@@ -1809,7 +1810,7 @@ void LockContentsView::LayoutAuth(LoginBigUserView* to_update,
       return;
     if (view->auth_user()) {
       view->auth_user()->SetAuthMethods(LoginAuthUserView::AUTH_NONE,
-                                        false /*can_use_pin*/);
+                                        false /*show_pinpad*/);
     } else if (view->public_account()) {
       view->public_account()->SetAuthEnabled(false /*enabled*/, animate);
     }

ash/login/ui/login_auth_user_view.cc

@@ -745,16 +745,14 @@ struct LoginAuthUserView::AnimationState {
     non_pin_y_start_in_screen = view->GetBoundsInScreen().y();
     pin_start_in_screen = view->pin_view_->GetBoundsInScreen().origin();
 
-    had_pin = (view->auth_methods() & LoginAuthUserView::AUTH_PIN) != 0;
-    had_password =
-        (view->auth_methods() & LoginAuthUserView::AUTH_PASSWORD) != 0;
-    had_fingerprint =
-        (view->auth_methods() & LoginAuthUserView::AUTH_FINGERPRINT) != 0;
+    had_pinpad = view->show_pinpad_;
+    had_password = view->HasAuthMethod(LoginAuthUserView::AUTH_PASSWORD);
+    had_fingerprint = view->HasAuthMethod(LoginAuthUserView::AUTH_FINGERPRINT);
   }
 
   int non_pin_y_start_in_screen = 0;
   gfx::Point pin_start_in_screen;
-  bool had_pin = false;
+  bool had_pinpad = false;
   bool had_password = false;
   bool had_fingerprint = false;
 };
@@ -980,20 +978,20 @@ LoginAuthUserView::LoginAuthUserView(const LoginUserInfo& user,
   add_padding(kDistanceFromPinKeyboardToBigUserViewBottomDp);
 
   // Update authentication UI.
-  SetAuthMethods(auth_methods_, false /*can_use_pin*/);
+  SetAuthMethods(auth_methods_, false /*show_pinpad*/);
   user_view_->UpdateForUser(user, false /*animate*/);
 }
 
 LoginAuthUserView::~LoginAuthUserView() = default;
 
 void LoginAuthUserView::SetAuthMethods(uint32_t auth_methods,
-                                       bool can_use_pin) {
-  can_use_pin_ = can_use_pin;
+                                       bool show_pinpad) {
+  show_pinpad_ = show_pinpad;
   bool had_password = HasAuthMethod(AUTH_PASSWORD);
 
   auth_methods_ = static_cast<AuthMethods>(auth_methods);
   bool has_password = HasAuthMethod(AUTH_PASSWORD);
-  bool has_pin_pad = HasAuthMethod(AUTH_PIN);
+  bool has_pin = HasAuthMethod(AUTH_PIN);
   bool has_tap = HasAuthMethod(AUTH_TAP);
   bool force_online_sign_in = HasAuthMethod(AUTH_ONLINE_SIGN_IN);
   bool has_fingerprint = HasAuthMethod(AUTH_FINGERPRINT);
@@ -1010,7 +1008,7 @@ void LoginAuthUserView::SetAuthMethods(uint32_t auth_methods,
   // Adjust the PIN keyboard visibility before the password textfield's one, so
   // that when both are about to be hidden the focus doesn't jump to the "1"
   // keyboard button, causing unexpected accessibility effects.
-  pin_view_->SetVisible(has_pin_pad);
+  pin_view_->SetVisible(show_pinpad);
 
   password_view_->SetEnabled(has_password);
   password_view_->SetEnabledOnEmptyPassword(has_tap);
@@ -1023,25 +1021,25 @@ void LoginAuthUserView::SetAuthMethods(uint32_t auth_methods,
     password_view_->RequestFocus();
 
   fingerprint_view_->SetVisible(has_fingerprint);
-  fingerprint_view_->SetCanUsePin(can_use_pin);
+  fingerprint_view_->SetCanUsePin(has_pin);
   challenge_response_view_->SetVisible(has_challenge_response);
 
   int padding_view_height = kDistanceBetweenPasswordFieldAndPinKeyboardDp;
-  if (has_fingerprint && !has_pin_pad) {
+  if (has_fingerprint && !show_pinpad) {
     padding_view_height = kDistanceBetweenPasswordFieldAndFingerprintViewDp;
-  } else if (has_challenge_response && !has_pin_pad) {
+  } else if (has_challenge_response && !show_pinpad) {
     padding_view_height =
         kDistanceBetweenPasswordFieldAndChallengeResponseViewDp;
   }
   padding_below_password_view_->SetPreferredSize(
       gfx::Size(kNonEmptyWidthDp, padding_view_height));
 
-  // Note: |has_tap| must have higher priority than |has_pin_pad| when
+  // Note: |has_tap| must have higher priority than |has_pin| when
   // determining the placeholder.
   if (has_tap) {
     password_view_->SetPlaceholderText(
         l10n_util::GetStringUTF16(IDS_ASH_LOGIN_POD_PASSWORD_TAP_PLACEHOLDER));
-  } else if (can_use_pin) {
+  } else if (has_pin) {
     password_view_->SetPlaceholderText(
         l10n_util::GetStringUTF16(IDS_ASH_LOGIN_POD_PASSWORD_PIN_PLACEHOLDER));
   } else {
@@ -1107,9 +1105,9 @@ void LoginAuthUserView::CaptureStateForAnimationPreLayout() {
 void LoginAuthUserView::ApplyAnimationPostLayout() {
   DCHECK(cached_animation_state_);
 
-  bool has_password = (auth_methods() & AUTH_PASSWORD) != 0;
-  bool has_pin = (auth_methods() & AUTH_PIN) != 0;
-  bool has_fingerprint = (auth_methods() & AUTH_FINGERPRINT) != 0;
+  bool has_password = HasAuthMethod(AUTH_PASSWORD);
+  bool has_pinpad = show_pinpad_;
+  bool has_fingerprint = HasAuthMethod(AUTH_FINGERPRINT);
 
   ////////
   // Animate the user info (ie, icon, name) up or down the screen.
@@ -1169,8 +1167,8 @@ void LoginAuthUserView::ApplyAnimationPostLayout() {
   ////////
   // Grow/shrink the PIN keyboard if it is being hidden or shown.
 
-  if (cached_animation_state_->had_pin != has_pin) {
-    if (!has_pin) {
+  if (cached_animation_state_->had_pinpad != has_pinpad) {
+    if (!has_pinpad) {
       gfx::Point pin_end_in_screen = pin_view_->GetBoundsInScreen().origin();
       gfx::Rect pin_bounds = pin_view_->bounds();
       pin_bounds.set_x(cached_animation_state_->pin_start_in_screen.x() -
@@ -1185,7 +1183,7 @@ void LoginAuthUserView::ApplyAnimationPostLayout() {
     }
 
     auto transition = std::make_unique<PinKeyboardAnimation>(
-        has_pin /*grow*/, pin_view_->height(),
+        has_pinpad /*grow*/, pin_view_->height(),
         // TODO(https://crbug.com/955119): Implement proper animation.
         base::TimeDelta::FromMilliseconds(
             login_constants::kChangeUserAnimationDurationMs / 2.0f),
@@ -1193,7 +1191,7 @@ void LoginAuthUserView::ApplyAnimationPostLayout() {
     auto* sequence = new ui::LayerAnimationSequence(std::move(transition));
 
     // Hide the PIN keyboard after animation if needed.
-    if (!has_pin) {
+    if (!has_pinpad) {
       auto* observer = BuildObserverToHideView(pin_view_);
       sequence->AddObserver(observer);
       observer->SetActive();
@@ -1291,7 +1289,7 @@ void LoginAuthUserView::OnAuthSubmit(const base::string16& password) {
   password_view_->SetReadOnly(true);
   Shell::Get()->login_screen_controller()->AuthenticateUserWithPasswordOrPin(
       current_user().basic_user_info.account_id, base::UTF16ToUTF8(password),
-      can_use_pin_,
+      HasAuthMethod(AUTH_PIN),
       base::BindOnce(&LoginAuthUserView::OnAuthComplete,
                      weak_factory_.GetWeakPtr()));
 }

ash/login/ui/login_auth_user_view.h

@@ -104,9 +104,9 @@ class ASH_EXPORT LoginAuthUserView : public NonAccessibleView,
   ~LoginAuthUserView() override;
 
   // Set the displayed set of auth methods. |auth_methods| contains or-ed
-  // together AuthMethod values. |can_use_pin| should be true if the user can
-  // authenticate using PIN, even if the PIN keyboard is not displayed.
-  void SetAuthMethods(uint32_t auth_methods, bool can_use_pin);
+  // together AuthMethod values. |show_pinpad| determines whether the pin pad
+  // should be visible.
+  void SetAuthMethods(uint32_t auth_methods, bool show_pinpad);
   AuthMethods auth_methods() const { return auth_methods_; }
 
   // Add an easy unlock icon.
@@ -184,10 +184,9 @@ class ASH_EXPORT LoginAuthUserView : public NonAccessibleView,
   void AttemptAuthenticateWithChallengeResponse();
 
   AuthMethods auth_methods_ = AUTH_NONE;
-  // True if the user's password might be a PIN. PIN is hashed differently from
-  // password. The PIN keyboard may not always be visible even when the user
-  // wants to submit a PIN, eg. the virtual keyboard hides the PIN keyboard.
-  bool can_use_pin_ = false;
+
+  // Whether to show the pinpad. Sometimes hidden by the on screen keyboard
+  bool show_pinpad_ = false;
   LoginUserView* user_view_ = nullptr;
   LoginPasswordView* password_view_ = nullptr;
   NonAccessibleView* password_view_container_ = nullptr;

ash/login/ui/login_auth_user_view_unittest.cc

@@ -57,9 +57,8 @@ class LoginAuthUserViewUnittest : public LoginTestBase {
     SetWidget(CreateWidgetWithContent(container_));
   }
 
-  void SetAuthMethods(uint32_t auth_methods) {
-    bool can_use_pin = (auth_methods & LoginAuthUserView::AUTH_PIN) != 0;
-    view_->SetAuthMethods(auth_methods, can_use_pin);
+  void SetAuthMethods(uint32_t auth_methods, bool show_pinpad) {
+    view_->SetAuthMethods(auth_methods, show_pinpad);
   }
 
   LoginUserInfo user_;
@@ -75,7 +74,7 @@ class LoginAuthUserViewUnittest : public LoginTestBase {
 // Verifies showing the PIN keyboard makes the user view grow.
 TEST_F(LoginAuthUserViewUnittest, ShowingPinExpandsView) {
   gfx::Size start_size = view_->size();
-  SetAuthMethods(LoginAuthUserView::AUTH_PIN);
+  SetAuthMethods(LoginAuthUserView::AUTH_PIN, true /*show_pinpad*/);
   container_->Layout();
   gfx::Size expanded_size = view_->size();
   EXPECT_GT(expanded_size.height(), start_size.height());
@@ -95,11 +94,11 @@ TEST_F(LoginAuthUserViewUnittest, ShowingPasswordForcesOpaque) {
   EXPECT_FALSE(auth_test.user_view()->HasFocus());
 
   // If the user view is showing a password it must be opaque.
-  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD);
+  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD, false /*show_pinpad*/);
   EXPECT_TRUE(user_test.is_opaque());
-  SetAuthMethods(LoginAuthUserView::AUTH_NONE);
+  SetAuthMethods(LoginAuthUserView::AUTH_NONE, false /*show_pinpad*/);
   EXPECT_FALSE(user_test.is_opaque());
-  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD);
+  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD, false /*show_pinpad*/);
   EXPECT_TRUE(user_test.is_opaque());
 }
 
@@ -119,8 +118,8 @@ TEST_F(LoginAuthUserViewUnittest, PressReturnWithTapToUnlockEnabled) {
   EXPECT_CALL(*client,
               AuthenticateUserWithEasyUnlock(
                   user_view->current_user().basic_user_info.account_id));
-  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD |
-                 LoginAuthUserView::AUTH_TAP);
+  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD | LoginAuthUserView::AUTH_TAP,
+                 false /*show_pinpad*/);
   password_view->Clear();
 
   generator->PressKey(ui::KeyboardCode::VKEY_RETURN, 0);
@@ -138,7 +137,7 @@ TEST_F(LoginAuthUserViewUnittest, OnlineSignInMessage) {
 
   // When auth method is |AUTH_ONLINE_SIGN_IN|, the online sign-in message is
   // visible. The password field and PIN keyboard are invisible.
-  SetAuthMethods(LoginAuthUserView::AUTH_ONLINE_SIGN_IN);
+  SetAuthMethods(LoginAuthUserView::AUTH_ONLINE_SIGN_IN, false /*show_pinpad*/);
   EXPECT_TRUE(online_sign_in_message->GetVisible());
   EXPECT_FALSE(password_view->GetVisible());
   EXPECT_FALSE(pin_view->GetVisible());
@@ -153,13 +152,13 @@ TEST_F(LoginAuthUserViewUnittest, OnlineSignInMessage) {
   base::RunLoop().RunUntilIdle();
 
   // The online sign-in message is invisible for all other auth methods.
-  SetAuthMethods(LoginAuthUserView::AUTH_NONE);
+  SetAuthMethods(LoginAuthUserView::AUTH_NONE, false /*show_pinpad*/);
   EXPECT_FALSE(online_sign_in_message->GetVisible());
-  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD);
+  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD, false /*show_pinpad*/);
   EXPECT_FALSE(online_sign_in_message->GetVisible());
-  SetAuthMethods(LoginAuthUserView::AUTH_PIN);
+  SetAuthMethods(LoginAuthUserView::AUTH_PIN, true /*show_pinpad*/);
   EXPECT_FALSE(online_sign_in_message->GetVisible());
-  SetAuthMethods(LoginAuthUserView::AUTH_TAP);
+  SetAuthMethods(LoginAuthUserView::AUTH_TAP, false /*show_pinpad*/);
   EXPECT_FALSE(online_sign_in_message->GetVisible());
 }
 
@@ -172,20 +171,20 @@ TEST_F(LoginAuthUserViewUnittest,
   };
 
   // Set a password.
-  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD);
+  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD, false /*show_pinpad*/);
   password_test.textfield()->SetText(base::ASCIIToUTF16("Hello"));
 
   // Enable some other auth method (PIN), password is not cleared.
   view_->CaptureStateForAnimationPreLayout();
-  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD |
-                 LoginAuthUserView::AUTH_PIN);
+  SetAuthMethods(LoginAuthUserView::AUTH_PASSWORD | LoginAuthUserView::AUTH_PIN,
+                 true /*show_pinpad*/);
   EXPECT_TRUE(has_password());
   view_->ApplyAnimationPostLayout();
   EXPECT_TRUE(has_password());
 
   // Disable password, password is cleared.
   view_->CaptureStateForAnimationPreLayout();
-  SetAuthMethods(LoginAuthUserView::AUTH_NONE);
+  SetAuthMethods(LoginAuthUserView::AUTH_NONE, false /*show_pinpad*/);
   EXPECT_TRUE(has_password());
   view_->ApplyAnimationPostLayout();
   EXPECT_FALSE(has_password());