Commit 1c9021b6 authored by Alex Newcomer's avatar Alex Newcomer Committed by Commit Bot

cros: Touchable App Context Menu dead code cleanup

Change-Id: Ifee152ae16c133e0d2dcf7f334e1ff7700a47167
Reviewed-on: https://chromium-review.googlesource.com/c/1341068Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarYury Khmel <khmel@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611011}
parent 48877479
......@@ -8,7 +8,6 @@
#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
#include "chrome/grit/generated_resources.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_base_features.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/vector_icon_types.h"
#include "ui/views/controls/menu/menu_config.h"
......@@ -37,9 +36,6 @@ void AppContextMenu::GetMenuModel(GetMenuModelCallback callback) {
void AppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
// Show Pin/Unpin option if shelf is available.
if (controller_->GetPinnable(app_id()) != AppListControllerDelegate::NO_PIN) {
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
AddContextMenuOption(menu_model, ash::TOGGLE_PIN,
controller_->IsAppPinned(app_id_)
? IDS_APP_LIST_CONTEXT_MENU_UNPIN
......@@ -93,14 +89,12 @@ void AppContextMenu::TogglePin(const std::string& shelf_app_id) {
void AppContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
ash::CommandId command_id,
int string_id) {
// Do not include disabled items in touchable menus.
if (features::IsTouchableAppContextMenuEnabled() &&
!IsCommandIdEnabled(command_id)) {
// Do not include disabled items.
if (!IsCommandIdEnabled(command_id))
return;
}
const gfx::VectorIcon& icon = GetMenuItemVectorIcon(command_id, string_id);
if (features::IsTouchableAppContextMenuEnabled() && !icon.is_empty()) {
if (!icon.is_empty()) {
const views::MenuConfig& menu_config = views::MenuConfig::instance();
menu_model->AddItemWithStringIdAndIcon(
command_id, string_id,
......@@ -178,9 +172,6 @@ void AppContextMenu::ExecuteCommand(int command_id, int event_flags) {
bool AppContextMenu::GetIconForCommandId(int command_id,
gfx::Image* icon) const {
if (!features::IsTouchableAppContextMenuEnabled())
return false;
if (command_id == ash::TOGGLE_PIN) {
const views::MenuConfig& menu_config = views::MenuConfig::instance();
*icon = gfx::Image(gfx::CreateVectorIcon(
......
......@@ -126,8 +126,7 @@ std::unique_ptr<ui::MenuModel> GetMenuModel(
} // namespace
class AppContextMenuTest : public AppListTestBase,
public testing::WithParamInterface<bool> {
class AppContextMenuTest : public AppListTestBase {
public:
AppContextMenuTest() = default;
~AppContextMenuTest() override {
......@@ -137,12 +136,6 @@ class AppContextMenuTest : public AppListTestBase,
void SetUp() override {
AppListTestBase::SetUp();
if (testing::UnitTest::GetInstance()->current_test_info()->value_param() &&
GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
features::kTouchableAppContextMenu);
arc::IconDecodeRequest::DisableSafeDecodingForTesting();
}
extensions::MenuManagerFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&MenuManagerFactory));
controller_ = std::make_unique<FakeAppListControllerDelegate>();
......@@ -202,26 +195,13 @@ class AppContextMenuTest : public AppListTestBase,
Profile* profile() { return profile_.get(); }
void AddSeparator(std::vector<MenuState>* states) {
// TODO(newcomer): Remove this function when touchable app context menus are
// enabled by default.
if (features::IsTouchableAppContextMenuEnabled())
return;
if (states->empty() || states->back().command_id == -1)
return;
states->push_back(MenuState());
}
void AddToStates(const app_list::ExtensionAppContextMenu& menu,
MenuState state,
std::vector<MenuState>* states) {
// If the command is not enabled, it is not added to touchable app context
// menus, so do not add it to states.
if (features::IsTouchableAppContextMenuEnabled() &&
!menu.IsCommandIdEnabled(state.command_id)) {
// If the command is not enabled do not add it to states.
if (!menu.IsCommandIdEnabled(state.command_id))
return;
}
states->push_back(state);
}
......@@ -241,53 +221,16 @@ class AppContextMenuTest : public AppListTestBase,
ASSERT_NE(nullptr, menu_model);
std::vector<MenuState> states;
if (!platform_app) {
if (!platform_app)
AddToStates(menu, MenuState(ash::LAUNCH_NEW), &states);
if (!features::IsTouchableAppContextMenuEnabled()) {
AddSeparator(&states);
if (extensions::util::CanHostedAppsOpenInWindows() &&
extensions::util::IsNewBookmarkAppsEnabled()) {
bool checked = launch_type == extensions::LAUNCH_TYPE_WINDOW ||
launch_type == extensions::LAUNCH_TYPE_FULLSCREEN;
AddToStates(menu,
MenuState(ash::USE_LAUNCH_TYPE_WINDOW, true, checked),
&states);
} else if (!extensions::util::IsNewBookmarkAppsEnabled()) {
bool regular_checked = launch_type == extensions::LAUNCH_TYPE_REGULAR;
AddToStates(
menu,
MenuState(ash::USE_LAUNCH_TYPE_REGULAR, true, regular_checked),
&states);
AddToStates(menu,
MenuState(ash::USE_LAUNCH_TYPE_PINNED, true,
launch_type == extensions::LAUNCH_TYPE_PINNED),
&states);
if (extensions::util::CanHostedAppsOpenInWindows()) {
AddToStates(
menu,
MenuState(ash::USE_LAUNCH_TYPE_WINDOW, true,
launch_type == extensions::LAUNCH_TYPE_WINDOW),
&states);
}
AddToStates(
menu,
MenuState(ash::USE_LAUNCH_TYPE_FULLSCREEN, true,
launch_type == extensions::LAUNCH_TYPE_FULLSCREEN),
&states);
}
}
}
if (pinnable != AppListControllerDelegate::NO_PIN) {
AddSeparator(&states);
AddToStates(
menu,
MenuState(ash::TOGGLE_PIN,
pinnable != AppListControllerDelegate::PIN_FIXED, false),
&states);
}
AddSeparator(&states);
if (!platform_app)
AddToStates(menu, MenuState(ash::OPTIONS, false, false), &states);
AddToStates(menu, MenuState(ash::UNINSTALL), &states);
......@@ -325,9 +268,7 @@ class AppContextMenuTest : public AppListTestBase,
DISALLOW_COPY_AND_ASSIGN(AppContextMenuTest);
};
INSTANTIATE_TEST_CASE_P(, AppContextMenuTest, testing::Bool());
TEST_P(AppContextMenuTest, ExtensionApp) {
TEST_F(AppContextMenuTest, ExtensionApp) {
app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
false);
for (extensions::LaunchType launch_type = extensions::LAUNCH_TYPE_FIRST;
......@@ -353,14 +294,14 @@ TEST_P(AppContextMenuTest, ExtensionApp) {
}
}
TEST_P(AppContextMenuTest, ChromeApp) {
TEST_F(AppContextMenuTest, ChromeApp) {
app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
true);
for (bool can_show_app_info : {true, false})
TestChromeApp(can_show_app_info);
}
TEST_P(AppContextMenuTest, NonExistingExtensionApp) {
TEST_F(AppContextMenuTest, NonExistingExtensionApp) {
app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
false);
app_list::ExtensionAppContextMenu menu(menu_delegate(), profile(),
......@@ -370,7 +311,7 @@ TEST_P(AppContextMenuTest, NonExistingExtensionApp) {
EXPECT_EQ(nullptr, menu_model);
}
TEST_P(AppContextMenuTest, ArcMenu) {
TEST_F(AppContextMenuTest, ArcMenu) {
ArcAppTest arc_test;
arc_test.SetUp(profile());
......@@ -389,17 +330,12 @@ TEST_P(AppContextMenuTest, ArcMenu) {
// Separators are not added to touchable app context menus. For touchable app
// context menus, arc app has double separator, three more app shortcuts
// provided by arc::FakeAppInstance and two separators between shortcuts.
const int expected_items =
features::IsTouchableAppContextMenuEnabled() ? 10 : 6;
const int expected_items = 10;
ASSERT_EQ(expected_items, menu->GetItemCount());
int index = 0;
ValidateItemState(menu.get(), index++, MenuState(ash::LAUNCH_NEW));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::UNINSTALL));
ValidateItemState(menu.get(), index++, MenuState(ash::SHOW_APP_INFO));
......@@ -421,19 +357,15 @@ TEST_P(AppContextMenuTest, ArcMenu) {
// Separators are not added to touchable app context menus except for arc app
// shortcuts, which have double separator, three more app shortcuts provided
// by arc::FakeAppInstance and two separators between shortcuts.
const int expected_items_app_open =
features::IsTouchableAppContextMenuEnabled() ? 9 : 4;
const int expected_items_app_open = 9;
ASSERT_EQ(expected_items_app_open, menu->GetItemCount());
index = 0;
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::UNINSTALL));
ValidateItemState(menu.get(), index++, MenuState(ash::SHOW_APP_INFO));
// Test that arc app shortcuts provided by arc::FakeAppInstance have a
// separator between each app shortcut.
if (features::IsTouchableAppContextMenuEnabled()) {
EXPECT_EQ(ui::DOUBLE_SEPARATOR, menu->GetSeparatorTypeAt(index++));
for (int shortcut_index = 0; index < menu->GetItemCount(); ++index) {
EXPECT_EQ(base::StringPrintf("ShortLabel %d", shortcut_index++),
......@@ -446,7 +378,6 @@ TEST_P(AppContextMenuTest, ArcMenu) {
EXPECT_EQ(0, arc_test.app_instance()->launch_app_shortcut_item_count());
menu->ActivatedAt(menu->GetItemCount() - 1);
EXPECT_EQ(1, arc_test.app_instance()->launch_app_shortcut_item_count());
}
// This makes all apps non-ready.
controller()->SetAppOpen(app_id, false);
......@@ -460,24 +391,14 @@ TEST_P(AppContextMenuTest, ArcMenu) {
// menus. For touchable app context menus, arc app has double separator,
// three more app shortcuts provided by arc::FakeAppInstance and two
// separators between shortcuts.
const int expected_items_reopen =
features::IsTouchableAppContextMenuEnabled() ? 8 : 6;
const int expected_items_reopen = 8;
ASSERT_EQ(expected_items_reopen, menu->GetItemCount());
index = 0;
ValidateItemState(menu.get(), index++, MenuState(ash::LAUNCH_NEW));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled()) {
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++,
MenuState(ash::UNINSTALL, false, false));
ValidateItemState(menu.get(), index++,
MenuState(ash::SHOW_APP_INFO, false, false));
}
// Test that arc app shortcuts provided by arc::FakeAppInstance have a
// separator between each app shortcut.
if (features::IsTouchableAppContextMenuEnabled()) {
EXPECT_EQ(ui::DOUBLE_SEPARATOR, menu->GetSeparatorTypeAt(index++));
for (int shortcut_index = 0; index < menu->GetItemCount(); ++index) {
EXPECT_EQ(base::StringPrintf("ShortLabel %d", shortcut_index++),
......@@ -485,7 +406,6 @@ TEST_P(AppContextMenuTest, ArcMenu) {
if (index < menu->GetItemCount())
EXPECT_EQ(ui::PADDED_SEPARATOR, menu->GetSeparatorTypeAt(index));
}
}
// Uninstall all apps.
arc_test.app_instance()->RefreshAppList();
......@@ -498,7 +418,7 @@ TEST_P(AppContextMenuTest, ArcMenu) {
EXPECT_EQ(0, menu->GetItemCount());
}
TEST_P(AppContextMenuTest, ArcMenuShortcut) {
TEST_F(AppContextMenuTest, ArcMenuShortcut) {
ArcAppTest arc_test;
arc_test.SetUp(profile());
......@@ -515,28 +435,21 @@ TEST_P(AppContextMenuTest, ArcMenuShortcut) {
// Separators are not added to touchable app context menus. For touchable app
// context menus, arc app has double separator, three more app shortcuts
// provided by arc::FakeAppInstance and two separators between shortcuts.
const int expected_items =
features::IsTouchableAppContextMenuEnabled() ? 10 : 6;
const int expected_items = 10;
int index = 0;
ASSERT_EQ(expected_items, menu->GetItemCount());
ValidateItemState(menu.get(), index++, MenuState(ash::LAUNCH_NEW));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::UNINSTALL));
ValidateItemState(menu.get(), index++, MenuState(ash::SHOW_APP_INFO));
// Test that arc app shortcuts provided by arc::FakeAppInstance have a
// separator between each app shortcut.
if (features::IsTouchableAppContextMenuEnabled()) {
EXPECT_EQ(ui::DOUBLE_SEPARATOR, menu->GetSeparatorTypeAt(index++));
for (int shortcut_index = 0; index < menu->GetItemCount(); ++index) {
EXPECT_EQ(base::StringPrintf("ShortLabel %d", shortcut_index++),
base::UTF16ToUTF8(menu->GetLabelAt(index++)));
if (index < menu->GetItemCount())
EXPECT_EQ(ui::PADDED_SEPARATOR, menu->GetSeparatorTypeAt(index));
}
}
// This makes all apps non-ready. Shortcut is still uninstall-able.
......@@ -549,24 +462,15 @@ TEST_P(AppContextMenuTest, ArcMenuShortcut) {
// menus. For touchable app context menus, arc app has double separator,
// three more app shortcuts provided by arc::FakeAppInstance and two
// separators between shortcuts.
const int expected_items_non_ready =
features::IsTouchableAppContextMenuEnabled() ? 9 : 6;
const int expected_items_non_ready = 9;
ASSERT_EQ(expected_items_non_ready, menu->GetItemCount());
index = 0;
ValidateItemState(menu.get(), index++, MenuState(ash::LAUNCH_NEW));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::UNINSTALL));
if (!features::IsTouchableAppContextMenuEnabled()) {
ValidateItemState(menu.get(), index++,
MenuState(ash::SHOW_APP_INFO, false, false));
}
// Test that arc app shortcuts provided by arc::FakeAppInstance have a
// separator between each app shortcut.
if (features::IsTouchableAppContextMenuEnabled()) {
EXPECT_EQ(ui::DOUBLE_SEPARATOR, menu->GetSeparatorTypeAt(index++));
for (int shortcut_index = 0; index < menu->GetItemCount(); ++index) {
EXPECT_EQ(base::StringPrintf("ShortLabel %d", shortcut_index++),
......@@ -574,10 +478,9 @@ TEST_P(AppContextMenuTest, ArcMenuShortcut) {
if (index < menu->GetItemCount())
EXPECT_EQ(ui::PADDED_SEPARATOR, menu->GetSeparatorTypeAt(index));
}
}
}
TEST_P(AppContextMenuTest, ArcMenuStickyItem) {
TEST_F(AppContextMenuTest, ArcMenuStickyItem) {
ArcAppTest arc_test;
arc_test.SetUp(profile());
......@@ -597,33 +500,27 @@ TEST_P(AppContextMenuTest, ArcMenuStickyItem) {
// Separators are not added to touchable app context menus. For touchable
// app context menus, arc app has double separator, three more app shortcuts
// provided by arc::FakeAppInstance and two separators between shortcuts.
int expected_items = features::IsTouchableAppContextMenuEnabled() ? 9 : 5;
int expected_items = 9;
ASSERT_EQ(expected_items, menu->GetItemCount());
int index = 0;
ValidateItemState(menu.get(), index++, MenuState(ash::LAUNCH_NEW));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::SHOW_APP_INFO));
// Test that arc app shortcuts provided by arc::FakeAppInstance have a
// separator between each app shortcut.
if (features::IsTouchableAppContextMenuEnabled()) {
EXPECT_EQ(ui::DOUBLE_SEPARATOR, menu->GetSeparatorTypeAt(index++));
for (int shortcut_index = 0; index < menu->GetItemCount(); ++index) {
EXPECT_EQ(base::StringPrintf("ShortLabel %d", shortcut_index++),
base::UTF16ToUTF8(menu->GetLabelAt(index++)));
if (index < menu->GetItemCount())
EXPECT_EQ(ui::PADDED_SEPARATOR, menu->GetSeparatorTypeAt(index));
}
}
}
}
// In suspended state app does not have launch item.
TEST_P(AppContextMenuTest, ArcMenuSuspendedItem) {
TEST_F(AppContextMenuTest, ArcMenuSuspendedItem) {
ArcAppTest arc_test;
arc_test.SetUp(profile());
......@@ -642,24 +539,20 @@ TEST_P(AppContextMenuTest, ArcMenuSuspendedItem) {
// Separators are not added to touchable app context menus. For touchable
// app context menus, arc app has double separator, three more app shortcuts
// provided by arc::FakeAppInstance and two separators between shortcuts.
int expected_items = features::IsTouchableAppContextMenuEnabled() ? 8 : 3;
int expected_items = 8;
ASSERT_EQ(expected_items, menu->GetItemCount());
int index = 0;
ValidateItemState(menu.get(), index++, MenuState(ash::TOGGLE_PIN));
if (!features::IsTouchableAppContextMenuEnabled())
ValidateItemState(menu.get(), index++, MenuState()); // separator
ValidateItemState(menu.get(), index++, MenuState(ash::SHOW_APP_INFO));
// Test that arc app shortcuts provided by arc::FakeAppInstance have a
// separator between each app shortcut.
if (features::IsTouchableAppContextMenuEnabled()) {
EXPECT_EQ(ui::DOUBLE_SEPARATOR, menu->GetSeparatorTypeAt(index++));
for (int shortcut_index = 0; index < menu->GetItemCount(); ++index) {
EXPECT_EQ(base::StringPrintf("ShortLabel %d", shortcut_index++),
base::UTF16ToUTF8(menu->GetLabelAt(index++)));
if (index < menu->GetItemCount())
EXPECT_EQ(ui::PADDED_SEPARATOR, menu->GetSeparatorTypeAt(index));
}
}
}
......@@ -684,7 +577,7 @@ TEST_F(AppContextMenuTest, CommandIdsMatchEnumsForHistograms) {
}
// Tests that internal app's context menu is correct.
TEST_P(AppContextMenuTest, InternalAppMenu) {
TEST_F(AppContextMenuTest, InternalAppMenu) {
for (const auto& internal_app : app_list::GetInternalAppList(profile())) {
if (!internal_app.show_in_launcher)
continue;
......
......@@ -17,7 +17,6 @@
#include "chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h"
#include "chrome/browser/ui/ash/tablet_mode_client.h"
#include "chrome/grit/generated_resources.h"
#include "ui/base/ui_base_features.h"
ArcAppContextMenu::ArcAppContextMenu(app_list::AppContextMenuDelegate* delegate,
Profile* profile,
......@@ -30,10 +29,6 @@ ArcAppContextMenu::~ArcAppContextMenu() = default;
void ArcAppContextMenu::GetMenuModel(GetMenuModelCallback callback) {
auto menu_model = std::make_unique<ui::SimpleMenuModel>(this);
BuildMenu(menu_model.get());
if (!features::IsTouchableAppContextMenuEnabled()) {
std::move(callback).Run(std::move(menu_model));
return;
}
BuildAppShortcutsMenu(std::move(menu_model), std::move(callback));
}
......@@ -50,14 +45,10 @@ void ArcAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
if (!controller()->IsAppOpen(app_id()) && !app_info->suspended) {
AddContextMenuOption(menu_model, ash::LAUNCH_NEW,
IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
// Create default items.
app_list::AppContextMenu::BuildMenu(menu_model);
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
if (arc_prefs->IsShortcut(app_id())) {
AddContextMenuOption(menu_model, ash::UNINSTALL,
IDS_APP_LIST_REMOVE_SHORTCUT);
......
......@@ -16,7 +16,6 @@
#include "chrome/grit/generated_resources.h"
#include "content/public/common/context_menu_params.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_base_features.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/controls/menu/menu_config.h"
#include "ui/views/vector_icons.h"
......@@ -92,45 +91,11 @@ void ExtensionAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
base::Bind(MenuItemHasLauncherContext));
// First, add the primary actions.
if (!is_platform_app_) {
if (features::IsTouchableAppContextMenuEnabled()) {
CreateOpenNewSubmenu(menu_model);
} else {
AddContextMenuOption(menu_model, ash::LAUNCH_NEW, GetLaunchStringId());
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
// When bookmark apps are enabled, hosted apps can only toggle between
// USE_LAUNCH_TYPE_WINDOW and USE_LAUNCH_TYPE_REGULAR.
if (extensions::util::CanHostedAppsOpenInWindows() &&
extensions::util::IsNewBookmarkAppsEnabled()) {
// When both flags are enabled, only allow toggling between
// USE_LAUNCH_TYPE_WINDOW and USE_LAUNCH_TYPE_REGULAR
AddContextMenuOption(menu_model, ash::USE_LAUNCH_TYPE_WINDOW,
IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
} else if (!extensions::util::IsNewBookmarkAppsEnabled()) {
// When new bookmark apps are disabled, add pinned and full screen
// options as well. Add open as window if CanHostedAppsOpenInWindows
// is enabled.
AddContextMenuOption(menu_model, ash::USE_LAUNCH_TYPE_REGULAR,
IDS_APP_CONTEXT_MENU_OPEN_REGULAR);
AddContextMenuOption(menu_model, ash::USE_LAUNCH_TYPE_PINNED,
IDS_APP_CONTEXT_MENU_OPEN_PINNED);
if (extensions::util::CanHostedAppsOpenInWindows()) {
AddContextMenuOption(menu_model, ash::USE_LAUNCH_TYPE_WINDOW,
IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
}
// Even though the launch type is Full Screen it is more accurately
// described as Maximized in Ash.
AddContextMenuOption(menu_model, ash::USE_LAUNCH_TYPE_FULLSCREEN,
IDS_APP_CONTEXT_MENU_OPEN_MAXIMIZED);
}
}
}
if (!is_platform_app_)
CreateOpenNewSubmenu(menu_model);
// Create default items.
AppContextMenu::BuildMenu(menu_model);
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
// Assign unique IDs to commands added by the app itself.
int index = ash::USE_LAUNCH_TYPE_COMMAND_END;
......@@ -139,13 +104,6 @@ void ExtensionAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
base::string16(),
&index,
false); // is_action_menu
// If at least 1 item was added, add another separator after the list.
if (index > ash::USE_LAUNCH_TYPE_COMMAND_END &&
!features::IsTouchableAppContextMenuEnabled()) {
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
if (!is_platform_app_)
AddContextMenuOption(menu_model, ash::OPTIONS, IDS_NEW_TAB_APP_OPTIONS);
......@@ -172,9 +130,6 @@ base::string16 ExtensionAppContextMenu::GetLabelForCommandId(
bool ExtensionAppContextMenu::GetIconForCommandId(int command_id,
gfx::Image* icon) const {
if (!features::IsTouchableAppContextMenuEnabled())
return false;
if (command_id == ash::TOGGLE_PIN)
return AppContextMenu::GetIconForCommandId(command_id, icon);
......
......@@ -16,7 +16,6 @@
#include "chrome/browser/ui/ash/launcher/arc_app_shelf_id.h"
#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
#include "chrome/grit/generated_resources.h"
#include "ui/base/ui_base_features.h"
ArcLauncherContextMenu::ArcLauncherContextMenu(
ChromeLauncherController* controller,
......@@ -63,8 +62,6 @@ void ArcLauncherContextMenu::BuildMenu(
DCHECK(app_info->launchable);
AddContextMenuOption(menu_model.get(), ash::MENU_OPEN_NEW,
IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
if (!app_id.has_shelf_group_id() && app_info->launchable)
......@@ -74,14 +71,6 @@ void ArcLauncherContextMenu::BuildMenu(
AddContextMenuOption(menu_model.get(), ash::MENU_CLOSE,
IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
}
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
// App shortcuts from Android are shown on touchable context menu only.
if (!features::IsTouchableAppContextMenuEnabled()) {
std::move(callback).Run(std::move(menu_model));
return;
}
DCHECK(!app_shortcuts_menu_builder_);
app_shortcuts_menu_builder_ =
......
......@@ -15,7 +15,6 @@
#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
#include "chrome/browser/ui/views/crostini/crostini_app_restart_view.h"
#include "chrome/grit/generated_resources.h"
#include "ui/base/ui_base_features.h"
#include "ui/strings/grit/ui_strings.h"
CrostiniShelfContextMenu::CrostiniShelfContextMenu(
......@@ -72,9 +71,6 @@ void CrostiniShelfContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
IDS_CROSTINI_USE_LOW_DENSITY);
}
}
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
void CrostiniShelfContextMenu::ExecuteCommand(int command_id, int event_flags) {
......
......@@ -21,7 +21,6 @@
#include "chrome/grit/generated_resources.h"
#include "content/public/common/context_menu_params.h"
#include "extensions/browser/extension_prefs.h"
#include "ui/base/ui_base_features.h"
#include "ui/display/screen.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/controls/menu/menu_config.h"
......@@ -179,40 +178,8 @@ void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
base::Bind(MenuItemHasLauncherContext)));
if (item().type == ash::TYPE_PINNED_APP || item().type == ash::TYPE_APP) {
// V1 apps can be started from the menu - but V2 apps should not.
if (!controller()->IsPlatformApp(item().id)) {
if (features::IsTouchableAppContextMenuEnabled()) {
CreateOpenNewSubmenu(menu_model);
} else {
AddContextMenuOption(menu_model, ash::MENU_OPEN_NEW,
GetLaunchTypeStringId());
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
// Touchable app context menus do not include these check items, their
// functionality is achieved by an actionable submenu.
if (item().type == ash::TYPE_PINNED_APP) {
if (extensions::util::IsNewBookmarkAppsEnabled()) {
// With bookmark apps enabled, hosted apps launch in a window by
// default. This menu item is re-interpreted as a single,
// toggle-able option to launch the hosted app as a tab.
AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_WINDOW,
IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
} else {
AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_REGULAR_TAB,
IDS_APP_CONTEXT_MENU_OPEN_REGULAR);
AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_PINNED_TAB,
IDS_APP_CONTEXT_MENU_OPEN_PINNED);
AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_WINDOW,
IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
// Even though the launch type is Full Screen it is more accurately
// described as Maximized in Ash.
AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_FULLSCREEN,
IDS_APP_CONTEXT_MENU_OPEN_MAXIMIZED);
}
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
}
}
if (!controller()->IsPlatformApp(item().id))
CreateOpenNewSubmenu(menu_model);
AddPinMenu(menu_model);
if (controller()->IsOpen(item().id)) {
......@@ -238,16 +205,12 @@ void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
AddContextMenuOption(menu_model, ash::MENU_CLOSE,
IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
}
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
if (item().type == ash::TYPE_PINNED_APP || item().type == ash::TYPE_APP) {
const extensions::MenuItem::ExtensionKey app_key(item().id.app_id);
if (!app_key.empty()) {
int index = 0;
extension_items_->AppendExtensionItems(app_key, base::string16(), &index,
false); // is_action_menu
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
}
}
......
......@@ -12,7 +12,6 @@
#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
#include "chrome/grit/generated_resources.h"
#include "ui/base/ui_base_features.h"
InternalAppShelfContextMenu::InternalAppShelfContextMenu(
ChromeLauncherController* controller,
......@@ -31,8 +30,6 @@ void InternalAppShelfContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
if (!app_is_open) {
AddContextMenuOption(menu_model, ash::MENU_OPEN_NEW,
IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
const auto* internal_app = app_list::FindInternalApp(item().id.app_id);
......@@ -44,8 +41,4 @@ void InternalAppShelfContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
AddContextMenuOption(menu_model, ash::MENU_CLOSE,
IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
}
// Default menu items, e.g. "Autohide shelf" are appended. Adding a separator.
if (!features::IsTouchableAppContextMenuEnabled())
menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
}
......@@ -22,7 +22,6 @@
#include "chrome/browser/ui/ash/launcher/internal_app_shelf_context_menu.h"
#include "chrome/browser/ui/ash/tablet_mode_client.h"
#include "chrome/grit/generated_resources.h"
#include "ui/base/ui_base_features.h"
#include "ui/display/types/display_constants.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/controls/menu/menu_config.h"
......@@ -163,7 +162,7 @@ void LauncherContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
ash::CommandId type,
int string_id) {
const gfx::VectorIcon& icon = GetCommandIdVectorIcon(type, string_id);
if (features::IsTouchableAppContextMenuEnabled() && !icon.is_empty()) {
if (!icon.is_empty()) {
const views::MenuConfig& menu_config = views::MenuConfig::instance();
menu_model->AddItemWithStringIdAndIcon(
type, string_id,
......
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