Commit 25ea1442 authored by Alex Newcomer's avatar Alex Newcomer Committed by Commit Bot

Reland "cros: Enable touchable app context menus by default."

This CL is exactly the same as the previously landed versions, but I
landed a CL which fixes the errors in asan tests, linked below, so this
should be safe.

https://chromium-review.googlesource.com/c/chromium/src/+/1180073

TBR=xiyuan@chromium.org,sky@chromium.org,newcomer@chromium.org

Bug: 871843
Change-Id: I259e2c1177d05b050cea5638818a01cc82f24ea7
Reviewed-on: https://chromium-review.googlesource.com/1181770Reviewed-by: default avatarAlex Newcomer <newcomer@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584789}
parent e3e9da53
...@@ -50,13 +50,11 @@ TEST(ShelfApplicationMenuModelTest, VerifyContentsWithNoMenuItems) { ...@@ -50,13 +50,11 @@ TEST(ShelfApplicationMenuModelTest, VerifyContentsWithNoMenuItems) {
base::string16 title = base::ASCIIToUTF16("title"); base::string16 title = base::ASCIIToUTF16("title");
ShelfApplicationMenuModel menu(title, std::vector<mojom::MenuItemPtr>(), ShelfApplicationMenuModel menu(title, std::vector<mojom::MenuItemPtr>(),
nullptr); nullptr);
// Expect the title with separators. // Expect the title.
ASSERT_EQ(static_cast<int>(3), menu.GetItemCount()); ASSERT_EQ(static_cast<int>(1), menu.GetItemCount());
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(0)); EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(0));
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(1)); EXPECT_EQ(title, menu.GetLabelAt(0));
EXPECT_EQ(title, menu.GetLabelAt(1)); EXPECT_FALSE(menu.IsEnabledAt(0));
EXPECT_FALSE(menu.IsEnabledAt(1));
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(2));
} }
// Verifies the menu contents given a non-empty item list. // Verifies the menu contents given a non-empty item list.
...@@ -76,23 +74,23 @@ TEST(ShelfApplicationMenuModelTest, VerifyContentsWithMenuItems) { ...@@ -76,23 +74,23 @@ TEST(ShelfApplicationMenuModelTest, VerifyContentsWithMenuItems) {
ShelfApplicationMenuModel menu(title, std::move(items), nullptr); ShelfApplicationMenuModel menu(title, std::move(items), nullptr);
ShelfApplicationMenuModelTestAPI menu_test_api(&menu); ShelfApplicationMenuModelTestAPI menu_test_api(&menu);
// Expect the title with separators, the enabled items, and another separator. // Expect the title and the enabled items.
ASSERT_EQ(static_cast<int>(7), menu.GetItemCount()); ASSERT_EQ(static_cast<int>(5), menu.GetItemCount());
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(0));
// The label title should not be enabled.
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(0));
EXPECT_EQ(title, menu.GetLabelAt(0));
EXPECT_FALSE(menu.IsEnabledAt(0));
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(1)); EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(1));
EXPECT_EQ(title, menu.GetLabelAt(1)); EXPECT_EQ(title1, menu.GetLabelAt(1));
EXPECT_FALSE(menu.IsEnabledAt(1)); EXPECT_TRUE(menu.IsEnabledAt(1));
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(2)); EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(2));
EXPECT_EQ(title2, menu.GetLabelAt(2));
EXPECT_TRUE(menu.IsEnabledAt(2));
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(3)); EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(3));
EXPECT_EQ(title1, menu.GetLabelAt(3)); EXPECT_EQ(title3, menu.GetLabelAt(3));
EXPECT_TRUE(menu.IsEnabledAt(3)); EXPECT_TRUE(menu.IsEnabledAt(3));
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(4));
EXPECT_EQ(title2, menu.GetLabelAt(4));
EXPECT_TRUE(menu.IsEnabledAt(4));
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(5));
EXPECT_EQ(title3, menu.GetLabelAt(5));
EXPECT_TRUE(menu.IsEnabledAt(5));
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(6));
} }
// Verifies RecordMenuItemSelectedMetrics uses the correct histogram buckets. // Verifies RecordMenuItemSelectedMetrics uses the correct histogram buckets.
......
...@@ -126,33 +126,53 @@ TEST_F(ShelfContextMenuModelTest, Invocation) { ...@@ -126,33 +126,53 @@ TEST_F(ShelfContextMenuModelTest, Invocation) {
// Check the shelf auto-hide behavior and menu interaction. // Check the shelf auto-hide behavior and menu interaction.
ShelfContextMenuModel menu1(MenuItemList(), nullptr, primary_id); ShelfContextMenuModel menu1(MenuItemList(), nullptr, primary_id);
EXPECT_FALSE(menu1.IsItemCheckedAt(0));
EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_NEVER, shelf->auto_hide_behavior()); EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_NEVER, shelf->auto_hide_behavior());
menu1.ActivatedAt(0); menu1.ActivatedAt(0);
EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS, shelf->auto_hide_behavior()); EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS, shelf->auto_hide_behavior());
// This menu shows auto-hide enabled; check alignment and menu interaction. // Recreate the menu, auto-hide should still be enabled.
ShelfContextMenuModel menu2(MenuItemList(), nullptr, primary_id); ShelfContextMenuModel menu2(MenuItemList(), nullptr, primary_id);
EXPECT_TRUE(menu2.IsItemCheckedAt(0)); EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS, shelf->auto_hide_behavior());
// By default the shelf should be on bottom, shelf alignment options in order:
// Left, Bottom, Right. Bottom should be checked.
ui::MenuModel* submenu = menu2.GetSubmenuModelAt(1); ui::MenuModel* submenu = menu2.GetSubmenuModelAt(1);
EXPECT_TRUE(submenu->IsItemCheckedAt(1)); EXPECT_TRUE(submenu->IsItemCheckedAt(1));
EXPECT_EQ(SHELF_ALIGNMENT_BOTTOM, shelf->alignment()); EXPECT_EQ(SHELF_ALIGNMENT_BOTTOM, shelf->alignment());
// Activate the left shelf alignment option.
submenu->ActivatedAt(0); submenu->ActivatedAt(0);
EXPECT_EQ(SHELF_ALIGNMENT_LEFT, shelf->alignment()); EXPECT_EQ(SHELF_ALIGNMENT_LEFT, shelf->alignment());
// This menu shows left alignment; check wallpaper item interaction. // Recreate the menu, it should show left alignment checked.
ShelfContextMenuModel menu3(MenuItemList(), nullptr, primary_id); ShelfContextMenuModel menu3(MenuItemList(), nullptr, primary_id);
submenu = menu3.GetSubmenuModelAt(1); submenu = menu3.GetSubmenuModelAt(1);
EXPECT_TRUE(submenu->IsItemCheckedAt(0)); EXPECT_TRUE(submenu->IsItemCheckedAt(0));
TestWallpaperControllerClient client; TestWallpaperControllerClient client;
Shell::Get()->wallpaper_controller()->SetClientForTesting( Shell::Get()->wallpaper_controller()->SetClientForTesting(
client.CreateInterfacePtr()); client.CreateInterfacePtr());
EXPECT_EQ(0u, client.open_count()); EXPECT_EQ(0u, client.open_count());
// Click the third option, wallpaper picker. It should open.
menu3.ActivatedAt(2); menu3.ActivatedAt(2);
Shell::Get()->wallpaper_controller()->FlushForTesting(); Shell::Get()->wallpaper_controller()->FlushForTesting();
EXPECT_EQ(1u, client.open_count()); EXPECT_EQ(1u, client.open_count());
} }
// Tests that a desktop context menu shows the default options.
TEST_F(ShelfContextMenuModelTest, DesktopMenu) {
MenuItemList items;
// Pass a null delegate to indicate the menu is not for an application.
ShelfContextMenuModel menu(std::move(items), nullptr,
GetPrimaryDisplay().id());
ASSERT_EQ(3, menu.GetItemCount());
EXPECT_EQ(CommandId::MENU_AUTO_HIDE, menu.GetCommandIdAt(0));
EXPECT_EQ(CommandId::MENU_ALIGNMENT_MENU, menu.GetCommandIdAt(1));
EXPECT_EQ(CommandId::MENU_CHANGE_WALLPAPER, menu.GetCommandIdAt(2));
}
// Tests the prepending of custom items in a shelf context menu. // Tests the prepending of custom items in a shelf context menu.
TEST_F(ShelfContextMenuModelTest, CustomItems) { TEST_F(ShelfContextMenuModelTest, CustomItems) {
// Make a list of custom items with a variety of values. // Make a list of custom items with a variety of values.
...@@ -163,9 +183,7 @@ TEST_F(ShelfContextMenuModelTest, CustomItems) { ...@@ -163,9 +183,7 @@ TEST_F(ShelfContextMenuModelTest, CustomItems) {
item->label = base::ASCIIToUTF16("item"); item->label = base::ASCIIToUTF16("item");
item->enabled = true; item->enabled = true;
items.push_back(std::move(item)); items.push_back(std::move(item));
mojom::MenuItemPtr separator(mojom::MenuItem::New());
separator->type = ui::MenuModel::TYPE_SEPARATOR;
items.push_back(std::move(separator));
mojom::MenuItemPtr check(mojom::MenuItem::New()); mojom::MenuItemPtr check(mojom::MenuItem::New());
check->type = ui::MenuModel::TYPE_CHECK; check->type = ui::MenuModel::TYPE_CHECK;
check->command_id = 999; check->command_id = 999;
...@@ -173,6 +191,7 @@ TEST_F(ShelfContextMenuModelTest, CustomItems) { ...@@ -173,6 +191,7 @@ TEST_F(ShelfContextMenuModelTest, CustomItems) {
check->enabled = true; check->enabled = true;
check->checked = false; check->checked = false;
items.push_back(std::move(check)); items.push_back(std::move(check));
mojom::MenuItemPtr radio(mojom::MenuItem::New()); mojom::MenuItemPtr radio(mojom::MenuItem::New());
radio->type = ui::MenuModel::TYPE_RADIO; radio->type = ui::MenuModel::TYPE_RADIO;
radio->command_id = 1337; radio->command_id = 1337;
...@@ -181,38 +200,35 @@ TEST_F(ShelfContextMenuModelTest, CustomItems) { ...@@ -181,38 +200,35 @@ TEST_F(ShelfContextMenuModelTest, CustomItems) {
radio->checked = true; radio->checked = true;
items.push_back(std::move(radio)); items.push_back(std::move(radio));
// Ensure the menu model's prepended contents match the items above.
TestShelfItemDelegate delegate; TestShelfItemDelegate delegate;
ShelfContextMenuModel menu(std::move(items), &delegate, ShelfContextMenuModel menu(std::move(items), &delegate,
GetPrimaryDisplay().id()); GetPrimaryDisplay().id());
ASSERT_EQ(7, menu.GetItemCount());
// Ensure the menu model's prepended contents match the items above. Because
// the delegate is not null, the menu is interpreted as an application menu.
// It will not have the desktop menu options which are autohide, shelf
// position, and wallpaper picker.
ASSERT_EQ(3, menu.GetItemCount());
EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(0)); EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(0));
EXPECT_EQ(123, menu.GetCommandIdAt(0)); EXPECT_EQ(123, menu.GetCommandIdAt(0));
EXPECT_EQ(base::ASCIIToUTF16("item"), menu.GetLabelAt(0)); EXPECT_EQ(base::ASCIIToUTF16("item"), menu.GetLabelAt(0));
EXPECT_TRUE(menu.IsEnabledAt(0)); EXPECT_TRUE(menu.IsEnabledAt(0));
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(1)); EXPECT_EQ(ui::MenuModel::TYPE_CHECK, menu.GetTypeAt(1));
EXPECT_EQ(999, menu.GetCommandIdAt(1));
EXPECT_EQ(ui::MenuModel::TYPE_CHECK, menu.GetTypeAt(2)); EXPECT_EQ(base::ASCIIToUTF16("check"), menu.GetLabelAt(1));
EXPECT_EQ(999, menu.GetCommandIdAt(2)); EXPECT_TRUE(menu.IsEnabledAt(1));
EXPECT_EQ(base::ASCIIToUTF16("check"), menu.GetLabelAt(2)); EXPECT_FALSE(menu.IsItemCheckedAt(1));
EXPECT_TRUE(menu.IsEnabledAt(2));
EXPECT_FALSE(menu.IsItemCheckedAt(2));
EXPECT_EQ(ui::MenuModel::TYPE_RADIO, menu.GetTypeAt(3)); EXPECT_EQ(ui::MenuModel::TYPE_RADIO, menu.GetTypeAt(2));
EXPECT_EQ(1337, menu.GetCommandIdAt(3)); EXPECT_EQ(1337, menu.GetCommandIdAt(2));
EXPECT_EQ(base::ASCIIToUTF16("radio"), menu.GetLabelAt(3)); EXPECT_EQ(base::ASCIIToUTF16("radio"), menu.GetLabelAt(2));
EXPECT_FALSE(menu.IsEnabledAt(3)); EXPECT_FALSE(menu.IsEnabledAt(2));
EXPECT_TRUE(menu.IsItemCheckedAt(3)); EXPECT_TRUE(menu.IsItemCheckedAt(2));
// The default contents should appear at the bottom.
EXPECT_EQ(CommandId::MENU_AUTO_HIDE, menu.GetCommandIdAt(4));
EXPECT_EQ(CommandId::MENU_ALIGNMENT_MENU, menu.GetCommandIdAt(5));
EXPECT_EQ(CommandId::MENU_CHANGE_WALLPAPER, menu.GetCommandIdAt(6));
// Invoking a custom item should execute the command id on the delegate. // Invoking a custom item should execute the command id on the delegate.
menu.ActivatedAt(2); menu.ActivatedAt(1);
EXPECT_EQ(999, delegate.last_executed_command()); EXPECT_EQ(999, delegate.last_executed_command());
} }
...@@ -259,44 +275,54 @@ TEST_F(ShelfContextMenuModelTest, AutohideShelfOptionOnExternalDisplay) { ...@@ -259,44 +275,54 @@ TEST_F(ShelfContextMenuModelTest, AutohideShelfOptionOnExternalDisplay) {
EXPECT_NE(-1, secondary_menu.GetIndexOfCommandId(CommandId::MENU_AUTO_HIDE)); EXPECT_NE(-1, secondary_menu.GetIndexOfCommandId(CommandId::MENU_AUTO_HIDE));
} }
TEST_F(ShelfContextMenuModelTest, DisableAutoHideOptionOnTabletMode) { // Tests that the autohide and alignment menu options are not included in tablet
// mode.
TEST_F(ShelfContextMenuModelTest, ExcludeClamshellOptionsOnTabletMode) {
TabletModeController* tablet_mode_controller = TabletModeController* tablet_mode_controller =
Shell::Get()->tablet_mode_controller(); Shell::Get()->tablet_mode_controller();
int64_t primary_id = GetPrimaryDisplay().id(); int64_t primary_id = GetPrimaryDisplay().id();
// Tests that in tablet mode, shelf auto-hide option is disabled. // In tablet mode, the wallpaper picker should be the only option because the
// other options are disabled.
tablet_mode_controller->EnableTabletModeWindowManager(true); tablet_mode_controller->EnableTabletModeWindowManager(true);
ShelfContextMenuModel menu1(MenuItemList(), nullptr, primary_id); ShelfContextMenuModel menu1(MenuItemList(), nullptr, primary_id);
ASSERT_EQ(CommandId::MENU_AUTO_HIDE, menu1.GetCommandIdAt(0)); EXPECT_EQ(1, menu1.GetItemCount());
EXPECT_FALSE(menu1.IsEnabledAt(0)); EXPECT_EQ(ShelfContextMenuModel::MENU_CHANGE_WALLPAPER,
EXPECT_TRUE(menu1.IsVisibleAt(0)); menu1.GetCommandIdAt(0));
// Tests that exiting tablet mode reenables the auto-hide context menu item. // Test that a menu shown out of tablet mode includes all three options:
// MENU_AUTO_HIDE, MENU_ALIGNMENT_MENU, and MENU_CHANGE_WALLPAPER.
tablet_mode_controller->EnableTabletModeWindowManager(false); tablet_mode_controller->EnableTabletModeWindowManager(false);
ShelfContextMenuModel menu2(MenuItemList(), nullptr, primary_id); ShelfContextMenuModel menu2(MenuItemList(), nullptr, primary_id);
ASSERT_EQ(CommandId::MENU_AUTO_HIDE, menu2.GetCommandIdAt(0)); EXPECT_EQ(3, menu2.GetItemCount());
EXPECT_TRUE(menu2.IsEnabledAt(0));
EXPECT_TRUE(menu2.IsVisibleAt(0));
}
TEST_F(ShelfContextMenuModelTest, DisableAlignmentMenuOnTabletMode) {
TabletModeController* tablet_mode_controller =
Shell::Get()->tablet_mode_controller();
int64_t primary_id = GetPrimaryDisplay().id();
// Tests that in tablet mode, shelf alignment menu is disabled. // Test the auto hide option.
tablet_mode_controller->EnableTabletModeWindowManager(true); EXPECT_EQ(ShelfContextMenuModel::MENU_AUTO_HIDE, menu2.GetCommandIdAt(0));
ShelfContextMenuModel menu1(MenuItemList(), nullptr, primary_id); EXPECT_TRUE(menu2.IsEnabledAt(0));
ASSERT_EQ(CommandId::MENU_ALIGNMENT_MENU, menu1.GetCommandIdAt(1));
EXPECT_FALSE(menu1.IsEnabledAt(1));
EXPECT_TRUE(menu1.IsVisibleAt(1));
// Tests that exiting tablet mode reenables the shelf alignment menu. // Test the shelf alignment menu option.
tablet_mode_controller->EnableTabletModeWindowManager(false); EXPECT_EQ(ShelfContextMenuModel::MENU_ALIGNMENT_MENU,
ShelfContextMenuModel menu2(MenuItemList(), nullptr, primary_id); menu2.GetCommandIdAt(1));
ASSERT_EQ(CommandId::MENU_ALIGNMENT_MENU, menu2.GetCommandIdAt(1));
EXPECT_TRUE(menu2.IsEnabledAt(1)); EXPECT_TRUE(menu2.IsEnabledAt(1));
EXPECT_TRUE(menu2.IsVisibleAt(1));
// Test the shelf alignment submenu.
ui::MenuModel* submenu = menu2.GetSubmenuModelAt(1);
EXPECT_EQ(ShelfContextMenuModel::MENU_ALIGNMENT_LEFT,
submenu->GetCommandIdAt(0));
EXPECT_TRUE(submenu->IsEnabledAt(0));
EXPECT_EQ(ShelfContextMenuModel::MENU_ALIGNMENT_BOTTOM,
submenu->GetCommandIdAt(1));
EXPECT_TRUE(submenu->IsEnabledAt(1));
EXPECT_EQ(ShelfContextMenuModel::MENU_ALIGNMENT_RIGHT,
submenu->GetCommandIdAt(2));
EXPECT_TRUE(submenu->IsEnabledAt(2));
// Test the wallpaper picker option.
EXPECT_EQ(ShelfContextMenuModel::MENU_CHANGE_WALLPAPER,
menu2.GetCommandIdAt(2));
EXPECT_TRUE(menu2.IsEnabledAt(2));
} }
TEST_F(ShelfContextMenuModelTest, CommandIdsMatchEnumsForHistograms) { TEST_F(ShelfContextMenuModelTest, CommandIdsMatchEnumsForHistograms) {
......
...@@ -1993,18 +1993,6 @@ TEST_F(ShelfViewTest, TabletModeStartAndEndClosesContextMenu) { ...@@ -1993,18 +1993,6 @@ TEST_F(ShelfViewTest, TabletModeStartAndEndClosesContextMenu) {
EXPECT_FALSE(test_api_->CloseMenu()); EXPECT_FALSE(test_api_->CloseMenu());
} }
// Tests that the app list button shows a context menu on right click when
// touchable app context menus are not enabled.
TEST_F(ShelfViewTest, AppListButtonShowsContextMenu) {
ui::test::EventGenerator* generator = GetEventGenerator();
AppListButton* app_list_button = shelf_view_->GetAppListButton();
generator->MoveMouseTo(app_list_button->GetBoundsInScreen().CenterPoint());
generator->PressRightButton();
EXPECT_TRUE(test_api_->CloseMenu());
}
// Tests that the back button does not show a context menu. // Tests that the back button does not show a context menu.
TEST_F(ShelfViewTest, NoContextMenuOnBackButton) { TEST_F(ShelfViewTest, NoContextMenuOnBackButton) {
ui::test::EventGenerator* generator = GetEventGenerator(); ui::test::EventGenerator* generator = GetEventGenerator();
...@@ -2034,23 +2022,6 @@ TEST_F(ShelfViewTest, NoContextMenuOnOverflowButton) { ...@@ -2034,23 +2022,6 @@ TEST_F(ShelfViewTest, NoContextMenuOnOverflowButton) {
EXPECT_FALSE(test_api_->CloseMenu()); EXPECT_FALSE(test_api_->CloseMenu());
} }
// Tests that a ShelfButton ink drop highlight is set to ACTIVATED when a menu
// is shown by mouse.
TEST_F(ShelfViewTest, ShelfButtonShowsInkDropHighlightOnMenuShow) {
const ShelfID id_0 = AddApp();
ShelfButton* button_0 = GetButtonByID(id_0);
ui::test::EventGenerator* generator = GetEventGenerator();
generator->MoveMouseTo(button_0->GetBoundsInScreen().CenterPoint());
generator->PressRightButton();
EXPECT_EQ(views::InkDropState::ACTIVATED,
button_0->GetInkDropForTesting()->GetTargetInkDropState());
// Close the menu, the InkDropState should transition to HIDDEN.
EXPECT_TRUE(test_api_->CloseMenu());
EXPECT_EQ(views::InkDropState::HIDDEN,
button_0->GetInkDropForTesting()->GetTargetInkDropState());
}
// Tests that ShelfWindowWatcher buttons show a context menu on right click. // Tests that ShelfWindowWatcher buttons show a context menu on right click.
TEST_F(ShelfViewTest, ShelfWindowWatcherButtonShowsContextMenu) { TEST_F(ShelfViewTest, ShelfWindowWatcherButtonShowsContextMenu) {
ui::test::EventGenerator* generator = GetEventGenerator(); ui::test::EventGenerator* generator = GetEventGenerator();
...@@ -2798,7 +2769,10 @@ TEST_F(ShelfViewInkDropTest, ShelfButtonWithMenuPressRelease) { ...@@ -2798,7 +2769,10 @@ TEST_F(ShelfViewInkDropTest, ShelfButtonWithMenuPressRelease) {
mouse_location, ui::EventTimeForNow(), mouse_location, ui::EventTimeForNow(),
ui::EF_LEFT_MOUSE_BUTTON, 0); ui::EF_LEFT_MOUSE_BUTTON, 0);
button->OnMouseReleased(release_event); button->OnMouseReleased(release_event);
test_api_->CloseMenu(); EXPECT_EQ(views::InkDropState::ACTIVATED,
browser_button_ink_drop_->GetTargetInkDropState());
EXPECT_TRUE(test_api_->CloseMenu());
EXPECT_EQ(views::InkDropState::HIDDEN, EXPECT_EQ(views::InkDropState::HIDDEN,
browser_button_ink_drop_->GetTargetInkDropState()); browser_button_ink_drop_->GetTargetInkDropState());
EXPECT_THAT(browser_button_ink_drop_->GetAndResetRequestedStates(), EXPECT_THAT(browser_button_ink_drop_->GetAndResetRequestedStates(),
......
...@@ -273,11 +273,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest, ...@@ -273,11 +273,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest,
VerifyMenuItem("parent", top_level_model_, top_level_index(), VerifyMenuItem("parent", top_level_model_, top_level_index(),
ui::MenuModel::TYPE_SUBMENU, true); ui::MenuModel::TYPE_SUBMENU, true);
// Since the extension submenu is shown, the previous separator should be in
// the model.
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR,
top_level_model_->GetTypeAt(top_level_index() - 1));
ui::MenuModel* submodel = ui::MenuModel* submodel =
top_level_model_->GetSubmenuModelAt(top_level_index()); top_level_model_->GetSubmenuModelAt(top_level_index());
ASSERT_TRUE(submodel); ASSERT_TRUE(submodel);
...@@ -306,11 +301,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest, ...@@ -306,11 +301,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest,
VerifyMenuItem("parent", top_level_model_, top_level_index(), VerifyMenuItem("parent", top_level_model_, top_level_index(),
ui::MenuModel::TYPE_SUBMENU, true); ui::MenuModel::TYPE_SUBMENU, true);
// Since the extension submenu is shown, the previous separator should be in
// the model.
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR,
top_level_model_->GetTypeAt(top_level_index() - 1));
ui::MenuModel* submodel = ui::MenuModel* submodel =
top_level_model_->GetSubmenuModelAt(top_level_index()); top_level_model_->GetSubmenuModelAt(top_level_index());
ASSERT_TRUE(submodel); ASSERT_TRUE(submodel);
...@@ -339,11 +329,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest, ...@@ -339,11 +329,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest,
VerifyMenuItem(extension()->name(), top_level_model_, top_level_index(), VerifyMenuItem(extension()->name(), top_level_model_, top_level_index(),
ui::MenuModel::TYPE_SUBMENU, true); ui::MenuModel::TYPE_SUBMENU, true);
// Since the extension submenu is shown, the previous separator should be in
// the model.
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR,
top_level_model_->GetTypeAt(top_level_index() - 1));
ui::MenuModel* submodel = ui::MenuModel* submodel =
top_level_model_->GetSubmenuModelAt(top_level_index()); top_level_model_->GetSubmenuModelAt(top_level_index());
ASSERT_TRUE(submodel); ASSERT_TRUE(submodel);
...@@ -378,11 +363,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest, ...@@ -378,11 +363,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuApiTest,
VerifyMenuItem(extension()->name(), top_level_model_, top_level_index(), VerifyMenuItem(extension()->name(), top_level_model_, top_level_index(),
ui::MenuModel::TYPE_SUBMENU, true); ui::MenuModel::TYPE_SUBMENU, true);
// Since the extension submenu is shown, the previous separator should be in
// the model.
EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR,
top_level_model_->GetTypeAt(top_level_index() - 1));
ui::MenuModel* submodel = ui::MenuModel* submodel =
top_level_model_->GetSubmenuModelAt(top_level_index()); top_level_model_->GetSubmenuModelAt(top_level_index());
ASSERT_TRUE(submodel); ASSERT_TRUE(submodel);
......
...@@ -571,28 +571,20 @@ static void VerifyMenuForSeparatorsTest(const MenuModel& menu) { ...@@ -571,28 +571,20 @@ static void VerifyMenuForSeparatorsTest(const MenuModel& menu) {
// We expect to see the following items in the menu: // We expect to see the following items in the menu:
// radio1 // radio1
// radio2 // radio2
// --separator-- (automatically added)
// normal1 // normal1
// --separator--
// normal2 // normal2
// --separator--
// radio3 // radio3
// radio4 // radio4
// --separator--
// normal3 // normal3
int index = 0; int index = 0;
ASSERT_EQ(11, menu.GetItemCount()); ASSERT_EQ(7, menu.GetItemCount());
ExpectLabelAndType("radio1", MenuModel::TYPE_RADIO, menu, index++); ExpectLabelAndType("radio1", MenuModel::TYPE_RADIO, menu, index++);
ExpectLabelAndType("radio2", MenuModel::TYPE_RADIO, menu, index++); ExpectLabelAndType("radio2", MenuModel::TYPE_RADIO, menu, index++);
EXPECT_EQ(MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(index++));
ExpectLabelAndType("normal1", MenuModel::TYPE_COMMAND, menu, index++); ExpectLabelAndType("normal1", MenuModel::TYPE_COMMAND, menu, index++);
EXPECT_EQ(MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(index++));
ExpectLabelAndType("normal2", MenuModel::TYPE_COMMAND, menu, index++); ExpectLabelAndType("normal2", MenuModel::TYPE_COMMAND, menu, index++);
EXPECT_EQ(MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(index++));
ExpectLabelAndType("radio3", MenuModel::TYPE_RADIO, menu, index++); ExpectLabelAndType("radio3", MenuModel::TYPE_RADIO, menu, index++);
ExpectLabelAndType("radio4", MenuModel::TYPE_RADIO, menu, index++); ExpectLabelAndType("radio4", MenuModel::TYPE_RADIO, menu, index++);
EXPECT_EQ(MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(index++));
ExpectLabelAndType("normal3", MenuModel::TYPE_COMMAND, menu, index++); ExpectLabelAndType("normal3", MenuModel::TYPE_COMMAND, menu, index++);
} }
......
...@@ -3530,25 +3530,25 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuExecution) { ...@@ -3530,25 +3530,25 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuExecution) {
model_->GetShelfItemDelegate(gmail_id); model_->GetShelfItemDelegate(gmail_id);
ASSERT_TRUE(item_delegate); ASSERT_TRUE(item_delegate);
EXPECT_EQ(1, browser()->tab_strip_model()->active_index()); EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
// Execute the second item in the menu, after the title and two separators, // Execute the second item in the menu, after the title,
// this shouldn't do anything since that item is already the active tab. // this shouldn't do anything since that item is already the active tab.
{ {
ash::ShelfApplicationMenuModel menu( ash::ShelfApplicationMenuModel menu(
base::string16(), base::string16(),
launcher_controller_->GetAppMenuItemsForTesting(item_gmail), launcher_controller_->GetAppMenuItemsForTesting(item_gmail),
item_delegate); item_delegate);
menu.ActivatedAt(4); menu.ActivatedAt(2);
} }
EXPECT_EQ(1, browser()->tab_strip_model()->active_index()); EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
// Execute the first item in the menu, after the title and two separators, // Execute the first item in the menu, after the title,
// this should activate the other tab. // this should activate the other tab.
{ {
ash::ShelfApplicationMenuModel menu( ash::ShelfApplicationMenuModel menu(
base::string16(), base::string16(),
launcher_controller_->GetAppMenuItemsForTesting(item_gmail), launcher_controller_->GetAppMenuItemsForTesting(item_gmail),
item_delegate); item_delegate);
menu.ActivatedAt(3); menu.ActivatedAt(1);
} }
EXPECT_EQ(0, browser()->tab_strip_model()->active_index()); EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
} }
...@@ -3558,7 +3558,6 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuDeletionExecution) { ...@@ -3558,7 +3558,6 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuDeletionExecution) {
InitLauncherControllerWithBrowser(); InitLauncherControllerWithBrowser();
// Add |extension3_| to the launcher and add two items. // Add |extension3_| to the launcher and add two items.
GURL gmail = GURL("https://mail.google.com/mail/u");
const ash::ShelfID gmail_id(extension3_->id()); const ash::ShelfID gmail_id(extension3_->id());
extension_service_->AddExtension(extension3_.get()); extension_service_->AddExtension(extension3_.get());
launcher_controller_->SetRefocusURLPatternForTest(gmail_id, GURL(kGmailUrl)); launcher_controller_->SetRefocusURLPatternForTest(gmail_id, GURL(kGmailUrl));
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/chromeos/arc/icon_decode_request.h"
#include "chrome/browser/prefs/incognito_mode_prefs.h" #include "chrome/browser/prefs/incognito_mode_prefs.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/arc/arc_app_test.h" #include "chrome/browser/ui/app_list/arc/arc_app_test.h"
...@@ -78,6 +79,10 @@ class LauncherContextMenuTest : public ash::AshTestBase { ...@@ -78,6 +79,10 @@ class LauncherContextMenuTest : public ash::AshTestBase {
tablet_mode_client_ = std::make_unique<TabletModeClient>(); tablet_mode_client_ = std::make_unique<TabletModeClient>();
tablet_mode_client_->InitForTesting( tablet_mode_client_->InitForTesting(
fake_tablet_mode_controller_.CreateInterfacePtr()); fake_tablet_mode_controller_.CreateInterfacePtr());
// Disable safe icon decoding to ensure ArcAppShortcutRequests returns in
// the test environment.
arc::IconDecodeRequest::DisableSafeDecodingForTesting();
} }
std::unique_ptr<LauncherContextMenu> CreateLauncherContextMenu( std::unique_ptr<LauncherContextMenu> CreateLauncherContextMenu(
...@@ -346,7 +351,7 @@ TEST_F(LauncherContextMenuTest, ArcLauncherMenusCheck) { ...@@ -346,7 +351,7 @@ TEST_F(LauncherContextMenuTest, ArcLauncherMenusCheck) {
} }
} }
TEST_F(LauncherContextMenuTest, ArcLauncherSuspenedAppMenu) { TEST_F(LauncherContextMenuTest, ArcLauncherSuspendAppMenu) {
arc::mojom::AppInfo app = arc_test().fake_apps()[0]; arc::mojom::AppInfo app = arc_test().fake_apps()[0];
app.suspended = true; app.suspended = true;
arc_test().app_instance()->RefreshAppList(); arc_test().app_instance()->RefreshAppList();
...@@ -436,7 +441,7 @@ TEST_F(LauncherContextMenuTest, CommandIdsMatchEnumsForHistograms) { ...@@ -436,7 +441,7 @@ TEST_F(LauncherContextMenuTest, CommandIdsMatchEnumsForHistograms) {
} }
TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) { TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) {
// Tests that there are 4 ARC app context menu options. If you're // Tests that there are 8 ARC app context menu options. If you're
// adding a context menu option ensure that you have added the enum to // adding a context menu option ensure that you have added the enum to
// tools/metrics/enums.xml and that you haven't modified the order of the // tools/metrics/enums.xml and that you haven't modified the order of the
// existing enums. // existing enums.
...@@ -457,8 +462,8 @@ TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) { ...@@ -457,8 +462,8 @@ TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) {
std::unique_ptr<ui::MenuModel> menu = std::unique_ptr<ui::MenuModel> menu =
GetContextMenu(item_delegate, primary_id); GetContextMenu(item_delegate, primary_id);
// Test that there are 4 items in an ARC app context menu. // Test that there are 8 items in an ARC app context menu.
EXPECT_EQ(4, menu->GetItemCount()); EXPECT_EQ(8, menu->GetItemCount());
} }
// Tests that the context menu of internal app is correct. // Tests that the context menu of internal app is correct.
...@@ -510,7 +515,7 @@ TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenuOptionsNumber) { ...@@ -510,7 +515,7 @@ TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenuOptionsNumber) {
std::unique_ptr<ui::MenuModel> menu = std::unique_ptr<ui::MenuModel> menu =
GetContextMenu(item_delegate, primary_id); GetContextMenu(item_delegate, primary_id);
const int expected_options_num = internal_app.show_in_launcher ? 4 : 2; const int expected_options_num = internal_app.show_in_launcher ? 2 : 1;
EXPECT_EQ(expected_options_num, menu->GetItemCount()); EXPECT_EQ(expected_options_num, menu->GetItemCount());
} }
} }
......
...@@ -71,8 +71,8 @@ void OpenBrowserUsingContextMenuOnRootWindow(aura::Window* root) { ...@@ -71,8 +71,8 @@ void OpenBrowserUsingContextMenuOnRootWindow(aura::Window* root) {
ChromeLauncherController::instance()->FlushForTesting(); ChromeLauncherController::instance()->FlushForTesting();
// Move the cursor up to the "New window" menu option - assumes menu content. // Move the cursor up to the "New window" menu option - assumes menu content.
generator.MoveMouseBy(0, -5 * views::MenuConfig::instance().item_min_height - generator.MoveMouseBy(
views::MenuConfig::instance().separator_height); 0, -3 * views::MenuConfig::instance().touchable_menu_height);
generator.ReleaseRightButton(); generator.ReleaseRightButton();
// Ash notifies Chrome's ShelfItemDelegate that the menu item was selected. // Ash notifies Chrome's ShelfItemDelegate that the menu item was selected.
......
...@@ -51,7 +51,7 @@ const base::Feature kSystemKeyboardLock{"SystemKeyboardLock", ...@@ -51,7 +51,7 @@ const base::Feature kSystemKeyboardLock{"SystemKeyboardLock",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kTouchableAppContextMenu = { const base::Feature kTouchableAppContextMenu = {
"EnableTouchableAppContextMenu", base::FEATURE_DISABLED_BY_DEFAULT}; "EnableTouchableAppContextMenu", base::FEATURE_ENABLED_BY_DEFAULT};
bool IsTouchableAppContextMenuEnabled() { bool IsTouchableAppContextMenuEnabled() {
return base::FeatureList::IsEnabled(kTouchableAppContextMenu) || return base::FeatureList::IsEnabled(kTouchableAppContextMenu) ||
......
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