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);
 
+  // Start syncing a remote (Chrome) ShelfModel and the local (Ash) ShelfModel.
+  // Chrome's supplied |observer| will be notified of Ash's ShelfModel changes.
+  // The callback passes Ash's corresponding |observer| of Chrome's ShelfModel.
+  // Chrome delays binding its observer until the callback is run, so it can
+  // prepend Ash's |items| and notify Ash's observer of existing Chrome items.
+  LinkShelfModels(ShelfModelObserver observer) => (ShelfModelObserver observer,

[sky, 2017/05/26 19:42:01]

Is there a reason you don't want LinkShelfModels to take both observers rather
than using a callback? The second one would be a ShelfModelObserver&. That said,
is there a reason you prefer an observer in both directions rather than explicit
functions on ShelfController for the mutation? Using observers in both
directions muddies who the real owner of the state is. If you instead have
explicit functions on ShelfController for the mutation and an observer in only
one direction it's clear who owns what.

[msw, 2017/05/27 00:04:38]

I've started pursuing changes like what you suggest (adding explicit ShelfModel
client functions on ShelfController, dropping Ash's 'observation' of chrome, and
consolidating ShelfModelObserver functions onto the associated ShelfObserver
interface). I suppose using observers on both sides had a somewhat nice symmetry
(the impls on either sides was nearly identical) and that pattern punts the
issue of ownership (the model is simply replicated, not 'owned' on one side and
'cached' on the other), but ultimately, Ash *should* own the model, and Chrome
should be a client/observer of it, so your suggestions are worth pursuing.

I'll need to refine the new initialization sequence next week, after fixing one
issue (disagreement about indices amid init), I revised the client interface to
allow [temporary] index disagreement between the models, but now I get the
profile-pinned apps out of order on the shelf. I'll ping here when I've
addressed that defect (etc.), but feel free to make other high-level suggestions
or do a preliminary review pass.

+                                                   array<ShelfItem> items);
+
   // 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.
 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);
 };
 
+// ShelfModelObserver is notified on changes to the shelf model and its items.
+interface ShelfModelObserver {
+  OnShelfItemAdded(int32 index, ShelfItem item);
+  OnShelfItemRemoved(int32 index, ShelfItem item);
+  OnShelfItemMoved(int32 start_index, int32 target_index);
+  OnShelfItemChanged(int32 index, ShelfItem item);
+  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.
   // TODO(msw): Remove |display_id| once panels are removed. crbug.com/691099
   ItemSelected(ui.mojom.Event event,
@@ skipping 29 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.
 };