mash: Support ShelfModel access in Chrome.

chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
index 465ef44ed504299fef3decf335f1ca363171376d..95f6167225e160dd06118c7794da0007334bd22f 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 
 #include "ash/multi_profile_uma.h"
+#include "ash/public/cpp/remote_shelf_item_delegate.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "ash/public/interfaces/constants.mojom.h"
 #include "ash/resources/grit/ash_resources.h"
@@ -20,6 +21,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/chromeos/ash_config.h"
 #include "chrome/browser/extensions/chrome_app_icon_loader.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/prefs/pref_service_syncable_util.h"
@@ -279,6 +281,9 @@ void ChromeLauncherController::Init() {
     ash::mojom::ShelfObserverAssociatedPtrInfo ptr_info;
     observer_binding_.Bind(mojo::MakeRequest(&ptr_info));
     shelf_controller_->AddObserver(std::move(ptr_info));
+    // ShelfModel's constructor should have initialized the app list item.
+    DCHECK_EQ(1, model_->item_count());
+    DCHECK_EQ(ash::kAppListId, model_->items()[0].id.app_id);
   }
 
   CreateBrowserShortcutLauncherItem();
@@ -1268,12 +1273,76 @@ void ChromeLauncherController::OnAutoHideBehaviorChanged(
   SetShelfAutoHideBehaviorPref(profile_->GetPrefs(), display_id, auto_hide);
 }
 
+void ChromeLauncherController::OnShelfItemAdded(int32_t index,
+                                                const ash::ShelfItem& item) {
+  // Ignore notifications of adding the AppList item; it should already exist.
+  if (item.id.app_id == ash::kAppListId) {
+    DCHECK_EQ(0, model_->ItemIndexByID(item.id));
+    return;
+  }
+
+  DCHECK(ash_util::IsRunningInMash()) << "Unexpected model synchronization";
+  DCHECK(!applying_remote_shelf_model_changes_) << "Unexpected model change";
+  DCHECK_GE(model_->item_count(), index) << "Models out of sync";
+  base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, true);
+  model_->AddAt(index, item);
+}
+
+void ChromeLauncherController::OnShelfItemRemoved(int32_t index,
+                                                  const ash::ShelfItem& item) {
+  DCHECK(ash_util::IsRunningInMash()) << "Unexpected model synchronization";
+  DCHECK(!applying_remote_shelf_model_changes_) << "Unexpected model change";
+  DCHECK_GT(model_->item_count(), index) << "Models out of sync";
+  DCHECK_EQ(model_->items()[index].id, item.id) << "Models out of sync";
+  base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, true);
+  model_->RemoveItemAt(index);
+}
+
+void ChromeLauncherController::OnShelfItemMoved(int32_t start_index,
+                                                int32_t target_index) {
+  DCHECK(ash_util::IsRunningInMash()) << "Unexpected model synchronization";
+  DCHECK(!applying_remote_shelf_model_changes_) << "Unexpected model change";
+  DCHECK_GT(model_->item_count(), start_index) << "Models out of sync";
+  DCHECK_GT(model_->item_count(), target_index) << "Models out of sync";
+  base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, true);
+  model_->Move(start_index, target_index);
+}
+
+void ChromeLauncherController::OnShelfItemChanged(int32_t index,
+                                                  const ash::ShelfItem& item) {
+  DCHECK(ash_util::IsRunningInMash()) << "Unexpected model synchronization";
+  DCHECK(!applying_remote_shelf_model_changes_) << "Unexpected model change";
+  DCHECK_GT(model_->item_count(), index) << "Models out of sync";
+  DCHECK_EQ(model_->items()[index].id, item.id) << "Models out of sync";
+  base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, true);
+  model_->Set(index, item);
+}
+
+void ChromeLauncherController::OnShelfItemDelegateChanged(
+    const ash::ShelfID& id,
+    ash::mojom::ShelfItemDelegatePtr delegate) {
+  DCHECK(ash_util::IsRunningInMash()) << "Unexpected model synchronization";
+  DCHECK(!applying_remote_shelf_model_changes_) << "Unexpected model change";
+  base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, true);
+  if (delegate.is_bound())

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

nit: curlies

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

Done.

+    model_->SetShelfItemDelegate(id,
+                                 base::MakeUnique<ash::RemoteShelfItemDelegate>(
+                                     id, std::move(delegate)));
+  else
+    model_->SetShelfItemDelegate(id, nullptr);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // ash::ShelfModelObserver:
 
 void ChromeLauncherController::ShelfItemAdded(int index) {
-  // Update the pin position preference as needed.
   ash::ShelfItem item = model_->items()[index];
+  if (shelf_controller_ && !applying_remote_shelf_model_changes_ &&
+      chromeos::GetAshConfig() == ash::Config::MASH) {

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

This (and the blocks below) is a little odd. So CLC is observing changes to its
own model and using that to report changes to ash? Naively I would expect code
in chrome to call functions on CLC, which would both update its model and send
mojo messages to ash.

Is that because code in chrome directly accesses CLC's ShelfModel instance? Or
because of the multiple updates done below?

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

Yes, code in chrome directly accesses CLC's ShelfModel instance, some even still
uses ash::Shell::shelf_model() . I'll try to refine that later on by making
those Chrome callsites use CLC/ShelfController instead.

+    shelf_controller_->AddShelfItem(index, item);
+  }
+
+  // Update the pin position preference as needed.
   if (ItemTypeIsPinned(item) && should_sync_pin_changes_)
     SyncPinPosition(item.id);
 
@@ -1301,12 +1370,17 @@ void ChromeLauncherController::ShelfItemAdded(int index) {
       needs_update = true;
       item.status = status;
     }
-    if (needs_update)
+    if (needs_update) {
+      // Ensure these changes are reported back to Ash.
+      base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, false);
       model_->Set(index, item);
+    }
   }
 
   // Construct a ShelfItemDelegate for the item if one does not yet exist.
   if (!model_->GetShelfItemDelegate(item.id)) {
+    // Ensure these changes are reported back to Ash.
+    base::AutoReset<bool> reset(&applying_remote_shelf_model_changes_, false);

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

Aside: It's kind of a bummer that adding an item can result in 3 IPCs from
chrome to ash.

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

Perhaps worth following up.

     model_->SetShelfItemDelegate(
         item.id, AppShortcutLauncherItemController::Create(item.id));
   }
@@ -1315,6 +1389,11 @@ void ChromeLauncherController::ShelfItemAdded(int index) {
 void ChromeLauncherController::ShelfItemRemoved(
     int index,
     const ash::ShelfItem& old_item) {
+  if (shelf_controller_ && !applying_remote_shelf_model_changes_ &&
+      chromeos::GetAshConfig() == ash::Config::MASH) {
+    shelf_controller_->RemoveShelfItem(old_item.id);
+  }
+
   // Remove the pin position from preferences as needed.
   if (ItemTypeIsPinned(old_item) && should_sync_pin_changes_)
     RemovePinPosition(profile(), old_item.id);
@@ -1326,8 +1405,13 @@ void ChromeLauncherController::ShelfItemRemoved(
 
 void ChromeLauncherController::ShelfItemMoved(int start_index,
                                               int target_index) {
-  // Update the pin position preference as needed.
   const ash::ShelfItem& item = model_->items()[target_index];
+  if (shelf_controller_ && !applying_remote_shelf_model_changes_ &&
+      chromeos::GetAshConfig() == ash::Config::MASH) {
+    shelf_controller_->MoveShelfItem(item.id, target_index);
+  }
+
+  // Update the pin position preference as needed.
   DCHECK_NE(ash::TYPE_APP_LIST, item.type);
   if (ItemTypeIsPinned(item) && should_sync_pin_changes_)
     SyncPinPosition(item.id);
@@ -1336,10 +1420,15 @@ void ChromeLauncherController::ShelfItemMoved(int start_index,
 void ChromeLauncherController::ShelfItemChanged(
     int index,
     const ash::ShelfItem& old_item) {
+  const ash::ShelfItem& item = model_->items()[index];
+  if (shelf_controller_ && !applying_remote_shelf_model_changes_ &&
+      chromeos::GetAshConfig() == ash::Config::MASH) {
+    shelf_controller_->UpdateShelfItem(item);
+  }
+
   if (!should_sync_pin_changes_)
     return;
 
-  const ash::ShelfItem& item = model_->items()[index];
   // Add or remove the pin position from preferences as needed.
   if (!ItemTypeIsPinned(old_item) && ItemTypeIsPinned(item))
     SyncPinPosition(item.id);
@@ -1347,6 +1436,17 @@ void ChromeLauncherController::ShelfItemChanged(
     RemovePinPosition(profile(), old_item.id);
 }
 
+void ChromeLauncherController::ShelfItemDelegateChanged(
+    const ash::ShelfID& id,
+    ash::ShelfItemDelegate* delegate) {
+  if (shelf_controller_ && !applying_remote_shelf_model_changes_ &&
+      chromeos::GetAshConfig() == ash::Config::MASH) {
+    shelf_controller_->SetShelfItemDelegate(
+        id, delegate ? delegate->CreateInterfacePtrAndBind()
+                     : ash::mojom::ShelfItemDelegatePtr());
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // ash::WindowTreeHostManager::Observer: