mash: Support ShelfModel access in Chrome.

ash/public/interfaces/shelf.mojom

Index: ash/public/interfaces/shelf.mojom
diff --git a/ash/public/interfaces/shelf.mojom b/ash/public/interfaces/shelf.mojom
index faaeddf3bd8ad28e5bbcba7b651b02694db0fd8c..8e8e5539b4541ff70c9440bd44f1128301c487eb 100644
--- a/ash/public/interfaces/shelf.mojom
+++ b/ash/public/interfaces/shelf.mojom
@@ -61,23 +61,36 @@ 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.
@@ -129,12 +142,12 @@ struct ShelfID {
 // 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.
 };