[SmartLock] Add SmartLockAuthModel, get basic flow working

Adds a new AuthFactorModel for Smart Lock, along with enough plumbing to get a basic authentication flow working. Does not handle error states or animations, and side-by-side display with Fingerprint will come later. Add an ArrowButtonView to LoginAuthFactorsView to handle the "click to enter" state. Also update icons and get rid of the arrow icon since it's identical to the existing arrow icon. This change is hidden behind the kSmartLockUIRevamp feature flag. Tests are WIP (crrev/c/3206472). Screenshot: https://screenshot.googleplex.com/8TsDMsQoUykm2u8.png Bug: 1233614 Change-Id: Idd440be1d38da45b921e6d59450f967956fceb41 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3205408 Commit-Queue: Curt Clemens <cclem@google.com> Reviewed-by: Thomas Tellier <tellier@google.com> Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Ryan Hansberry <hansberry@chromium.org> Cr-Commit-Position: refs/heads/main@{#931090}
marcosholgado · Oct 13, 2021 · bd24612 · bd24612
1 parent 217f533
commit bd24612
Show file tree

Hide file tree

Showing 20 changed files with 665 additions and 144 deletions.
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
@@ -643,6 +643,8 @@ component("ash") {
     "login/ui/public_account_monitoring_info_dialog.h",
     "login/ui/scrollable_users_list_view.cc",
     "login/ui/scrollable_users_list_view.h",
+    "login/ui/smart_lock_auth_model.cc",
+    "login/ui/smart_lock_auth_model.h",
     "login/ui/system_label_button.cc",
     "login/ui/system_label_button.h",
     "login/ui/user_switch_flip_animation.cc",
@@ -2320,6 +2322,7 @@ test("ash_unittests") {
     "login/ui/login_user_view_unittest.cc",
     "login/ui/note_action_launch_button_unittest.cc",
     "login/ui/pin_request_view_unittest.cc",
+    "login/ui/smart_lock_auth_model_unittest.cc",
     "login/ui/views_utils_unittest.cc",
     "marker/marker_controller_unittest.cc",
     "media/media_controller_unittest.cc",

diff --git a/ash/login/ui/auth_factor_model.h b/ash/login/ui/auth_factor_model.h
@@ -5,8 +5,7 @@
 #ifndef ASH_LOGIN_UI_AUTH_FACTOR_MODEL_H_
 #define ASH_LOGIN_UI_AUTH_FACTOR_MODEL_H_
 
-#include <string>
-
+#include "ash/ash_export.h"
 #include "base/callback.h"
 
 namespace ash {
@@ -25,7 +24,7 @@ int operator|(AuthFactorType type1, AuthFactorType type2);
 
 // Base class representing an auth factor. Used by LoginAuthFactorsView to
 // display a list of auth factors.
-class AuthFactorModel {
+class ASH_EXPORT AuthFactorModel {
  public:
   enum class AuthFactorState {
     // The feature is disabled, disallowed by policy, or requires
@@ -62,19 +61,20 @@ class AuthFactorModel {
 
   virtual AuthFactorType GetType() = 0;
 
-  // The label that should be shown in the current state.
-  virtual std::u16string GetLabel() = 0;
+  // The ID of the label that should be shown in the current state.
+  virtual int GetLabelId() = 0;
 
   // Controls whether the label is announced by Chromevox.
   virtual bool ShouldAnnounceLabel() = 0;
 
   // Alternative text to be provided to screen readers.
-  virtual std::u16string GetAccessibleName() = 0;
+  virtual int GetAccessibleNameId() = 0;
 
   // Update an AuthIconView to represent the current state of the auth factor.
   // Should call SetIcon() or set up an animation.
   virtual void UpdateIcon(AuthIconView* icon_view) = 0;
 
+  // This will be called when the auth factor view is tapped.
   virtual void OnTapEvent() {}
 
   // Set a callback that will be called by the AuthFactorModel whenever its

diff --git a/ash/login/ui/auth_icon_view.cc b/ash/login/ui/auth_icon_view.cc
@@ -12,7 +12,7 @@
 namespace ash {
 
 namespace {
-constexpr int kAuthIconSizeDp = 28;
+constexpr int kAuthIconSizeDp = 32;
 }
 
 AuthIconView::AuthIconView()

diff --git a/ash/login/ui/fingerprint_auth_model.cc b/ash/login/ui/fingerprint_auth_model.cc
@@ -10,7 +10,6 @@
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
-#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -80,31 +79,27 @@ AuthFactorType FingerprintAuthModel::GetType() {
   return AuthFactorType::kFingerprint;
 }
 
-std::u16string FingerprintAuthModel::GetLabel() {
-  auto get_displayed_id = [&]() {
-    if (auth_result_.has_value()) {
-      return auth_result_.value()
-                 ? IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_AUTH_SUCCESS
-                 : IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_AUTH_FAILED;
-    }
-
-    switch (state_) {
-      case FingerprintState::UNAVAILABLE:
-      case FingerprintState::AVAILABLE_DEFAULT:
-        return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_AVAILABLE;
-      case FingerprintState::AVAILABLE_WITH_TOUCH_SENSOR_WARNING:
-        return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_TOUCH_SENSOR;
-      case FingerprintState::DISABLED_FROM_ATTEMPTS:
-        return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_DISABLED_FROM_ATTEMPTS;
-      case FingerprintState::DISABLED_FROM_TIMEOUT:
-        if (can_use_pin_)
-          return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_PIN_OR_PASSWORD_REQUIRED;
-        return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_PASSWORD_REQUIRED;
-    }
-    NOTREACHED();
-  };
+int FingerprintAuthModel::GetLabelId() {
+  if (auth_result_.has_value()) {
+    return auth_result_.value() ? IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_AUTH_SUCCESS
+                                : IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_AUTH_FAILED;
+  }
 
-  return l10n_util::GetStringUTF16(get_displayed_id());
+  switch (state_) {
+    case FingerprintState::UNAVAILABLE:
+      FALLTHROUGH;
+    case FingerprintState::AVAILABLE_DEFAULT:
+      return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_AVAILABLE;
+    case FingerprintState::AVAILABLE_WITH_TOUCH_SENSOR_WARNING:
+      return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_TOUCH_SENSOR;
+    case FingerprintState::DISABLED_FROM_ATTEMPTS:
+      return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_DISABLED_FROM_ATTEMPTS;
+    case FingerprintState::DISABLED_FROM_TIMEOUT:
+      if (can_use_pin_)
+        return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_PIN_OR_PASSWORD_REQUIRED;
+      return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_PASSWORD_REQUIRED;
+  }
+  NOTREACHED();
 }
 
 bool FingerprintAuthModel::ShouldAnnounceLabel() {
@@ -113,11 +108,11 @@ bool FingerprintAuthModel::ShouldAnnounceLabel() {
          (auth_result_.has_value() && !auth_result_.value());
 }
 
-std::u16string FingerprintAuthModel::GetAccessibleName() {
+int FingerprintAuthModel::GetAccessibleNameId() {
   if (state_ == FingerprintState::DISABLED_FROM_ATTEMPTS)
-    return l10n_util::GetStringUTF16(
-        IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_ACCESSIBLE_AUTH_DISABLED_FROM_ATTEMPTS);
-  return GetLabel();
+    return IDS_ASH_LOGIN_FINGERPRINT_UNLOCK_ACCESSIBLE_AUTH_DISABLED_FROM_ATTEMPTS;
+
+  return GetLabelId();
 }
 
 void FingerprintAuthModel::UpdateIcon(AuthIconView* icon_view) {
@@ -150,8 +145,11 @@ void FingerprintAuthModel::UpdateIcon(AuthIconView* icon_view) {
           : AshColorProvider::Get()->GetDisabledColor(icon_color);
   switch (state_) {
     case FingerprintState::UNAVAILABLE:
+      FALLTHROUGH;
     case FingerprintState::AVAILABLE_DEFAULT:
+      FALLTHROUGH;
     case FingerprintState::AVAILABLE_WITH_TOUCH_SENSOR_WARNING:
+      FALLTHROUGH;
     case FingerprintState::DISABLED_FROM_TIMEOUT:
       icon_view->SetImage(gfx::CreateVectorIcon(kLockScreenFingerprintIcon,
                                                 kFingerprintIconSizeDp, color));

diff --git a/ash/login/ui/fingerprint_auth_model.h b/ash/login/ui/fingerprint_auth_model.h
@@ -5,8 +5,6 @@
 #ifndef ASH_LOGIN_UI_FINGERPRINT_AUTH_MODEL_H_
 #define ASH_LOGIN_UI_FINGERPRINT_AUTH_MODEL_H_
 
-#include <string>
-
 #include "ash/login/ui/auth_factor_model.h"
 #include "ash/public/cpp/login_types.h"
 #include "base/timer/timer.h"
@@ -32,9 +30,9 @@ class FingerprintAuthModel : public AuthFactorModel {
   // AuthFactorModel:
   AuthFactorState GetAuthFactorState() override;
   AuthFactorType GetType() override;
-  std::u16string GetLabel() override;
+  int GetLabelId() override;
   bool ShouldAnnounceLabel() override;
-  std::u16string GetAccessibleName() override;
+  int GetAccessibleNameId() override;
   void UpdateIcon(AuthIconView* icon_view) override;
   void OnTapEvent() override;
 

diff --git a/ash/login/ui/lock_contents_view.cc b/ash/login/ui/lock_contents_view.cc
@@ -908,7 +908,8 @@ void LockContentsView::OnUsersChanged(const std::vector<LoginUserInfo>& users) {
   // getting focused. Make sure to clear internal references before that happens
   // so there is not stale-pointer usage. See crbug.com/884402.
   // TODO(crbug.com/1222096): We should figure out a better way of handling
-  // user info changes such as avatar changes. They should not cause view re-layouting.
+  // user info changes such as avatar changes. They should not cause view
+  // re-layouting.
   main_view_->RemoveAllChildViews();
 
   // Build user state list. Preserve previous state if the user already exists.
@@ -1170,18 +1171,18 @@ void LockContentsView::OnForceOnlineSignInForUser(const AccountId& user) {
 void LockContentsView::OnShowEasyUnlockIcon(
     const AccountId& user,
     const EasyUnlockIconInfo& icon_info) {
-  // Do not update EasyUnlockIconState if the Smart Lock revamp is enabled since
-  // it will be removed post launch.
-  if (base::FeatureList::IsEnabled(ash::features::kSmartLockUIRevamp))
-    return;
-
   UserState* state = FindStateForUser(user);
   if (!state)
     return;
 
   state->easy_unlock_icon_info = icon_info;
   UpdateEasyUnlockIconForUser(user);
 
+  // Do not show tooltip if the Smart Lock revamp is enabled since it will be
+  // removed post launch.
+  if (base::FeatureList::IsEnabled(ash::features::kSmartLockUIRevamp))
+    return;
+
   // Show tooltip only if the user is actively showing auth.
   LoginBigUserView* big_user =
       TryToFindBigUser(user, true /*require_auth_active*/);
@@ -2083,11 +2084,6 @@ void LockContentsView::OnBigUserChanged() {
 }
 
 void LockContentsView::UpdateEasyUnlockIconForUser(const AccountId& user) {
-  // Do not update EasyUnlockIconState if the Smart Lock revamp is enabled since
-  // it will be removed post launch.
-  if (base::FeatureList::IsEnabled(ash::features::kSmartLockUIRevamp))
-    return;
-
   // Try to find an big view for |user|. If there is none, there is no state to
   // update.
   LoginBigUserView* big_view =

diff --git a/ash/login/ui/lock_debug_view.cc b/ash/login/ui/lock_debug_view.cc
@@ -292,12 +292,9 @@ class LockDebugView::DebugDataDispatcherTransformer
 
   // Enables click to auth for the user at |user_index|.
   void CycleEasyUnlockForUserIndex(size_t user_index) {
-    // Do not cycle EasyUnlockIconState if the Smart Lock revamp is enabled
-    // since it will be removed post launch.
-    // TODO(crbug.com/1233614) Update this to cycle the states of the new UI
-    // once ready.
-    DCHECK(!base::FeatureList::IsEnabled(ash::features::kSmartLockUIRevamp));
-
+    // TODO(crbug.com/1233614) Update this to cycle through SmartLockState
+    // values instead of EasyUnlockIconState values if Smart Lock UI revamp
+    // feature is enabled.
     DCHECK(user_index >= 0 && user_index < debug_users_.size());
     UserMetadata* debug_user = &debug_users_[user_index];
 

diff --git a/ash/login/ui/login_auth_factors_view.cc b/ash/login/ui/login_auth_factors_view.cc
@@ -4,14 +4,18 @@
 
 #include "ash/login/ui/login_auth_factors_view.h"
 
+#include "ash/login/ui/arrow_button_view.h"
 #include "ash/login/ui/auth_factor_model.h"
 #include "ash/login/ui/auth_icon_view.h"
+#include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
 #include "base/callback.h"
 #include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/accessibility/ax_node_data.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/compositor/layer.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/highlight_path_generator.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 
@@ -24,6 +28,7 @@ using AuthFactorState = AuthFactorModel::AuthFactorState;
 constexpr int kAuthFactorsViewWidthDp = 204;
 constexpr int kSpacingBetweenIconsAndLabelDp = 15;
 constexpr int kIconTopSpacingDp = 20;
+constexpr int kArrowButtonSizeDp = 32;
 
 }  // namespace
 
@@ -35,6 +40,7 @@ class AuthFactorsLabel : public views::Label {
     SetEnabledColor(AshColorProvider::Get()->GetContentLayerColor(
         AshColorProvider::ContentLayerType::kTextColorSecondary));
     SetMultiLine(true);
+    SizeToFit(kAuthFactorsViewWidthDp);
   }
 
   AuthFactorsLabel(const AuthFactorsLabel&) = delete;
@@ -53,6 +59,13 @@ class AuthFactorsLabel : public views::Label {
         AshColorProvider::ContentLayerType::kTextColorSecondary));
   }
 
+  // views::View:
+  gfx::Size CalculatePreferredSize() const override {
+    gfx::Size size = views::View::CalculatePreferredSize();
+    size.set_width(kAuthFactorsViewWidthDp);
+    return size;
+  }
+
   void SetAccessibleName(const std::u16string& name) {
     accessible_name_ = name;
     NotifyAccessibilityEvent(ax::mojom::Event::kTextChanged,
@@ -63,7 +76,9 @@ class AuthFactorsLabel : public views::Label {
   std::u16string accessible_name_;
 };
 
-LoginAuthFactorsView::LoginAuthFactorsView() {
+LoginAuthFactorsView::LoginAuthFactorsView(
+    base::RepeatingClosure on_click_to_enter)
+    : on_click_to_enter_callback_(on_click_to_enter) {
   SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
   SetBorder(views::CreateEmptyBorder(kIconTopSpacingDp, 0, 0, 0));
@@ -72,8 +87,19 @@ LoginAuthFactorsView::LoginAuthFactorsView() {
       views::BoxLayout::Orientation::kVertical, gfx::Insets(),
       kSpacingBetweenIconsAndLabelDp));
   layout->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kCenter);
+  layout->set_cross_axis_alignment(
+      views::BoxLayout::CrossAxisAlignment::kCenter);
 
   icon_ = AddChildView(std::make_unique<AuthIconView>());
+
+  arrow_button_ = AddChildView(std::make_unique<ArrowButtonView>(
+      base::BindRepeating(&LoginAuthFactorsView::ArrowButtonPressed,
+                          base::Unretained(this)),
+      kArrowButtonSizeDp));
+  arrow_button_->SetInstallFocusRingOnFocus(true);
+  views::InstallCircleHighlightPathGenerator(arrow_button_);
+  arrow_button_->SetVisible(false);
+
   label_ = AddChildView(std::make_unique<AuthFactorsLabel>());
 }
 
@@ -95,12 +121,29 @@ void LoginAuthFactorsView::UpdateState() {
 
   SetVisible(auth_factor->GetAuthFactorState() !=
              AuthFactorState::kUnavailable);
-  auth_factor->UpdateIcon(icon_);
-  label_->SetText(auth_factor->GetLabel());
-  label_->SetAccessibleName(auth_factor->GetAccessibleName());
 
-  if (auth_factor->ShouldAnnounceLabel())
+  if (auth_factor->GetAuthFactorState() == AuthFactorState::kClickRequired) {
+    icon_->SetVisible(false);
+    arrow_button_->SetVisible(true);
+    arrow_button_->EnableLoadingAnimation(false);
+    const std::u16string msg =
+        l10n_util::GetStringUTF16(IDS_AUTH_FACTOR_LABEL_CLICK_TO_ENTER);
+    label_->SetText(msg);
+    label_->SetAccessibleName(msg);
     FireAlert();
+  } else {
+    icon_->SetVisible(true);
+    arrow_button_->SetVisible(false);
+    arrow_button_->SetAccessibleName(
+        l10n_util::GetStringUTF16(auth_factor->GetAccessibleNameId()));
+    auth_factor->UpdateIcon(icon_);
+    label_->SetText(l10n_util::GetStringUTF16(auth_factor->GetLabelId()));
+    label_->SetAccessibleName(
+        l10n_util::GetStringUTF16(auth_factor->GetAccessibleNameId()));
+
+    if (auth_factor->ShouldAnnounceLabel())
+      FireAlert();
+  }
 }
 
 // views::View:
@@ -135,4 +178,11 @@ void LoginAuthFactorsView::FireAlert() {
                                    /*send_native_event=*/true);
 }
 
+void LoginAuthFactorsView::ArrowButtonPressed(const ui::Event& event) {
+  if (on_click_to_enter_callback_) {
+    arrow_button_->EnableLoadingAnimation(true);
+    on_click_to_enter_callback_.Run();
+  }
+}
+
 }  // namespace ash