CrOS Shelf: Hide back button when not in-app

When the 'shelf-hotseat' flag is enabled, the back button will only be
shown in tablet mode when in-app.

Some unittests have been changed to ensure the back button functionality
still works both with and without the 'shelf-hotseat' flag enabled.

Bug: 1012043
Change-Id: I9754f7a3d08473d4ecdd1573aa1ecf6ce33186ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857186
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708054}

ash/shelf/back_button_unittest.cc

@@ -18,13 +18,16 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/test_accelerator_target.h"
 #include "ui/events/test/event_generator.h"
 
 namespace ash {
 
-class BackButtonTest : public AshTestBase {
+class BackButtonTest : public AshTestBase,
+                       public testing::WithParamInterface<bool> {
  public:
   BackButtonTest() = default;
   ~BackButtonTest() override = default;
@@ -35,6 +38,14 @@ class BackButtonTest : public AshTestBase {
   ShelfViewTestAPI* test_api() { return test_api_.get(); }
 
   void SetUp() override {
+    if (GetParam()) {
+      scoped_feature_list_.InitAndEnableFeature(
+          chromeos::features::kShelfHotseat);
+    } else {
+      scoped_feature_list_.InitAndDisableFeature(
+          chromeos::features::kShelfHotseat);
+    }
+
     AshTestBase::SetUp();
     test_api_ = std::make_unique<ShelfViewTestAPI>(
         GetPrimaryShelf()->GetShelfViewForTesting());
@@ -49,15 +60,29 @@ class BackButtonTest : public AshTestBase {
  protected:
   std::unique_ptr<ShelfViewTestAPI> test_api_;
 
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
   DISALLOW_COPY_AND_ASSIGN(BackButtonTest);
 };
 
+// The parameter indicates whether the kShelfHotseat feature is enabled.
+INSTANTIATE_TEST_SUITE_P(, BackButtonTest, testing::Bool());
+
 // Verify that the back button is visible in tablet mode.
-TEST_F(BackButtonTest, Visibility) {
+TEST_P(BackButtonTest, Visibility) {
   ASSERT_TRUE(back_button()->layer());
   EXPECT_EQ(0.f, back_button()->layer()->opacity());
 
   Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+  if (GetParam()) {
+    // Ensure the back button is not yet visible when hotseat is enabled.
+    EXPECT_EQ(0.f, back_button()->layer()->opacity());
+
+    // When hotseat is enabled, the back button is only usable in in-app shelf.
+    std::unique_ptr<views::Widget> widget = CreateTestWidget();
+  }
+
   test_api()->RunMessageLoopUntilAnimationsDone();
   EXPECT_EQ(1.f, back_button()->layer()->opacity());
 
@@ -68,12 +93,16 @@ TEST_F(BackButtonTest, Visibility) {
 
 // Verify that the back button is visible in tablet mode, if the initial shelf
 // alignment is on the left or right.
-TEST_F(BackButtonTest, VisibilityWithVerticalShelf) {
+TEST_P(BackButtonTest, VisibilityWithVerticalShelf) {
   test_api()->shelf_view()->shelf()->SetAlignment(SHELF_ALIGNMENT_LEFT);
   ASSERT_TRUE(back_button()->layer());
   EXPECT_EQ(0.f, back_button()->layer()->opacity());
 
   Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+  // When hotseat is enabled, the back button is only usable in in-app shelf.
+  if (GetParam())
+    std::unique_ptr<views::Widget> widget = CreateTestWidget();
+
   test_api()->RunMessageLoopUntilAnimationsDone();
   EXPECT_EQ(1.f, back_button()->layer()->opacity());
 
@@ -82,9 +111,13 @@ TEST_F(BackButtonTest, VisibilityWithVerticalShelf) {
   EXPECT_EQ(0.f, back_button()->layer()->opacity());
 }
 
-TEST_F(BackButtonTest, BackKeySequenceGenerated) {
+TEST_P(BackButtonTest, BackKeySequenceGenerated) {
   // Enter tablet mode; the back button is not visible in non tablet mode.
   Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+  // When hotseat is enabled, the back button is only usable in in-app shelf.
+  if (GetParam())
+    std::unique_ptr<views::Widget> widget = CreateTestWidget();
+
   // Wait for the navigation widget's animation.
   test_api()->RunMessageLoopUntilAnimationsDone(
       GetPrimaryShelf()
@@ -128,4 +161,30 @@ TEST_F(BackButtonTest, BackKeySequenceGenerated) {
   EXPECT_EQ(1, target_back_release.accelerator_count());
 }
 
+// Tests that the back button does not show a context menu.
+TEST_P(BackButtonTest, NoContextMenuOnBackButton) {
+  ui::test::EventGenerator* generator = GetEventGenerator();
+
+  // Enable tablet mode to show the back button. Wait for tablet mode animations
+  // to finish in order for the back button to move out from under the
+  // home button.
+  Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+
+  // When hotseat is enabled, the back button is only usable in in-app shelf.
+  if (GetParam())
+    std::unique_ptr<views::Widget> widget = CreateTestWidget();
+
+  // We need to wait for the navigation widget's animation to be done.
+  test_api_->RunMessageLoopUntilAnimationsDone(
+      GetPrimaryShelf()
+          ->shelf_widget()
+          ->navigation_widget()
+          ->get_bounds_animator_for_testing());
+
+  generator->MoveMouseTo(back_button()->GetBoundsInScreen().CenterPoint());
+  generator->PressRightButton();
+
+  EXPECT_FALSE(test_api_->CloseMenu());
+}
+
 }  // namespace ash

ash/shelf/home_button_unittest.cc

@@ -27,7 +27,7 @@
 #include "base/run_loop.h"
 #include "base/test/scoped_command_line.h"
 #include "base/test/scoped_feature_list.h"
-#include "chromeos/constants/chromeos_switches.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/events/test/event_generator.h"
 
@@ -37,13 +37,24 @@ ui::GestureEvent CreateGestureEvent(ui::GestureEventDetails details) {
   return ui::GestureEvent(0, 0, ui::EF_NONE, base::TimeTicks(), details);
 }
 
-class HomeButtonTest : public AshTestBase {
+class HomeButtonTest : public AshTestBase,
+                       public testing::WithParamInterface<bool> {
  public:
   HomeButtonTest() = default;
   ~HomeButtonTest() override = default;
 
   // AshTestBase:
-  void SetUp() override { AshTestBase::SetUp(); }
+  void SetUp() override {
+    if (GetParam()) {
+      scoped_feature_list_.InitAndEnableFeature(
+          chromeos::features::kShelfHotseat);
+    } else {
+      scoped_feature_list_.InitAndDisableFeature(
+          chromeos::features::kShelfHotseat);
+    }
+
+    AshTestBase::SetUp();
+  }
 
   void SendGestureEvent(ui::GestureEvent* event) {
     GetPrimaryShelf()->shelf_widget()->GetHomeButton()->OnGestureEvent(event);
@@ -70,10 +81,15 @@ class HomeButtonTest : public AshTestBase {
   }
 
  private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
   DISALLOW_COPY_AND_ASSIGN(HomeButtonTest);
 };
 
-TEST_F(HomeButtonTest, SwipeUpToOpenFullscreenAppList) {
+// The parameter indicates whether the kShelfHotseat feature is enabled.
+INSTANTIATE_TEST_SUITE_P(, HomeButtonTest, testing::Bool());
+
+TEST_P(HomeButtonTest, SwipeUpToOpenFullscreenAppList) {
   Shelf* shelf = GetPrimaryShelf();
   EXPECT_EQ(SHELF_ALIGNMENT_BOTTOM, shelf->alignment());
 
@@ -108,7 +124,7 @@ TEST_F(HomeButtonTest, SwipeUpToOpenFullscreenAppList) {
   GetAppListTestHelper()->CheckState(ash::AppListViewState::kFullscreenAllApps);
 }
 
-TEST_F(HomeButtonTest, ClickToOpenAppList) {
+TEST_P(HomeButtonTest, ClickToOpenAppList) {
   Shelf* shelf = GetPrimaryShelf();
   EXPECT_EQ(SHELF_ALIGNMENT_BOTTOM, shelf->alignment());
 
@@ -143,7 +159,7 @@ TEST_F(HomeButtonTest, ClickToOpenAppList) {
   GetAppListTestHelper()->CheckState(ash::AppListViewState::kClosed);
 }
 
-TEST_F(HomeButtonTest, ButtonPositionInTabletMode) {
+TEST_P(HomeButtonTest, ButtonPositionInTabletMode) {
   // Finish all setup tasks. In particular we want to finish the
   // GetSwitchStates post task in (Fake)PowerManagerClient which is triggered
   // by TabletModeController otherwise this will cause tablet mode to exit
@@ -152,6 +168,23 @@ TEST_F(HomeButtonTest, ButtonPositionInTabletMode) {
 
   ShelfViewTestAPI test_api(GetPrimaryShelf()->GetShelfViewForTesting());
   Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+
+  // When hotseat is enabled, home button position changes between in-app shelf
+  // and home shelf, so test in-app when hotseat is enabled.
+  if (GetParam()) {
+    // Wait for the navigation widget's animation.
+    test_api.RunMessageLoopUntilAnimationsDone(
+        GetPrimaryShelf()
+            ->shelf_widget()
+            ->navigation_widget()
+            ->get_bounds_animator_for_testing());
+
+    EXPECT_EQ(home_button()->bounds().x(), 0);
+
+    // Switch to in-app shelf.
+    std::unique_ptr<views::Widget> widget = CreateTestWidget();
+  }
+
   // Wait for the navigation widget's animation.
   test_api.RunMessageLoopUntilAnimationsDone(
       GetPrimaryShelf()
@@ -171,7 +204,7 @@ TEST_F(HomeButtonTest, ButtonPositionInTabletMode) {
   EXPECT_EQ(0, home_button()->bounds().x());
 }
 
-TEST_F(HomeButtonTest, LongPressGesture) {
+TEST_P(HomeButtonTest, LongPressGesture) {
   ui::ScopedAnimationDurationScaleMode animation_duration_mode(
       ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
   // Simulate two user with primary user as active.
@@ -205,7 +238,7 @@ TEST_F(HomeButtonTest, LongPressGesture) {
                                                ->visibility());
 }
 
-TEST_F(HomeButtonTest, LongPressGestureWithSecondaryUser) {
+TEST_P(HomeButtonTest, LongPressGestureWithSecondaryUser) {
   // Disallowed by secondary user.
   assistant_state()->NotifyFeatureAllowed(
       mojom::AssistantAllowedState::DISALLOWED_BY_NONPRIMARY_USER);
@@ -232,7 +265,7 @@ TEST_F(HomeButtonTest, LongPressGestureWithSecondaryUser) {
                                                ->visibility());
 }
 
-TEST_F(HomeButtonTest, LongPressGestureWithSettingsDisabled) {
+TEST_P(HomeButtonTest, LongPressGestureWithSettingsDisabled) {
   // Simulate two user with primary user as active.
   CreateUserSessions(2);
 

ash/shelf/shelf_navigation_widget.cc

@@ -50,6 +50,12 @@ gfx::Rect GetSecondButtonBounds() {
       ShelfConfig::Get()->control_size());
 }
 
+bool IsBackButtonShown() {
+  return chromeos::switches::ShouldShowShelfHotseat()
+             ? IsTabletMode() && ShelfConfig::Get()->is_in_app()
+             : IsTabletMode();
+}
+
 }  // namespace
 
 class ShelfNavigationWidget::Delegate : public views::AccessiblePaneView,
@@ -228,10 +234,11 @@ gfx::Size ShelfNavigationWidget::GetIdealSize() const {
   const int control_size = ShelfConfig::Get()->control_size();
   if (!shelf_->IsHorizontalAlignment())
     return gfx::Size(control_size, control_size);
-  return gfx::Size(
-      IsTabletMode() ? (2 * control_size + ShelfConfig::Get()->button_spacing())
-                     : control_size,
-      control_size);
+
+  return gfx::Size(IsBackButtonShown() ? (2 * control_size +
+                                          ShelfConfig::Get()->button_spacing())
+                                       : control_size,
+                   control_size);
 }
 
 bool ShelfNavigationWidget::OnNativeWidgetActivationChanged(bool active) {
@@ -276,7 +283,7 @@ void ShelfNavigationWidget::OnShelfAlignmentChanged(aura::Window* root_window) {
 
 void ShelfNavigationWidget::OnImplicitAnimationsCompleted() {
   // Hide the back button once it has become fully transparent.
-  if (!IsTabletMode())
+  if (!IsTabletMode() || !IsBackButtonShown())
     GetBackButton()->SetVisible(false);
 }
 
@@ -285,22 +292,23 @@ void ShelfNavigationWidget::OnShelfConfigUpdated() {
 }
 
 void ShelfNavigationWidget::UpdateLayout() {
-  const bool tablet_mode = IsTabletMode();
+  bool is_back_button_shown = IsBackButtonShown();
+
   // Show the back button right away so that the animation is visible.
-  if (tablet_mode)
+  if (is_back_button_shown)
     GetBackButton()->SetVisible(true);
-  GetBackButton()->SetFocusBehavior(tablet_mode
+  GetBackButton()->SetFocusBehavior(is_back_button_shown
                                         ? views::View::FocusBehavior::ALWAYS
                                         : views::View::FocusBehavior::NEVER);
   ui::ScopedLayerAnimationSettings settings(
       GetBackButton()->layer()->GetAnimator());
   settings.SetTransitionDuration(kBackButtonOpacityAnimationDuration);
   settings.AddObserver(this);
-  GetBackButton()->layer()->SetOpacity(tablet_mode ? 1 : 0);
+  GetBackButton()->layer()->SetOpacity(is_back_button_shown ? 1 : 0);
 
   bounds_animator_->AnimateViewTo(
       GetHomeButton(),
-      tablet_mode ? GetSecondButtonBounds() : GetFirstButtonBounds());
+      is_back_button_shown ? GetSecondButtonBounds() : GetFirstButtonBounds());
   GetBackButton()->SetBoundsRect(GetFirstButtonBounds());
 
   delegate_->UpdateOpaqueBackground();

ash/shelf/shelf_view_unittest.cc

@@ -2254,28 +2254,6 @@ TEST_F(ShelfViewTest, TabletModeStartAndEndClosesContextMenu) {
   EXPECT_FALSE(test_api_->CloseMenu());
 }
 
-// Tests that the back button does not show a context menu.
-TEST_F(ShelfViewTest, NoContextMenuOnBackButton) {
-  ui::test::EventGenerator* generator = GetEventGenerator();
-
-  // Enable tablet mode to show the back button. Wait for tablet mode animations
-  // to finish in order for the BackButton to move out from under the
-  // HomeButton.
-  Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
-
-  // We need to wait for the navigation widget's animation to be done.
-  test_api_->RunMessageLoopUntilAnimationsDone(
-      shelf_view_->shelf_widget()
-          ->navigation_widget()
-          ->get_bounds_animator_for_testing());
-
-  views::View* back_button = shelf_view_->shelf_widget()->GetBackButton();
-  generator->MoveMouseTo(back_button->GetBoundsInScreen().CenterPoint());
-  generator->PressRightButton();
-
-  EXPECT_FALSE(test_api_->CloseMenu());
-}
-
 // Tests that the overflow button does not show a context menu.
 TEST_F(ShelfViewTestNotScrollable, NoContextMenuOnOverflowButton) {
   ui::test::EventGenerator* generator = GetEventGenerator();