mash: Use mojo for ShelfItemDelegate and [app] MenuItem.

ash/public/interfaces/shelf.mojom

 // Copyright 2016 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.
 
 module ash.mojom;
 
+import "mojo/common/string16.mojom";
 import "skia/public/interfaces/bitmap.mojom";
+import "ui/events/mojo/event.mojom";
+
+// These values match ash::ShelfAction.
+enum ShelfAction {
+  NONE,

[James Cook, 2017/03/09 01:09:47]

nit: document these. Everything in mojoms needs to be well documented.

[msw, 2017/03/10 06:17:56]

Done.

+  WINDOW_CREATED,
+  WINDOW_ACTIVATED,
+  WINDOW_MINIMIZED,
+  APP_LIST_SHOWN,
+};
 
 // These values match ash::ShelfAlignment.
 enum ShelfAlignment { BOTTOM, LEFT, RIGHT, BOTTOM_LOCKED, };
 
 // These values match ash::ShelfAutoHideBehavior.
 enum ShelfAutoHideBehavior { ALWAYS, NEVER, HIDDEN, };
 
+// These values match ash::ShelfLaunchSource.
+enum ShelfLaunchSource { UNKNOWN, APP_LIST, APP_LIST_SEARCH, };

[James Cook, 2017/03/09 01:09:46]

ditto

arguably the mojo docs are more important than the c++ docs

[msw, 2017/03/10 06:17:56]

Done.

+
 // The Shelf controller allows clients (eg. Chrome) to control the ash shelf.
 interface ShelfController {
   // Observers are immediately notified of the current shelf states when added.
   AddObserver(associated ShelfObserver observer);
 
   // Set the shelf alignment and auto-hide behavior. See WmShelf for details.
   SetAlignment(ShelfAlignment alignment, int64 display_id);
   SetAutoHideBehavior(ShelfAutoHideBehavior auto_hide, int64 display_id);
 
   // Pin and unpin items on the shelf, or update shelf item images.
   PinItem(ShelfItem item, associated ShelfItemDelegate delegate);
   UnpinItem(string app_id);
   SetItemImage(string app_id, skia.mojom.Bitmap image);
 };
 
 // ShelfObserver is notified on shelf changes; used to persist profile settings.
 interface ShelfObserver {
   OnShelfCreated(int64 display_id);
   OnAlignmentChanged(ShelfAlignment alignment, int64 display_id);
   OnAutoHideBehaviorChanged(ShelfAutoHideBehavior auto_hide, int64 display_id);
 };
 
-// ShelfItemDelegate handles command execution and observes shelf item changes.
+// ShelfItemDelegate handles shelf item selection, menu command execution, etc.
 interface ShelfItemDelegate {
-  // Called when a pinned shelf item is invoked without an open window.
-  LaunchItem();
+  // Called when the user selects a shelf item. The event, display, and source
+  // info should be provided if known; some implementations use these arguments.
+  // Defaults: (nullptr, kInvalidDisplayId, LAUNCH_FROM_UNKNOWN)
+  // The callback reports the action taken and any app menu items to show.

[James Cook, 2017/03/09 01:09:47]

Is this used for context menus? Either way it would be nice to say something,
especially since MenuItem below implies it's used for context menus.

(I agree with using the same data structures for context and non-context menus,
I just think the docs need to be super clear that that's how it works.)

[msw, 2017/03/10 06:17:56]

Done. This and MenuItem are not used for context menus yet.

+  //
+  // TODO(msw): Use the Event location to determine the display id? 
+  ItemSelected(ui.mojom.Event event,
+               int64 display_id,
+               ShelfLaunchSource source) => (ShelfAction action,
+                                             array<MenuItem> menu_items);

[James Cook, 2017/03/09 01:09:47]

I like how you return both pieces of data atomically. Nice docs too.

[msw, 2017/03/10 06:17:56]

Acknowledged.

 
-  // Called on invocation of a shelf item's context menu command.
+  // Called on invocation of a shelf item's context or application menu command.
   ExecuteCommand(uint32 command_id, int32 event_flags);
 
-  // Called when a shelf item is pinned or unpinned.
-  ItemPinned();
-  ItemUnpinned();
-
-  // Called when a pinned shelf item is reordered.
-  // |order| is the index of the item on the shelf.
-  ItemReordered(uint32 order);
+  // Closes all windows associated with this shelf item.
+  Close();
 };
 
-// ContextMenuItems may be used to supplement ash shelf item context menus.
-struct ContextMenuItem {
+// MenuItems are used to populate context and application menus for shelf items.
+struct MenuItem {
   enum Type { ITEM, CHECK, RADIO, SEPARATOR, SUBMENU };
 
   Type type;
   uint32 command_id;
-  string? label;
-  array<ContextMenuItem>? submenu;
+  mojo.common.mojom.String16 label;
+  skia.mojom.Bitmap image;

[James Cook, 2017/03/09 01:09:47]

Is image optional? How is it used? Can it be a null image?

[msw, 2017/03/10 06:17:57]

It's not 'optional' in the sense of having a question mark here and being
base::Optional in code, but it definitely supports null SkBitmap objects (that
return true for isNull), added member comments, and mentioned this.

+  array<MenuItem>? submenu;
   bool enabled;
   bool checked;
   uint32 radio_group_id;
 };
 
 // ShelfItem contains the basic fields needed to pin shortcut items.
 struct ShelfItem {
   // An app id, used to correlate windows and shortcuts (eg. 'mojo:foo').
   string app_id;
 
   // A app title, used for tooltips, etc. (eg. 'Foo Application').
   string app_title;
 
   // An icon image Bitmap, shown on the shelf.
   skia.mojom.Bitmap image;
 
   // Additional context menu items (eg. 'New Incognito Window').
-  array<ContextMenuItem>? context_menu_items;
+  array<MenuItem>? context_menu_items;
 };