mash: Support ShelfModel access in Chrome.

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";
 
@@ skipping 43 matching lines @@
   UNKNOWN,          // The item was launched from outside the app list.
   APP_LIST,         // The item was launched from a generic app list view.
   APP_LIST_SEARCH,  // The item was launched from an app list search view.
 };
 
 // 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);
 
+  // Functions to modify ShelfModel items and their delegates. Observers are not
+  // notified of these changes, since Chrome is the solitary ShelfObserver and
+  // client of these functions; notifications would be cyclical and problematic.
+  AddShelfItem(int32 index, ShelfItem item);  // Use a negative index to append.
+  RemoveShelfItem(ShelfID id);
+  MoveShelfItem(ShelfID id, int32 target_index);

[James Cook, 2017/05/30 22:47:47]

nit: document what happens for negative/out-of-range index. Also, I presume
"target_index" means "make this the index value" not "put it after |target|".

Also, I presume 0 is invalid, due to app list icon being there?

[msw, 2017/05/31 16:55:21]

Done.

+  UpdateShelfItem(ShelfItem item);
+  SetShelfItemDelegate(ShelfID id, ShelfItemDelegate delegate);
+
   // Set the shelf alignment and auto-hide behavior. See Shelf for details.
+  // TODO(jamescook): Eliminate all these methods and use the ash pref service
+  // to observe and to set prefs. http://crbug.com/723085
   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.
+// A Shelf observer, used to persist profile settings and cache a ShelfModel.
 interface ShelfObserver {
   // TODO(jamescook): Eliminate all these methods and use the ash pref service
   // to observe and to set prefs. http://crbug.com/723085
   OnShelfInitialized(int64 display_id);
   OnAlignmentChanged(ShelfAlignment alignment, int64 display_id);
   OnAutoHideBehaviorChanged(ShelfAutoHideBehavior auto_hide, int64 display_id);
+
+  // Functions to observe changes to ShelfModel items and their delegates.
+  OnShelfItemAdded(int32 index, ShelfItem item);
+  OnShelfItemRemoved(int32 index, ShelfItem item);
+  OnShelfItemMoved(int32 start_index, int32 target_index);

[James Cook, 2017/05/30 22:47:47]

naively it seems like this should pass an ID to verify the destination really
has the right item in the right location

[msw, 2017/05/31 16:55:21]

I'll pass the item's id instead of the original index.

+  OnShelfItemChanged(int32 index, ShelfItem item);

[James Cook, 2017/05/30 22:47:47]

It's a little odd that this takes an index. I would expect an ID. I think it
needs docs explaining what counts as a change (does changing index mean a
change? I'm guessing no)

Also, should this be OnShelfItemUpdated() to correspond with UpdateShelfItem, or
is it something different?

[msw, 2017/05/31 16:55:21]

I removed that index param, fixed the name, added comments throughout.

+  OnShelfItemDelegateChanged(ShelfID id, ShelfItemDelegate delegate);
 };
 
 // ShelfItemDelegate handles shelf item selection, menu command execution, etc.
 interface ShelfItemDelegate {
   // 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.
   //
   // NOTE: This codepath is not currently used for context menu triggering.
@@ skipping 31 matching lines @@
 struct ShelfID {
   string app_id;            // An app id string, used to match app windows.
                             // (eg. extension ids, arc ids, "AppList", etc.)
   string launch_id;         // A string used to support multiple items per app.
                             // (eg. Citrix may use 'Word' or 'Excel' launch ids)
 };
 
 // ShelfItems are used to populate the shelf.
 // This structure matches ash::ShelfItem.
 struct ShelfItem {
-  ShelfItemType type;       // The type of the shelf item.
-  skia.mojom.Bitmap image;  // An icon image Bitmap, shown on the shelf.
-  ShelfItemStatus status;   // The running/closed/etc. status of the item.
-  ShelfID shelf_id;         // The id for the shelf item and its windows.
+  ShelfItemType type;        // The type of the shelf item.
+  skia.mojom.Bitmap? image;  // An icon image Bitmap, shown on the shelf.
+  ShelfItemStatus status;    // The running/closed/etc. status of the item.
+  ShelfID shelf_id;          // The id for the shelf item and its windows.
   mojo.common.mojom.String16 title;  // The title to display for tooltips, etc.
-  bool shows_tooltip;       // Whether the tooltip should be shown on hover.
-  bool pinned_by_policy;    // Whether the item is pinned by policy preferences,
-                            // the user cannot un-pin these items.
+  bool shows_tooltip;        // Whether the tooltip should be shown on hover.
+  bool pinned_by_policy;     // Whether the item is pinned by policy prefs, the
+                             // user cannot un-pin these items.
 };