Refactor ParentAccessUI to general UI for PIN requests.
The new UI is used both for parent access and by smartcard login.
Several options were added to accommodate the second use case.
All parent access specific logic moved into ParentAccessController, which is
now the single point of access to the UI widget for parent access.
Bug: 1001288
Change-Id: Iee728bb5ae77d387ae999741101e356602618d6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003169
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740603}
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
index dc9552a6..2195264 100644
--- a/ash/BUILD.gn
+++ b/ash/BUILD.gn
@@ -414,6 +414,8 @@
"lock_screen_action/lock_screen_note_launcher.h",
"login/login_screen_controller.cc",
"login/login_screen_controller.h",
+ "login/parent_access_controller.cc",
+ "login/parent_access_controller.h",
"login/ui/animated_rounded_image_view.cc",
"login/ui/animated_rounded_image_view.h",
"login/ui/animation_frame.h",
@@ -1785,6 +1787,7 @@
"login/login_screen_controller_unittest.cc",
"login/mock_login_screen_client.cc",
"login/mock_login_screen_client.h",
+ "login/parent_access_controller_unittest.cc",
"login/ui/fake_login_detachable_base_model.cc",
"login/ui/fake_login_detachable_base_model.h",
"login/ui/lock_contents_view_unittest.cc",
diff --git a/ash/login/login_screen_controller.cc b/ash/login/login_screen_controller.cc
index b18c70d..25b81f6 100644
--- a/ash/login/login_screen_controller.cc
+++ b/ash/login/login_screen_controller.cc
@@ -7,6 +7,7 @@
#include <utility>
#include "ash/focus_cycler.h"
+#include "ash/login/parent_access_controller.h"
#include "ash/login/ui/lock_screen.h"
#include "ash/login/ui/login_data_dispatcher.h"
#include "ash/public/cpp/ash_pref_names.h"
@@ -199,8 +200,10 @@
bool LoginScreenController::ValidateParentAccessCode(
const AccountId& account_id,
- const std::string& code,
- base::Time validation_time) {
+ base::Time validation_time,
+ const std::string& code) {
+ DCHECK(!validation_time.is_null());
+
if (!client_)
return false;
@@ -378,17 +381,6 @@
->ShowParentAccessButton(show);
}
-void LoginScreenController::ShowParentAccessWidget(
- const AccountId& child_account_id,
- ParentAccessWidget::OnExitCallback callback,
- ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time) {
- DCHECK(!ParentAccessWidget::Get());
- ParentAccessWidget::Show(child_account_id, std::move(callback), reason,
- extra_dimmer, validation_time);
-}
-
void LoginScreenController::SetAllowLoginAsGuest(bool allow_guest) {
Shelf::ForWindow(Shell::Get()->GetPrimaryRootWindow())
->shelf_widget()
@@ -404,6 +396,18 @@
->GetScopedGuestButtonBlocker();
}
+void LoginScreenController::ShowParentAccessWidget(
+ const AccountId& child_account_id,
+ ParentAccessRequest::OnParentAccessDone callback,
+ ParentAccessRequestReason reason,
+ bool extra_dimmer,
+ base::Time validation_time) {
+ DCHECK(!ParentAccessWidget::Get());
+ Shell::Get()->parent_access_controller()->ShowWidget(
+ child_account_id, std::move(callback), reason, extra_dimmer,
+ validation_time);
+}
+
void LoginScreenController::RequestSecurityTokenPin(
SecurityTokenPinRequest request) {
if (LockScreen::HasInstance() && !security_token_pin_request_cancelled_) {
diff --git a/ash/login/login_screen_controller.h b/ash/login/login_screen_controller.h
index f9d2fd1..1dad569 100644
--- a/ash/login/login_screen_controller.h
+++ b/ash/login/login_screen_controller.h
@@ -9,7 +9,6 @@
#include "ash/ash_export.h"
#include "ash/login/ui/login_data_dispatcher.h"
-#include "ash/login/ui/parent_access_widget.h"
#include "ash/public/cpp/kiosk_app_menu.h"
#include "ash/public/cpp/login_screen.h"
#include "ash/public/cpp/system_tray_focus_observer.h"
@@ -22,7 +21,6 @@
namespace ash {
-class ParentAccessWidget;
class SystemTrayNotifier;
// LoginScreenController implements LoginScreen and wraps the LoginScreenClient
@@ -71,8 +69,8 @@
void AuthenticateUserWithChallengeResponse(const AccountId& account_id,
OnAuthenticateCallback callback);
bool ValidateParentAccessCode(const AccountId& account_id,
- const std::string& code,
- base::Time validation_time);
+ base::Time validation_time,
+ const std::string& code);
void OnSecurityTokenPinRequestCancelledByUser();
bool GetSecurityTokenPinRequestCancelled() const;
void HardlockPod(const AccountId& account_id);
@@ -118,14 +116,18 @@
void EnableShutdownButton(bool enable) override;
void ShowGuestButtonInOobe(bool show) override;
void ShowParentAccessButton(bool show) override;
- void ShowParentAccessWidget(const AccountId& child_account_id,
- ParentAccessWidget::OnExitCallback callback,
- ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time) override;
void SetAllowLoginAsGuest(bool allow_guest) override;
std::unique_ptr<ScopedGuestButtonBlocker> GetScopedGuestButtonBlocker()
override;
+
+ // TODO(agawronska): Change all callers of this to use
+ // Shell::Get()->parent_access_controller()->ShowWidget() directly and delete
+ // this method.
+ void ShowParentAccessWidget(const AccountId& child_account_id,
+ base::OnceCallback<void(bool success)> callback,
+ ParentAccessRequestReason reason,
+ bool extra_dimmer,
+ base::Time validation_time) override;
void RequestSecurityTokenPin(SecurityTokenPinRequest request) override;
void ClearSecurityTokenPinRequest() override;
diff --git a/ash/login/parent_access_controller.cc b/ash/login/parent_access_controller.cc
new file mode 100644
index 0000000..c9cf21ab
--- /dev/null
+++ b/ash/login/parent_access_controller.cc
@@ -0,0 +1,167 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/login/parent_access_controller.h"
+
+#include "ash/login/login_screen_controller.h"
+#include "ash/session/session_controller_impl.h"
+#include "ash/shell.h"
+#include "ash/strings/grit/ash_strings.h"
+#include "base/bind.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string16.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace ash {
+
+namespace {
+
+// Number of digits displayed in parent access code input.
+constexpr int kParentAccessCodePinLength = 6;
+
+base::string16 GetTitle(ParentAccessRequestReason reason) {
+ int title_id;
+ switch (reason) {
+ case ParentAccessRequestReason::kUnlockTimeLimits:
+ title_id = IDS_ASH_LOGIN_PARENT_ACCESS_TITLE;
+ break;
+ case ParentAccessRequestReason::kChangeTime:
+ title_id = IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_CHANGE_TIME;
+ break;
+ case ParentAccessRequestReason::kChangeTimezone:
+ title_id = IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_CHANGE_TIMEZONE;
+ break;
+ }
+ return l10n_util::GetStringUTF16(title_id);
+}
+
+base::string16 GetDescription(ParentAccessRequestReason reason) {
+ int description_id;
+ switch (reason) {
+ case ParentAccessRequestReason::kUnlockTimeLimits:
+ description_id = IDS_ASH_LOGIN_PARENT_ACCESS_DESCRIPTION;
+ break;
+ case ParentAccessRequestReason::kChangeTime:
+ case ParentAccessRequestReason::kChangeTimezone:
+ description_id = IDS_ASH_LOGIN_PARENT_ACCESS_GENERIC_DESCRIPTION;
+ break;
+ }
+ return l10n_util::GetStringUTF16(description_id);
+}
+
+base::string16 GetAccessibleTitle() {
+ return l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PARENT_ACCESS_DIALOG_NAME);
+}
+
+} // namespace
+
+ParentAccessController::ParentAccessController() {}
+
+ParentAccessController::~ParentAccessController() = default;
+
+// static
+constexpr char ParentAccessController::kUMAParentAccessCodeAction[];
+
+// static
+constexpr char ParentAccessController::kUMAParentAccessCodeUsage[];
+
+void RecordParentAccessAction(ParentAccessController::UMAAction action) {
+ UMA_HISTOGRAM_ENUMERATION(ParentAccessController::kUMAParentAccessCodeAction,
+ action);
+}
+
+void RecordParentAccessUsage(ParentAccessRequestReason reason) {
+ switch (reason) {
+ case ParentAccessRequestReason::kUnlockTimeLimits: {
+ UMA_HISTOGRAM_ENUMERATION(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimeLimits);
+ return;
+ }
+ case ParentAccessRequestReason::kChangeTime: {
+ bool is_login = Shell::Get()->session_controller()->GetSessionState() ==
+ session_manager::SessionState::LOGIN_PRIMARY;
+ UMA_HISTOGRAM_ENUMERATION(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ is_login ? ParentAccessController::UMAUsage::kTimeChangeLoginScreen
+ : ParentAccessController::UMAUsage::kTimeChangeInSession);
+ return;
+ }
+ case ParentAccessRequestReason::kChangeTimezone: {
+ UMA_HISTOGRAM_ENUMERATION(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimezoneChange);
+ return;
+ }
+ }
+ NOTREACHED() << "Unknown ParentAccessRequestReason";
+}
+
+ParentAccessView::SubmissionResult ParentAccessController::OnPinSubmitted(
+ const std::string& pin) {
+ bool pin_is_valid =
+ Shell::Get()->login_screen_controller()->ValidateParentAccessCode(
+ account_id_, validation_time_, pin);
+
+ if (pin_is_valid) {
+ VLOG(1) << "Parent access code successfully validated";
+ RecordParentAccessAction(
+ ParentAccessController::UMAAction::kValidationSuccess);
+ return ParentAccessView::SubmissionResult::kPinAccepted;
+ }
+
+ VLOG(1) << "Invalid parent access code entered";
+ RecordParentAccessAction(ParentAccessController::UMAAction::kValidationError);
+ ParentAccessWidget::Get()->UpdateState(
+ ParentAccessRequestViewState::kError,
+ l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_ERROR),
+ GetDescription(reason_));
+ return ParentAccessView::SubmissionResult::kPinError;
+}
+
+void ParentAccessController::OnBack() {
+ RecordParentAccessAction(ParentAccessController::UMAAction::kCanceledByUser);
+}
+
+void ParentAccessController::OnHelp(gfx::NativeWindow parent_window) {
+ RecordParentAccessAction(ParentAccessController::UMAAction::kGetHelp);
+ // TODO(https://crbug.com/999387): Remove this when handling touch
+ // cancellation is fixed for system modal windows.
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::BindOnce(
+ [](gfx::NativeWindow parent_window) {
+ Shell::Get()->login_screen_controller()->ShowParentAccessHelpApp(
+ parent_window);
+ },
+ parent_window));
+}
+
+bool ParentAccessController::ShowWidget(
+ const AccountId& child_account_id,
+ ParentAccessRequest::OnParentAccessDone on_exit_callback,
+ ParentAccessRequestReason reason,
+ bool extra_dimmer,
+ base::Time validation_time) {
+ if (ParentAccessWidget::Get())
+ return false;
+
+ account_id_ = child_account_id;
+ reason_ = reason;
+ validation_time_ = validation_time;
+ ParentAccessRequest request;
+ request.on_parent_access_done = std::move(on_exit_callback);
+ request.help_button_enabled = true;
+ request.extra_dimmer = extra_dimmer;
+ request.pin_length = kParentAccessCodePinLength;
+ request.obscure_pin = false;
+ request.title = GetTitle(reason);
+ request.description = GetDescription(reason);
+ request.accessible_title = GetAccessibleTitle();
+ ParentAccessWidget::Show(std::move(request), this);
+ RecordParentAccessUsage(reason);
+ return true;
+}
+
+} // namespace ash
diff --git a/ash/login/parent_access_controller.h b/ash/login/parent_access_controller.h
new file mode 100644
index 0000000..a4672ba
--- /dev/null
+++ b/ash/login/parent_access_controller.h
@@ -0,0 +1,97 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_LOGIN_PARENT_ACCESS_CONTROLLER_H_
+#define ASH_LOGIN_PARENT_ACCESS_CONTROLLER_H_
+
+#include "ash/login/ui/parent_access_view.h"
+#include "ash/login/ui/parent_access_widget.h"
+#include "ash/public/cpp/login_types.h"
+
+namespace ash {
+
+enum class ParentAccessRequestViewState;
+
+// ParentAccessController serves as a single point of access for PIN requests
+// regarding parent access. It takes care of showing and hiding the PIN UI, as
+// well as logging usage metrics.
+class ASH_EXPORT ParentAccessController : ParentAccessView::Delegate {
+ public:
+ // Actions that originated in parent access dialog. These values are persisted
+ // to metrics. Entries should not be renumbered and numeric values should
+ // never be reused.
+ enum class UMAAction {
+ kValidationSuccess = 0,
+ kValidationError = 1,
+ kCanceledByUser = 2,
+ kGetHelp = 3,
+ kMaxValue = kGetHelp,
+ };
+
+ // Context in which parent access code was used. These values are persisted to
+ // metrics. Entries should not be reordered and numeric values should never be
+ // reused.
+ enum class UMAUsage {
+ kTimeLimits = 0,
+ kTimeChangeLoginScreen = 1,
+ kTimeChangeInSession = 2,
+ kTimezoneChange = 3,
+ kMaxValue = kTimezoneChange,
+ };
+
+ // Histogram to log actions that originated in parent access dialog.
+ static constexpr char kUMAParentAccessCodeAction[] =
+ "Supervision.ParentAccessCode.Action";
+
+ // Histogram to log context in which parent access code was used.
+ static constexpr char kUMAParentAccessCodeUsage[] =
+ "Supervision.ParentAccessCode.Usage";
+
+ ParentAccessController();
+ ParentAccessController(const ParentAccessController&) = delete;
+ ParentAccessController& operator=(const ParentAccessController&) = delete;
+ ~ParentAccessController() override;
+
+ // ParentAccessView::Delegate interface.
+ ParentAccessView::SubmissionResult OnPinSubmitted(
+ const std::string& pin) override;
+ void OnBack() override;
+ void OnHelp(gfx::NativeWindow parent_window) override;
+
+ // Shows a standalone ParentAccess dialog. If |child_account_id| is valid, it
+ // validates the parent access code for that child only, when it is empty it
+ // validates the code for any child signed in the device.
+ // |on_exit_callback| is invoked when the back button is clicked or the
+ // correct code is entered.
+ // |reason| contains information about why the parent
+ // access view is necessary, it is used to modify the view appearance by
+ // changing the title and description strings and background color.
+ // The parent access widget is a modal and already contains a dimmer, however
+ // when another modal is the parent of the widget, the dimmer will be placed
+ // behind the two windows.
+ // |extra_dimmer| will create an extra dimmer between the two.
+ // |validation_time| is the time that will be used to validate the
+ // code, if null the system's time will be used. Note: this is intended for
+ // children only. If a non child account id is provided, the validation will
+ // necessarily fail.
+ // Returns whether opening the dialog was successful. Will fail if another PIN
+ // dialog is already opened.
+ bool ShowWidget(const AccountId& child_account_id,
+ ParentAccessRequest::OnParentAccessDone on_exit_callback,
+ ParentAccessRequestReason reason,
+ bool extra_dimmer,
+ base::Time validation_time);
+
+ private:
+ AccountId account_id_;
+ ParentAccessRequestReason reason_ =
+ ParentAccessRequestReason::kUnlockTimeLimits;
+ base::Time validation_time_;
+
+ base::WeakPtrFactory<ParentAccessController> weak_factory_{this};
+};
+
+} // namespace ash
+
+#endif // ASH_LOGIN_PARENT_ACCESS_CONTROLLER_H_
diff --git a/ash/login/parent_access_controller_unittest.cc b/ash/login/parent_access_controller_unittest.cc
new file mode 100644
index 0000000..4c4ad76d
--- /dev/null
+++ b/ash/login/parent_access_controller_unittest.cc
@@ -0,0 +1,219 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/login/parent_access_controller.h"
+
+#include "ash/login/mock_login_screen_client.h"
+#include "ash/login/ui/login_button.h"
+#include "ash/login/ui/login_test_base.h"
+#include "ash/login/ui/parent_access_view.h"
+#include "ash/login/ui/parent_access_widget.h"
+#include "ash/login/ui/views_utils.h"
+#include "base/bind.h"
+#include "base/test/metrics/histogram_tester.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/views/controls/button/label_button.h"
+
+using ::testing::_;
+
+namespace ash {
+
+namespace {
+
+class ParentAccessControllerTest : public LoginTestBase {
+ protected:
+ ParentAccessControllerTest()
+ : account_id_(AccountId::FromUserEmail("child@gmail.com")) {}
+ ~ParentAccessControllerTest() override = default;
+
+ // LoginScreenTest:
+ void SetUp() override {
+ LoginTestBase::SetUp();
+ login_client_ = std::make_unique<MockLoginScreenClient>();
+ controller_ = std::make_unique<ParentAccessController>();
+ }
+
+ void TearDown() override {
+ LoginTestBase::TearDown();
+
+ // If the test did not explicitly dismissed the widget, destroy it now.
+ ParentAccessWidget* parent_access_widget = ParentAccessWidget::Get();
+ if (parent_access_widget)
+ parent_access_widget->Close(false /* validation success */);
+ }
+
+ // Simulates mouse press event on a |button|.
+ void SimulateButtonPress(views::Button* button) {
+ ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+ ui::EventTimeForNow(), 0, 0);
+ view_->ButtonPressed(button, event);
+ }
+
+ // Called when ParentAccessView finished processing.
+ void OnFinished(bool access_granted) {
+ access_granted ? ++successful_validation_ : ++back_action_;
+ }
+
+ void StartParentAccess(ParentAccessRequestReason reason =
+ ParentAccessRequestReason::kUnlockTimeLimits) {
+ validation_time_ = base::Time::Now();
+ controller_->ShowWidget(
+ account_id_,
+ base::BindRepeating(&ParentAccessControllerTest::OnFinished,
+ base::Unretained(this)),
+ reason, false, validation_time_);
+ view_ = ParentAccessWidget::TestApi(ParentAccessWidget::Get())
+ .parent_access_view();
+ }
+
+ // Verifies expectation that UMA |action| was logged.
+ void ExpectUMAActionReported(ParentAccessController::UMAAction action,
+ int bucket_count,
+ int total_count) {
+ histogram_tester_.ExpectBucketCount(
+ ParentAccessController::kUMAParentAccessCodeAction, action,
+ bucket_count);
+ histogram_tester_.ExpectTotalCount(
+ ParentAccessController::kUMAParentAccessCodeAction, total_count);
+ }
+
+ // Simulates entering a code. |success| determines whether the code will be
+ // accepted.
+ void SimulateValidation(bool success) {
+ login_client_->set_validate_parent_access_code_result(success);
+ EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
+ validation_time_))
+ .Times(1);
+
+ ui::test::EventGenerator* generator = GetEventGenerator();
+ for (int i = 0; i < 6; ++i) {
+ generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
+ ui::EF_NONE);
+ base::RunLoop().RunUntilIdle();
+ }
+ }
+
+ std::unique_ptr<ParentAccessController> controller_;
+
+ const AccountId account_id_;
+ std::unique_ptr<MockLoginScreenClient> login_client_;
+
+ // Number of times the view was dismissed with back button.
+ int back_action_ = 0;
+
+ // Number of times the view was dismissed after successful validation.
+ int successful_validation_ = 0;
+
+ // Time that will be used on the code validation.
+ base::Time validation_time_;
+
+ base::HistogramTester histogram_tester_;
+
+ ParentAccessView* view_ = nullptr; // Owned by test widget view hierarchy.
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ParentAccessControllerTest);
+};
+
+// Tests parent access dialog showing/hiding and focus behavior for parent
+// access.
+TEST_F(ParentAccessControllerTest, ParentAccessDialogFocus) {
+ EXPECT_FALSE(ParentAccessWidget::Get());
+
+ StartParentAccess();
+ ParentAccessView::TestApi view_test_api = ParentAccessView::TestApi(view_);
+
+ ASSERT_TRUE(ParentAccessWidget::Get());
+ EXPECT_TRUE(login_views_utils::HasFocusInAnyChildView(
+ view_test_api.access_code_view()));
+
+ ParentAccessWidget::Get()->Close(false /* validation success */);
+
+ EXPECT_FALSE(ParentAccessWidget::Get());
+}
+
+// Tests correct UMA reporting for parent access.
+TEST_F(ParentAccessControllerTest, ParentAccessUMARecording) {
+ StartParentAccess(ParentAccessRequestReason::kUnlockTimeLimits);
+ histogram_tester_.ExpectBucketCount(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimeLimits, 1);
+ SimulateButtonPress(ParentAccessView::TestApi(view_).back_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kCanceledByUser, 1,
+ 1);
+
+ StartParentAccess(ParentAccessRequestReason::kChangeTimezone);
+ histogram_tester_.ExpectBucketCount(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimezoneChange, 1);
+ SimulateButtonPress(ParentAccessView::TestApi(view_).back_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kCanceledByUser, 2,
+ 2);
+
+ // The below usage depends on the session state.
+ GetSessionControllerClient()->SetSessionState(
+ session_manager::SessionState::ACTIVE);
+ StartParentAccess(ParentAccessRequestReason::kChangeTime);
+ histogram_tester_.ExpectBucketCount(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimeChangeInSession, 1);
+ SimulateButtonPress(ParentAccessView::TestApi(view_).back_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kCanceledByUser, 3,
+ 3);
+
+ GetSessionControllerClient()->SetSessionState(
+ session_manager::SessionState::LOGIN_PRIMARY);
+ StartParentAccess(ParentAccessRequestReason::kChangeTime);
+ histogram_tester_.ExpectBucketCount(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimeChangeLoginScreen, 1);
+ SimulateButtonPress(ParentAccessView::TestApi(view_).back_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kCanceledByUser, 4,
+ 4);
+
+ GetSessionControllerClient()->SetSessionState(
+ session_manager::SessionState::ACTIVE);
+ StartParentAccess(ParentAccessRequestReason::kChangeTime);
+ histogram_tester_.ExpectBucketCount(
+ ParentAccessController::kUMAParentAccessCodeUsage,
+ ParentAccessController::UMAUsage::kTimeChangeInSession, 2);
+ SimulateButtonPress(ParentAccessView::TestApi(view_).back_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kCanceledByUser, 5,
+ 5);
+
+ histogram_tester_.ExpectTotalCount(
+ ParentAccessController::kUMAParentAccessCodeUsage, 5);
+ EXPECT_EQ(5, back_action_);
+}
+
+// Tests successful parent access validation flow.
+TEST_F(ParentAccessControllerTest, ParentAccessSuccessfulValidation) {
+ StartParentAccess();
+ SimulateValidation(true);
+
+ EXPECT_EQ(1, successful_validation_);
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kValidationSuccess,
+ 1, 1);
+}
+
+// Tests unsuccessful parent access flow, including help button and cancelling
+// the request.
+TEST_F(ParentAccessControllerTest, ParentAccessUnsuccessfulValidation) {
+ StartParentAccess();
+ SimulateValidation(false);
+
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kValidationError,
+ 1, 1);
+
+ EXPECT_CALL(*login_client_, ShowParentAccessHelpApp(_)).Times(1);
+ SimulateButtonPress(ParentAccessView::TestApi(view_).help_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kGetHelp, 1, 2);
+
+ SimulateButtonPress(ParentAccessView::TestApi(view_).back_button());
+ ExpectUMAActionReported(ParentAccessController::UMAAction::kCanceledByUser, 1,
+ 3);
+}
+
+} // namespace
+} // namespace ash
diff --git a/ash/login/ui/lock_contents_view.cc b/ash/login/ui/lock_contents_view.cc
index 113193c..c941264 100644
--- a/ash/login/ui/lock_contents_view.cc
+++ b/ash/login/ui/lock_contents_view.cc
@@ -13,6 +13,7 @@
#include "ash/focus_cycler.h"
#include "ash/ime/ime_controller_impl.h"
#include "ash/login/login_screen_controller.h"
+#include "ash/login/parent_access_controller.h"
#include "ash/login/ui/bottom_status_indicator.h"
#include "ash/login/ui/lock_screen.h"
#include "ash/login/ui/lock_screen_media_controls_view.h"
@@ -24,7 +25,6 @@
#include "ash/login/ui/login_user_view.h"
#include "ash/login/ui/non_accessible_view.h"
#include "ash/login/ui/note_action_launch_button.h"
-#include "ash/login/ui/parent_access_widget.h"
#include "ash/login/ui/scrollable_users_list_view.h"
#include "ash/login/ui/system_label_button.h"
#include "ash/login/ui/views_utils.h"
@@ -625,12 +625,11 @@
const AccountId account_id =
CurrentBigUserView()->GetCurrentUser().basic_user_info.account_id;
- DCHECK(!ParentAccessWidget::Get());
- ParentAccessWidget::Show(
+ Shell::Get()->parent_access_controller()->ShowWidget(
account_id,
base::BindRepeating(&LockContentsView::OnParentAccessValidationFinished,
weak_ptr_factory_.GetWeakPtr(), account_id),
- ParentAccessRequestReason::kUnlockTimeLimits);
+ ParentAccessRequestReason::kUnlockTimeLimits, false, base::Time::Now());
Shell::Get()->login_screen_controller()->ShowParentAccessButton(false);
}
diff --git a/ash/login/ui/lock_contents_view_unittest.cc b/ash/login/ui/lock_contents_view_unittest.cc
index d5f6ce3..9daa95f 100644
--- a/ash/login/ui/lock_contents_view_unittest.cc
+++ b/ash/login/ui/lock_contents_view_unittest.cc
@@ -12,6 +12,7 @@
#include "ash/detachable_base/detachable_base_pairing_status.h"
#include "ash/login/login_screen_controller.h"
#include "ash/login/mock_login_screen_client.h"
+#include "ash/login/parent_access_controller.h"
#include "ash/login/ui/arrow_button_view.h"
#include "ash/login/ui/fake_login_detachable_base_model.h"
#include "ash/login/ui/lock_screen.h"
@@ -26,8 +27,6 @@
#include "ash/login/ui/login_test_base.h"
#include "ash/login/ui/login_test_utils.h"
#include "ash/login/ui/login_user_view.h"
-#include "ash/login/ui/parent_access_view.h"
-#include "ash/login/ui/parent_access_widget.h"
#include "ash/login/ui/scrollable_users_list_view.h"
#include "ash/login/ui/views_utils.h"
#include "ash/public/cpp/ash_features.h"
@@ -2158,21 +2157,13 @@
contents->ShowParentAccessDialog();
EXPECT_TRUE(primary_view->auth_user());
- ASSERT_TRUE(ParentAccessWidget::Get());
- ParentAccessWidget::TestApi widget =
- ParentAccessWidget::TestApi(ParentAccessWidget::Get());
EXPECT_FALSE(LoginPasswordView::TestApi(auth_user.password_view())
.textfield()
->HasFocus());
- EXPECT_TRUE(HasFocusInAnyChildView(
- ParentAccessView::TestApi(widget.parent_access_view())
- .access_code_view()));
- ParentAccessWidget::TestApi(ParentAccessWidget::Get())
- .SimulateValidationFinished(false);
+ ParentAccessWidget::Get()->Close(false /* validation success */);
EXPECT_TRUE(primary_view->auth_user());
- EXPECT_FALSE(ParentAccessWidget::Get());
EXPECT_TRUE(LoginPasswordView::TestApi(auth_user.password_view())
.textfield()
->HasFocus());
@@ -2199,15 +2190,13 @@
// Validation failed - show the button.
contents->ShowParentAccessDialog();
EXPECT_FALSE(LoginScreenTestApi::IsParentAccessButtonShown());
- ParentAccessWidget::TestApi(ParentAccessWidget::Get())
- .SimulateValidationFinished(false);
+ ParentAccessWidget::Get()->Close(false /* validation success */);
EXPECT_TRUE(LoginScreenTestApi::IsParentAccessButtonShown());
// Validation succeeded - hide the button.
contents->ShowParentAccessDialog();
EXPECT_FALSE(LoginScreenTestApi::IsParentAccessButtonShown());
- ParentAccessWidget::TestApi(ParentAccessWidget::Get())
- .SimulateValidationFinished(true);
+ ParentAccessWidget::Get()->Close(true /* validation success */);
EXPECT_FALSE(LoginScreenTestApi::IsParentAccessButtonShown());
// Validation failed but user auth got enabled - hide button.
@@ -2215,8 +2204,7 @@
contents->ShowParentAccessDialog();
EXPECT_FALSE(LoginScreenTestApi::IsParentAccessButtonShown());
DataDispatcher()->EnableAuthForUser(child_id);
- ParentAccessWidget::TestApi(ParentAccessWidget::Get())
- .SimulateValidationFinished(false);
+ ParentAccessWidget::Get()->Close(false /* validation success */);
EXPECT_FALSE(LoginScreenTestApi::IsParentAccessButtonShown());
}
diff --git a/ash/login/ui/parent_access_view.cc b/ash/login/ui/parent_access_view.cc
index 3f91f23..b406efe 100644
--- a/ash/login/ui/parent_access_view.cc
+++ b/ash/login/ui/parent_access_view.cc
@@ -13,6 +13,7 @@
#include "ash/login/ui/login_button.h"
#include "ash/login/ui/login_pin_view.h"
#include "ash/login/ui/non_accessible_view.h"
+#include "ash/login/ui/parent_access_widget.h"
#include "ash/public/cpp/login_constants.h"
#include "ash/public/cpp/login_types.h"
#include "ash/public/cpp/shelf_config.h"
@@ -24,7 +25,6 @@
#include "ash/wallpaper/wallpaper_controller_impl.h"
#include "base/bind.h"
#include "base/logging.h"
-#include "base/metrics/histogram_macros.h"
#include "base/optional.h"
#include "base/strings/strcat.h"
#include "base/strings/string16.h"
@@ -64,12 +64,9 @@
// Identifier of parent access input views group used for focus traversal.
constexpr int kParentAccessInputGroup = 1;
-// Number of digits displayed in access code input.
-constexpr int kParentAccessCodePinLength = 6;
-
constexpr int kParentAccessViewWidthDp = 340;
constexpr int kParentAccessViewHeightDp = 340;
-constexpr int kParentAccessViewTabletModeHeightDp = 580;
+constexpr int kParentAccessViewPinKeyboardModeHeightDp = 580;
constexpr int kParentAccessViewRoundedCornerRadiusDp = 8;
constexpr int kParentAccessViewVerticalInsetDp = 8;
// Inset for all elements except the back button.
@@ -84,7 +81,7 @@
constexpr int kDescriptionToAccessCodeDistanceDp = 28;
constexpr int kAccessCodeToPinKeyboardDistanceDp = 5;
constexpr int kPinKeyboardToFooterDistanceDp = 57;
-constexpr int kPinKeyboardToFooterTabletModeDistanceDp = 17;
+constexpr int kPinKeyboardToFooterPinKeyboardModeDistanceDp = 17;
constexpr int kSubmitButtonBottomMarginDp = 28;
constexpr int kTitleFontSizeDeltaDp = 3;
@@ -116,83 +113,29 @@
return Shell::Get()->tablet_mode_controller()->InTabletMode();
}
-gfx::Size GetPinKeyboardToFooterSpacerSize() {
- return gfx::Size(0, IsTabletMode() ? kPinKeyboardToFooterTabletModeDistanceDp
- : kPinKeyboardToFooterDistanceDp);
-}
-
-gfx::Size GetParentAccessViewSize() {
- return gfx::Size(kParentAccessViewWidthDp,
- IsTabletMode() ? kParentAccessViewTabletModeHeightDp
- : kParentAccessViewHeightDp);
-}
-
-base::string16 GetTitle(ParentAccessRequestReason reason) {
- int title_id;
- switch (reason) {
- case ParentAccessRequestReason::kUnlockTimeLimits:
- title_id = IDS_ASH_LOGIN_PARENT_ACCESS_TITLE;
- break;
- case ParentAccessRequestReason::kChangeTime:
- title_id = IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_CHANGE_TIME;
- break;
- case ParentAccessRequestReason::kChangeTimezone:
- title_id = IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_CHANGE_TIMEZONE;
- break;
- }
- return l10n_util::GetStringUTF16(title_id);
-}
-
-base::string16 GetDescription(ParentAccessRequestReason reason) {
- int description_id;
- switch (reason) {
- case ParentAccessRequestReason::kUnlockTimeLimits:
- description_id = IDS_ASH_LOGIN_PARENT_ACCESS_DESCRIPTION;
- break;
- case ParentAccessRequestReason::kChangeTime:
- case ParentAccessRequestReason::kChangeTimezone:
- description_id = IDS_ASH_LOGIN_PARENT_ACCESS_GENERIC_DESCRIPTION;
- break;
- }
- return l10n_util::GetStringUTF16(description_id);
-}
-
-base::string16 GetAccessibleTitle() {
- return l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PARENT_ACCESS_DIALOG_NAME);
-}
-
-void RecordAction(ParentAccessView::UMAAction action) {
- UMA_HISTOGRAM_ENUMERATION(ParentAccessView::kUMAParentAccessCodeAction,
- action);
-}
-
-void RecordUsage(ParentAccessRequestReason reason) {
- switch (reason) {
- case ParentAccessRequestReason::kUnlockTimeLimits: {
- UMA_HISTOGRAM_ENUMERATION(ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimeLimits);
- return;
- }
- case ParentAccessRequestReason::kChangeTime: {
- bool is_login = Shell::Get()->session_controller()->GetSessionState() ==
- session_manager::SessionState::LOGIN_PRIMARY;
- UMA_HISTOGRAM_ENUMERATION(
- ParentAccessView::kUMAParentAccessCodeUsage,
- is_login ? ParentAccessView::UMAUsage::kTimeChangeLoginScreen
- : ParentAccessView::UMAUsage::kTimeChangeInSession);
- return;
- }
- case ParentAccessRequestReason::kChangeTimezone: {
- UMA_HISTOGRAM_ENUMERATION(ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimezoneChange);
- return;
- }
- }
- NOTREACHED() << "Unknown ParentAccessRequestReason";
-}
-
} // namespace
+ParentAccessRequest::ParentAccessRequest() = default;
+ParentAccessRequest::ParentAccessRequest(ParentAccessRequest&&) = default;
+ParentAccessRequest& ParentAccessRequest::operator=(ParentAccessRequest&&) =
+ default;
+ParentAccessRequest::~ParentAccessRequest() = default;
+
+// Label button that displays focus ring.
+class ParentAccessView::FocusableLabelButton : public views::LabelButton {
+ public:
+ FocusableLabelButton(views::ButtonListener* listener,
+ const base::string16& text)
+ : views::LabelButton(listener, text) {
+ SetInstallFocusRingOnFocus(true);
+ focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
+ }
+
+ FocusableLabelButton(const FocusableLabelButton&) = delete;
+ FocusableLabelButton& operator=(const FocusableLabelButton&) = delete;
+ ~FocusableLabelButton() override = default;
+};
+
class ParentAccessView::AccessCodeInput : public views::View,
public views::TextfieldController {
public:
@@ -236,6 +179,8 @@
on_enter_(std::move(on_enter)),
on_escape_(std::move(on_escape)) {
DCHECK(on_input_change_);
+ DCHECK(on_enter_);
+ DCHECK(on_escape_);
SetLayoutManager(std::make_unique<views::FillLayout>());
@@ -260,9 +205,9 @@
}
}
- ~FlexCodeInput() override = default;
FlexCodeInput(const FlexCodeInput&) = delete;
FlexCodeInput& operator=(const FlexCodeInput&) = delete;
+ ~FlexCodeInput() override = default;
// Appends |value| to the code
void InsertDigit(int value) override {
@@ -418,21 +363,6 @@
DISALLOW_COPY_AND_ASSIGN(AccessibleInputField);
};
-// Label button that displays focus ring.
-class ParentAccessView::FocusableLabelButton : public views::LabelButton {
- public:
- FocusableLabelButton(views::ButtonListener* listener,
- const base::string16& text)
- : views::LabelButton(listener, text) {
- SetInstallFocusRingOnFocus(true);
- focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
- }
- ~FocusableLabelButton() override = default;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(FocusableLabelButton);
-};
-
// Digital access code input view for variable length of input codes.
// Displays a separate underscored field for every input code digit.
class ParentAccessView::FixedLengthCodeInput : public AccessCodeInput {
@@ -747,22 +677,10 @@
.GetInputTextField(index);
}
-ParentAccessView::State ParentAccessView::TestApi::state() const {
+ParentAccessRequestViewState ParentAccessView::TestApi::state() const {
return view_->state_;
}
-ParentAccessView::Callbacks::Callbacks() = default;
-
-ParentAccessView::Callbacks::Callbacks(const Callbacks& other) = default;
-
-ParentAccessView::Callbacks::~Callbacks() = default;
-
-// static
-constexpr char ParentAccessView::kUMAParentAccessCodeAction[];
-
-// static
-constexpr char ParentAccessView::kUMAParentAccessCodeUsage[];
-
// static
SkColor ParentAccessView::GetChildUserDialogColor(bool using_blur) {
SkColor color = AshColorProvider::Get()->GetBaseLayerColor(
@@ -783,15 +701,16 @@
return using_blur ? SkColorSetA(color, kAlpha74Percent) : color;
}
-ParentAccessView::ParentAccessView(const AccountId& account_id,
- const Callbacks& callbacks,
- ParentAccessRequestReason reason,
- base::Time validation_time)
- : callbacks_(callbacks),
- account_id_(account_id),
- request_reason_(reason),
- validation_time_(validation_time) {
- DCHECK(callbacks.on_finished);
+ParentAccessView::ParentAccessView(ParentAccessRequest request,
+ Delegate* delegate)
+ : delegate_(delegate),
+ on_parent_access_done_(std::move(request.on_parent_access_done)),
+ pin_keyboard_always_enabled_(request.pin_keyboard_always_enabled),
+ default_title_(request.title),
+ default_description_(request.description),
+ default_accessible_title_(request.accessible_title.empty()
+ ? request.title
+ : request.accessible_title) {
// Main view contains all other views aligned vertically and centered.
auto layout = std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical,
@@ -864,9 +783,8 @@
};
// Main view title.
- title_label_ =
- new views::Label(GetTitle(request_reason_), views::style::CONTEXT_LABEL,
- views::style::STYLE_PRIMARY);
+ title_label_ = new views::Label(default_title_, views::style::CONTEXT_LABEL,
+ views::style::STYLE_PRIMARY);
title_label_->SetFontList(gfx::FontList().Derive(
kTitleFontSizeDeltaDp, gfx::Font::NORMAL, gfx::Font::Weight::MEDIUM));
decorate_label(title_label_);
@@ -875,9 +793,9 @@
add_spacer(kTitleToDescriptionDistanceDp);
// Main view description.
- description_label_ = new views::Label(GetDescription(request_reason_),
- views::style::CONTEXT_LABEL,
- views::style::STYLE_PRIMARY);
+ description_label_ =
+ new views::Label(default_description_, views::style::CONTEXT_LABEL,
+ views::style::STYLE_PRIMARY);
description_label_->SetMultiLine(true);
description_label_->SetLineHeight(kDescriptionTextLineHeightDp);
description_label_->SetFontList(
@@ -889,15 +807,25 @@
add_spacer(kDescriptionToAccessCodeDistanceDp);
// Access code input view.
- access_code_view_ = AddChildView(std::make_unique<FixedLengthCodeInput>(
- kParentAccessCodePinLength,
- base::BindRepeating(&ParentAccessView::OnInputChange,
- base::Unretained(this)),
- base::BindRepeating(&ParentAccessView::SubmitCode,
- base::Unretained(this)),
- base::BindRepeating(&ParentAccessView::OnBack, base::Unretained(this)),
- false /*obscure_pin*/));
-
+ if (request.pin_length.has_value()) {
+ CHECK_GT(request.pin_length.value(), 0);
+ access_code_view_ = AddChildView(std::make_unique<FixedLengthCodeInput>(
+ request.pin_length.value(),
+ base::BindRepeating(&ParentAccessView::OnInputChange,
+ base::Unretained(this)),
+ base::BindRepeating(&ParentAccessView::SubmitCode,
+ base::Unretained(this)),
+ base::BindRepeating(&ParentAccessView::OnBack, base::Unretained(this)),
+ request.obscure_pin));
+ } else {
+ access_code_view_ = AddChildView(std::make_unique<FlexCodeInput>(
+ base::BindRepeating(&ParentAccessView::OnInputChange,
+ base::Unretained(this), false),
+ base::BindRepeating(&ParentAccessView::SubmitCode,
+ base::Unretained(this)),
+ base::BindRepeating(&ParentAccessView::OnBack, base::Unretained(this)),
+ request.obscure_pin));
+ }
access_code_view_->SetFocusBehavior(FocusBehavior::ALWAYS);
add_spacer(kAccessCodeToPinKeyboardDistanceDp);
@@ -940,7 +868,7 @@
help_button_->SetTextColor(views::Button::STATE_HOVERED, kTextColor);
help_button_->SetTextColor(views::Button::STATE_PRESSED, kTextColor);
help_button_->SetFocusBehavior(FocusBehavior::ALWAYS);
-
+ help_button_->SetVisible(request.help_button_enabled);
footer->AddChildView(help_button_);
auto* horizontal_spacer = new NonAccessibleView();
@@ -958,12 +886,9 @@
footer->AddChildView(submit_button_);
add_spacer(kSubmitButtonBottomMarginDp);
- // Pin keyboard is only shown in tablet mode.
- pin_keyboard_view_->SetVisible(IsTabletMode());
+ pin_keyboard_view_->SetVisible(PinKeyboardVisible());
tablet_mode_observer_.Add(Shell::Get()->tablet_mode_controller());
-
- RecordUsage(request_reason_);
}
ParentAccessView::~ParentAccessView() = default;
@@ -999,7 +924,7 @@
}
base::string16 ParentAccessView::GetAccessibleWindowTitle() const {
- return GetAccessibleTitle();
+ return default_accessible_title_;
}
void ParentAccessView::ButtonPressed(views::Button* sender,
@@ -1007,37 +932,31 @@
if (sender == back_button_) {
OnBack();
} else if (sender == help_button_) {
- RecordAction(ParentAccessView::UMAAction::kGetHelp);
- // TODO(https://crbug.com/999387): Remove this when handling touch
- // cancellation is fixed for system modal windows.
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(
- [](gfx::NativeWindow parent_window) {
- Shell::Get()->login_screen_controller()->ShowParentAccessHelpApp(
- parent_window);
- },
- GetWidget()->GetNativeWindow()));
+ delegate_->OnHelp(GetWidget()->GetNativeWindow());
} else if (sender == submit_button_) {
SubmitCode();
}
}
void ParentAccessView::OnTabletModeStarted() {
- VLOG(1) << "Showing PIN keyboard in ParentAccessView";
- pin_keyboard_view_->SetVisible(true);
- // This will trigger ChildPreferredSizeChanged in parent view and Layout() in
- // view. As the result whole hierarchy will go through re-layout.
- UpdatePreferredSize();
+ if (!pin_keyboard_always_enabled_) {
+ VLOG(1) << "Showing PIN keyboard in ParentAccessView";
+ pin_keyboard_view_->SetVisible(true);
+ // This will trigger ChildPreferredSizeChanged in parent view and Layout()
+ // in view. As the result whole hierarchy will go through re-layout.
+ UpdatePreferredSize();
+ }
}
void ParentAccessView::OnTabletModeEnded() {
- VLOG(1) << "Hiding PIN keyboard in ParentAccessView";
- DCHECK(pin_keyboard_view_);
- pin_keyboard_view_->SetVisible(false);
- // This will trigger ChildPreferredSizeChanged in parent view and Layout() in
- // view. As the result whole hierarchy will go through re-layout.
- UpdatePreferredSize();
+ if (!pin_keyboard_always_enabled_) {
+ VLOG(1) << "Hiding PIN keyboard in ParentAccessView";
+ DCHECK(pin_keyboard_view_);
+ pin_keyboard_view_->SetVisible(false);
+ // This will trigger ChildPreferredSizeChanged in parent view and Layout()
+ // in view. As the result whole hierarchy will go through re-layout.
+ UpdatePreferredSize();
+ }
}
void ParentAccessView::OnTabletControllerDestroyed() {
@@ -1048,45 +967,47 @@
base::Optional<std::string> code = access_code_view_->GetCode();
DCHECK(code.has_value());
- bool result =
- Shell::Get()->login_screen_controller()->ValidateParentAccessCode(
- account_id_, *code,
- validation_time_.is_null() ? base::Time::Now() : validation_time_);
-
- if (result) {
- VLOG(1) << "Parent access code successfully validated";
- RecordAction(ParentAccessView::UMAAction::kValidationSuccess);
- callbacks_.on_finished.Run(true);
- return;
+ SubmissionResult result = delegate_->OnPinSubmitted(*code);
+ switch (result) {
+ case SubmissionResult::kPinAccepted: {
+ std::move(on_parent_access_done_).Run(true /* success */);
+ return;
+ }
+ case SubmissionResult::kPinError: {
+ // Caller is expected to call UpdateState() to allow for customization of
+ // error messages.
+ return;
+ }
+ case SubmissionResult::kSubmitPending: {
+ // Waiting on validation result - do nothing for now.
+ return;
+ }
}
-
- VLOG(1) << "Invalid parent access code entered";
- RecordAction(ParentAccessView::UMAAction::kValidationError);
- UpdateState(State::kError);
}
void ParentAccessView::OnBack() {
- RecordAction(ParentAccessView::UMAAction::kCanceledByUser);
- callbacks_.on_finished.Run(false /*access_granted*/);
+ delegate_->OnBack();
+ if (ParentAccessWidget::Get()) {
+ ParentAccessWidget::Get()->Close(false /* success */);
+ }
}
-void ParentAccessView::UpdateState(State state) {
- if (state_ == state)
- return;
-
+void ParentAccessView::UpdateState(ParentAccessRequestViewState state,
+ const base::string16& title,
+ const base::string16& description) {
state_ = state;
+ title_label_->SetText(title);
+ description_label_->SetText(description);
switch (state_) {
- case State::kNormal: {
+ case ParentAccessRequestViewState::kNormal: {
access_code_view_->SetInputColor(kTextColor);
title_label_->SetEnabledColor(kTextColor);
- title_label_->SetText(GetTitle(request_reason_));
return;
}
- case State::kError: {
+ case ParentAccessRequestViewState::kError: {
access_code_view_->SetInputColor(kErrorColor);
title_label_->SetEnabledColor(kErrorColor);
- title_label_->SetText(
- l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_ERROR));
+ // Read out the error.
title_label_->NotifyAccessibilityEvent(ax::mojom::Event::kAlert, true);
return;
}
@@ -1110,8 +1031,10 @@
}
void ParentAccessView::OnInputChange(bool last_field_active, bool complete) {
- if (state_ == State::kError)
- UpdateState(State::kNormal);
+ if (state_ == ParentAccessRequestViewState::kError) {
+ UpdateState(ParentAccessRequestViewState::kNormal, default_title_,
+ default_description_);
+ }
submit_button_->SetEnabled(complete);
@@ -1133,7 +1056,26 @@
void ParentAccessView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
views::View::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kDialog;
- node_data->SetName(GetAccessibleTitle());
+ node_data->SetName(default_accessible_title_);
+}
+
+// If |pin_keyboard_always_enabled_| is not set, pin keyboard is only shown in
+// tablet mode.
+bool ParentAccessView::PinKeyboardVisible() const {
+ return pin_keyboard_always_enabled_ || IsTabletMode();
+}
+
+gfx::Size ParentAccessView::GetPinKeyboardToFooterSpacerSize() const {
+ return gfx::Size(0, PinKeyboardVisible()
+ ? kPinKeyboardToFooterPinKeyboardModeDistanceDp
+ : kPinKeyboardToFooterDistanceDp);
+}
+
+gfx::Size ParentAccessView::GetParentAccessViewSize() const {
+ return gfx::Size(kParentAccessViewWidthDp,
+ PinKeyboardVisible()
+ ? kParentAccessViewPinKeyboardModeHeightDp
+ : kParentAccessViewHeightDp);
}
} // namespace ash
diff --git a/ash/login/ui/parent_access_view.h b/ash/login/ui/parent_access_view.h
index 70dda6d..ffb7f1a1 100644
--- a/ash/login/ui/parent_access_view.h
+++ b/ash/login/ui/parent_access_view.h
@@ -8,6 +8,7 @@
#include <string>
#include "ash/ash_export.h"
+#include "ash/public/cpp/login_types.h"
#include "ash/public/cpp/tablet_mode_observer.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "base/callback.h"
@@ -33,7 +34,48 @@
class LoginPinView;
class NonAccessibleView;
-enum class ParentAccessRequestReason;
+// State of the ParentAccessView.
+enum class ParentAccessRequestViewState {
+ kNormal,
+ kError,
+};
+
+struct ASH_EXPORT ParentAccessRequest {
+ ParentAccessRequest();
+ ParentAccessRequest(ParentAccessRequest&&);
+ ParentAccessRequest& operator=(ParentAccessRequest&&);
+ ~ParentAccessRequest();
+
+ // Callback for PIN validations. It is called when the validation has finished
+ // and the view is closing.
+ // |success| indicates whether the validation was successful.
+ using success)>;
+ OnParentAccessDone on_parent_access_done = base::NullCallback();
+
+ // Whether the help button is displayed.
+ bool help_button_enabled = false;
+
+ base::Optional<int> pin_length;
+
+ // When |pin_keyboard_always_enabled| is set, the PIN keyboard is displayed at
+ // all times. Otherwise, it is only displayed when the device is in tablet
+ // mode.
+ bool pin_keyboard_always_enabled = false;
+
+ // The parent access widget is a modal and already contains a dimmer, however
+ // when another modal is the parent of the widget, the dimmer will be placed
+ // behind the two windows. |extra_dimmer| will create an extra dimmer between
+ // the two.
+ bool extra_dimmer = false;
+
+ // Whether the entered PIN should be displayed clearly or only as bullets.
+ bool obscure_pin = true;
+
+ // Strings for UI.
+ base::string16 title;
+ base::string16 description;
+ base::string16 accessible_title;
+};
// The view that allows for input of parent access code to authorize certain
// actions on child's device.
@@ -41,10 +83,23 @@
public views::ButtonListener,
public TabletModeObserver {
public:
- // ParentAccessView state.
- enum class State {
- kNormal, // View with default texts and colors.
- kError // View with texts and color signalizing input error.
+ enum class SubmissionResult {
+ // Closes the UI and calls |on_parent_access_done_|.
+ kPinAccepted,
+ // PIN rejected - keeps the UI in its current state.
+ kPinError,
+ // Async waiting for result - keeps the UI in its current state.
+ kSubmitPending,
+ };
+
+ class Delegate {
+ public:
+ virtual SubmissionResult OnPinSubmitted(const std::string& pin) = 0;
+ virtual void OnBack() = 0;
+ virtual void OnHelp(gfx::NativeWindow parent_window) = 0;
+
+ protected:
+ virtual ~Delegate() = default;
};
class ASH_EXPORT TestApi {
@@ -62,75 +117,20 @@
views::Textfield* GetInputTextField(int index);
- State state() const;
+ ParentAccessRequestViewState state() const;
private:
ParentAccessView* const view_;
};
- using access_granted)>;
-
- // Parent access view callbacks.
- struct Callbacks {
- Callbacks();
- Callbacks(const Callbacks& other);
- ~Callbacks();
-
- // Called when ParentAccessView finshed processing and should be dismissed.
- // If access code was successfully validated, |access_granted| will
- // contain true. If access code was not entered or not successfully
- // validated and user pressed back button, |access_granted| will contain
- // false.
- OnFinished on_finished;
- };
-
- // Actions that originated in parent access dialog. These values are persisted
- // to metrics. Entries should not be renumbered and numeric values should
- // never be reused.
- enum class UMAAction {
- kValidationSuccess = 0,
- kValidationError = 1,
- kCanceledByUser = 2,
- kGetHelp = 3,
- kMaxValue = kGetHelp,
- };
-
- // Context in which parent access code was used. These values are persisted to
- // metrics. Entries should not be reordered and numeric values should never be
- // reused.
- enum class UMAUsage {
- kTimeLimits = 0,
- kTimeChangeLoginScreen = 1,
- kTimeChangeInSession = 2,
- kTimezoneChange = 3,
- kMaxValue = kTimezoneChange,
- };
-
- // Histogram to log actions that originated in parent access dialog.
- static constexpr char kUMAParentAccessCodeAction[] =
- "Supervision.ParentAccessCode.Action";
-
- // Histogram to log context in which parent access code was used.
- static constexpr char kUMAParentAccessCodeUsage[] =
- "Supervision.ParentAccessCode.Usage";
-
// Returns color used for dialog and UI elements specific for child user.
// |using_blur| should be true if the UI element is using background blur
// (color transparency depends on it).
static SkColor GetChildUserDialogColor(bool using_blur);
- // Creates parent access view that will validate the parent access code for a
- // specific child, when |account_id| is set, or to any child signed in the
- // device, when it is empty. |callbacks| will be called when user performs
- // certain actions. |reason| contains information about why the parent access
- // view is necessary, it is used to modify the view appearance by changing the
- // title and description strings and background color. |validation_time| is
- // the time that will be used to validate the code, if null the system's
- // current time will be used.
- ParentAccessView(const AccountId& account_id,
- const Callbacks& callbacks,
- ParentAccessRequestReason reason,
- base::Time validation_time);
+ // Creates parent access view that will enable the user to enter a pin.
+ // |request| is used to configure callbacks and UI details.
+ ParentAccessView(ParentAccessRequest request, Delegate* delegate);
~ParentAccessView() override;
// views::View:
@@ -155,6 +155,11 @@
// Sets whether the user can enter a PIN.
void SetInputEnabled(bool input_enabled);
+ // Updates state of the view.
+ void UpdateState(ParentAccessRequestViewState state,
+ const base::string16& title,
+ const base::string16& description);
+
private:
class FocusableLabelButton;
class AccessCodeInput;
@@ -164,9 +169,6 @@
// Submits access code for validation.
void SubmitCode();
- // Updates state of the view.
- void UpdateState(State state);
-
// Closes the view.
void OnBack();
@@ -181,26 +183,32 @@
// information whether last input field is currently active.
void OnInputChange(bool last_field_active, bool complete);
- // Callbacks to be called when user performs certain actions.
- const Callbacks callbacks_;
+ // Returns if the pin keyboard should be visible.
+ bool PinKeyboardVisible() const;
- // Account id of the user that parent access code is processed for. When
- // empty, the code is processed for all the children signed in the device.
- const AccountId account_id_;
+ // Sizes that depend on the pin keyboards visibility.
+ gfx::Size GetPinKeyboardToFooterSpacerSize() const;
+ gfx::Size GetParentAccessViewSize() const;
- // Indicates what action will be authorized with parent access code.
- // The appearance of the view depends on |request_reason_|.
- const ParentAccessRequestReason request_reason_;
+ ParentAccessRequestViewState state_ = ParentAccessRequestViewState::kNormal;
- // Time used to validate the code. When this is null, the current system time
- // is used.
- const base::Time validation_time_;
+ // Unowned pointer to the delegate. The delegate should outlive this instance.
+ Delegate* delegate_;
- State state_ = State::kNormal;
+ // Callback to close the UI.
+ ParentAccessRequest::OnParentAccessDone on_parent_access_done_;
// Auto submit code when the last input has been inserted.
bool auto_submit_enabled_ = true;
+ // If false, |pin_keyboard_view| is only displayed in tablet mode.
+ bool pin_keyboard_always_enabled_ = true;
+
+ // Strings as on view construction to enable restoring the original state.
+ base::string16 default_title_;
+ base::string16 default_description_;
+ base::string16 default_accessible_title_;
+
views::Label* title_label_ = nullptr;
views::Label* description_label_ = nullptr;
AccessCodeInput* access_code_view_ = nullptr;
diff --git a/ash/login/ui/parent_access_view_unittest.cc b/ash/login/ui/parent_access_view_unittest.cc
index 9750a8a..baf479fc 100644
--- a/ash/login/ui/parent_access_view_unittest.cc
+++ b/ash/login/ui/parent_access_view_unittest.cc
@@ -16,6 +16,7 @@
#include "ash/login/ui/login_test_base.h"
#include "ash/login/ui/login_test_utils.h"
#include "ash/login/ui/parent_access_widget.h"
+#include "ash/public/cpp/login_types.h"
#include "ash/session/test_session_controller_client.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
@@ -36,41 +37,17 @@
#include "ui/events/event.h"
#include "ui/events/test/event_generator.h"
#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/native_widget_types.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/controls/textfield/textfield.h"
#include "ui/views/widget/widget.h"
namespace ash {
-namespace {
-
-// Struct containing the correct title and description that are displayed when
-// the dialog is instantiated with a given ParentAccessRequestReason.
-struct ViewModifiersTestData {
- ParentAccessRequestReason reason;
- // The title string id.
- int title;
- // The description string id.
- int description;
-};
-
-const ViewModifiersTestData kViewModifiersTestData[] = {
- {ParentAccessRequestReason::kUnlockTimeLimits,
- IDS_ASH_LOGIN_PARENT_ACCESS_TITLE,
- IDS_ASH_LOGIN_PARENT_ACCESS_DESCRIPTION},
- {ParentAccessRequestReason::kChangeTime,
- IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_CHANGE_TIME,
- IDS_ASH_LOGIN_PARENT_ACCESS_GENERIC_DESCRIPTION},
- {ParentAccessRequestReason::kChangeTimezone,
- IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_CHANGE_TIMEZONE,
- IDS_ASH_LOGIN_PARENT_ACCESS_GENERIC_DESCRIPTION}};
-
-// TODO(crbug.com/996828): Make (at least some of) the tests use
-// ParentAccessWidget.
-class ParentAccessViewTest : public LoginTestBase {
+class ParentAccessViewTest : public LoginTestBase,
+ public ParentAccessView::Delegate {
protected:
- ParentAccessViewTest()
- : account_id_(AccountId::FromUserEmail("child@gmail.com")) {}
+ ParentAccessViewTest() {}
~ParentAccessViewTest() override = default;
// LoginScreenTest:
@@ -83,8 +60,9 @@
LoginTestBase::TearDown();
// If the test did not explicitly dismissed the widget, destroy it now.
- if (ParentAccessWidget::Get())
- ParentAccessWidget::Get()->Destroy();
+ ParentAccessWidget* parent_access_widget = ParentAccessWidget::Get();
+ if (parent_access_widget)
+ parent_access_widget->Close(false /* validation success */);
}
// Simulates mouse press event on a |button|.
@@ -101,32 +79,41 @@
button->OnEvent(&event);
}
- // Called when ParentAccessView finished processing.
- void OnFinished(bool access_granted) {
- access_granted ? ++successful_validation_ : ++back_action_;
+ ParentAccessView::SubmissionResult OnPinSubmitted(
+ const std::string& code) override {
+ ++pin_submitted_;
+ last_code_submitted_ = code;
+ if (!will_authenticate_) {
+ view_->UpdateState(ParentAccessRequestViewState::kError, base::string16(),
+ base::string16());
+ return ParentAccessView::SubmissionResult::kPinError;
+ }
+ return ParentAccessView::SubmissionResult::kPinAccepted;
}
- void StartView(ParentAccessRequestReason reason =
- ParentAccessRequestReason::kUnlockTimeLimits) {
- ParentAccessView::Callbacks callbacks;
- callbacks.on_finished = base::BindRepeating(
- &ParentAccessViewTest::OnFinished, base::Unretained(this));
+ void OnBack() override { ++back_action_; }
- validation_time_ = base::Time::Now();
- view_ =
- new ParentAccessView(account_id_, callbacks, reason, validation_time_);
+ void OnHelp(gfx::NativeWindow parent_window) override {
+ ++help_dialog_opened_;
+ }
+
+ void StartView(base::Optional<int> pin_length = 6) {
+ ParentAccessRequest request;
+ request.help_button_enabled = true;
+ request.pin_length = pin_length;
+ request.on_parent_access_done = base::DoNothing::Once<bool>();
+ view_ = new ParentAccessView(std::move(request), this);
+
SetWidget(CreateWidgetWithContent(view_));
}
// Shows parent access widget with the specified |reason|.
- void ShowWidget(ParentAccessRequestReason reason =
- ParentAccessRequestReason::kUnlockTimeLimits) {
- validation_time_ = base::Time::Now();
- ParentAccessWidget::Show(
- account_id_,
- base::BindRepeating(&ParentAccessViewTest::OnFinished,
- base::Unretained(this)),
- reason, false /*extra_dimmer*/, validation_time_);
+ void ShowWidget(base::Optional<int> pin_length = 6) {
+ ParentAccessRequest request;
+ request.help_button_enabled = true;
+ request.pin_length = pin_length;
+ request.on_parent_access_done = base::DoNothing::Once<bool>();
+ ParentAccessWidget::Show(std::move(request), this);
ParentAccessWidget* widget = ParentAccessWidget::Get();
ASSERT_TRUE(widget);
}
@@ -145,43 +132,35 @@
view->ButtonPressed(test_api.back_button(), event);
}
- // Verifies expectation that UMA |action| was logged.
- void ExpectUMAActionReported(ParentAccessView::UMAAction action,
- int bucket_count,
- int total_count) {
- histogram_tester_.ExpectBucketCount(
- ParentAccessView::kUMAParentAccessCodeAction, action, bucket_count);
- histogram_tester_.ExpectTotalCount(
- ParentAccessView::kUMAParentAccessCodeAction, total_count);
- }
-
void SimulateFailedValidation() {
- login_client_->set_validate_parent_access_code_result(false);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
- validation_time_))
- .Times(1);
-
+ will_authenticate_ = false;
ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012345", last_code_submitted_);
+ pin_submitted_ = 0;
}
- const AccountId account_id_;
std::unique_ptr<MockLoginScreenClient> login_client_;
+ // Number of times the Pin was submitted.
+ int pin_submitted_ = 0;
+
+ // The code that was submitted the last time.
+ std::string last_code_submitted_ = "";
+
// Number of times the view was dismissed with back button.
int back_action_ = 0;
- // Number of times the view was dismissed after successful validation.
- int successful_validation_ = 0;
+ // Number of times the help dialog was opened.
+ int help_dialog_opened_ = 0;
- // Time that will be used on the code validation.
- base::Time validation_time_;
-
- base::HistogramTester histogram_tester_;
+ // Whether the next pin submission will trigger setting an error state.
+ bool will_authenticate_ = true;
ParentAccessView* view_ = nullptr; // Owned by test widget view hierarchy.
@@ -189,30 +168,11 @@
DISALLOW_COPY_AND_ASSIGN(ParentAccessViewTest);
};
-class ParentAccessViewModifiersTest
- : public ParentAccessViewTest,
- public ::testing::WithParamInterface<ViewModifiersTestData> {};
-
-} // namespace
-
-// Tests that title and description are correctly set.
-TEST_P(ParentAccessViewModifiersTest, CheckStrings) {
- const ViewModifiersTestData& test_data = GetParam();
- StartView(test_data.reason);
- ParentAccessView::TestApi test_api(view_);
- EXPECT_EQ(l10n_util::GetStringUTF16(test_data.title),
- test_api.title_label()->GetText());
- EXPECT_EQ(l10n_util::GetStringUTF16(test_data.description),
- test_api.description_label()->GetText());
-}
-
-INSTANTIATE_TEST_SUITE_P(All,
- ParentAccessViewModifiersTest,
- testing::ValuesIn(kViewModifiersTestData));
-
// Tests that back button works.
TEST_F(ParentAccessViewTest, BackButton) {
- StartView();
+ ShowWidget();
+ ParentAccessWidget* widget = ParentAccessWidget::Get();
+ view_ = ParentAccessWidget::TestApi(widget).parent_access_view();
ParentAccessView::TestApi test_api(view_);
EXPECT_TRUE(test_api.back_button()->GetEnabled());
EXPECT_EQ(0, back_action_);
@@ -220,8 +180,7 @@
SimulateButtonPress(test_api.back_button());
EXPECT_EQ(1, back_action_);
- EXPECT_EQ(0, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kCanceledByUser, 1, 1);
+ EXPECT_EQ(nullptr, ParentAccessWidget::Get());
}
// Tests that the code is autosubmitted when input is complete.
@@ -230,11 +189,6 @@
ParentAccessView::TestApi test_api(view_);
EXPECT_FALSE(test_api.submit_button()->GetEnabled());
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
- validation_time_))
- .Times(1);
-
ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
@@ -242,9 +196,8 @@
base::RunLoop().RunUntilIdle();
}
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 1);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012345", last_code_submitted_);
}
// Tests that submit button submits code from code input.
@@ -257,35 +210,25 @@
auto* generator = GetEventGenerator();
// Updating input code (here last digit) should clear error state.
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
EXPECT_TRUE(test_api.submit_button()->GetEnabled());
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
- validation_time_))
- .Times(1);
-
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 2);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012346", last_code_submitted_);
}
// Tests that help button opens help app.
TEST_F(ParentAccessViewTest, HelpButton) {
- auto client = std::make_unique<MockLoginScreenClient>();
StartView();
ParentAccessView::TestApi test_api(view_);
EXPECT_TRUE(test_api.help_button()->GetEnabled());
- EXPECT_CALL(*client, ShowParentAccessHelpApp(widget()->GetNativeWindow()))
- .Times(1);
SimulateButtonPress(test_api.help_button());
base::RunLoop().RunUntilIdle();
-
- ExpectUMAActionReported(ParentAccessView::UMAAction::kGetHelp, 1, 1);
+ EXPECT_EQ(1, help_dialog_opened_);
}
// Tests that access code can be entered with numpad.
@@ -293,19 +236,14 @@
StartView();
ParentAccessView::TestApi test_api(view_);
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
- validation_time_))
- .Times(1);
ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::VKEY_NUMPAD0 + i), ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->GetEnabled());
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 1);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012345", last_code_submitted_);
}
// Tests that access code can be submitted with press of 'enter' key.
@@ -319,18 +257,12 @@
// Updating input code (here last digit) should clear error state.
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
-
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
- validation_time_))
- .Times(1);
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 2);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012346", last_code_submitted_);
}
// Tests that 'enter' key does not submit incomplete code.
@@ -348,38 +280,27 @@
}
EXPECT_FALSE(test_api.submit_button()->GetEnabled());
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_).Times(0);
-
// Pressing enter should not submit incomplete code.
generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(0, successful_validation_);
+ EXPECT_EQ(0, pin_submitted_);
- login_client_->set_validate_parent_access_code_result(false);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012349",
- validation_time_))
- .Times(1);
-
- // Fill in last digit of the code.
+ // Fill in last digit of the code, set error on submission.
+ will_authenticate_ = false;
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_9), ui::EF_NONE);
base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012349", last_code_submitted_);
// Updating input code (here last digit) should clear error state.
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
-
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
- validation_time_))
- .Times(1);
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
// Now the code should be submitted with enter key.
generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 2);
+ EXPECT_EQ(2, pin_submitted_);
+ EXPECT_EQ("012346", last_code_submitted_);
}
// Tests that backspace button works.
@@ -389,7 +310,7 @@
EXPECT_FALSE(test_api.submit_button()->GetEnabled());
SimulateFailedValidation();
- EXPECT_EQ(ParentAccessView::State::kError, test_api.state());
+ EXPECT_EQ(ParentAccessRequestViewState::kError, test_api.state());
ui::test::EventGenerator* generator = GetEventGenerator();
@@ -412,16 +333,35 @@
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(test_api.submit_button()->GetEnabled());
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012323",
- validation_time_))
- .Times(1);
-
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 2);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012323", last_code_submitted_);
+}
+
+// Tests input with unknown pin length.
+TEST_F(ParentAccessViewTest, FlexCodeInput) {
+ StartView(base::nullopt);
+ ParentAccessView::TestApi test_api(view_);
+ ui::test::EventGenerator* generator = GetEventGenerator();
+ will_authenticate_ = false;
+
+ EXPECT_FALSE(test_api.submit_button()->GetEnabled());
+ for (int i = 0; i < 8; ++i) {
+ generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
+ ui::EF_NONE);
+ base::RunLoop().RunUntilIdle();
+ }
+ EXPECT_TRUE(test_api.submit_button()->GetEnabled());
+ SimulateButtonPress(test_api.submit_button());
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("01234567", last_code_submitted_);
+
+ // Test Backspace.
+ generator->PressKey(ui::KeyboardCode::VKEY_BACK, ui::EF_NONE);
+ SimulateButtonPress(test_api.submit_button());
+ EXPECT_EQ(2, pin_submitted_);
+ EXPECT_EQ("0123456", last_code_submitted_);
}
// Tests input with virtual pin keyboard.
@@ -435,18 +375,12 @@
LoginPinView::TestApi test_pin_keyboard(test_api.pin_keyboard_view());
EXPECT_FALSE(test_api.submit_button()->GetEnabled());
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
- validation_time_))
- .Times(1);
-
for (int i = 0; i < 6; ++i) {
SimulatePinKeyboardPress(test_pin_keyboard.GetButton(i));
base::RunLoop().RunUntilIdle();
}
- EXPECT_EQ(1, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
- 1);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012345", last_code_submitted_);
}
// Tests that pin keyboard visibility changes upon tablet mode changes.
@@ -467,30 +401,21 @@
TEST_F(ParentAccessViewTest, ErrorState) {
StartView();
ParentAccessView::TestApi test_api(view_);
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
// Error should be shown after unsuccessful validation.
SimulateFailedValidation();
- EXPECT_EQ(ParentAccessView::State::kError, test_api.state());
-
- EXPECT_EQ(0, successful_validation_);
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationError, 1, 1);
+ EXPECT_EQ(ParentAccessRequestViewState::kError, test_api.state());
// Updating input code (here last digit) should clear error state.
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
-
- login_client_->set_validate_parent_access_code_result(true);
- EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
- validation_time_))
- .Times(1);
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(1, successful_validation_);
-
- ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationError, 1, 2);
+ EXPECT_EQ(1, pin_submitted_);
+ EXPECT_EQ("012346", last_code_submitted_);
}
// Tests children views traversal with tab key.
@@ -505,7 +430,7 @@
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
EXPECT_TRUE(test_api.submit_button()->HasFocus());
generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_NONE);
@@ -528,7 +453,7 @@
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
+ EXPECT_EQ(ParentAccessRequestViewState::kNormal, test_api.state());
EXPECT_TRUE(test_api.submit_button()->HasFocus());
generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_SHIFT_DOWN);
@@ -552,54 +477,11 @@
using ParentAccessWidgetTest = ParentAccessViewTest;
-// Tests that correct usage metric is reported.
-TEST_F(ParentAccessWidgetTest, UMAUsageMetric) {
- ShowWidget(ParentAccessRequestReason::kUnlockTimeLimits);
- DismissWidget();
- histogram_tester_.ExpectBucketCount(
- ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimeLimits, 1);
-
- ShowWidget(ParentAccessRequestReason::kChangeTimezone);
- DismissWidget();
- histogram_tester_.ExpectBucketCount(
- ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimezoneChange, 1);
-
- // The below usage depends on the session state.
- GetSessionControllerClient()->SetSessionState(
- session_manager::SessionState::ACTIVE);
- ShowWidget(ParentAccessRequestReason::kChangeTime);
- DismissWidget();
- histogram_tester_.ExpectBucketCount(
- ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimeChangeInSession, 1);
-
- GetSessionControllerClient()->SetSessionState(
- session_manager::SessionState::LOGIN_PRIMARY);
- ShowWidget(ParentAccessRequestReason::kChangeTime);
- DismissWidget();
- histogram_tester_.ExpectBucketCount(
- ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimeChangeLoginScreen, 1);
-
- GetSessionControllerClient()->SetSessionState(
- session_manager::SessionState::ACTIVE);
- ShowWidget(ParentAccessRequestReason::kChangeTime);
- DismissWidget();
- histogram_tester_.ExpectBucketCount(
- ParentAccessView::kUMAParentAccessCodeUsage,
- ParentAccessView::UMAUsage::kTimeChangeInSession, 2);
-
- histogram_tester_.ExpectTotalCount(
- ParentAccessView::kUMAParentAccessCodeUsage, 5);
-}
-
// Tests that the widget is properly resized when tablet mode changes.
TEST_F(ParentAccessWidgetTest, WidgetResizingInTabletMode) {
// Set display large enough to fit preferred view sizes.
UpdateDisplay("1200x800");
- ShowWidget(ParentAccessRequestReason::kUnlockTimeLimits);
+ ShowWidget();
ParentAccessWidget* widget = ParentAccessWidget::Get();
ASSERT_TRUE(widget);
@@ -636,7 +518,7 @@
EXPECT_EQ(kClamshellModeSize, view->size());
EXPECT_EQ(kClamshellModeSize, widget_size());
EXPECT_EQ(user_work_area_center(), widget_center());
- widget->Destroy();
+ widget->Close(false /* validation success */);
}
TEST_F(ParentAccessViewTest, VirtualKeyboardHidden) {
@@ -647,7 +529,7 @@
keyboard::KeyboardEnableFlag::kCommandLineEnabled);
// Show widget.
- ShowWidget(ParentAccessRequestReason::kUnlockTimeLimits);
+ ShowWidget();
auto* view = ParentAccessWidget::TestApi(ParentAccessWidget::Get())
.parent_access_view();
ParentAccessView::TestApi test_api(view);
@@ -669,7 +551,7 @@
// Tests that spoken feedback keycombo starts screen reader.
TEST_F(ParentAccessWidgetTest, SpokenFeedbackKeyCombo) {
- ShowWidget(ParentAccessRequestReason::kUnlockTimeLimits);
+ ShowWidget();
AccessibilityControllerImpl* controller =
Shell::Get()->accessibility_controller();
diff --git a/ash/login/ui/parent_access_widget.cc b/ash/login/ui/parent_access_widget.cc
index f879282..9b8b3c0f 100644
--- a/ash/login/ui/parent_access_widget.cc
+++ b/ash/login/ui/parent_access_widget.cc
@@ -7,7 +7,6 @@
#include <utility>
#include "ash/keyboard/keyboard_controller_impl.h"
-#include "ash/login/ui/parent_access_view.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
@@ -33,32 +32,11 @@
parent_access_widget_->widget_->widget_delegate());
}
-void ParentAccessWidget::TestApi::SimulateValidationFinished(
- bool access_granted) {
- parent_access_widget_->OnExit(access_granted);
-}
-
// static
-void ParentAccessWidget::Show(const AccountId& child_account_id,
- OnExitCallback callback,
- ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time) {
- if (instance_) {
- VLOG(1) << "Showing existing instance of ParentAccessWidget.";
- instance_->Show();
- return;
- }
-
- instance_ = new ParentAccessWidget(child_account_id, std::move(callback),
- reason, extra_dimmer, validation_time);
-}
-
-// static
-void ParentAccessWidget::Show(const AccountId& account_id,
- OnExitCallback callback,
- ParentAccessRequestReason reason) {
- Show(account_id, std::move(callback), reason, false, base::Time());
+void ParentAccessWidget::Show(ParentAccessRequest request,
+ ParentAccessView::Delegate* delegate) {
+ DCHECK(!instance_);
+ instance_ = new ParentAccessWidget(std::move(request), delegate);
}
// static
@@ -66,20 +44,26 @@
return instance_;
}
-void ParentAccessWidget::Destroy() {
+void ParentAccessWidget::UpdateState(ParentAccessRequestViewState state,
+ const base::string16& title,
+ const base::string16& description) {
DCHECK_EQ(instance_, this);
- widget_->Close();
-
- delete instance_;
- instance_ = nullptr;
+ static_cast<ParentAccessView*>(widget_->widget_delegate())
+ ->UpdateState(state, title, description);
}
-ParentAccessWidget::ParentAccessWidget(const AccountId& account_id,
- OnExitCallback callback,
- ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time)
- : callback_(std::move(callback)) {
+void ParentAccessWidget::Close(bool success) {
+ DCHECK_EQ(instance_, this);
+ ParentAccessWidget* instance = instance_;
+ instance_ = nullptr;
+ std::move(on_parent_access_done_).Run(success);
+ widget_->Close();
+ delete instance;
+}
+
+ParentAccessWidget::ParentAccessWidget(ParentAccessRequest request,
+ ParentAccessView::Delegate* delegate)
+ : on_parent_access_done_(std::move(request.on_parent_access_done)) {
views::Widget::InitParams widget_params;
// Using window frameless to be able to get focus on the view input fields,
// which does not work with popup type.
@@ -97,15 +81,11 @@
: kShellWindowId_LockSystemModalContainer;
widget_params.parent =
Shell::GetPrimaryRootWindow()->GetChildById(parent_window_id);
-
- ParentAccessView::Callbacks callbacks;
- callbacks.on_finished = base::BindRepeating(&ParentAccessWidget::OnExit,
- weak_factory_.GetWeakPtr());
- widget_params.delegate =
- new ParentAccessView(account_id, callbacks, reason, validation_time);
-
- if (extra_dimmer)
+ if (request.extra_dimmer)
dimmer_ = std::make_unique<WindowDimmer>(widget_params.parent);
+ request.on_parent_access_done =
+ base::BindOnce(&ParentAccessWidget::Close, weak_factory_.GetWeakPtr());
+ widget_params.delegate = new ParentAccessView(std::move(request), delegate);
widget_ = std::make_unique<views::Widget>();
widget_->Init(std::move(widget_params));
@@ -127,9 +107,4 @@
keyboard_controller->HideKeyboard(HideReason::kSystem);
}
-void ParentAccessWidget::OnExit(bool success) {
- callback_.Run(success);
- Destroy();
-}
-
} // namespace ash
diff --git a/ash/login/ui/parent_access_widget.h b/ash/login/ui/parent_access_widget.h
index 35cb666..4cb90eee 100644
--- a/ash/login/ui/parent_access_widget.h
+++ b/ash/login/ui/parent_access_widget.h
@@ -8,14 +8,13 @@
#include <memory>
#include "ash/ash_export.h"
+#include "ash/login/ui/parent_access_view.h"
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/time/time.h"
-class AccountId;
-
namespace views {
class Widget;
}
@@ -23,9 +22,9 @@
namespace ash {
class WindowDimmer;
-class ParentAccessView;
enum class ParentAccessRequestReason;
+enum class ParentAccessRequestViewState;
// Widget to display the Parent Access View in a standalone container.
// This widget is modal and only one instance can be created at a time. It will
@@ -48,61 +47,39 @@
ParentAccessWidget* const parent_access_widget_;
};
- // Callback for Parent Access Code validations. It is called when the widget
- // is about to be dismissed. |success| tells whether the validation was
- // successful.
- using success)>;
-
// Creates and shows the instance of ParentAccessWidget.
// This widget is modal and only one instance can be created at a time. It
// will be destroyed when dismissed.
- // When |account_id| is valid, the parent access code is validated using the
- // configuration for the provided account, when it is empty it tries to
- // validate the access code to any child signed in the device.
- // The |callback| is called when (a) the validation is successful or (b) the
- // back button is pressed.
- // |reason| contains information about why the parent access view is
- // necessary, it is used to modify the widget appearance by changing the title
- // and description strings and background color. The parent access widget is a
- // modal and already contains a dimmer, however when another modal is the
- // parent of the widget, the dimmer will be placed behind the two windows.
- // |extra_dimmer| will create an extra dimmer between the two.
- // |validation_time| is the time that will be used to validate the code, if
- // null the system's current time will be used.
- static void Show(const AccountId& account_id,
- OnExitCallback callback,
- ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time);
- static void Show(const AccountId& account_id,
- OnExitCallback callback,
- ParentAccessRequestReason reason);
+ static void Show(ParentAccessRequest request,
+ ParentAccessView::Delegate* delegate);
// Returns the instance of ParentAccessWidget or nullptr if it does not exits.
static ParentAccessWidget* Get();
- // Destroys the instance of ParentAccessWidget.
- void Destroy();
+ // Toggles showing an error state and updates displayed strings.
+ void UpdateState(ParentAccessRequestViewState state,
+ const base::string16& title,
+ const base::string16& description);
+
+ // Closes the widget.
+ // |success| describes whether the validation was successful and is passed to
+ // |on_parent_access_done_|.
+ void Close(bool success);
private:
- ParentAccessWidget(const AccountId& account_id,
- OnExitCallback callback,
- ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time);
+ ParentAccessWidget(ParentAccessRequest request,
+ ParentAccessView::Delegate* delegate);
~ParentAccessWidget();
// Shows the |widget_| and |dimmer_| if applicable.
void Show();
- // Closes the widget and forwards the result to the validation to |callback_|.
- void OnExit(bool success);
+ // Callback invoked when closing the widget.
+ ParentAccessRequest::OnParentAccessDone on_parent_access_done_;
std::unique_ptr<views::Widget> widget_;
std::unique_ptr<WindowDimmer> dimmer_;
- OnExitCallback callback_;
-
base::WeakPtrFactory<ParentAccessWidget> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(ParentAccessWidget);
diff --git a/ash/public/cpp/login_screen.h b/ash/public/cpp/login_screen.h
index 0ccefc8..53e1545 100644
--- a/ash/public/cpp/login_screen.h
+++ b/ash/public/cpp/login_screen.h
@@ -63,7 +63,7 @@
virtual void ShowParentAccessButton(bool show) = 0;
// Shows a standalone Parent Access dialog. If |child_account_id| is valid, it
- // validates the parent access code for that child only, when it is empty it
+ // validates the parent access code for that child only, when it is empty it
// validates the code for any child signed in the device. |callback| is
// invoked when the back button is clicked or the correct code was entered.
// |reason| contains information about why the parent access view is
@@ -78,7 +78,7 @@
// necessarily fail.
virtual void ShowParentAccessWidget(
const AccountId& child_account_id,
- base::RepeatingCallback<void(bool success)> callback,
+ base::OnceCallback<void(bool success)> callback,
ParentAccessRequestReason reason,
bool extra_dimmer,
base::Time validation_time) = 0;
diff --git a/ash/shell.cc b/ash/shell.cc
index 42c4eef0..ae9c7d4 100644
--- a/ash/shell.cc
+++ b/ash/shell.cc
@@ -58,6 +58,7 @@
#include "ash/keyboard/ui/keyboard_ui_factory.h"
#include "ash/laser/laser_pointer_controller.h"
#include "ash/login/login_screen_controller.h"
+#include "ash/login/parent_access_controller.h"
#include "ash/login_status.h"
#include "ash/magnifier/docked_magnifier_controller_impl.h"
#include "ash/magnifier/magnification_controller.h"
@@ -533,6 +534,7 @@
keyboard_brightness_control_delegate_(
std::make_unique<KeyboardBrightnessController>()),
locale_update_controller_(std::make_unique<LocaleUpdateControllerImpl>()),
+ parent_access_controller_(std::make_unique<ParentAccessController>()),
ash_color_provider_(std::make_unique<AshColorProvider>()),
session_controller_(std::make_unique<SessionControllerImpl>()),
shell_delegate_(std::move(shell_delegate)),
diff --git a/ash/shell.h b/ash/shell.h
index 2ce575e8..42fbedab 100644
--- a/ash/shell.h
+++ b/ash/shell.h
@@ -141,6 +141,7 @@
class NightLightControllerImpl;
class OverlayEventFilter;
class OverviewController;
+class ParentAccessController;
class PartialMagnificationController;
class PeripheralBatteryNotifier;
class PeripheralBatteryTracker;
@@ -410,6 +411,9 @@
return privacy_screen_controller_.get();
}
OverlayEventFilter* overlay_filter() { return overlay_filter_.get(); }
+ ParentAccessController* parent_access_controller() {
+ return parent_access_controller_.get();
+ }
PartialMagnificationController* partial_magnification_controller() {
return partial_magnification_controller_.get();
}
@@ -654,6 +658,7 @@
std::unique_ptr<MruWindowTracker> mru_window_tracker_;
std::unique_ptr<MultiDeviceNotificationPresenter>
multidevice_notification_presenter_;
+ std::unique_ptr<ParentAccessController> parent_access_controller_;
std::unique_ptr<ResizeShadowController> resize_shadow_controller_;
std::unique_ptr<AshColorProvider> ash_color_provider_;
std::unique_ptr<SessionControllerImpl> session_controller_;
diff --git a/chrome/browser/ui/ash/test_login_screen.cc b/chrome/browser/ui/ash/test_login_screen.cc
index 9b6fcdb..17b227daf 100644
--- a/chrome/browser/ui/ash/test_login_screen.cc
+++ b/chrome/browser/ui/ash/test_login_screen.cc
@@ -48,7 +48,7 @@
void TestLoginScreen::ShowParentAccessWidget(
const AccountId& child_account_id,
- base::RepeatingCallback<void(bool success)> callback,
+ base::OnceCallback<void(bool success)> callback,
ash::ParentAccessRequestReason reason,
bool extra_dimmer,
base::Time validation_time) {}
diff --git a/chrome/browser/ui/ash/test_login_screen.h b/chrome/browser/ui/ash/test_login_screen.h
index 4153812..64c0f74 100644
--- a/chrome/browser/ui/ash/test_login_screen.h
+++ b/chrome/browser/ui/ash/test_login_screen.h
@@ -34,12 +34,11 @@
void EnableShutdownButton(bool enable) override;
void ShowGuestButtonInOobe(bool show) override;
void ShowParentAccessButton(bool show) override;
- void ShowParentAccessWidget(
- const AccountId& child_account_id,
- base::RepeatingCallback<void(bool success)> callback,
- ash::ParentAccessRequestReason reason,
- bool extra_dimmer,
- base::Time validation_time) override;
+ void ShowParentAccessWidget(const AccountId& child_account_id,
+ base::OnceCallback<void(bool success)> callback,
+ ash::ParentAccessRequestReason reason,
+ bool extra_dimmer,
+ base::Time validation_time) override;
void SetAllowLoginAsGuest(bool allow_guest) override;
std::unique_ptr<ash::ScopedGuestButtonBlocker> GetScopedGuestButtonBlocker()
override;
diff --git a/chrome/browser/ui/webui/chromeos/set_time_ui.cc b/chrome/browser/ui/webui/chromeos/set_time_ui.cc
index ec4bb54..511b8d6 100644
--- a/chrome/browser/ui/webui/chromeos/set_time_ui.cc
+++ b/chrome/browser/ui/webui/chromeos/set_time_ui.cc
@@ -144,8 +144,8 @@
}
ash::LoginScreen::Get()->ShowParentAccessWidget(
account_id,
- base::BindRepeating(&SetTimeMessageHandler::OnParentAccessValidation,
- weak_factory_.GetWeakPtr()),
+ base::BindOnce(&SetTimeMessageHandler::OnParentAccessValidation,
+ weak_factory_.GetWeakPtr()),
ash::ParentAccessRequestReason::kChangeTime,
!is_user_logged_in /* extra_dimmer */,
base::Time::FromDoubleT(seconds));
diff --git a/chrome/browser/ui/webui/settings/chromeos/date_time_handler.cc b/chrome/browser/ui/webui/settings/chromeos/date_time_handler.cc
index 59656e6..39ceb89 100644
--- a/chrome/browser/ui/webui/settings/chromeos/date_time_handler.cc
+++ b/chrome/browser/ui/webui/settings/chromeos/date_time_handler.cc
@@ -168,8 +168,8 @@
ash::LoginScreen::Get()->ShowParentAccessWidget(
user_manager::UserManager::Get()->GetActiveUser()->GetAccountId(),
- base::BindRepeating(&DateTimeHandler::OnParentAccessValidation,
- weak_ptr_factory_.GetWeakPtr()),
+ base::BindOnce(&DateTimeHandler::OnParentAccessValidation,
+ weak_ptr_factory_.GetWeakPtr()),
ash::ParentAccessRequestReason::kChangeTimezone, false /* extra_dimmer */,
base::Time());
}