cros: Move all app command id enums to app_menu_constants.h

Move all app command id enums to app_menu_constants.h.

Bug: 853927
Change-Id: Ib269c5b35dfad205912a4d69eb84d84fd7c86857
Reviewed-on: https://chromium-review.googlesource.com/1107199Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569643}

ash/public/cpp/app_menu_constants.h

@@ -7,10 +7,51 @@
 
 namespace ash {
 
-// NOTIFICATION_CONTAINER is 9 so it matches
-// LauncherContextMenu::MenuItem::NOTIFICATION_CONTAINER.
+// Defines command ids used in Shelf and AppList app context menus. These are
+// used in histograms, do not remove/renumber entries. If you're adding to this
+// enum with the intention that it will be logged, add checks to ensure
+// stability of the enum and update the ChromeOSUICommands enum listing in
+// tools/metrics/histograms/enums.xml.
+// TODO(newcomer): Deprecate duplicate CommandIds after the Touchable App
+// Context Menu launch. Delay deprecating these because it will disrupt
+// histograms. https://crbug.com/854433
 enum CommandId {
+  // Used by LauncherContextMenu.
+  MENU_OPEN_NEW = 0,
+  MENU_CLOSE = 1,
+  MENU_PIN = 2,
+  LAUNCH_TYPE_PINNED_TAB = 3,
+  LAUNCH_TYPE_REGULAR_TAB = 4,
+  LAUNCH_TYPE_FULLSCREEN = 5,
+  LAUNCH_TYPE_WINDOW = 6,
+  MENU_NEW_WINDOW = 7,
+  MENU_NEW_INCOGNITO_WINDOW = 8,
+
+  // Used by AppMenuModelAdapter.
   NOTIFICATION_CONTAINER = 9,
+
+  // Used by AppContextMenu.
+  LAUNCH_NEW = 100,
+  TOGGLE_PIN = 101,
+  SHOW_APP_INFO = 102,
+  OPTIONS = 103,
+  UNINSTALL = 104,
+  REMOVE_FROM_FOLDER = 105,
+  APP_CONTEXT_MENU_NEW_WINDOW = 106,
+  APP_CONTEXT_MENU_NEW_INCOGNITO_WINDOW = 107,
+  INSTALL = 108,
+  USE_LAUNCH_TYPE_COMMAND_START = 200,
+  USE_LAUNCH_TYPE_PINNED = USE_LAUNCH_TYPE_COMMAND_START,
+  USE_LAUNCH_TYPE_REGULAR = 201,
+  USE_LAUNCH_TYPE_FULLSCREEN = 202,
+  USE_LAUNCH_TYPE_WINDOW = 203,
+  USE_LAUNCH_TYPE_COMMAND_END,
+
+  // Range of command ids reserved for launching app shortcuts from context
+  // menu for Android app. Used by AppContextMenu and LauncherContextMenu.
+  LAUNCH_APP_SHORTCUT_FIRST = 1000,
+  LAUNCH_APP_SHORTCUT_LAST = 1999,
+  COMMAND_ID_COUNT
 };
 
 // Minimum padding for children of NotificationMenuView in dips.

chrome/browser/ui/app_list/app_context_menu.cc

@@ -40,7 +40,7 @@ void AppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
     if (!features::IsTouchableAppContextMenuEnabled())
       menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
 
-    AddContextMenuOption(menu_model, TOGGLE_PIN,
+    AddContextMenuOption(menu_model, ash::TOGGLE_PIN,
                          controller_->IsAppPinned(app_id_)
                              ? IDS_APP_LIST_CONTEXT_MENU_UNPIN
                              : IDS_APP_LIST_CONTEXT_MENU_PIN);
@@ -48,11 +48,11 @@ void AppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
 }
 
 bool AppContextMenu::IsItemForCommandIdDynamic(int command_id) const {
-  return command_id == TOGGLE_PIN;
+  return command_id == ash::TOGGLE_PIN;
 }
 
 base::string16 AppContextMenu::GetLabelForCommandId(int command_id) const {
-  if (command_id == TOGGLE_PIN) {
+  if (command_id == ash::TOGGLE_PIN) {
     // Return "{Pin to, Unpin from} shelf" or "Pinned by administrator".
     // Note this only exists on Ash desktops.
     if (controller_->GetPinnable(app_id()) ==
@@ -74,7 +74,7 @@ bool AppContextMenu::IsCommandIdChecked(int command_id) const {
 }
 
 bool AppContextMenu::IsCommandIdEnabled(int command_id) const {
-  if (command_id == TOGGLE_PIN) {
+  if (command_id == ash::TOGGLE_PIN) {
     return controller_->GetPinnable(app_id_) ==
            AppListControllerDelegate::PIN_EDITABLE;
   }
@@ -91,7 +91,7 @@ void AppContextMenu::TogglePin(const std::string& shelf_app_id) {
 }
 
 void AppContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
-                                          CommandId command_id,
+                                          ash::CommandId command_id,
                                           int string_id) {
   // Do not include disabled items in touchable menus.
   if (features::IsTouchableAppContextMenuEnabled() &&
@@ -109,14 +109,14 @@ void AppContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
     return;
   }
   // Check items use default icons.
-  if (command_id == USE_LAUNCH_TYPE_PINNED ||
-      command_id == USE_LAUNCH_TYPE_REGULAR ||
-      command_id == USE_LAUNCH_TYPE_FULLSCREEN ||
-      command_id == USE_LAUNCH_TYPE_WINDOW) {
+  if (command_id == ash::USE_LAUNCH_TYPE_PINNED ||
+      command_id == ash::USE_LAUNCH_TYPE_REGULAR ||
+      command_id == ash::USE_LAUNCH_TYPE_FULLSCREEN ||
+      command_id == ash::USE_LAUNCH_TYPE_WINDOW) {
     menu_model->AddCheckItemWithStringId(command_id, string_id);
     return;
   }
-  if (command_id == NOTIFICATION_CONTAINER) {
+  if (command_id == ash::NOTIFICATION_CONTAINER) {
     NOTREACHED()
         << "NOTIFICATION_CONTAINER is added by NotificationMenuController.";
     return;
@@ -129,36 +129,36 @@ const gfx::VectorIcon& AppContextMenu::GetMenuItemVectorIcon(
     int string_id) const {
   static const gfx::VectorIcon blank = {};
   switch (command_id) {
-    case LAUNCH_NEW:
+    case ash::LAUNCH_NEW:
       if (string_id == IDS_APP_LIST_CONTEXT_MENU_NEW_WINDOW)
         return views::kNewWindowIcon;
       if (string_id == IDS_APP_LIST_CONTEXT_MENU_NEW_TAB)
         return views::kNewTabIcon;
       // The LAUNCH_NEW command is for an ARC app.
       return views::kLaunchIcon;
-    case TOGGLE_PIN:
+    case ash::TOGGLE_PIN:
       return string_id == IDS_APP_LIST_CONTEXT_MENU_PIN ? views::kPinIcon
                                                         : views::kUnpinIcon;
-    case SHOW_APP_INFO:
+    case ash::SHOW_APP_INFO:
       return views::kInfoIcon;
-    case OPTIONS:
+    case ash::OPTIONS:
       return views::kOptionsIcon;
-    case UNINSTALL:
+    case ash::UNINSTALL:
       return views::kUninstallIcon;
-    case MENU_NEW_WINDOW:
+    case ash::APP_CONTEXT_MENU_NEW_WINDOW:
       return views::kNewWindowIcon;
-    case MENU_NEW_INCOGNITO_WINDOW:
+    case ash::APP_CONTEXT_MENU_NEW_INCOGNITO_WINDOW:
       return views::kNewIncognitoWindowIcon;
-    case INSTALL:
+    case ash::INSTALL:
       // Deprecated.
       return blank;
-    case USE_LAUNCH_TYPE_PINNED:
-    case USE_LAUNCH_TYPE_REGULAR:
-    case USE_LAUNCH_TYPE_FULLSCREEN:
-    case USE_LAUNCH_TYPE_WINDOW:
+    case ash::USE_LAUNCH_TYPE_PINNED:
+    case ash::USE_LAUNCH_TYPE_REGULAR:
+    case ash::USE_LAUNCH_TYPE_FULLSCREEN:
+    case ash::USE_LAUNCH_TYPE_WINDOW:
       // Check items use the default icon.
       return blank;
-    case NOTIFICATION_CONTAINER:
+    case ash::NOTIFICATION_CONTAINER:
       NOTREACHED() << "NOTIFICATION_CONTAINER does not have an icon, and it is "
                       "added to the model by NotificationMenuController.";
       return blank;
@@ -170,7 +170,7 @@ const gfx::VectorIcon& AppContextMenu::GetMenuItemVectorIcon(
 
 void AppContextMenu::ExecuteCommand(int command_id, int event_flags) {
   switch (command_id) {
-    case TOGGLE_PIN:
+    case ash::TOGGLE_PIN:
       TogglePin(app_id_);
       break;
   }
@@ -181,7 +181,7 @@ bool AppContextMenu::GetIconForCommandId(int command_id,
   if (!features::IsTouchableAppContextMenuEnabled())
     return false;
 
-  if (command_id == TOGGLE_PIN) {
+  if (command_id == ash::TOGGLE_PIN) {
     const views::MenuConfig& menu_config = views::MenuConfig::instance();
     *icon = gfx::Image(gfx::CreateVectorIcon(
         GetMenuItemVectorIcon(command_id, controller_->IsAppPinned(app_id_)

chrome/browser/ui/app_list/app_context_menu.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "base/callback.h"
 #include "base/macros.h"
 #include "ui/base/models/simple_menu_model.h"
@@ -22,38 +23,6 @@ class AppContextMenuDelegate;
 // Base class of all context menus in app list view.
 class AppContextMenu : public ui::SimpleMenuModel::Delegate {
  public:
-  // Defines command ids, used in context menu of all types.
-  // These are used in histograms, do not remove/renumber entries. Only add at
-  // the end just before USE_LAUNCH_TYPE_COMMAND_END or after INSTALL and before
-  // USE_LAUNCH_TYPE_COMMAND_START or after LAUNCH_APP_SHORTCUT_LAST. If you're
-  // adding to this enum with the intention that it will be logged, add checks
-  // to ensure stability of the enum and update the ChromeOSUICommands enum
-  // listing in tools/metrics/histograms/enums.xml.
-  enum CommandId {
-    // This must match ash::CommandId::NOTIFICATION_CONTAINER.
-    NOTIFICATION_CONTAINER = 9,
-    LAUNCH_NEW = 100,
-    TOGGLE_PIN = 101,
-    SHOW_APP_INFO = 102,
-    OPTIONS = 103,
-    UNINSTALL = 104,
-    REMOVE_FROM_FOLDER = 105,
-    MENU_NEW_WINDOW = 106,
-    MENU_NEW_INCOGNITO_WINDOW = 107,
-    INSTALL = 108,
-    // Order matters in USE_LAUNCH_TYPE_* and must match the LaunchType enum.
-    USE_LAUNCH_TYPE_COMMAND_START = 200,
-    USE_LAUNCH_TYPE_PINNED = USE_LAUNCH_TYPE_COMMAND_START,
-    USE_LAUNCH_TYPE_REGULAR = 201,
-    USE_LAUNCH_TYPE_FULLSCREEN = 202,
-    USE_LAUNCH_TYPE_WINDOW = 203,
-    USE_LAUNCH_TYPE_COMMAND_END,
-    // Range of command ids reserved for launching app shortcuts from context
-    // menu for Android app.
-    LAUNCH_APP_SHORTCUT_FIRST = 1000,
-    LAUNCH_APP_SHORTCUT_LAST = 1999,
-  };
-
   AppContextMenu(AppContextMenuDelegate* delegate,
                  Profile* profile,
                  const std::string& app_id,
@@ -82,7 +51,7 @@ class AppContextMenu : public ui::SimpleMenuModel::Delegate {
 
   // Helper method to add touchable or normal context menu options.
   void AddContextMenuOption(ui::SimpleMenuModel* menu_model,
-                            CommandId command_id,
+                            ash::CommandId command_id,
                             int string_id);
 
   // Helper method to get the gfx::VectorIcon for a |command_id|. Returns an

chrome/browser/ui/app_list/arc/arc_app_context_menu.cc

@@ -47,7 +47,7 @@ void ArcAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
   }
 
   if (!controller()->IsAppOpen(app_id())) {
-    AddContextMenuOption(menu_model, LAUNCH_NEW,
+    AddContextMenuOption(menu_model, ash::LAUNCH_NEW,
                          IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
     if (!features::IsTouchableAppContextMenuEnabled())
       menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
@@ -58,12 +58,14 @@ void ArcAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
   if (!features::IsTouchableAppContextMenuEnabled())
     menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
   if (arc_prefs->IsShortcut(app_id()))
-    AddContextMenuOption(menu_model, UNINSTALL, IDS_APP_LIST_REMOVE_SHORTCUT);
+    AddContextMenuOption(menu_model, ash::UNINSTALL,
+                         IDS_APP_LIST_REMOVE_SHORTCUT);
   else if (!app_info->sticky)
-    AddContextMenuOption(menu_model, UNINSTALL, IDS_APP_LIST_UNINSTALL_ITEM);
+    AddContextMenuOption(menu_model, ash::UNINSTALL,
+                         IDS_APP_LIST_UNINSTALL_ITEM);
 
   // App Info item.
-  AddContextMenuOption(menu_model, SHOW_APP_INFO,
+  AddContextMenuOption(menu_model, ash::SHOW_APP_INFO,
                        IDS_APP_CONTEXT_MENU_SHOW_INFO);
 }
 
@@ -74,11 +76,11 @@ bool ArcAppContextMenu::IsCommandIdEnabled(int command_id) const {
       arc_prefs->GetApp(app_id());
 
   switch (command_id) {
-    case UNINSTALL:
+    case ash::UNINSTALL:
       return app_info &&
           !app_info->sticky &&
           (app_info->ready || app_info->shortcut);
-    case SHOW_APP_INFO:
+    case ash::SHOW_APP_INFO:
       return app_info && app_info->ready;
     default:
       return app_list::AppContextMenu::IsCommandIdEnabled(command_id);
@@ -88,14 +90,14 @@ bool ArcAppContextMenu::IsCommandIdEnabled(int command_id) const {
 }
 
 void ArcAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
-  if (command_id == LAUNCH_NEW) {
+  if (command_id == ash::LAUNCH_NEW) {
     delegate()->ExecuteLaunchCommand(event_flags);
-  } else if (command_id == UNINSTALL) {
+  } else if (command_id == ash::UNINSTALL) {
     arc::ShowArcAppUninstallDialog(profile(), app_id());
-  } else if (command_id == SHOW_APP_INFO) {
+  } else if (command_id == ash::SHOW_APP_INFO) {
     ShowPackageInfo();
-  } else if (command_id >= LAUNCH_APP_SHORTCUT_FIRST &&
-             command_id <= LAUNCH_APP_SHORTCUT_LAST) {
+  } else if (command_id >= ash::LAUNCH_APP_SHORTCUT_FIRST &&
+             command_id <= ash::LAUNCH_APP_SHORTCUT_LAST) {
     DCHECK(app_shortcuts_menu_builder_);
     app_shortcuts_menu_builder_->ExecuteCommand(command_id);
   } else {
@@ -120,7 +122,7 @@ void ArcAppContextMenu::BuildAppShortcutsMenu(
   app_shortcuts_menu_builder_ =
       std::make_unique<arc::ArcAppShortcutsMenuBuilder>(
           profile(), app_id(), controller()->GetAppListDisplayId(),
-          LAUNCH_APP_SHORTCUT_FIRST, LAUNCH_APP_SHORTCUT_LAST);
+          ash::LAUNCH_APP_SHORTCUT_FIRST, ash::LAUNCH_APP_SHORTCUT_LAST);
   app_shortcuts_menu_builder_->BuildMenu(
       app_info->package_name, std::move(menu_model), std::move(callback));
 }

chrome/browser/ui/app_list/arc/arc_playstore_app_context_menu.cc

@@ -19,12 +19,13 @@ ArcPlayStoreAppContextMenu::~ArcPlayStoreAppContextMenu() = default;
 
 void ArcPlayStoreAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
   // App Info item.
-  menu_model->AddItemWithStringId(INSTALL, IDS_APP_CONTEXT_MENU_INSTALL_ARC);
+  menu_model->AddItemWithStringId(ash::INSTALL,
+                                  IDS_APP_CONTEXT_MENU_INSTALL_ARC);
 }
 
 bool ArcPlayStoreAppContextMenu::IsCommandIdEnabled(int command_id) const {
   switch (command_id) {
-    case INSTALL:
+    case ash::INSTALL:
       return true;
     default:
       return app_list::AppContextMenu::IsCommandIdEnabled(command_id);
@@ -34,7 +35,7 @@ bool ArcPlayStoreAppContextMenu::IsCommandIdEnabled(int command_id) const {
 void ArcPlayStoreAppContextMenu::ExecuteCommand(int command_id,
                                                 int event_flags) {
   switch (command_id) {
-    case INSTALL:
+    case ash::INSTALL:
       delegate()->ExecuteLaunchCommand(event_flags);
       break;
     default:

chrome/browser/ui/app_list/crostini/crostini_app_context_menu.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/app_list/crostini/crostini_app_context_menu.h"
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
 #include "chrome/grit/generated_resources.h"
@@ -26,12 +27,13 @@ void CrostiniAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
     if (!features::IsTouchableAppContextMenuEnabled())
       menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
 
-    AddContextMenuOption(menu_model, UNINSTALL, IDS_APP_LIST_UNINSTALL_ITEM);
+    AddContextMenuOption(menu_model, ash::UNINSTALL,
+                         IDS_APP_LIST_UNINSTALL_ITEM);
   }
 }
 
 bool CrostiniAppContextMenu::IsCommandIdEnabled(int command_id) const {
-  if (command_id == UNINSTALL) {
+  if (command_id == ash::UNINSTALL) {
     if (app_id() == kCrostiniTerminalId) {
       return IsCrostiniEnabled(profile());
     }
@@ -41,7 +43,7 @@ bool CrostiniAppContextMenu::IsCommandIdEnabled(int command_id) const {
 
 void CrostiniAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
   switch (command_id) {
-    case UNINSTALL:
+    case ash::UNINSTALL:
       if (app_id() == kCrostiniTerminalId) {
         ShowCrostiniUninstallerView(profile(), CrostiniUISurface::kAppList);
         return;

chrome/browser/ui/ash/launcher/arc_launcher_context_menu.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcuts_menu_builder.h"
 #include "chrome/browser/profiles/profile.h"
@@ -30,8 +31,8 @@ void ArcLauncherContextMenu::GetMenuModel(GetMenuModelCallback callback) {
 }
 
 void ArcLauncherContextMenu::ExecuteCommand(int command_id, int event_flags) {
-  if (command_id >= LAUNCH_APP_SHORTCUT_FIRST &&
-      command_id <= LAUNCH_APP_SHORTCUT_LAST) {
+  if (command_id >= ash::LAUNCH_APP_SHORTCUT_FIRST &&
+      command_id <= ash::LAUNCH_APP_SHORTCUT_LAST) {
     DCHECK(app_shortcuts_menu_builder_);
     app_shortcuts_menu_builder_->ExecuteCommand(command_id);
     return;
@@ -60,7 +61,7 @@ void ArcLauncherContextMenu::BuildMenu(
   const bool app_is_open = controller()->IsOpen(item().id);
   if (!app_is_open) {
     DCHECK(app_info->launchable);
-    AddContextMenuOption(menu_model.get(), MENU_OPEN_NEW,
+    AddContextMenuOption(menu_model.get(), ash::MENU_OPEN_NEW,
                          IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
     if (!features::IsTouchableAppContextMenuEnabled())
       menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
@@ -70,7 +71,7 @@ void ArcLauncherContextMenu::BuildMenu(
     AddPinMenu(menu_model.get());
 
   if (app_is_open) {
-    AddContextMenuOption(menu_model.get(), MENU_CLOSE,
+    AddContextMenuOption(menu_model.get(), ash::MENU_CLOSE,
                          IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
   }
   if (!features::IsTouchableAppContextMenuEnabled())
@@ -86,7 +87,7 @@ void ArcLauncherContextMenu::BuildMenu(
   app_shortcuts_menu_builder_ =
       std::make_unique<arc::ArcAppShortcutsMenuBuilder>(
           controller()->profile(), item().id.app_id, display_id(),
-          LAUNCH_APP_SHORTCUT_FIRST, LAUNCH_APP_SHORTCUT_LAST);
+          ash::LAUNCH_APP_SHORTCUT_FIRST, ash::LAUNCH_APP_SHORTCUT_LAST);
   app_shortcuts_menu_builder_->BuildMenu(
       app_info->package_name, std::move(menu_model), std::move(callback));
 }

chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc

@@ -8,6 +8,7 @@
 
 #include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/app_list/app_list_switches.h"
+#include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item_delegate.h"
 #include "ash/public/cpp/shelf_model.h"
 #include "ash/public/cpp/window_properties.h"
@@ -1929,17 +1930,15 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest,
   // Open a context menu for the existing browser window.
   std::unique_ptr<LauncherContextMenu> menu1 = CreateBrowserItemContextMenu();
   // Check if "Close" is added to in the context menu.
-  ASSERT_TRUE(
-      IsItemPresentInMenu(menu1.get(), LauncherContextMenu::MENU_CLOSE));
+  ASSERT_TRUE(IsItemPresentInMenu(menu1.get(), ash::MENU_CLOSE));
 
   // Close all windows via the menu item.
-  CloseBrowserWindow(browser(), menu1.get(), LauncherContextMenu::MENU_CLOSE);
+  CloseBrowserWindow(browser(), menu1.get(), ash::MENU_CLOSE);
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
 
   // Check if "Close" is removed from the context menu.
   std::unique_ptr<LauncherContextMenu> menu2 = CreateBrowserItemContextMenu();
-  ASSERT_FALSE(
-      IsItemPresentInMenu(menu2.get(), LauncherContextMenu::MENU_CLOSE));
+  ASSERT_FALSE(IsItemPresentInMenu(menu2.get(), ash::MENU_CLOSE));
 }
 
 // Chrome's ShelfModel should have back button, AppList and browser items and

chrome/browser/ui/ash/launcher/crostini_shelf_context_menu.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "chrome/browser/chromeos/crostini/crostini_registry_service.h"
 #include "chrome/browser/chromeos/crostini/crostini_registry_service_factory.h"
@@ -38,13 +39,14 @@ void CrostiniShelfContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
   if (registration)
     AddPinMenu(menu_model);
 
-  menu_model->AddItemWithStringId(MENU_NEW_WINDOW, IDS_APP_LIST_NEW_WINDOW);
+  menu_model->AddItemWithStringId(ash::MENU_NEW_WINDOW,
+                                  IDS_APP_LIST_NEW_WINDOW);
 
   if (controller()->IsOpen(item().id)) {
-    menu_model->AddItemWithStringId(MENU_CLOSE,
+    menu_model->AddItemWithStringId(ash::MENU_CLOSE,
                                     IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
   } else {
-    menu_model->AddItemWithStringId(MENU_OPEN_NEW,
+    menu_model->AddItemWithStringId(ash::MENU_OPEN_NEW,
                                     IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
   }
 
@@ -56,7 +58,7 @@ void CrostiniShelfContextMenu::ExecuteCommand(int command_id, int event_flags) {
   if (ExecuteCommonCommand(command_id, event_flags))
     return;
 
-  if (command_id == MENU_NEW_WINDOW) {
+  if (command_id == ash::MENU_NEW_WINDOW) {
     LaunchCrostiniApp(controller()->profile(), item().id.app_id, display_id());
     return;
   }

chrome/browser/ui/ash/launcher/extension_launcher_context_menu.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "base/bind.h"
 #include "chrome/browser/extensions/context_menu_matcher.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ -71,16 +72,16 @@ void ExtensionLauncherContextMenu::GetMenuModel(GetMenuModelCallback callback) {
 
 bool ExtensionLauncherContextMenu::IsCommandIdChecked(int command_id) const {
   switch (command_id) {
-    case LAUNCH_TYPE_PINNED_TAB:
+    case ash::LAUNCH_TYPE_PINNED_TAB:
       return GetLaunchType() == extensions::LAUNCH_TYPE_PINNED;
-    case LAUNCH_TYPE_REGULAR_TAB:
+    case ash::LAUNCH_TYPE_REGULAR_TAB:
       return GetLaunchType() == extensions::LAUNCH_TYPE_REGULAR;
-    case LAUNCH_TYPE_WINDOW:
+    case ash::LAUNCH_TYPE_WINDOW:
       return GetLaunchType() == extensions::LAUNCH_TYPE_WINDOW;
-    case LAUNCH_TYPE_FULLSCREEN:
+    case ash::LAUNCH_TYPE_FULLSCREEN:
       return GetLaunchType() == extensions::LAUNCH_TYPE_FULLSCREEN;
     default:
-      if (command_id < MENU_ITEM_COUNT)
+      if (command_id < ash::COMMAND_ID_COUNT)
         return LauncherContextMenu::IsCommandIdChecked(command_id);
       return (extension_items_ &&
               extension_items_->IsCommandIdChecked(command_id));
@@ -89,18 +90,18 @@ bool ExtensionLauncherContextMenu::IsCommandIdChecked(int command_id) const {
 
 bool ExtensionLauncherContextMenu::IsCommandIdEnabled(int command_id) const {
   switch (command_id) {
-    case MENU_NEW_WINDOW:
+    case ash::MENU_NEW_WINDOW:
       // "Normal" windows are not allowed when incognito is enforced.
       return IncognitoModePrefs::GetAvailability(
                  controller()->profile()->GetPrefs()) !=
              IncognitoModePrefs::FORCED;
-    case MENU_NEW_INCOGNITO_WINDOW:
+    case ash::MENU_NEW_INCOGNITO_WINDOW:
       // Incognito windows are not allowed when incognito is disabled.
       return IncognitoModePrefs::GetAvailability(
                  controller()->profile()->GetPrefs()) !=
              IncognitoModePrefs::DISABLED;
     default:
-      if (command_id < MENU_ITEM_COUNT)
+      if (command_id < ash::COMMAND_ID_COUNT)
         return LauncherContextMenu::IsCommandIdEnabled(command_id);
       return (extension_items_ &&
               extension_items_->IsCommandIdEnabled(command_id));
@@ -115,14 +116,14 @@ void ExtensionLauncherContextMenu::ExecuteCommand(int command_id,
   // Place new windows on the same display as the context menu.
   ScopedDisplayIdForNewWindows scoped_display(display_id());
 
-  switch (static_cast<MenuItem>(command_id)) {
-    case LAUNCH_TYPE_PINNED_TAB:
+  switch (static_cast<ash::CommandId>(command_id)) {
+    case ash::LAUNCH_TYPE_PINNED_TAB:
       SetLaunchType(extensions::LAUNCH_TYPE_PINNED);
       break;
-    case LAUNCH_TYPE_REGULAR_TAB:
+    case ash::LAUNCH_TYPE_REGULAR_TAB:
       SetLaunchType(extensions::LAUNCH_TYPE_REGULAR);
       break;
-    case LAUNCH_TYPE_WINDOW: {
+    case ash::LAUNCH_TYPE_WINDOW: {
       extensions::LaunchType launch_type = extensions::LAUNCH_TYPE_WINDOW;
       // With bookmark apps enabled, hosted apps can only toggle between
       // LAUNCH_WINDOW and LAUNCH_REGULAR.
@@ -134,13 +135,13 @@ void ExtensionLauncherContextMenu::ExecuteCommand(int command_id,
       SetLaunchType(launch_type);
       break;
     }
-    case LAUNCH_TYPE_FULLSCREEN:
+    case ash::LAUNCH_TYPE_FULLSCREEN:
       SetLaunchType(extensions::LAUNCH_TYPE_FULLSCREEN);
       break;
-    case MENU_NEW_WINDOW:
+    case ash::MENU_NEW_WINDOW:
       chrome::NewEmptyWindow(controller()->profile());
       break;
-    case MENU_NEW_INCOGNITO_WINDOW:
+    case ash::MENU_NEW_INCOGNITO_WINDOW:
       chrome::NewEmptyWindow(controller()->profile()->GetOffTheRecordProfile());
       break;
     default:
@@ -156,16 +157,18 @@ void ExtensionLauncherContextMenu::CreateOpenNewSubmenu(
   // Touchable extension context menus use an actionable submenu for
   // MENU_OPEN_NEW.
   const gfx::VectorIcon& icon =
-      GetMenuItemVectorIcon(MENU_OPEN_NEW, GetLaunchTypeStringId());
+      GetCommandIdVectorIcon(ash::MENU_OPEN_NEW, GetLaunchTypeStringId());
   const views::MenuConfig& menu_config = views::MenuConfig::instance();
   const int kGroupId = 1;
   open_new_submenu_model_ = std::make_unique<ui::SimpleMenuModel>(this);
   open_new_submenu_model_->AddRadioItemWithStringId(
-      LAUNCH_TYPE_REGULAR_TAB, IDS_APP_LIST_CONTEXT_MENU_NEW_TAB, kGroupId);
+      ash::LAUNCH_TYPE_REGULAR_TAB, IDS_APP_LIST_CONTEXT_MENU_NEW_TAB,
+      kGroupId);
   open_new_submenu_model_->AddRadioItemWithStringId(
-      LAUNCH_TYPE_WINDOW, IDS_APP_LIST_CONTEXT_MENU_NEW_WINDOW, kGroupId);
+      ash::LAUNCH_TYPE_WINDOW, IDS_APP_LIST_CONTEXT_MENU_NEW_WINDOW, kGroupId);
   menu_model->AddActionableSubmenuWithStringIdAndIcon(
-      MENU_OPEN_NEW, GetLaunchTypeStringId(), open_new_submenu_model_.get(),
+      ash::MENU_OPEN_NEW, GetLaunchTypeStringId(),
+      open_new_submenu_model_.get(),
       gfx::CreateVectorIcon(icon, menu_config.touchable_icon_size,
                             menu_config.touchable_icon_color));
 }
@@ -180,7 +183,7 @@ void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
       if (features::IsTouchableAppContextMenuEnabled()) {
         CreateOpenNewSubmenu(menu_model);
       } else {
-        AddContextMenuOption(menu_model, MENU_OPEN_NEW,
+        AddContextMenuOption(menu_model, ash::MENU_OPEN_NEW,
                              GetLaunchTypeStringId());
         menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
 
@@ -191,18 +194,18 @@ void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
             // 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, LAUNCH_TYPE_WINDOW,
+            AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_WINDOW,
                                  IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
           } else {
-            AddContextMenuOption(menu_model, LAUNCH_TYPE_REGULAR_TAB,
+            AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_REGULAR_TAB,
                                  IDS_APP_CONTEXT_MENU_OPEN_REGULAR);
-            AddContextMenuOption(menu_model, LAUNCH_TYPE_PINNED_TAB,
+            AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_PINNED_TAB,
                                  IDS_APP_CONTEXT_MENU_OPEN_PINNED);
-            AddContextMenuOption(menu_model, LAUNCH_TYPE_WINDOW,
+            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, LAUNCH_TYPE_FULLSCREEN,
+            AddContextMenuOption(menu_model, ash::LAUNCH_TYPE_FULLSCREEN,
                                  IDS_APP_CONTEXT_MENU_OPEN_MAXIMIZED);
           }
           menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
@@ -213,25 +216,26 @@ void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
     AddPinMenu(menu_model);
 
     if (controller()->IsOpen(item().id)) {
-      AddContextMenuOption(menu_model, MENU_CLOSE,
+      AddContextMenuOption(menu_model, ash::MENU_CLOSE,
                            IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
     }
   } else if (item().type == ash::TYPE_BROWSER_SHORTCUT) {
-    AddContextMenuOption(menu_model, MENU_NEW_WINDOW, IDS_APP_LIST_NEW_WINDOW);
+    AddContextMenuOption(menu_model, ash::MENU_NEW_WINDOW,
+                         IDS_APP_LIST_NEW_WINDOW);
     if (!controller()->profile()->IsGuestSession()) {
-      AddContextMenuOption(menu_model, MENU_NEW_INCOGNITO_WINDOW,
+      AddContextMenuOption(menu_model, ash::MENU_NEW_INCOGNITO_WINDOW,
                            IDS_APP_LIST_NEW_INCOGNITO_WINDOW);
     }
     if (!BrowserShortcutLauncherItemController(controller()->shelf_model())
              .IsListOfActiveBrowserEmpty()) {
-      AddContextMenuOption(menu_model, MENU_CLOSE,
+      AddContextMenuOption(menu_model, ash::MENU_CLOSE,
                            IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
     }
   } else if (item().type == ash::TYPE_DIALOG) {
-    AddContextMenuOption(menu_model, MENU_CLOSE,
+    AddContextMenuOption(menu_model, ash::MENU_CLOSE,
                          IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
   } else if (controller()->IsOpen(item().id)) {
-    AddContextMenuOption(menu_model, MENU_CLOSE,
+    AddContextMenuOption(menu_model, ash::MENU_CLOSE,
                          IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
   }
   if (!features::IsTouchableAppContextMenuEnabled())

chrome/browser/ui/ash/launcher/internal_app_shelf_context_menu.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
@@ -28,7 +29,7 @@ void InternalAppShelfContextMenu::GetMenuModel(GetMenuModelCallback callback) {
 void InternalAppShelfContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
   const bool app_is_open = controller()->IsOpen(item().id);
   if (!app_is_open) {
-    AddContextMenuOption(menu_model, MENU_OPEN_NEW,
+    AddContextMenuOption(menu_model, ash::MENU_OPEN_NEW,
                          IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
     if (!features::IsTouchableAppContextMenuEnabled())
       menu_model->AddSeparator(ui::NORMAL_SEPARATOR);
@@ -40,7 +41,7 @@ void InternalAppShelfContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
     AddPinMenu(menu_model);
 
   if (app_is_open) {
-    AddContextMenuOption(menu_model, MENU_CLOSE,
+    AddContextMenuOption(menu_model, ash::MENU_CLOSE,
                          IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
   }
 

chrome/browser/ui/ash/launcher/launcher_context_menu.cc

@@ -74,30 +74,30 @@ LauncherContextMenu::LauncherContextMenu(ChromeLauncherController* controller,
 LauncherContextMenu::~LauncherContextMenu() = default;
 
 bool LauncherContextMenu::IsCommandIdChecked(int command_id) const {
-  DCHECK(command_id < MENU_ITEM_COUNT);
+  DCHECK(command_id < ash::COMMAND_ID_COUNT);
   return false;
 }
 
 bool LauncherContextMenu::IsCommandIdEnabled(int command_id) const {
-  if (command_id == MENU_PIN) {
+  if (command_id == ash::MENU_PIN) {
     // Users cannot modify the pinned state of apps pinned by policy.
     return !item_.pinned_by_policy &&
            (item_.type == ash::TYPE_PINNED_APP || item_.type == ash::TYPE_APP);
   }
 
-  DCHECK(command_id < MENU_ITEM_COUNT);
+  DCHECK(command_id < ash::COMMAND_ID_COUNT);
   return true;
 }
 
 void LauncherContextMenu::ExecuteCommand(int command_id, int event_flags) {
-  switch (static_cast<MenuItem>(command_id)) {
-    case MENU_OPEN_NEW:
+  switch (static_cast<ash::CommandId>(command_id)) {
+    case ash::MENU_OPEN_NEW:
       // Use a copy of the id to avoid crashes, as this menu's owner will be
       // destroyed if LaunchApp replaces the ShelfItemDelegate instance.
       controller_->LaunchApp(ash::ShelfID(item_.id), ash::LAUNCH_FROM_UNKNOWN,
                              ui::EF_NONE, display_id_);
       break;
-    case MENU_CLOSE:
+    case ash::MENU_CLOSE:
       if (item_.type == ash::TYPE_DIALOG) {
         ash::ShelfItemDelegate* item_delegate =
             controller_->shelf_model()->GetShelfItemDelegate(item_.id);
@@ -113,7 +113,7 @@ void LauncherContextMenu::ExecuteCommand(int command_id, int event_flags) {
             base::UserMetricsAction("Tablet_WindowCloseFromContextMenu"));
       }
       break;
-    case MENU_PIN:
+    case ash::MENU_PIN:
       if (controller_->IsAppPinned(item_.id.app_id))
         controller_->UnpinAppWithID(item_.id.app_id);
       else
@@ -143,15 +143,15 @@ void LauncherContextMenu::AddPinMenu(ui::SimpleMenuModel* menu_model) {
       NOTREACHED();
       return;
   }
-  AddContextMenuOption(menu_model, MENU_PIN, menu_pin_string_id);
+  AddContextMenuOption(menu_model, ash::MENU_PIN, menu_pin_string_id);
 }
 
 bool LauncherContextMenu::ExecuteCommonCommand(int command_id,
                                                int event_flags) {
   switch (command_id) {
-    case MENU_OPEN_NEW:
-    case MENU_CLOSE:
-    case MENU_PIN:
+    case ash::MENU_OPEN_NEW:
+    case ash::MENU_CLOSE:
+    case ash::MENU_PIN:
       LauncherContextMenu::ExecuteCommand(command_id, event_flags);
       return true;
     default:
@@ -160,9 +160,9 @@ bool LauncherContextMenu::ExecuteCommonCommand(int command_id,
 }
 
 void LauncherContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
-                                               MenuItem type,
+                                               ash::CommandId type,
                                                int string_id) {
-  const gfx::VectorIcon& icon = GetMenuItemVectorIcon(type, string_id);
+  const gfx::VectorIcon& icon = GetCommandIdVectorIcon(type, string_id);
   if (features::IsTouchableAppContextMenuEnabled() && !icon.is_empty()) {
     const views::MenuConfig& menu_config = views::MenuConfig::instance();
     menu_model->AddItemWithStringIdAndIcon(
@@ -172,13 +172,14 @@ void LauncherContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
     return;
   }
   // If the MenuType is a check item.
-  if (type == LAUNCH_TYPE_REGULAR_TAB || type == LAUNCH_TYPE_PINNED_TAB ||
-      type == LAUNCH_TYPE_WINDOW || type == LAUNCH_TYPE_FULLSCREEN) {
+  if (type == ash::LAUNCH_TYPE_REGULAR_TAB ||
+      type == ash::LAUNCH_TYPE_PINNED_TAB || type == ash::LAUNCH_TYPE_WINDOW ||
+      type == ash::LAUNCH_TYPE_FULLSCREEN) {
     menu_model->AddCheckItemWithStringId(type, string_id);
     return;
   }
   // NOTIFICATION_CONTAINER is added by NotificationMenuController.
-  if (type == NOTIFICATION_CONTAINER) {
+  if (type == ash::NOTIFICATION_CONTAINER) {
     NOTREACHED()
         << "NOTIFICATION_CONTAINER is added by NotificationMenuController.";
     return;
@@ -186,39 +187,42 @@ void LauncherContextMenu::AddContextMenuOption(ui::SimpleMenuModel* menu_model,
   menu_model->AddItemWithStringId(type, string_id);
 }
 
-const gfx::VectorIcon& LauncherContextMenu::GetMenuItemVectorIcon(
-    MenuItem type,
+const gfx::VectorIcon& LauncherContextMenu::GetCommandIdVectorIcon(
+    ash::CommandId type,
     int string_id) const {
   static const gfx::VectorIcon blank = {};
   switch (type) {
-    case MENU_OPEN_NEW:
+    case ash::MENU_OPEN_NEW:
       if (string_id == IDS_APP_LIST_CONTEXT_MENU_NEW_TAB)
         return views::kNewTabIcon;
       if (string_id == IDS_APP_LIST_CONTEXT_MENU_NEW_WINDOW)
         return views::kNewWindowIcon;
       return views::kOpenIcon;
-    case MENU_CLOSE:
+    case ash::MENU_CLOSE:
       return views::kCloseIcon;
-    case MENU_PIN:
+    case ash::MENU_PIN:
       return controller_->IsPinned(item_.id) ? views::kUnpinIcon
                                              : views::kPinIcon;
-    case MENU_NEW_WINDOW:
+    case ash::MENU_NEW_WINDOW:
       return views::kNewWindowIcon;
-    case MENU_NEW_INCOGNITO_WINDOW:
+    case ash::MENU_NEW_INCOGNITO_WINDOW:
       return views::kNewIncognitoWindowIcon;
-    case LAUNCH_TYPE_PINNED_TAB:
-    case LAUNCH_TYPE_REGULAR_TAB:
-    case LAUNCH_TYPE_FULLSCREEN:
-    case LAUNCH_TYPE_WINDOW:
+    case ash::LAUNCH_TYPE_PINNED_TAB:
+    case ash::LAUNCH_TYPE_REGULAR_TAB:
+    case ash::LAUNCH_TYPE_FULLSCREEN:
+    case ash::LAUNCH_TYPE_WINDOW:
       // Check items use a default icon in touchable and default context menus.
       return blank;
-    case NOTIFICATION_CONTAINER:
+    case ash::NOTIFICATION_CONTAINER:
       NOTREACHED() << "NOTIFICATION_CONTAINER does not have an icon, and it is "
                       "added to the model by NotificationMenuController.";
       return blank;
-    case LAUNCH_APP_SHORTCUT_FIRST:
-    case LAUNCH_APP_SHORTCUT_LAST:
-    case MENU_ITEM_COUNT:
+    case ash::LAUNCH_APP_SHORTCUT_FIRST:
+    case ash::LAUNCH_APP_SHORTCUT_LAST:
+    case ash::COMMAND_ID_COUNT:
+      NOTREACHED();
+      return blank;
+    default:
       NOTREACHED();
       return blank;
   }

chrome/browser/ui/ash/launcher/launcher_context_menu.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_ASH_LAUNCHER_LAUNCHER_CONTEXT_MENU_H_
 #define CHROME_BROWSER_UI_ASH_LAUNCHER_LAUNCHER_CONTEXT_MENU_H_
 
+#include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "base/macros.h"
 #include "ui/base/models/simple_menu_model.h"
@@ -15,33 +16,6 @@ class ChromeLauncherController;
 // A base class for browser, extension, and ARC shelf item context menus.
 class LauncherContextMenu : public ui::SimpleMenuModel::Delegate {
  public:
-  // Menu item command ids, used by subclasses and tests.
-  // These are used in histograms, do not remove/renumber entries. Only add at
-  // the end just before MENU_ITEM_COUNT. If you're adding to this enum with the
-  // intention that it will be logged, add checks to ensure stability of the
-  // enum and update the ChromeOSUICommands enum listing in
-  // tools/metrics/histograms/enums.xml. Besides LAUNCH_APP_SHORTCUT_FIRST and
-  // LAUNCH_APP_SHORTCUT_LAST, these enums must not overlap with enums in
-  // AppContextMenu::CommandId.
-  enum MenuItem {
-    MENU_OPEN_NEW = 0,
-    MENU_CLOSE = 1,
-    MENU_PIN = 2,
-    LAUNCH_TYPE_PINNED_TAB = 3,
-    LAUNCH_TYPE_REGULAR_TAB = 4,
-    LAUNCH_TYPE_FULLSCREEN = 5,
-    LAUNCH_TYPE_WINDOW = 6,
-    MENU_NEW_WINDOW = 7,
-    MENU_NEW_INCOGNITO_WINDOW = 8,
-    // This must match ash::CommandId::NOTIFICATION_CONTAINER.
-    NOTIFICATION_CONTAINER = 9,
-    // Range of command ids reserved for launching app shortcuts from context
-    // menu for Android app. Must overlap with AppContextMenu::CommandId.
-    LAUNCH_APP_SHORTCUT_FIRST = 1000,
-    LAUNCH_APP_SHORTCUT_LAST = 1999,
-    MENU_ITEM_COUNT
-  };
-
   ~LauncherContextMenu() override;
 
   // Static function to create a context menu instance.
@@ -75,12 +49,12 @@ class LauncherContextMenu : public ui::SimpleMenuModel::Delegate {
 
   // Helper method to add touchable or normal context menu options.
   void AddContextMenuOption(ui::SimpleMenuModel* menu_model,
-                            MenuItem type,
+                            ash::CommandId type,
                             int string_id);
 
   // Helper method to get the gfx::VectorIcon for a |type|. Returns an empty
   // gfx::VectorIcon if there is no icon for this |type|.
-  const gfx::VectorIcon& GetMenuItemVectorIcon(MenuItem type,
+  const gfx::VectorIcon& GetCommandIdVectorIcon(ash::CommandId type,
                                                 int string_id) const;
 
   int64_t display_id() const { return display_id_; }

chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc

@@ -164,10 +164,9 @@ TEST_F(LauncherContextMenuTest,
       CreateLauncherContextMenu(ash::TYPE_BROWSER_SHORTCUT, display_id);
   std::unique_ptr<ui::MenuModel> menu =
       GetMenuModel(launcher_context_menu.get());
-  ASSERT_TRUE(IsItemPresentInMenu(
-      menu.get(), LauncherContextMenu::MENU_NEW_INCOGNITO_WINDOW));
+  ASSERT_TRUE(IsItemPresentInMenu(menu.get(), ash::MENU_NEW_INCOGNITO_WINDOW));
   EXPECT_TRUE(launcher_context_menu->IsCommandIdEnabled(
-      LauncherContextMenu::MENU_NEW_INCOGNITO_WINDOW));
+      ash::MENU_NEW_INCOGNITO_WINDOW));
 
   // Disable Incognito mode.
   IncognitoModePrefs::SetAvailability(profile()->GetPrefs(),
@@ -176,10 +175,9 @@ TEST_F(LauncherContextMenuTest,
       CreateLauncherContextMenu(ash::TYPE_BROWSER_SHORTCUT, display_id);
   menu = GetMenuModel(launcher_context_menu.get());
   // The item should be disabled.
-  ASSERT_TRUE(IsItemPresentInMenu(
-      menu.get(), LauncherContextMenu::MENU_NEW_INCOGNITO_WINDOW));
+  ASSERT_TRUE(IsItemPresentInMenu(menu.get(), ash::MENU_NEW_INCOGNITO_WINDOW));
   EXPECT_FALSE(launcher_context_menu->IsCommandIdEnabled(
-      LauncherContextMenu::MENU_NEW_INCOGNITO_WINDOW));
+      ash::MENU_NEW_INCOGNITO_WINDOW));
 }
 
 // Verifies that "New window" menu item in the launcher context
@@ -192,10 +190,8 @@ TEST_F(LauncherContextMenuTest,
       CreateLauncherContextMenu(ash::TYPE_BROWSER_SHORTCUT, display_id);
   std::unique_ptr<ui::MenuModel> menu =
       GetMenuModel(launcher_context_menu.get());
-  ASSERT_TRUE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_NEW_WINDOW));
-  EXPECT_TRUE(launcher_context_menu->IsCommandIdEnabled(
-      LauncherContextMenu::MENU_NEW_WINDOW));
+  ASSERT_TRUE(IsItemPresentInMenu(menu.get(), ash::MENU_NEW_WINDOW));
+  EXPECT_TRUE(launcher_context_menu->IsCommandIdEnabled(ash::MENU_NEW_WINDOW));
 
   // Disable Incognito mode.
   IncognitoModePrefs::SetAvailability(profile()->GetPrefs(),
@@ -203,10 +199,8 @@ TEST_F(LauncherContextMenuTest,
   launcher_context_menu =
       CreateLauncherContextMenu(ash::TYPE_BROWSER_SHORTCUT, display_id);
   menu = GetMenuModel(launcher_context_menu.get());
-  ASSERT_TRUE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_NEW_WINDOW));
-  EXPECT_FALSE(launcher_context_menu->IsCommandIdEnabled(
-      LauncherContextMenu::MENU_NEW_WINDOW));
+  ASSERT_TRUE(IsItemPresentInMenu(menu.get(), ash::MENU_NEW_WINDOW));
+  EXPECT_FALSE(launcher_context_menu->IsCommandIdEnabled(ash::MENU_NEW_WINDOW));
 }
 
 // Verifies that "Close" is not shown in context menu if no browser window is
@@ -218,8 +212,7 @@ TEST_F(LauncherContextMenuTest,
       CreateLauncherContextMenu(ash::TYPE_BROWSER_SHORTCUT, display_id);
   std::unique_ptr<ui::MenuModel> menu =
       GetMenuModel(launcher_context_menu.get());
-  ASSERT_FALSE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  ASSERT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_CLOSE));
 }
 
 // Verifies context menu and app menu items for ARC app.
@@ -248,11 +241,9 @@ TEST_F(LauncherContextMenuTest, ArcLauncherMenusCheck) {
   ASSERT_TRUE(menu);
 
   // ARC app is pinned but not running.
-  EXPECT_TRUE(
-      IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-  EXPECT_FALSE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_OPEN_NEW));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_PIN));
+  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_CLOSE));
 
   // ARC app is running.
   std::string window_app_id1("org.chromium.arc.1");
@@ -270,10 +261,9 @@ TEST_F(LauncherContextMenuTest, ArcLauncherMenusCheck) {
   menu = GetContextMenu(item_delegate, display_id);
   ASSERT_TRUE(menu);
 
-  EXPECT_FALSE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_OPEN_NEW));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_PIN));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_CLOSE));
 
   // ARC non-launchable app is running.
   const std::string app_id2 = ArcAppTest::GetAppId(arc_test().fake_apps()[1]);
@@ -297,10 +287,9 @@ TEST_F(LauncherContextMenuTest, ArcLauncherMenusCheck) {
   menu = GetContextMenu(item_delegate2, display_id);
   ASSERT_TRUE(menu);
 
-  EXPECT_FALSE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_OPEN_NEW));
+  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_PIN));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_CLOSE));
 
   // Shelf group context menu.
   std::vector<arc::mojom::ShortcutInfo> shortcuts = arc_test().fake_shortcuts();
@@ -339,12 +328,9 @@ TEST_F(LauncherContextMenuTest, ArcLauncherMenusCheck) {
     menu = GetContextMenu(item_delegate3, display_id);
     ASSERT_TRUE(menu);
 
-    EXPECT_FALSE(
-        IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-    EXPECT_FALSE(
-        IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-    EXPECT_TRUE(
-        IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+    EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_OPEN_NEW));
+    EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_PIN));
+    EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_CLOSE));
 
     menu_list = item_delegate3->GetAppMenuItems(0 /* event_flags */);
     ASSERT_EQ(i + 1, menu_list.size());
@@ -389,44 +375,33 @@ TEST_F(LauncherContextMenuTest, ArcDeferredLauncherContextMenuItemCheck) {
       GetContextMenu(item_delegate, 0 /* display_id */);
   ASSERT_TRUE(menu);
 
-  EXPECT_FALSE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_OPEN_NEW));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_PIN));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_CLOSE));
 
   item_delegate = model()->GetShelfItemDelegate(shelf_id2);
   ASSERT_TRUE(item_delegate);
   menu = GetContextMenu(item_delegate, 0 /* display_id */);
   ASSERT_TRUE(menu);
 
-  EXPECT_FALSE(
-      IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_OPEN_NEW));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_PIN));
+  EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_CLOSE));
 }
 
 TEST_F(LauncherContextMenuTest, CommandIdsMatchEnumsForHistograms) {
   // Tests that CommandId enums are not changed as the values are used in
   // histograms.
-  EXPECT_EQ(0, LauncherContextMenu::MENU_OPEN_NEW);
-  EXPECT_EQ(1, LauncherContextMenu::MENU_CLOSE);
-  EXPECT_EQ(2, LauncherContextMenu::MENU_PIN);
-  EXPECT_EQ(3, LauncherContextMenu::LAUNCH_TYPE_PINNED_TAB);
-  EXPECT_EQ(4, LauncherContextMenu::LAUNCH_TYPE_REGULAR_TAB);
-  EXPECT_EQ(5, LauncherContextMenu::LAUNCH_TYPE_FULLSCREEN);
-  EXPECT_EQ(6, LauncherContextMenu::LAUNCH_TYPE_WINDOW);
-  EXPECT_EQ(7, LauncherContextMenu::MENU_NEW_WINDOW);
-  EXPECT_EQ(8, LauncherContextMenu::MENU_NEW_INCOGNITO_WINDOW);
-  EXPECT_EQ(9, LauncherContextMenu::NOTIFICATION_CONTAINER);
-}
-
-// Tests that LauncherContextMenu::NOTIFICATION_CONTAINER matches
-// ash::NOTIFICATION_CONTAINER.
-TEST_F(LauncherContextMenuTest, CommandIdEquivalency) {
-  // TODO(newcomer): Remove this test when NOTIFICATION_CONTAINER and other
-  // command ids are consolidated.
-  EXPECT_EQ(static_cast<int>(LauncherContextMenu::NOTIFICATION_CONTAINER),
-            static_cast<int>(ash::NOTIFICATION_CONTAINER));
+  EXPECT_EQ(0, ash::MENU_OPEN_NEW);
+  EXPECT_EQ(1, ash::MENU_CLOSE);
+  EXPECT_EQ(2, ash::MENU_PIN);
+  EXPECT_EQ(3, ash::LAUNCH_TYPE_PINNED_TAB);
+  EXPECT_EQ(4, ash::LAUNCH_TYPE_REGULAR_TAB);
+  EXPECT_EQ(5, ash::LAUNCH_TYPE_FULLSCREEN);
+  EXPECT_EQ(6, ash::LAUNCH_TYPE_WINDOW);
+  EXPECT_EQ(7, ash::MENU_NEW_WINDOW);
+  EXPECT_EQ(8, ash::MENU_NEW_INCOGNITO_WINDOW);
+  EXPECT_EQ(9, ash::NOTIFICATION_CONTAINER);
 }
 
 TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) {
@@ -479,11 +454,9 @@ TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenu) {
     ASSERT_TRUE(menu);
 
     // Internal app is pinned but not running.
-    EXPECT_TRUE(
-        IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
-    EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
-    EXPECT_FALSE(
-        IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+    EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_OPEN_NEW));
+    EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), ash::MENU_PIN));
+    EXPECT_FALSE(IsItemPresentInMenu(menu.get(), ash::MENU_CLOSE));
   }
 }