cros: Touchable App Context Menu dead code cleanup

Remove code that is dead now that touchable app menus are enabled by
default.

Bug: 906684
Change-Id: I28bb46da596ba93e0bbc01c0685ed1cc1b7ec0ac
Reviewed-on: https://chromium-review.googlesource.com/c/1341064
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611008}

ash/shelf/shelf_application_menu_model.cc

@@ -11,7 +11,6 @@
 
 #include "ash/public/cpp/shelf_item_delegate.h"
 #include "base/metrics/histogram_macros.h"
-#include "ui/base/ui_base_features.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/gfx/image/image.h"
 
@@ -28,11 +27,7 @@ ShelfApplicationMenuModel::ShelfApplicationMenuModel(
     std::vector<mojom::MenuItemPtr> items,
     ShelfItemDelegate* delegate)
     : ui::SimpleMenuModel(this), items_(std::move(items)), delegate_(delegate) {
-  if (!features::IsTouchableAppContextMenuEnabled())
-    AddSeparator(ui::SPACING_SEPARATOR);
   AddItem(kInvalidCommandId, title);
-  if (!features::IsTouchableAppContextMenuEnabled())
-    AddSeparator(ui::SPACING_SEPARATOR);
 
   for (size_t i = 0; i < items_.size(); i++) {
     mojom::MenuItem* item = items_[i].get();

ash/shelf/shelf_context_menu_model.cc

@@ -25,7 +25,6 @@
 #include "base/numerics/safe_conversions.h"
 #include "components/prefs/pref_service.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/ui_base_features.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/menu/menu_config.h"
@@ -62,11 +61,6 @@ void AddLocalMenuItems(MenuItemList* menu, int64_t display_id) {
                                   ->tablet_mode_controller()
                                   ->IsTabletModeWindowManagerEnabled();
 
-  // When touchable app context menus are enabled in tablet mode, shelf
-  // alignment option is not shown.
-  const bool skip_clamshell_only_options =
-      features::IsTouchableAppContextMenuEnabled() && is_tablet_mode;
-
   const views::MenuConfig& menu_config = views::MenuConfig::instance();
 
   // In fullscreen, the shelf is either hidden or auto-hidden, depending on
@@ -78,7 +72,6 @@ void AddLocalMenuItems(MenuItemList* menu, int64_t display_id) {
     const bool is_autohide_set =
         GetShelfAutoHideBehaviorPref(prefs, display_id) ==
         SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS;
-    if (features::IsTouchableAppContextMenuEnabled()) {
       auto_hide->type = ui::MenuModel::TYPE_COMMAND;
       auto_hide->image = gfx::CreateVectorIcon(
           is_autohide_set ? kAlwaysShowShelfIcon : kAutoHideIcon,
@@ -86,20 +79,17 @@ void AddLocalMenuItems(MenuItemList* menu, int64_t display_id) {
       auto_hide->label = GetStringUTF16(
           is_autohide_set ? IDS_ASH_SHELF_CONTEXT_MENU_ALWAYS_SHOW_SHELF
                           : IDS_ASH_SHELF_CONTEXT_MENU_AUTO_HIDE);
-    } else {
-      auto_hide->label = GetStringUTF16(IDS_ASH_SHELF_CONTEXT_MENU_AUTO_HIDE);
-      auto_hide->type = ui::MenuModel::TYPE_CHECK;
-      auto_hide->checked = is_autohide_set;
-    }
+
     auto_hide->command_id = ShelfContextMenuModel::MENU_AUTO_HIDE;
     auto_hide->enabled = true;
     menu->push_back(std::move(auto_hide));
   }
 
-  // Only allow shelf alignment modifications by the owner or user.
+  // Only allow shelf alignment modifications by the owner or user. In tablet
+  // mode, the shelf alignment option is not shown.
   LoginStatus status = Shell::Get()->session_controller()->login_status();
   if ((status == LoginStatus::USER || status == LoginStatus::OWNER) &&
-      !skip_clamshell_only_options) {
+      !is_tablet_mode) {
     const ShelfAlignment alignment = GetShelfAlignmentPref(prefs, display_id);
     mojom::MenuItemPtr alignment_menu(mojom::MenuItem::New());
     alignment_menu->type = ui::MenuModel::TYPE_SUBMENU;
@@ -107,11 +97,9 @@ void AddLocalMenuItems(MenuItemList* menu, int64_t display_id) {
     alignment_menu->label = GetStringUTF16(IDS_ASH_SHELF_CONTEXT_MENU_POSITION);
     alignment_menu->submenu = MenuItemList();
     alignment_menu->enabled = !is_tablet_mode;
-    if (features::IsTouchableAppContextMenuEnabled()) {
-      alignment_menu->image = gfx::CreateVectorIcon(
-          kShelfPositionIcon, menu_config.touchable_icon_size,
-          menu_config.touchable_icon_color);
-    }
+    alignment_menu->image = gfx::CreateVectorIcon(
+        kShelfPositionIcon, menu_config.touchable_icon_size,
+        menu_config.touchable_icon_color);
 
     mojom::MenuItemPtr left(mojom::MenuItem::New());
     left->type = ui::MenuModel::TYPE_RADIO;
@@ -146,11 +134,9 @@ void AddLocalMenuItems(MenuItemList* menu, int64_t display_id) {
     wallpaper->command_id = ShelfContextMenuModel::MENU_CHANGE_WALLPAPER;
     wallpaper->label = GetStringUTF16(IDS_AURA_SET_DESKTOP_WALLPAPER);
     wallpaper->enabled = true;
-    if (features::IsTouchableAppContextMenuEnabled()) {
-      wallpaper->image =
-          gfx::CreateVectorIcon(kWallpaperIcon, menu_config.touchable_icon_size,
-                                menu_config.touchable_icon_color);
-    }
+    wallpaper->image =
+        gfx::CreateVectorIcon(kWallpaperIcon, menu_config.touchable_icon_size,
+                              menu_config.touchable_icon_color);
     menu->push_back(std::move(wallpaper));
   }
 }
@@ -165,7 +151,7 @@ ShelfContextMenuModel::ShelfContextMenuModel(MenuItemList menu_items,
       delegate_(delegate),
       display_id_(display_id) {
   // Append shelf settings and wallpaper items if no shelf item was selected.
-  if (!features::IsTouchableAppContextMenuEnabled() || !delegate)
+  if (!delegate)
     AddLocalMenuItems(&menu_items_, display_id);
   menu_utils::PopulateMenuFromMojoMenuItems(this, this, menu_items_,
                                             &submenus_);

ash/shelf/shelf_controller.h

@@ -102,7 +102,7 @@ class ASH_EXPORT ShelfController : public message_center::MessageCenterObserver,
   // Changes to the local ShelfModel should not be reported during this time.
   bool applying_remote_shelf_model_changes_ = false;
 
-  // Whether touchable context menus have been enabled for app icons on the
+  // Whether notification indicators have been enabled for app icons in the
   // shelf.
   const bool is_notification_indicator_enabled_;
 

ash/shelf/shelf_controller_unittest.cc

@@ -209,27 +209,26 @@ TEST_F(ShelfControllerTest, ShelfItemImageSynchronization) {
   EXPECT_FALSE(controller->model()->items()[index].image.isNull());
 }
 
-class ShelfControllerTouchableContextMenuTest : public AshTestBase {
+class ShelfControllerNotificationIndicatorTest : public AshTestBase {
  public:
-  ShelfControllerTouchableContextMenuTest() = default;
-  ~ShelfControllerTouchableContextMenuTest() override = default;
+  ShelfControllerNotificationIndicatorTest() = default;
+  ~ShelfControllerNotificationIndicatorTest() override = default;
 
   void SetUp() override {
-    scoped_feature_list_.InitWithFeatures(
-        {features::kTouchableAppContextMenu, features::kNotificationIndicator},
-        {});
+    scoped_feature_list_.InitWithFeatures({features::kNotificationIndicator},
+                                          {});
     AshTestBase::SetUp();
   }
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 
-  DISALLOW_COPY_AND_ASSIGN(ShelfControllerTouchableContextMenuTest);
+  DISALLOW_COPY_AND_ASSIGN(ShelfControllerNotificationIndicatorTest);
 };
 
 // Tests that the ShelfController keeps the ShelfModel updated on new
 // notifications.
-TEST_F(ShelfControllerTouchableContextMenuTest, HasNotificationBasic) {
+TEST_F(ShelfControllerNotificationIndicatorTest, HasNotificationBasic) {
   ShelfController* controller = Shell::Get()->shelf_controller();
   const std::string app_id("app_id");
   ShelfItem item;

ash/shelf/shelf_view.cc

@@ -1622,19 +1622,14 @@ void ShelfView::UpdateOverflowRange(ShelfView* overflow_view) const {
 gfx::Rect ShelfView::GetMenuAnchorRect(const views::View& source,
                                        const gfx::Point& location,
                                        bool context_menu) const {
-  const bool for_item = ShelfItemForView(&source);
+  // Application menus for items are anchored on the icon bounds.
+  if (ShelfItemForView(&source) || !context_menu)
+    return source.GetBoundsInScreen();
+
   const gfx::Rect shelf_bounds_in_screen =
       is_overflow_mode()
           ? owner_overflow_bubble_->bubble_view()->GetBubbleBounds()
           : GetBoundsInScreen();
-  const gfx::Rect& source_bounds_in_screen = source.GetBoundsInScreen();
-  // Application menus and touchable menus for items are anchored on the icon
-  // bounds.
-  if ((features::IsTouchableAppContextMenuEnabled() && for_item) ||
-      !context_menu) {
-    return source_bounds_in_screen;
-  }
-
   gfx::Point origin;
   switch (shelf_->alignment()) {
     case SHELF_ALIGNMENT_BOTTOM:
@@ -1648,31 +1643,7 @@ gfx::Rect ShelfView::GetMenuAnchorRect(const views::View& source,
       origin = gfx::Point(shelf_bounds_in_screen.x(), location.y());
       break;
   }
-  return gfx::Rect(origin,
-                   for_item ? source_bounds_in_screen.size() : gfx::Size());
-}
-
-views::MenuAnchorPosition ShelfView::GetMenuAnchorPosition(
-    bool for_item,
-    bool context_menu) const {
-  if (features::IsTouchableAppContextMenuEnabled()) {
-    return shelf_->IsHorizontalAlignment()
-               ? views::MENU_ANCHOR_BUBBLE_TOUCHABLE_ABOVE
-               : views::MENU_ANCHOR_BUBBLE_TOUCHABLE_LEFT;
-  }
-  if (!context_menu) {
-    switch (shelf_->alignment()) {
-      case SHELF_ALIGNMENT_BOTTOM:
-      case SHELF_ALIGNMENT_BOTTOM_LOCKED:
-        return views::MENU_ANCHOR_BUBBLE_ABOVE;
-      case SHELF_ALIGNMENT_LEFT:
-        return views::MENU_ANCHOR_BUBBLE_RIGHT;
-      case SHELF_ALIGNMENT_RIGHT:
-        return views::MENU_ANCHOR_BUBBLE_LEFT;
-    }
-  }
-  return shelf_->IsHorizontalAlignment() ? views::MENU_ANCHOR_FIXED_BOTTOMCENTER
-                                         : views::MENU_ANCHOR_FIXED_SIDECENTER;
+  return gfx::Rect(origin, gfx::Size());
 }
 
 gfx::Rect ShelfView::GetBoundsForDragInsertInScreen() {
@@ -2091,16 +2062,12 @@ void ShelfView::ShowMenu(std::unique_ptr<ui::SimpleMenuModel> menu_model,
   closing_event_time_ = base::TimeTicks();
 
   // NOTE: If you convert to HAS_MNEMONICS be sure to update menu building code.
-  int run_types = 0;
+  int run_types = views::MenuRunner::USE_TOUCHABLE_LAYOUT;
   if (context_menu) {
     run_types |=
         views::MenuRunner::CONTEXT_MENU | views::MenuRunner::FIXED_ANCHOR;
   }
 
-  // Only use the touchable layout if the menu is for an app.
-  if (features::IsTouchableAppContextMenuEnabled())
-    run_types |= views::MenuRunner::USE_TOUCHABLE_LAYOUT;
-
   const ShelfItem* item = ShelfItemForView(source);
   // Only selected shelf items with context menu opened can be dragged.
   if (context_menu && item && ShelfButtonIsInDrag(item->type, source) &&
@@ -2114,7 +2081,10 @@ void ShelfView::ShowMenu(std::unique_ptr<ui::SimpleMenuModel> menu_model,
       base::BindOnce(&ShelfView::OnMenuClosed, base::Unretained(this), source));
   shelf_menu_model_adapter_->Run(
       GetMenuAnchorRect(*source, click_point, context_menu),
-      GetMenuAnchorPosition(item, context_menu), run_types);
+      shelf_->IsHorizontalAlignment()
+          ? views::MENU_ANCHOR_BUBBLE_TOUCHABLE_ABOVE
+          : views::MENU_ANCHOR_BUBBLE_TOUCHABLE_LEFT,
+      run_types);
 }
 
 void ShelfView::OnMenuClosed(views::View* source) {

ash/shelf/shelf_view.h

@@ -381,13 +381,6 @@ class ASH_EXPORT ShelfView : public views::View,
                               const gfx::Point& location,
                               bool context_menu) const;
 
-  // Gets the menu anchor position for a menu. |for_item| is true if the menu is
-  // for an item on the shelf, or false if the menu is for the shelf view
-  // itself, |context_menu| is whether the menu will be an application menu or
-  // context menu, and |touch_menu| is whether the menu was initiated by touch.
-  views::MenuAnchorPosition GetMenuAnchorPosition(bool for_item,
-                                                  bool context_menu) const;
-
   // Overridden from views::View:
   gfx::Size CalculatePreferredSize() const override;
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;

ash/shelf/shelf_view_unittest.cc

@@ -2190,69 +2190,30 @@ TEST_F(ShelfViewTest, DragAppAfterContextMenuIsShownInAutoHideShelf) {
   EXPECT_EQ(first_app_id, model_->items()[last_index].id);
 }
 
-struct TouchableAppContextMenuTestParams {
-  TouchableAppContextMenuTestParams(bool enable_touchable_app_context_menu,
-                                    bool context_menu)
-      : enable_touchable_app_context_menu(enable_touchable_app_context_menu),
-        context_menu(context_menu) {}
-  // Whether to enable the touchable app context menu feature.
-  bool enable_touchable_app_context_menu;
-  // Whether the menu is shown as an application or context menu.
-  bool context_menu;
-};
-
-class ShelfViewTouchableContextMenuTest
-    : public ShelfViewTest,
-      public testing::WithParamInterface<TouchableAppContextMenuTestParams> {
+// Tests that the app list button does not show a context menu on right click.
+TEST_F(ShelfViewTest, AppListButtonDoesNotShowContextMenu) {
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  const AppListButton* app_list_button = shelf_view_->GetAppListButton();
+  generator->MoveMouseTo(app_list_button->GetBoundsInScreen().CenterPoint());
+  generator->PressRightButton();
+  EXPECT_FALSE(test_api_->CloseMenu());
+}
+// Test class that tests both context and application menus.
+class ShelfViewMenuTest : public ShelfViewTest,
+                          public testing::WithParamInterface<bool> {
  public:
-  ShelfViewTouchableContextMenuTest() = default;
-  ~ShelfViewTouchableContextMenuTest() override = default;
-
-  void SetUp() override {
-    // If the test is parameterized, respect the parameter. Otherwise enable
-    // touchable app context menus by default.
-    const bool enable_touchable_app_context_menu =
-        testing::UnitTest::GetInstance()->current_test_info()->value_param()
-            ? GetParam().enable_touchable_app_context_menu
-            : true;
-    std::vector<base::Feature> enabled_features = {
-        features::kNotificationIndicator};
-    if (enable_touchable_app_context_menu)
-      enabled_features.push_back(features::kTouchableAppContextMenu);
-    scoped_feature_list_.InitWithFeatures(enabled_features, {});
-    ShelfViewTest::SetUp();
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
+  ShelfViewMenuTest() = default;
+  ~ShelfViewMenuTest() override = default;
 
-  DISALLOW_COPY_AND_ASSIGN(ShelfViewTouchableContextMenuTest);
+  DISALLOW_COPY_AND_ASSIGN(ShelfViewMenuTest);
 };
 
-INSTANTIATE_TEST_CASE_P(
-    TouchableDisabledAppMenu,
-    ShelfViewTouchableContextMenuTest,
-    ::testing::Values(TouchableAppContextMenuTestParams(false, false)));
-
-INSTANTIATE_TEST_CASE_P(
-    TouchableEnabledAppMenu,
-    ShelfViewTouchableContextMenuTest,
-    ::testing::Values(TouchableAppContextMenuTestParams(true, false)));
-
-INSTANTIATE_TEST_CASE_P(
-    TouchableDisabledContextMenu,
-    ShelfViewTouchableContextMenuTest,
-    ::testing::Values(TouchableAppContextMenuTestParams(false, true)));
-
-INSTANTIATE_TEST_CASE_P(
-    TouchableEnabledContextMenu,
-    ShelfViewTouchableContextMenuTest,
-    ::testing::Values(TouchableAppContextMenuTestParams(true, true)));
+INSTANTIATE_TEST_CASE_P(, ShelfViewMenuTest, testing::Bool());
 
 // Tests that menu anchor points are aligned with the shelf button bounds.
-TEST_P(ShelfViewTouchableContextMenuTest, ShelfViewMenuAnchorPoint) {
+TEST_P(ShelfViewMenuTest, ShelfViewMenuAnchorPoint) {
   const ShelfButton* shelf_button = GetButtonByID(AddApp());
-  const bool context_menu = GetParam().context_menu;
+  const bool context_menu = GetParam();
   EXPECT_EQ(ash::ShelfAlignment::SHELF_ALIGNMENT_BOTTOM,
             GetPrimaryShelf()->alignment());
 
@@ -2265,13 +2226,8 @@ TEST_P(ShelfViewTouchableContextMenuTest, ShelfViewMenuAnchorPoint) {
   // Test for left shelf.
   GetPrimaryShelf()->SetAlignment(ash::ShelfAlignment::SHELF_ALIGNMENT_LEFT);
 
-  int expected_x = shelf_button->GetBoundsInScreen().x();
-  // Left shelf context menus when TouchableAppContextMenu is disabled anchor
-  // off of the right edge of the shelf.
-  if (context_menu && !features::IsTouchableAppContextMenuEnabled())
-    expected_x = shelf_button->GetBoundsInScreen().right();
   EXPECT_EQ(
-      expected_x,
+      shelf_button->GetBoundsInScreen().x(),
       test_api_->GetMenuAnchorRect(*shelf_button, gfx::Point(), context_menu)
           .x());
 
@@ -2284,19 +2240,27 @@ TEST_P(ShelfViewTouchableContextMenuTest, ShelfViewMenuAnchorPoint) {
           .x());
 }
 
-// Tests that the app list button does not show a context menu on right click
-// when touchable app context menus are enabled.
-TEST_F(ShelfViewTouchableContextMenuTest, AppListButtonDoesNotShowContextMenu) {
-  ui::test::EventGenerator* generator = GetEventGenerator();
-  const AppListButton* app_list_button = shelf_view_->GetAppListButton();
-  generator->MoveMouseTo(app_list_button->GetBoundsInScreen().CenterPoint());
-  generator->PressRightButton();
-  EXPECT_FALSE(test_api_->CloseMenu());
-}
+// Test class that enables notification indicators.
+class NotificationIndicatorTest : public ShelfViewTest {
+ public:
+  NotificationIndicatorTest() = default;
+  ~NotificationIndicatorTest() override = default;
+
+  void SetUp() override {
+    scoped_feature_list_.InitWithFeatures({features::kNotificationIndicator},
+                                          {});
+    ShelfViewTest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(NotificationIndicatorTest);
+};
 
 // Tests that an item has a notification indicator when it recieves a
 // notification.
-TEST_F(ShelfViewTouchableContextMenuTest, AddedItemHasNotificationIndicator) {
+TEST_F(NotificationIndicatorTest, AddedItemHasNotificationIndicator) {
   const ShelfID id_0 = AddApp();
   const std::string notification_id_0("notification_id_0");
   const ShelfButton* button_0 = GetButtonByID(id_0);
@@ -2336,7 +2300,7 @@ TEST_F(ShelfViewTouchableContextMenuTest, AddedItemHasNotificationIndicator) {
 
 // Tests that the notification indicator is active until all notifications have
 // been removed.
-TEST_F(ShelfViewTouchableContextMenuTest,
+TEST_F(NotificationIndicatorTest,
        NotificationIndicatorStaysActiveUntilNotificationsAreGone) {
   const ShelfID app = AddApp();
   const ShelfButton* button = GetButtonByID(app);

ash/shelf/shelf_window_watcher_item_delegate.cc

@@ -17,7 +17,6 @@
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/ui_base_features.h"
 #include "ui/events/event_constants.h"
 #include "ui/wm/core/window_animations.h"
 
@@ -70,11 +69,6 @@ void ShelfWindowWatcherItemDelegate::GetContextMenuItems(
   close->label = l10n_util::GetStringUTF16(IDS_CLOSE);
   close->enabled = true;
   items.push_back(std::move(close));
-  if (!features::IsTouchableAppContextMenuEnabled()) {
-    ash::mojom::MenuItemPtr separator(ash::mojom::MenuItem::New());
-    separator->type = ui::MenuModel::TYPE_SEPARATOR;
-    items.push_back(std::move(separator));
-  }
   std::move(callback).Run(std::move(items));
 }