Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

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

Index: chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
index 495d79bba6d55bdb4027ba6ce8b1afba0af35c67..6031b05cccc75cffe60df8bc15f01110d7b1595d 100644
--- a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
+++ b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
@@ -94,8 +94,16 @@ AppShortcutLauncherItemController::AppShortcutLauncherItemController(
 
 AppShortcutLauncherItemController::~AppShortcutLauncherItemController() {}
 
-ash::ShelfItemDelegate::PerformedAction
-AppShortcutLauncherItemController::Activate(ash::LaunchSource source) {
+ash::ShelfAction AppShortcutLauncherItemController::ItemSelected(
+    ui::EventType event_type,
+    int event_flags,
+    int64_t display_id,
+    ash::ShelfLaunchSource source) {
+  // In case of a keyboard event, we were called by a hotkey. In that case we
+  // activate the next item in line if an item of our list is already active.
+  if (event_type == ui::ET_KEY_RELEASED && AdvanceToNextApp())
+    return ash::kExistingWindowActivated;
+
   content::WebContents* content = GetLRUApplication();
   if (!content) {
     // Ideally we come here only once. After that ShellLauncherItemController
@@ -104,26 +112,17 @@ AppShortcutLauncherItemController::Activate(ash::LaunchSource source) {
     // they open a window. Since there is currently no other way to detect if an
     // app was started we suppress any further clicks within a special time out.
     if (IsV2App() && !AllowNextLaunchAttempt())
-      return kNoAction;
+      return ash::kNoAction;
 
     // Launching some items replaces this item controller instance, which
     // destroys the app and launch id strings; making copies avoid crashes.
     launcher_controller()->LaunchApp(ash::AppLauncherId(app_id(), launch_id()),
-                                     source, ui::EF_NONE);
-    return kNewWindowCreated;
+                                     source, event_flags);

[James Cook, 2017/02/15 00:44:40]

Is this change needed?

[msw, 2017/02/15 19:59:18]

Reverted; no, afaict, this would have no behavioral difference at the moment. A
separate subclass, BrowserShortcutLauncherItemController uses the CTRL key
modifier flag to create new browser windows (instead of activating an existing
one, etc.). I *may* need to make a similar change in a follow up CL to merge
ItemSelected and GetAppMenuItems, but perhaps that also won't matter for some
time. It also just seems silly to disregard the flags available here.

+    return ash::kNewWindowCreated;
   }
   return ActivateContent(content);
 }
 
-ash::ShelfItemDelegate::PerformedAction
-AppShortcutLauncherItemController::ItemSelected(const ui::Event& event) {
-  // In case of a keyboard event, we were called by a hotkey. In that case we
-  // activate the next item in line if an item of our list is already active.
-  if (event.type() == ui::ET_KEY_RELEASED && AdvanceToNextApp())
-    return kExistingWindowActivated;
-  return Activate(ash::LAUNCH_FROM_UNKNOWN);
-}
-
 ash::ShelfAppMenuItemList AppShortcutLauncherItemController::GetAppMenuItems(
     int event_flags) {
   ash::ShelfAppMenuItemList items;
@@ -273,8 +272,7 @@ bool AppShortcutLauncherItemController::WebContentMatchesApp(
                                                                   app_id()));
 }
 
-ash::ShelfItemDelegate::PerformedAction
-AppShortcutLauncherItemController::ActivateContent(
+ash::ShelfAction AppShortcutLauncherItemController::ActivateContent(
     content::WebContents* content) {
   Browser* browser = chrome::FindBrowserWithWebContents(content);
   TabStripModel* tab_strip = browser->tab_strip_model();