Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h"
 
 #include <stddef.h>
 
 #include "ash/public/cpp/shelf_application_menu_item.h"
 #include "ash/wm/window_util.h"
@@ skipping 76 matching lines @@
       GetExtensionForAppID(app_id, controller->profile());
   // Some unit tests have no real extension.
   if (extension) {
     set_refocus_url(GURL(
         extensions::AppLaunchInfo::GetLaunchWebURL(extension).spec() + "*"));
   }
 }
 
 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
     // will take over when the shell window gets opened. However there are apps
     // which take a lot of time for pre-processing (like the files app) before
     // 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;
   std::vector<content::WebContents*> content_list = GetRunningApplications();
   for (size_t i = 0; i < content_list.size(); i++) {
     content::WebContents* web_contents = content_list[i];
     gfx::Image app_icon = launcher_controller()->GetAppListIcon(web_contents);
     base::string16 title = launcher_controller()->GetAppListTitle(web_contents);
     items.push_back(base::MakeUnique<ChromeLauncherAppMenuItemTab>(
         title, &app_icon, web_contents));
@@ skipping 129 matching lines @@
   // - The launcher controller knows that the tab got created for this app.
   const GURL tab_url = web_contents->GetURL();
   return ((!refocus_pattern.match_all_urls() &&
            refocus_pattern.MatchesURL(tab_url)) ||
           (extension->OverlapsWithOrigin(tab_url) &&
            extension->web_extent().MatchesURL(tab_url)) ||
           launcher_controller()->IsWebContentHandledByApplication(web_contents,
                                                                   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();
   int index = tab_strip->GetIndexOfWebContents(content);
   DCHECK_NE(TabStripModel::kNoTab, index);
 
   int old_index = tab_strip->active_index();
   if (index != old_index)
     tab_strip->ActivateTabAt(index, false);
   return launcher_controller()->ActivateWindowOrMinimizeIfActive(
@@ skipping 37 matching lines @@
 
 bool AppShortcutLauncherItemController::AllowNextLaunchAttempt() {
   if (last_launch_attempt_.is_null() ||
       last_launch_attempt_ + base::TimeDelta::FromMilliseconds(
           kClickSuppressionInMS) < base::Time::Now()) {
     last_launch_attempt_ = base::Time::Now();
     return true;
   }
   return false;
 }