[Welcome Tour] Handle the actions that change the session state
This CL ensures that the accelerator actions that modify the session
states, such as the lock screen action, are handled properly.
When `WelcomeTourAcceleratorHandler` receives these accelerator
actions, the running tour is aborted. Meanwhile, these actions
continue to perform as usual.
Bug: b/287483131
Change-Id: I1b0f2307d693946d0ef7150de060a83b59941efd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4712365
Reviewed-by: David Black <dmblack@google.com>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1175826}
diff --git a/ash/ash_element_identifiers.cc b/ash/ash_element_identifiers.cc
index 2bdb308..0f70592 100644
--- a/ash/ash_element_identifiers.cc
+++ b/ash/ash_element_identifiers.cc
@@ -14,6 +14,7 @@
DEFINE_ELEMENT_IDENTIFIER_VALUE(kExploreAppElementId);
DEFINE_ELEMENT_IDENTIFIER_VALUE(kHoldingSpaceTrayElementId);
DEFINE_ELEMENT_IDENTIFIER_VALUE(kHomeButtonElementId);
+DEFINE_ELEMENT_IDENTIFIER_VALUE(kLoginUserViewElementId);
DEFINE_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsSettingsButtonElementId);
DEFINE_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsViewElementId);
DEFINE_ELEMENT_IDENTIFIER_VALUE(kSearchBoxViewElementId);
diff --git a/ash/ash_element_identifiers.h b/ash/ash_element_identifiers.h
index 61a9da3..d5693b4 100644
--- a/ash/ash_element_identifiers.h
+++ b/ash/ash_element_identifiers.h
@@ -33,6 +33,9 @@
// Uniquely identifies the home (launcher) button.
DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(ASH_EXPORT, kHomeButtonElementId);
+// Uniquely identifies the `LoginUserView`.
+DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(ASH_EXPORT, kLoginUserViewElementId);
+
DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(
ASH_EXPORT,
kQuickSettingsSettingsButtonElementId);
diff --git a/ash/login/ui/login_user_view.cc b/ash/login/ui/login_user_view.cc
index cecc001..f9e5374 100644
--- a/ash/login/ui/login_user_view.cc
+++ b/ash/login/ui/login_user_view.cc
@@ -6,6 +6,7 @@
#include <memory>
+#include "ash/ash_element_identifiers.h"
#include "ash/login/ui/animated_rounded_image_view.h"
#include "ash/login/ui/hover_notifier.h"
#include "ash/login/ui/image_parser.h"
@@ -416,6 +417,8 @@
// `on_dropdown_pressed` is only available iff `show_dropdown` is true.
DCHECK(show_dropdown == !!on_dropdown_pressed);
+ SetProperty(views::kElementIdentifierKey, kLoginUserViewElementId);
+
user_image_ = new UserImage(style);
int label_width =
WidthForLayoutStyle(style) -
diff --git a/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.cc b/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.cc
index 1b4d918..25b09be 100644
--- a/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.cc
+++ b/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.cc
@@ -6,17 +6,20 @@
#include "ash/accelerators/ash_accelerator_configuration.h"
#include "ash/shell.h"
-#include "base/containers/contains.h"
+#include "base/ranges/algorithm.h"
+#include "base/task/sequenced_task_runner.h"
#include "ui/events/event.h"
#include "ui/events/event_target.h"
namespace ash {
// static
-constexpr std::array<AcceleratorAction, 7>
+constexpr std::array<WelcomeTourAcceleratorHandler::AllowedAction, 12>
WelcomeTourAcceleratorHandler::kAllowedActions;
-WelcomeTourAcceleratorHandler::WelcomeTourAcceleratorHandler() {
+WelcomeTourAcceleratorHandler::WelcomeTourAcceleratorHandler(
+ base::RepeatingClosure abort_tour_callback)
+ : abort_tour_callback_(abort_tour_callback) {
Shell::Get()->AddPreTargetHandler(this, ui::EventTarget::Priority::kSystem);
}
@@ -28,10 +31,22 @@
const AcceleratorAction* const action =
Shell::Get()->ash_accelerator_configuration()->FindAcceleratorAction(
ui::Accelerator(*event));
+ if (!action) {
+ // Return early if `event` does not trigger any accelerator action.
+ return;
+ }
- // Block `event` if the action corresponding to `event` is not allowed.
- if (action && !base::Contains(kAllowedActions, *action)) {
+ auto* action_it =
+ base::ranges::find(kAllowedActions, *action, &AllowedAction::action);
+
+ if (action_it == kAllowedActions.cend()) {
+ // Block `event` if `action` is not allowed.
event->StopPropagation();
+ } else if (action_it->aborts_tour) {
+ // Aborting the Welcome Tour could affect the enabling of `action`.
+ // Therefore, abort the Welcome Tour asynchronously.
+ base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
+ FROM_HERE, abort_tour_callback_);
}
}
diff --git a/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.h b/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.h
index 28cd4eb..a1cfdfb 100644
--- a/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.h
+++ b/ash/user_education/welcome_tour/welcome_tour_accelerator_handler.h
@@ -8,6 +8,7 @@
#include <array>
#include "ash/public/cpp/accelerator_actions.h"
+#include "base/functional/callback.h"
#include "ui/events/event_handler.h"
namespace ash {
@@ -15,18 +16,32 @@
// Handles the accelerator key events during the Welcome Tour.
class WelcomeTourAcceleratorHandler : public ui::EventHandler {
public:
- // The accelerator actions allowed during the Welcome Tour.
- static constexpr std::array<AcceleratorAction, 7> kAllowedActions = {
- AcceleratorAction::kBrightnessDown,
- AcceleratorAction::kBrightnessUp,
- AcceleratorAction::kPrintUiHierarchies,
- AcceleratorAction::kTakeScreenshot,
- AcceleratorAction::kVolumeDown,
- AcceleratorAction::kVolumeMute,
- AcceleratorAction::kVolumeUp,
+ struct AllowedAction {
+ const AcceleratorAction action;
+
+ // If true, when `action` is triggered during the Welcome Tour, the tour is
+ // aborted and `action` continues to perform as usual.
+ const bool aborts_tour;
};
- WelcomeTourAcceleratorHandler();
+ // The accelerator actions allowed during the Welcome Tour.
+ static constexpr std::array<AllowedAction, 12> kAllowedActions = {{
+ {AcceleratorAction::kBrightnessDown, /*aborts_tour=*/false},
+ {AcceleratorAction::kBrightnessUp, /*aborts_tour=*/false},
+ {AcceleratorAction::kExit, /*aborts_tour=*/true},
+ {AcceleratorAction::kLockScreen, /*aborts_tour=*/true},
+ {AcceleratorAction::kPowerPressed, /*aborts_tour=*/true},
+ {AcceleratorAction::kPowerReleased, /*aborts_tour=*/true},
+ {AcceleratorAction::kPrintUiHierarchies, /*aborts_tour=*/false},
+ {AcceleratorAction::kSuspend, /*aborts_tour=*/true},
+ {AcceleratorAction::kTakeScreenshot, /*aborts_tour=*/false},
+ {AcceleratorAction::kVolumeDown, /*aborts_tour=*/false},
+ {AcceleratorAction::kVolumeMute, /*aborts_tour=*/false},
+ {AcceleratorAction::kVolumeUp, /*aborts_tour=*/false},
+ }};
+
+ explicit WelcomeTourAcceleratorHandler(
+ base::RepeatingClosure abort_tour_callback);
WelcomeTourAcceleratorHandler(const WelcomeTourAcceleratorHandler&) = delete;
WelcomeTourAcceleratorHandler& operator=(
const WelcomeTourAcceleratorHandler&) = delete;
@@ -35,6 +50,9 @@
private:
// ui::EventHandler:
void OnKeyEvent(ui::KeyEvent* event) override;
+
+ // The callback to abort the Welcome Tour.
+ base::RepeatingClosure abort_tour_callback_;
};
} // namespace ash
diff --git a/ash/user_education/welcome_tour/welcome_tour_controller.cc b/ash/user_education/welcome_tour/welcome_tour_controller.cc
index b5b5c45..a28f473 100644
--- a/ash/user_education/welcome_tour/welcome_tour_controller.cc
+++ b/ash/user_education/welcome_tour/welcome_tour_controller.cc
@@ -329,7 +329,9 @@
// TODO(http://b/277091619): Stabilize wallpaper.
// TODO(http://b/277091624): Stabilize nudges/toasts.
void WelcomeTourController::OnWelcomeTourStarted() {
- accelerator_handler_ = std::make_unique<WelcomeTourAcceleratorHandler>();
+ accelerator_handler_ = std::make_unique<WelcomeTourAcceleratorHandler>(
+ base::BindRepeating(&WelcomeTourController::MaybeAbortWelcomeTour,
+ weak_ptr_factory_.GetWeakPtr()));
notification_blocker_ = std::make_unique<WelcomeTourNotificationBlocker>();
notification_blocker_->Init();
diff --git a/ash/user_education/welcome_tour/welcome_tour_controller_unittest.cc b/ash/user_education/welcome_tour/welcome_tour_controller_unittest.cc
index c8aa4251..e0b6c67 100644
--- a/ash/user_education/welcome_tour/welcome_tour_controller_unittest.cc
+++ b/ash/user_education/welcome_tour/welcome_tour_controller_unittest.cc
@@ -34,14 +34,12 @@
#include "base/functional/callback.h"
#include "base/functional/overloaded.h"
#include "base/scoped_observation.h"
-#include "base/strings/string_number_conversions.h"
#include "base/test/bind.h"
#include "base/test/gmock_callback_support.h"
#include "base/test/gmock_move_support.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
#include "components/account_id/account_id.h"
-#include "components/user_education/common/help_bubble.h"
#include "components/user_education/common/tutorial_description.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -733,7 +731,8 @@
void VerifyActionsInAllowedList() {
for (auto allowed_action : WelcomeTourAcceleratorHandler::kAllowedActions) {
- PerformActionAndCheckKeyEvents(allowed_action, /*received=*/true);
+ PerformActionAndCheckKeyEvents(allowed_action.action,
+ /*received=*/true);
}
}
@@ -783,6 +782,33 @@
/*received=*/true);
}
+// Verifies the accelerator actions that abort the Welcome Tour when performed.
+TEST_F(WelcomeTourAcceleratorHandlerRunTest, CheckActionsThatAbortTour) {
+ ASSERT_NO_FATAL_FAILURE(
+ Run(/*in_progress_callback=*/base::BindLambdaForTesting([&]() {
+ for (const auto& allowed_action :
+ WelcomeTourAcceleratorHandler::kAllowedActions) {
+ if (!allowed_action.aborts_tour) {
+ // Skip `allowed_action` if it does not need to abort the tour.
+ continue;
+ }
+
+ base::test::TestFuture<void> aborted_future;
+ EXPECT_CALL(*user_education_delegate(), AbortTutorial)
+ .WillOnce(RunOnceClosure(aborted_future.GetCallback()));
+
+ // During the Welcome Tour, the key events for `action` should be
+ // received by the mock event handler.
+ PerformActionAndCheckKeyEvents(allowed_action.action,
+ /*received=*/true);
+
+ // The delegate API that aborts the Welcome Tour should be called.
+ EXPECT_TRUE(aborted_future.Wait());
+ Mock::VerifyAndClearExpectations(user_education_delegate());
+ }
+ })));
+}
+
// WelcomeTourControllerTabletTest ---------------------------------------------
// Base class for tests of the `WelcomeTourController` that verify the tour does
diff --git a/chrome/browser/ui/ash/user_education/welcome_tour/welcome_tour_interactive_uitest.cc b/chrome/browser/ui/ash/user_education/welcome_tour/welcome_tour_interactive_uitest.cc
index c498a7f..e058a8fd 100644
--- a/chrome/browser/ui/ash/user_education/welcome_tour/welcome_tour_interactive_uitest.cc
+++ b/chrome/browser/ui/ash/user_education/welcome_tour/welcome_tour_interactive_uitest.cc
@@ -10,7 +10,6 @@
#include "ash/style/system_dialog_delegate_view.h"
#include "ash/user_education/views/help_bubble_view_ash.h"
#include "ash/user_education/welcome_tour/welcome_tour_controller.h"
-#include "ash/user_education/welcome_tour/welcome_tour_dialog.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/ash/app_list/app_list_client_impl.h"
#include "chrome/browser/ash/system_web_apps/system_web_app_manager.h"
@@ -24,8 +23,10 @@
#include "chrome/test/interaction/interactive_browser_test.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/test/browser_test.h"
-#include "ui/aura/window.h"
#include "ui/base/l10n/l10n_util.h"
+#include "ui/events/event_constants.h"
+#include "ui/events/keycodes/keyboard_codes_posix.h"
+#include "ui/events/test/event_generator.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/controls/label.h"
#include "ui/views/interaction/element_tracker_views.h"
@@ -98,9 +99,11 @@
return WaitForShow(kBrowserViewElementId);
}
- // Returns a builder for an interaction step that waits for the dialog.
- [[nodiscard]] static auto WaitForDialog() {
- return WaitForShow(ash::kWelcomeTourDialogElementId);
+ // Returns a builder for an interaction step that waits for the dialog to have
+ // the expected visibility.
+ [[nodiscard]] static auto WaitForDialogVisibility(bool visible) {
+ return visible ? WaitForShow(ash::kWelcomeTourDialogElementId)
+ : WaitForHide(ash::kWelcomeTourDialogElementId);
}
// Returns a builder for an interaction step that waits for a help bubble.
@@ -108,6 +111,11 @@
return WaitForShow(ash::HelpBubbleViewAsh::kHelpBubbleElementIdForTesting);
}
+ // Returns a builder for an interaction step that waits for login user view.
+ [[nodiscard]] static auto WaitForLoginUserView() {
+ return WaitForShow(ash::kLoginUserViewElementId);
+ }
+
// Returns a builder for an interaction step that checks the visibility of
// app list bubble.
[[nodiscard]] static auto CheckAppListBubbleVisibility(bool visible) {
@@ -230,7 +238,7 @@
IN_PROC_BROWSER_TEST_F(WelcomeTourInteractiveUiTest, WelcomeTour) {
RunTestSequence(
// Step 0: Dialog.
- InAnyContext(WaitForDialog()),
+ InAnyContext(WaitForDialogVisibility(true)),
InSameContext(Steps(
CheckDialogAcceptButtonFocus(true), CheckDialogAcceptButtonText(),
CheckDialogCancelButtonText(), CheckDialogDescription(),
@@ -303,3 +311,24 @@
InSameContext(Steps(WaitForAppListBubbleToHide(),
CheckBrowserIsForWebApp(web_app::kHelpAppId))));
}
+
+IN_PROC_BROWSER_TEST_F(WelcomeTourInteractiveUiTest,
+ LockScreenDuringWelcomeTour) {
+ RunTestSequence(
+ // Wait for the Welcome Tour dialog to show.
+ InAnyContext(WaitForDialogVisibility(true)),
+
+ InSameContext(Steps(
+ // Lock screen through accelerator.
+ Do([]() {
+ ui::test::EventGenerator(ash::Shell::GetPrimaryRootWindow())
+ .PressAndReleaseKey(ui::KeyboardCode::VKEY_L,
+ ui::EF_COMMAND_DOWN);
+ }),
+
+ // Wait for the Welcome Tour dialog to hide.
+ WaitForDialogVisibility(false))),
+
+ // Wait for the login user view to show.
+ InAnyContext(WaitForLoginUserView()));
+}