Commit f674ebc6 authored by Toni Barzic's avatar Toni Barzic Committed by Commit Bot

Improve logic for when to show home to overview nudge

Do not show the nudge unless the user has at least two (minimized)
app windows, so the user does not see empty overview UI.
Also, add an extra check that the nudges are not shown if the user
session is blocked.

BUG=1008963

Change-Id: I8a9b5380e011f0b84b3c5d76c24faf032931bc26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076449
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: default avatarManu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745131}
parent d7386169
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "ash/shelf/scrollable_shelf_view.h" #include "ash/shelf/scrollable_shelf_view.h"
#include "ash/shelf/shelf.h" #include "ash/shelf/shelf.h"
#include "ash/strings/grit/ash_strings.h" #include "ash/strings/grit/ash_strings.h"
#include "ash/wm/mru_window_tracker.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/location.h" #include "base/location.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -98,6 +99,13 @@ void HomeToOverviewNudgeController::SetNudgeAllowedForCurrentShelf( ...@@ -98,6 +99,13 @@ void HomeToOverviewNudgeController::SetNudgeAllowedForCurrentShelf(
return; return;
} }
// Make sure that the overview, if opened, would show at least two app
// windows.
MruWindowTracker::WindowList windows =
Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk);
if (windows.size() < 2)
return;
DCHECK(!nudge_); DCHECK(!nudge_);
PrefService* pref_service = PrefService* pref_service =
......
...@@ -88,6 +88,8 @@ class HomeToOverviewNudgeControllerTest : public AshTestBase { ...@@ -88,6 +88,8 @@ class HomeToOverviewNudgeControllerTest : public AshTestBase {
// AshTestBase: // AshTestBase:
void SetUp() override { void SetUp() override {
AshTestBase::SetUp(); AshTestBase::SetUp();
GetSessionControllerClient()->SetSessionState(
session_manager::SessionState::LOGIN_PRIMARY);
test_clock_.Advance(base::TimeDelta::FromHours(2)); test_clock_.Advance(base::TimeDelta::FromHours(2));
contextual_tooltip::OverrideClockForTesting(&test_clock_); contextual_tooltip::OverrideClockForTesting(&test_clock_);
} }
...@@ -112,6 +114,21 @@ class HomeToOverviewNudgeControllerTest : public AshTestBase { ...@@ -112,6 +114,21 @@ class HomeToOverviewNudgeControllerTest : public AshTestBase {
return GetPrimaryShelf()->shelf_widget()->hotseat_widget(); return GetPrimaryShelf()->shelf_widget()->hotseat_widget();
} }
// Helper that creates and minimzes |count| number of windows.
using ScopedWindowList = std::vector<std::unique_ptr<aura::Window>>;
ScopedWindowList CreateAndMinimizeWindows(int count) {
ScopedWindowList windows;
for (int i = 0; i < count; ++i) {
std::unique_ptr<aura::Window> window =
CreateTestWindow(gfx::Rect(0, 0, 400, 400));
WindowState::Get(window.get())->Minimize();
windows.push_back(std::move(window));
}
return windows;
}
void SanityCheckNudgeBounds() { void SanityCheckNudgeBounds() {
views::Widget* const nudge_widget = GetNudgeWidget(); views::Widget* const nudge_widget = GetNudgeWidget();
ASSERT_TRUE(nudge_widget); ASSERT_TRUE(nudge_widget);
...@@ -164,23 +181,50 @@ TEST_F(HomeToOverviewNudgeControllerWithNudgesDisabledTest, ...@@ -164,23 +181,50 @@ TEST_F(HomeToOverviewNudgeControllerWithNudgesDisabledTest,
->shelf_layout_manager() ->shelf_layout_manager()
->home_to_overview_nudge_controller_for_testing()); ->home_to_overview_nudge_controller_for_testing());
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
std::unique_ptr<aura::Window> window_1 =
CreateTestWindow(gfx::Rect(0, 0, 400, 400));
WindowState::Get(window_1.get())->Minimize();
std::unique_ptr<aura::Window> window_2 =
CreateTestWindow(gfx::Rect(0, 0, 400, 400));
WindowState::Get(window_2.get())->Minimize();
EXPECT_FALSE(GetPrimaryShelf() EXPECT_FALSE(GetPrimaryShelf()
->shelf_layout_manager() ->shelf_layout_manager()
->home_to_overview_nudge_controller_for_testing()); ->home_to_overview_nudge_controller_for_testing());
} }
// Tests that home to overview nudge is not shown before user logs in.
TEST_F(HomeToOverviewNudgeControllerTest, NoNudgeBeforeLogin) {
TabletModeControllerTestApi().EnterTabletMode();
EXPECT_FALSE(GetNudgeController());
CreateUserSessions(1);
EXPECT_TRUE(GetNudgeController());
}
// Test the flow for showing the home to overview gesture nudge - when shown the // Test the flow for showing the home to overview gesture nudge - when shown the
// first time, nudge should remain visible until the hotseat state changes. On // first time, nudge should remain visible until the hotseat state changes. On
// subsequent shows, the nudge should be hidden after a timeout. // subsequent shows, the nudge should be hidden after a timeout.
TEST_F(HomeToOverviewNudgeControllerTest, ShownOnHomeScreen) { TEST_F(HomeToOverviewNudgeControllerTest, ShownOnHomeScreen) {
CreateUserSessions(1);
// The nudge should not be shown in clamshell. // The nudge should not be shown in clamshell.
EXPECT_FALSE(GetNudgeController()); EXPECT_FALSE(GetNudgeController());
// Entering tablt mode should schedule the nudge to get shown. // In tablet mode, the nudge should be shown after at least 2 windows are
// minimized.
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
EXPECT_FALSE(GetNudgeController()->HasShowTimerForTesting());
ScopedWindowList window_1 = CreateAndMinimizeWindows(1);
EXPECT_FALSE(GetNudgeController()->HasShowTimerForTesting());
ScopedWindowList window_2 = CreateAndMinimizeWindows(1);
ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting()); ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting());
GetNudgeController()->FireShowTimerForTesting(); GetNudgeController()->FireShowTimerForTesting();
...@@ -208,7 +252,7 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShownOnHomeScreen) { ...@@ -208,7 +252,7 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShownOnHomeScreen) {
// The nudge should not show up unless the user actually transitions to home. // The nudge should not show up unless the user actually transitions to home.
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
// Create and delete a test window to force a transition to home. // Create and minimize another test window to force a transition to home.
std::unique_ptr<aura::Window> window = std::unique_ptr<aura::Window> window =
CreateTestWindow(gfx::Rect(0, 0, 400, 400)); CreateTestWindow(gfx::Rect(0, 0, 400, 400));
wm::ActivateWindow(window.get()); wm::ActivateWindow(window.get());
...@@ -234,6 +278,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShownOnHomeScreen) { ...@@ -234,6 +278,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShownOnHomeScreen) {
// Tests that the nudge eventually stops showing. // Tests that the nudge eventually stops showing.
TEST_F(HomeToOverviewNudgeControllerTest, ShownLimitedNumberOfTimes) { TEST_F(HomeToOverviewNudgeControllerTest, ShownLimitedNumberOfTimes) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
// Show the nudge kNotificationLimit amount of time. // Show the nudge kNotificationLimit amount of time.
...@@ -262,6 +308,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShownLimitedNumberOfTimes) { ...@@ -262,6 +308,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShownLimitedNumberOfTimes) {
// Tests that the nudge is hidden when tablet mode exits. // Tests that the nudge is hidden when tablet mode exits.
TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnTabletModeExit) { TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnTabletModeExit) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
...@@ -277,6 +325,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnTabletModeExit) { ...@@ -277,6 +325,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnTabletModeExit) {
// Tests that the nudge show is canceled when tablet mode exits. // Tests that the nudge show is canceled when tablet mode exits.
TEST_F(HomeToOverviewNudgeControllerTest, ShowCanceledOnTabletModeExit) { TEST_F(HomeToOverviewNudgeControllerTest, ShowCanceledOnTabletModeExit) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
...@@ -293,6 +343,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShowCanceledOnTabletModeExit) { ...@@ -293,6 +343,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ShowCanceledOnTabletModeExit) {
TEST_F(HomeToOverviewNudgeControllerTest, TEST_F(HomeToOverviewNudgeControllerTest,
ShowAnimationCanceledOnTabletModeExit) { ShowAnimationCanceledOnTabletModeExit) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
...@@ -314,6 +366,8 @@ TEST_F(HomeToOverviewNudgeControllerTest, ...@@ -314,6 +366,8 @@ TEST_F(HomeToOverviewNudgeControllerTest,
// Tests that the nudge is hidden when the screen is locked. // Tests that the nudge is hidden when the screen is locked.
TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnScreenLock) { TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnScreenLock) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
...@@ -325,12 +379,21 @@ TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnScreenLock) { ...@@ -325,12 +379,21 @@ TEST_F(HomeToOverviewNudgeControllerTest, HiddenOnScreenLock) {
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
EXPECT_EQ(gfx::Transform(), EXPECT_EQ(gfx::Transform(),
GetHotseatWidget()->GetLayer()->GetTargetTransform()); GetHotseatWidget()->GetLayer()->GetTargetTransform());
// Nudge should not be shown if a window is shown and hidden behind a lock
// screen.
test_clock_.Advance(base::TimeDelta::FromHours(25));
ScopedWindowList locked_session_window = CreateAndMinimizeWindows(1);
EXPECT_FALSE(GetNudgeController()->HasShowTimerForTesting());
} }
// Tests that the nudge show is canceled if the in-app shelf is shown before the // Tests that the nudge show is canceled if the in-app shelf is shown before the
// show timer runs. // show timer runs.
TEST_F(HomeToOverviewNudgeControllerTest, InAppShelfShownBeforeShowTimer) { TEST_F(HomeToOverviewNudgeControllerTest, InAppShelfShownBeforeShowTimer) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
EXPECT_FALSE(GetNudgeController()->nudge_for_testing()); EXPECT_FALSE(GetNudgeController()->nudge_for_testing());
...@@ -357,6 +420,9 @@ TEST_F(HomeToOverviewNudgeControllerTest, InAppShelfShownBeforeShowTimer) { ...@@ -357,6 +420,9 @@ TEST_F(HomeToOverviewNudgeControllerTest, InAppShelfShownBeforeShowTimer) {
// animation to show the nudge. // animation to show the nudge.
TEST_F(HomeToOverviewNudgeControllerTest, NudgeHiddenDuringShowAnimation) { TEST_F(HomeToOverviewNudgeControllerTest, NudgeHiddenDuringShowAnimation) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList extra_windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting()); ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting());
...@@ -407,6 +473,9 @@ TEST_F(HomeToOverviewNudgeControllerTest, NudgeHiddenDuringShowAnimation) { ...@@ -407,6 +473,9 @@ TEST_F(HomeToOverviewNudgeControllerTest, NudgeHiddenDuringShowAnimation) {
// Tests that there is no crash if the nudge widget gets closed unexpectedly. // Tests that there is no crash if the nudge widget gets closed unexpectedly.
TEST_F(HomeToOverviewNudgeControllerTest, NoCrashIfNudgeWidgetGetsClosed) { TEST_F(HomeToOverviewNudgeControllerTest, NoCrashIfNudgeWidgetGetsClosed) {
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting()); ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting());
...@@ -426,6 +495,9 @@ TEST_F(HomeToOverviewNudgeControllerTest, ...@@ -426,6 +495,9 @@ TEST_F(HomeToOverviewNudgeControllerTest,
NudgeBoundsUpdatedOnDisplayBoundsChange) { NudgeBoundsUpdatedOnDisplayBoundsChange) {
UpdateDisplay("768x1200"); UpdateDisplay("768x1200");
TabletModeControllerTestApi().EnterTabletMode(); TabletModeControllerTestApi().EnterTabletMode();
CreateUserSessions(1);
ScopedWindowList windows = CreateAndMinimizeWindows(2);
ASSERT_TRUE(GetNudgeController()); ASSERT_TRUE(GetNudgeController());
ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting()); ASSERT_TRUE(GetNudgeController()->HasShowTimerForTesting());
GetNudgeController()->FireShowTimerForTesting(); GetNudgeController()->FireShowTimerForTesting();
......
...@@ -595,6 +595,12 @@ void ShelfLayoutManager::UpdateContextualNudges() { ...@@ -595,6 +595,12 @@ void ShelfLayoutManager::UpdateContextualNudges() {
if (!ash::features::AreContextualNudgesEnabled()) if (!ash::features::AreContextualNudgesEnabled())
return; return;
// Do not allow nudges outside of an active session.
if (Shell::Get()->session_controller()->GetSessionState() !=
session_manager::SessionState::ACTIVE) {
return;
}
const bool in_app_shelf = ShelfConfig::Get()->is_in_app(); const bool in_app_shelf = ShelfConfig::Get()->is_in_app();
const bool in_tablet_mode = Shell::Get()->IsInTabletMode(); const bool in_tablet_mode = Shell::Get()->IsInTabletMode();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment