Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

ash/common/shelf/shelf_item_delegate.h

Index: ash/common/shelf/shelf_item_delegate.h
diff --git a/ash/common/shelf/shelf_item_delegate.h b/ash/common/shelf/shelf_item_delegate.h
index 43605db24411b7426b6bafa0293ccc6141350283..0551e073c793d44a6302ad90866d04e21acbd4b6 100644
--- a/ash/common/shelf/shelf_item_delegate.h
+++ b/ash/common/shelf/shelf_item_delegate.h
@@ -7,40 +7,24 @@
 
 #include "ash/ash_export.h"
 #include "ash/public/cpp/shelf_application_menu_item.h"
-
-namespace ui {
-class Event;
-}
+#include "ash/public/cpp/shelf_types.h"
+#include "ui/events/event_constants.h"
 
 namespace ash {
 
 // Delegate for the ShelfItem.
 class ASH_EXPORT ShelfItemDelegate {
  public:
-  // The return type for the ShelfItemDelegate::ItemSelected method.
-  enum PerformedAction {
-    // No action was taken.
-    kNoAction,
-    // A new window was created.
-    kNewWindowCreated,
-    // An existing window which was not currently active was activated.
-    kExistingWindowActivated,
-    // The currently active window was minimized.
-    kExistingWindowMinimized,
-    // The app list launcher menu was shown.
-    kAppListMenuShown,
-  };
-
   virtual ~ShelfItemDelegate() {}
 
-  // Invoked when the user clicks on a window entry in the launcher.
-  // |event| is the click event. The |event| is dispatched by a view
-  // and has an instance of |views::View| as the event target
-  // but not |aura::Window|. If the |event| is of type KeyEvent, it is assumed
-  // that this was triggered by keyboard action (Alt+<number>) and special
-  // handling might happen.
+  // Called when the user selects a shelf item. The event type and flags, the
+  // relevant display id, and the source of the selection are sometimes known.

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

uh, "sometimes known"? Maybe tell the reader which fields are optional and which
aren't? And document what the default values should be?

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

Heh, I didn't want to enumerate the various codepaths that call this function
and the different levels of context that they have. No fields are strictly
necessary, but when the information is available, it should be provided, since
some shelf item delegate implementations will vary the behavior based on the
arguments. I've revised the comment with suggested defaults.

   // Returns the action performed by selecting the item.
-  virtual PerformedAction ItemSelected(const ui::Event& event) = 0;
+  // TODO(msw): Replace ui::EventType with ui::mojom::EventType.
+  virtual ShelfAction ItemSelected(ui::EventType event_type,
+                                   int event_flags,
+                                   int64_t display_id,

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

You could also provide two versions, one of which does not take display_id, and
require that it handle that case correctly.

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

I'd rather not overload this function; I'm hoping to merge it with
GetAppMenuItems and convert it to mojo asap.

+                                   ShelfLaunchSource source) = 0;
 
   // Returns any application menu items that should appear for this shelf item.
   // |event_flags| specifies the flags of the event which triggered this menu.