[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