CrOS - Remove submit button from pinpad

Remove the submit button on the pin pad shown on the login/lock
screen. Change LoginPinView construction to only show the submit
button on the pinpad when the |on_submit| callback is valid.

Verified locally that it removes the memory leak LoginPinView from:
https://crbug.com/1099306#c1

Bug: 1075994, 1101326, 1099306
Change-Id: Ib7de615c9f5c6faec168e1205e44a981f739f9a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339524
Commit-Queue: Renato Silva <rrsilva@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Auto-Submit: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/heads/master@{#795437}

ash/login/ui/login_auth_user_view.cc

@@ -834,26 +834,12 @@ LoginAuthUserView::LoginAuthUserView(const LoginUserInfo& user,
   password_view_->SetDisplayPasswordButtonVisible(
       user.show_display_password_button);
 
-  std::unique_ptr<LoginPinView> pin_view;
-  // If the display password button feature is disabled, the PIN view does not
-  // need a submit button as the password view already has one.
-  if (chromeos::features::IsLoginDisplayPasswordButtonEnabled()) {
-    pin_view = std::make_unique<LoginPinView>(
-        LoginPinView::Style::kAlphanumeric,
-        base::BindRepeating(&LoginPasswordView::InsertNumber,
-                            base::Unretained(password_view.get())),
-        base::BindRepeating(&LoginPasswordView::Backspace,
-                            base::Unretained(password_view.get())),
-        base::BindRepeating(&LoginPasswordView::SubmitPassword,
-                            base::Unretained(password_view.get())));
-  } else {
-    pin_view = std::make_unique<LoginPinView>(
-        LoginPinView::Style::kAlphanumeric,
-        base::BindRepeating(&LoginPasswordView::InsertNumber,
-                            base::Unretained(password_view.get())),
-        base::BindRepeating(&LoginPasswordView::Backspace,
-                            base::Unretained(password_view.get())));
-  }
+  auto pin_view = std::make_unique<LoginPinView>(
+      LoginPinView::Style::kAlphanumeric,
+      base::BindRepeating(&LoginPasswordView::InsertNumber,
+                          base::Unretained(password_view.get())),
+      base::BindRepeating(&LoginPasswordView::Backspace,
+                          base::Unretained(password_view.get())));
   pin_view_ = pin_view.get();
   DCHECK(pin_view_->layer());
 

ash/login/ui/login_pin_view.cc

@@ -426,14 +426,9 @@ LoginPinView::LoginPinView(Style keyboard_style,
                            const OnPinKey& on_key,
                            const OnPinBackspace& on_backspace,
                            const OnPinSubmit& on_submit)
-    : NonAccessibleView(kLoginPinViewClassName),
-      backspace_(new BackspacePinButton(kButtonSize, on_backspace)),
-      submit_button_(new SubmitPinButton(kButtonSize, on_submit)),
-      on_key_(on_key),
-      on_backspace_(on_backspace),
-      on_submit_(on_submit) {
-  DCHECK(on_key_);
-  DCHECK(on_backspace_);
+    : NonAccessibleView(kLoginPinViewClassName) {
+  DCHECK(on_key);
+  DCHECK(on_backspace);
 
   // Layer rendering.
   SetPaintToLayer(ui::LayerType::LAYER_NOT_DRAWN);
@@ -445,7 +440,7 @@ LoginPinView::LoginPinView(Style keyboard_style,
 
   auto add_digit_button = [&](View* row, int value) {
     row->AddChildView(
-        new DigitPinButton(value, show_letters, kButtonSize, on_key_));
+        new DigitPinButton(value, show_letters, kButtonSize, on_key));
   };
 
   // 1-2-3
@@ -468,19 +463,16 @@ LoginPinView::LoginPinView(Style keyboard_style,
 
   // backspace-0-submit
   row = BuildAndAddRow();
-  row->AddChildView(backspace_);
+  backspace_ = row->AddChildView(
+      std::make_unique<BackspacePinButton>(kButtonSize, on_backspace));
   add_digit_button(row, 0);
-  if (!on_submit_.is_null())
-    row->AddChildView(submit_button_);
-}
 
-LoginPinView::LoginPinView(Style keyboard_style,
-                           const OnPinKey& on_key,
-                           const OnPinBackspace& on_backspace)
-    : LoginPinView(keyboard_style,
-                   on_key,
-                   on_backspace,
-                   base::RepeatingClosure()) {}
+  // Only add the submit button if the callback is valid.
+  if (!on_submit.is_null()) {
+    submit_button_ = row->AddChildView(
+        std::make_unique<SubmitPinButton>(kButtonSize, on_submit));
+  }
+}
 
 LoginPinView::~LoginPinView() = default;
 
@@ -495,7 +487,8 @@ void LoginPinView::NotifyAccessibilityLocationChanged() {
 
 void LoginPinView::OnPasswordTextChanged(bool is_empty) {
   backspace_->SetEnabled(!is_empty);
-  submit_button_->SetEnabled(!is_empty);
+  if (submit_button_)
+    submit_button_->SetEnabled(!is_empty);
 }
 
 NonAccessibleView* LoginPinView::BuildAndAddRow() {

ash/login/ui/login_pin_view.h

@@ -10,6 +10,7 @@
 
 #include "ash/ash_export.h"
 #include "ash/login/ui/non_accessible_view.h"
+#include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/macros.h"
 #include "ui/views/view.h"
@@ -46,7 +47,7 @@ namespace ash {
 //    -------    -------    -------
 //
 // The <- represents the delete button while -> represents the submit button.
-// The submit button can be hidden.
+// The submit button is optional.
 //
 class ASH_EXPORT LoginPinView : public NonAccessibleView {
  public:
@@ -91,18 +92,12 @@ class ASH_EXPORT LoginPinView : public NonAccessibleView {
   // non-null.
   // |on_backspace| is called when the user wants to erase the most recently
   // tapped key; must be non-null.
-  // |on_submit| is called when the user wants to submit the PIN / password.
+  // If |on_submit| is valid, there will be a submit button on the pinpad that
+  // calls it when the user wants to submit the PIN / password.
   LoginPinView(Style keyboard_style,
                const OnPinKey& on_key,
                const OnPinBackspace& on_backspace,
-               const OnPinSubmit& on_submit);
-
-  // Creates PIN view without submit button. This is useful when a submit button
-  // is already present in the password view, which is the case when the display
-  // password button feature is disabled.
-  LoginPinView(Style keyboard_style,
-               const OnPinKey& on_key,
-               const OnPinBackspace& on_backspace);
+               const OnPinSubmit& on_submit = base::NullCallback());
 
   ~LoginPinView() override;
 
@@ -119,11 +114,9 @@ class ASH_EXPORT LoginPinView : public NonAccessibleView {
   // Builds and returns a new view which contains a row of the PIN keyboard.
   NonAccessibleView* BuildAndAddRow();
 
-  BackspacePinButton* backspace_;
-  SubmitPinButton* submit_button_;
-  OnPinKey on_key_;
-  OnPinBackspace on_backspace_;
-  OnPinSubmit on_submit_;
+  BackspacePinButton* backspace_ = nullptr;
+  // The submit button does not exist when no |on_submit| callback is passed.
+  SubmitPinButton* submit_button_ = nullptr;
 
   std::vector<NonAccessibleView*> rows;
 

ash/login/ui/pin_request_view_unittest.cc

@@ -220,7 +220,7 @@ TEST_F(PinRequestViewTest, SubmitButton) {
 
   // The submit button on the PIN keyboard shouldn't be shown.
   LoginPinView::TestApi test_pin_keyboard(test_api.pin_keyboard_view());
-  EXPECT_FALSE(test_pin_keyboard.GetSubmitButton()->parent());
+  EXPECT_FALSE(test_pin_keyboard.GetSubmitButton());
 
   auto* generator = GetEventGenerator();
   // Updating input code (here last digit) should clear error state.