mash: Remove ChromeLauncherController's |id_to_item_controller_map_|.

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

 // Copyright 2013 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.
 
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 72 matching lines @@
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/keyboard/keyboard_util.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/wm/core/window_animations.h"
 
 using extensions::Extension;
+using extension_misc::kChromeAppId;
 using extension_misc::kGmailAppId;
 using content::WebContents;
 
 namespace {
 
 int64_t GetDisplayIDForShelf(ash::WmShelf* shelf) {
   display::Display display =
       shelf->GetWindow()->GetRootWindow()->GetDisplayNearestWindow();
   DCHECK(display.is_valid());
   return display.id();
@@ skipping 164 matching lines @@
 ChromeLauncherControllerImpl::~ChromeLauncherControllerImpl() {
   // Reset the BrowserStatusMonitor as it has a weak pointer to this.
   browser_status_monitor_.reset();
 
   // Reset the app window controllers here since it has a weak pointer to this.
   app_window_controllers_.clear();
 
   model_->RemoveObserver(this);
   if (ash::Shell::HasInstance())
     ash::Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
-  for (const auto& pair : id_to_item_controller_map_) {
-    int index = model_->ItemIndexByID(pair.first);
-    // A "browser proxy" is not known to the model and this removal does
-    // therefore not need to be propagated to the model.
-    if (index != -1 &&
-        model_->items()[index].type != ash::TYPE_BROWSER_SHORTCUT)
-      model_->RemoveItemAt(index);
-  }
 
   // Release all profile dependent resources.
   ReleaseProfile();
 
   // Get rid of the multi user window manager instance.
   chrome::MultiUserWindowManager::DeleteInstance();
 }
 
 ash::ShelfID ChromeLauncherControllerImpl::CreateAppLauncherItem(
     std::unique_ptr<ash::ShelfItemDelegate> item_delegate,
@@ skipping 23 matching lines @@
 void ChromeLauncherControllerImpl::SetItemStatus(ash::ShelfID id,
                                                  ash::ShelfItemStatus status) {
   const ash::ShelfItem* item = GetItem(id);
   if (item && item->status != status) {
     ash::ShelfItem new_item = *item;
     new_item.status = status;
     model_->Set(model_->ItemIndexByID(id), new_item);
   }
 }
 
-void ChromeLauncherControllerImpl::SetShelfItemDelegate(
-    ash::ShelfID id,
-    std::unique_ptr<ash::ShelfItemDelegate> item_delegate) {
-  CHECK(item_delegate);
-  IDToItemControllerMap::iterator iter = id_to_item_controller_map_.find(id);
-  CHECK(iter != id_to_item_controller_map_.end());
-  item_delegate->set_shelf_id(id);
-  iter->second = item_delegate.get();
-  model_->SetShelfItemDelegate(id, std::move(item_delegate));
-}
-
 void ChromeLauncherControllerImpl::CloseLauncherItem(ash::ShelfID id) {
   CHECK(id);
   if (IsPinned(id)) {
     // Create a new shortcut delegate.
     SetItemStatus(id, ash::STATUS_CLOSED);
-    SetShelfItemDelegate(id, AppShortcutLauncherItemController::Create(
-                                 GetItem(id)->app_launch_id));
+    model_->SetShelfItemDelegate(id, AppShortcutLauncherItemController::Create(
+                                         GetItem(id)->app_launch_id));
   } else {
-    LauncherItemClosed(id);
+    RemoveShelfItem(id);
   }
 }
 
 void ChromeLauncherControllerImpl::UnpinShelfItemInternal(ash::ShelfID id) {
   const ash::ShelfItem* item = GetItem(id);
   if (item && item->status != ash::STATUS_CLOSED)
     UnpinRunningAppInternal(model_->ItemIndexByID(id));
   else
-    LauncherItemClosed(id);
+    RemoveShelfItem(id);
 }
 
 bool ChromeLauncherControllerImpl::IsPinned(ash::ShelfID id) {
   const ash::ShelfItem* item = GetItem(id);
   return item && ItemTypeIsPinned(*item);
 }
 
 void ChromeLauncherControllerImpl::SetV1AppStatus(const std::string& app_id,
                                                   ash::ShelfItemStatus status) {
   ash::ShelfID id = GetShelfIDForAppID(app_id);
   const ash::ShelfItem* item = GetItem(id);
   if (item) {
     if (!IsPinned(id) && status == ash::STATUS_CLOSED)
-      LauncherItemClosed(id);
+      RemoveShelfItem(id);
     else
       SetItemStatus(id, status);
   } else if (status != ash::STATUS_CLOSED && !app_id.empty()) {
     InsertAppLauncherItem(
         AppShortcutLauncherItemController::Create(ash::AppLaunchId(app_id)),
         status, model_->item_count(), ash::TYPE_APP);
   }
 }
 
 void ChromeLauncherControllerImpl::Launch(ash::ShelfID id, int event_flags) {
-  ash::ShelfItemDelegate* delegate = GetShelfItemDelegate(id);
+  ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(id);
   if (!delegate)
     return;  // In case invoked from menu and item closed while menu up.
 
   // Launching some items replaces the associated item delegate instance,
   // which destroys the app and launch id strings; making copies avoid crashes.
   LaunchApp(ash::AppLaunchId(delegate->app_id(), delegate->launch_id()),
             ash::LAUNCH_FROM_UNKNOWN, event_flags);
 }
 
 void ChromeLauncherControllerImpl::Close(ash::ShelfID id) {
@@ skipping 82 matching lines @@
   if (id) {
     SetItemStatus(id, (app_state == APP_STATE_WINDOW_ACTIVE ||
                        app_state == APP_STATE_ACTIVE)
                           ? ash::STATUS_ACTIVE
                           : GetAppState(app_id));
   }
 }
 
 ash::ShelfID ChromeLauncherControllerImpl::GetShelfIDForWebContents(
     content::WebContents* contents) {
-  DCHECK(contents);
-
   std::string app_id = launcher_controller_helper()->GetAppID(contents);
-
   if (app_id.empty() && ContentCanBeHandledByGmailApp(contents))
     app_id = kGmailAppId;
 
   ash::ShelfID id = GetShelfIDForAppID(app_id);
-
-  if (app_id.empty() || !id) {
-    int browser_index = model_->GetItemIndexForType(ash::TYPE_BROWSER_SHORTCUT);
-    return model_->items()[browser_index].id;
-  }
-
-  return id;
+  return id == ash::kInvalidShelfID ? GetShelfIDForAppID(kChromeAppId) : id;

[James Cook, 2017/04/07 20:42:22]

nit: maybe a comment here about why invalid id means browser shortcut? (I
presume it's to show the white dot under the browser shortcut when |contents| is
not due to a web app.

[msw, 2017/04/07 22:02:16]

Done.

 }
 
 void ChromeLauncherControllerImpl::SetRefocusURLPatternForTest(
     ash::ShelfID id,
     const GURL& url) {
   const ash::ShelfItem* item = GetItem(id);
   if (item && !IsPlatformApp(id) &&
       (item->type == ash::TYPE_PINNED_APP || item->type == ash::TYPE_APP)) {
-    ash::ShelfItemDelegate* item_delegate = GetShelfItemDelegate(id);
+    ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(id);
     AppShortcutLauncherItemController* item_controller =
-        static_cast<AppShortcutLauncherItemController*>(item_delegate);
+        static_cast<AppShortcutLauncherItemController*>(delegate);
     item_controller->set_refocus_url(url);
   } else {
     NOTREACHED() << "Invalid launcher item or type";
   }
 }
 
 ash::ShelfAction ChromeLauncherControllerImpl::ActivateWindowOrMinimizeIfActive(
     ui::BaseWindow* window,
     bool allow_minimize) {
   // In separated desktop mode we might have to teleport a window back to the
@@ skipping 56 matching lines @@
 
 void ChromeLauncherControllerImpl::AdditionalUserAddedToSession(
     Profile* profile) {
   // Switch the running applications to the new user.
   for (auto& controller : app_window_controllers_)
     controller->AdditionalUserAddedToSession(profile);
 }
 
 ash::MenuItemList ChromeLauncherControllerImpl::GetAppMenuItemsForTesting(
     const ash::ShelfItem& item) {
-  ash::ShelfItemDelegate* item_delegate = GetShelfItemDelegate(item.id);
-  return item_delegate ? item_delegate->GetAppMenuItems(ui::EF_NONE)
-                       : ash::MenuItemList();
+  ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(item.id);
+  return delegate ? delegate->GetAppMenuItems(ui::EF_NONE)
+                  : ash::MenuItemList();
 }
 
 std::vector<content::WebContents*>
 ChromeLauncherControllerImpl::GetV1ApplicationsFromAppId(
     const std::string& app_id) {
   const ash::ShelfItem* item = GetItem(GetShelfIDForAppID(app_id));
   // If there is no such item pinned to the launcher, no menu gets created.
   if (!item || item->type != ash::TYPE_PINNED_APP)
     return std::vector<content::WebContents*>();
-  ash::ShelfItemDelegate* item_delegate = GetShelfItemDelegate(item->id);
+  ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(item->id);
   AppShortcutLauncherItemController* item_controller =
-      static_cast<AppShortcutLauncherItemController*>(item_delegate);
+      static_cast<AppShortcutLauncherItemController*>(delegate);
   return item_controller->GetRunningApplications();
 }
 
 void ChromeLauncherControllerImpl::ActivateShellApp(const std::string& app_id,
                                                     int window_index) {
   const ash::ShelfItem* item = GetItem(GetShelfIDForAppID(app_id));
   if (item &&
       (item->type == ash::TYPE_APP || item->type == ash::TYPE_PINNED_APP)) {
-    ash::ShelfItemDelegate* item_delegate = GetShelfItemDelegate(item->id);
+    ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(item->id);
     AppWindowLauncherItemController* item_controller =
-        item_delegate->AsAppWindowLauncherItemController();
+        delegate->AsAppWindowLauncherItemController();
     item_controller->ActivateIndexedApp(window_index);
   }
 }
 
 bool ChromeLauncherControllerImpl::IsWebContentHandledByApplication(
     content::WebContents* web_contents,
     const std::string& app_id) {
   if ((web_contents_to_app_id_.find(web_contents) !=
        web_contents_to_app_id_.end()) &&
       (web_contents_to_app_id_[web_contents] == app_id))
@@ skipping 43 matching lines @@
     const extensions::Extension* extension =
         GetExtensionForAppID(app_id, profile());
     if (extension)
       return base::UTF8ToUTF16(extension->name());
   }
   return l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE);
 }
 
 BrowserShortcutLauncherItemController*
 ChromeLauncherControllerImpl::GetBrowserShortcutLauncherItemController() {
-  for (const auto& pair : id_to_item_controller_map_) {
-    const ash::ShelfItem* item = GetItem(pair.first);
-    if (item && item->type == ash::TYPE_BROWSER_SHORTCUT)
-      return static_cast<BrowserShortcutLauncherItemController*>(pair.second);
-  }
-  NOTREACHED()
-      << "There should be always be a BrowserShortcutLauncherItemController.";
-  return nullptr;
-}
-
-ash::ShelfItemDelegate* ChromeLauncherControllerImpl::GetShelfItemDelegate(
-    const ash::ShelfID id) {
-  IDToItemControllerMap::iterator iter = id_to_item_controller_map_.find(id);
-  return iter == id_to_item_controller_map_.end() ? nullptr : iter->second;
+  ash::ShelfID id = GetShelfIDForAppID(kChromeAppId);
+  ash::mojom::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(id);
+  DCHECK(delegate) << "There should be always be a browser shortcut item.";
+  return static_cast<BrowserShortcutLauncherItemController*>(delegate);
 }
 
 bool ChromeLauncherControllerImpl::ShelfBoundsChangesProbablyWithUser(
     ash::WmShelf* shelf,
     const AccountId& account_id) const {
   Profile* other_profile = multi_user_util::GetProfileFromAccountId(account_id);
   if (!other_profile || other_profile == profile())
     return false;
 
   // Note: The Auto hide state from preferences is not the same as the actual
@@ skipping 20 matching lines @@
     user_switch_observer_->OnUserProfileReadyToSwitch(profile);
 }
 
 ArcAppDeferredLauncherController*
 ChromeLauncherControllerImpl::GetArcDeferredLauncher() {
   return arc_deferred_launcher_.get();
 }
 
 const std::string& ChromeLauncherControllerImpl::GetLaunchIDForShelfID(
     ash::ShelfID id) {
-  ash::ShelfItemDelegate* delegate = GetShelfItemDelegate(id);
+  ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(id);
   return delegate ? delegate->launch_id() : base::EmptyString();
 }
 
 void ChromeLauncherControllerImpl::AttachProfile(Profile* profile_to_attach) {
   // The base class implementation updates the helper and app icon loaders.
   ChromeLauncherController::AttachProfile(profile_to_attach);
 
   pref_change_registrar_.Init(profile()->GetPrefs());
   pref_change_registrar_.Add(
       prefs::kPolicyPinnedLauncherApps,
@@ skipping 224 matching lines @@
     if (item && item->type == ash::TYPE_APP) {
       int app_index = model_->ItemIndexByID(item->id);
       DCHECK_GE(app_index, 0);
       if (running_index != app_index)
         model_->Move(running_index, app_index);
       running_index++;
     }
   }
 }
 
-void ChromeLauncherControllerImpl::LauncherItemClosed(ash::ShelfID id) {
-  IDToItemControllerMap::iterator iter = id_to_item_controller_map_.find(id);
-  CHECK(iter != id_to_item_controller_map_.end());
-  CHECK(iter->second);
-  const std::string& app_id = iter->second->app_id();
+void ChromeLauncherControllerImpl::RemoveShelfItem(ash::ShelfID id) {
+  const std::string& app_id = GetAppIDForShelfID(id);
   AppIconLoader* app_icon_loader = GetAppIconLoaderForApp(app_id);
   if (app_icon_loader)
     app_icon_loader->ClearImage(app_id);
-  id_to_item_controller_map_.erase(iter);
-  int index = model_->ItemIndexByID(id);
+  const int index = model_->ItemIndexByID(id);
   // A "browser proxy" is not known to the model and this removal does
   // therefore not need to be propagated to the model.
   if (index != -1)
     model_->RemoveItemAt(index);
 }
 
 void ChromeLauncherControllerImpl::PinRunningAppInternal(
     int index,
     ash::ShelfID shelf_id) {
   DCHECK_EQ(GetItem(shelf_id)->type, ash::TYPE_APP);
@@ skipping 99 matching lines @@
     if (shelf_app_id != app_id)
       continue;
 
     // Update apps icon if applicable.
     OnAppUpdated(profile(), app_id);
 
     // Find existing pin or app from the right of current |index|.
     int app_index = index;
     for (; app_index < model_->item_count(); ++app_index) {
       const ash::ShelfItem& item = model_->items()[app_index];
-      const IDToItemControllerMap::iterator it =
-          id_to_item_controller_map_.find(item.id);
-      if (it != id_to_item_controller_map_.end() &&
-          it->second->app_id() == app_id &&
-          it->second->launch_id() == pref_app_launch_id.launch_id()) {
+      if (item.app_launch_id.app_id() == app_id &&
+          item.app_launch_id.launch_id() == pref_app_launch_id.launch_id()) {
         break;
       }
     }
     if (app_index < model_->item_count()) {
       // Found existing pin or running app.
       const ash::ShelfItem item = model_->items()[app_index];
-      if (item.type == ash::TYPE_PINNED_APP ||
-          item.type == ash::TYPE_BROWSER_SHORTCUT) {
+      if (ItemTypeIsPinned(item)) {
         // Just move to required position or keep it inplace.
         model_->Move(app_index, index);
       } else {
         PinRunningAppInternal(index, item.id);
       }
       DCHECK_EQ(model_->ItemIndexByID(item.id), index);
     } else {
       // This is fresh pin. Create new one.
-      DCHECK_NE(app_id, extension_misc::kChromeAppId);
+      DCHECK_NE(app_id, kChromeAppId);
       CreateAppShortcutLauncherItem(pref_app_launch_id, index);
     }
     ++index;
   }
 
   // At second step remove any pin to the right from the current index.
   while (index < model_->item_count()) {
     const ash::ShelfItem item = model_->items()[index];
     if (item.type == ash::TYPE_PINNED_APP)
       UnpinShelfItemInternal(item.id);
@@ skipping 63 matching lines @@
 ash::ShelfID ChromeLauncherControllerImpl::InsertAppLauncherItem(
     std::unique_ptr<ash::ShelfItemDelegate> item_delegate,
     ash::ShelfItemStatus status,
     int index,
     ash::ShelfItemType shelf_item_type) {
   ash::ShelfID id = model_->next_id();
   CHECK(!GetItem(id));
   CHECK(item_delegate);
   // Ash's ShelfWindowWatcher handles app panel windows separately.
   DCHECK_NE(ash::TYPE_APP_PANEL, shelf_item_type);
-  id_to_item_controller_map_[id] = item_delegate.get();
-  item_delegate->set_shelf_id(id);
 
   ash::ShelfItem item;
   item.type = shelf_item_type;
   item.app_launch_id = item_delegate->app_launch_id();
   item.image = extensions::util::GetDefaultAppIcon();
 
   const std::string& app_id = item_delegate->app_id();
   item.title = LauncherControllerHelper::GetAppTitle(profile(), app_id);
 
   ash::ShelfItemStatus new_state = GetAppState(app_id);
@@ skipping 17 matching lines @@
   // Do not sync the pin position of the browser shortcut item when it is added;
   // its initial position before prefs have loaded is unimportant and the sync
   // service may not yet be initialized.
   ScopedPinSyncDisabler scoped_pin_sync_disabler = GetScopedPinSyncDisabler();
 
   ash::ShelfItem browser_shortcut;
   browser_shortcut.type = ash::TYPE_BROWSER_SHORTCUT;
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   browser_shortcut.image = *rb.GetImageSkiaNamed(IDR_PRODUCT_LOGO_32);
   browser_shortcut.title = l10n_util::GetStringUTF16(IDS_PRODUCT_NAME);
-  browser_shortcut.app_launch_id =
-      ash::AppLaunchId(extension_misc::kChromeAppId);
+  browser_shortcut.app_launch_id = ash::AppLaunchId(kChromeAppId);
   ash::ShelfID id = model_->next_id();
   model_->AddAt(0, browser_shortcut);
   std::unique_ptr<BrowserShortcutLauncherItemController> item_delegate =
       base::MakeUnique<BrowserShortcutLauncherItemController>(model_);
   BrowserShortcutLauncherItemController* item_controller = item_delegate.get();
-  item_controller->set_shelf_id(id);
-  id_to_item_controller_map_[id] = item_controller;
   model_->SetShelfItemDelegate(id, std::move(item_delegate));
   item_controller->UpdateBrowserItemState();
 }
 
 bool ChromeLauncherControllerImpl::IsIncognito(
     const content::WebContents* web_contents) const {
   const Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   return profile->IsOffTheRecord() && !profile->IsGuestSession() &&
          !profile->IsSystemProfile();
 }
 
 int ChromeLauncherControllerImpl::FindInsertionPoint() {
-  DCHECK_GT(model_->item_count(), 0);
   for (int i = model_->item_count() - 1; i > 0; --i) {
-    ash::ShelfItemType type = model_->items()[i].type;
-    DCHECK_NE(ash::TYPE_APP_LIST, type);
-    if (type == ash::TYPE_PINNED_APP || type == ash::TYPE_BROWSER_SHORTCUT)
+    if (ItemTypeIsPinned(model_->items()[i]))
       return i;
   }
   return 0;
 }
 
 void ChromeLauncherControllerImpl::CloseWindowedAppsFromRemovedExtension(
     const std::string& app_id,
     const Profile* profile) {
   // This function cannot rely on the controller's enumeration functionality
   // since the extension has already been unloaded.
@@ skipping 53 matching lines @@
   // Remove the pin position from preferences as needed.
   if (ItemTypeIsPinned(old_item) && should_sync_pin_changes()) {
     // TODO(khmel): Fix this Arc application id mapping. See http://b/31703859
     const std::string shelf_app_id =
         ArcAppWindowLauncherController::GetShelfAppIdFromArcAppId(
             old_item.app_launch_id.app_id());
     ash::AppLaunchId app_launch_id(shelf_app_id,
                                    old_item.app_launch_id.launch_id());
     ash::launcher::RemovePinPosition(profile(), app_launch_id);
   }
-
-  // TODO(skuhne): This fixes crbug.com/429870, but it does not answer why we
-  // get into this state in the first place.
-  if (id_to_item_controller_map_.count(old_item.id) > 0)
-    id_to_item_controller_map_.erase(old_item.id);
 }
 
 void ChromeLauncherControllerImpl::ShelfItemMoved(int start_index,
                                                   int target_index) {
   // Update the pin position preference as needed.
   const ash::ShelfItem& item = model_->items()[target_index];
   DCHECK_NE(ash::TYPE_APP_LIST, item.type);
   if (ItemTypeIsPinned(item) && should_sync_pin_changes())
     SyncPinPosition(item.id);
 }
@@ skipping 13 matching lines @@
     const std::string shelf_app_id =
         ArcAppWindowLauncherController::GetShelfAppIdFromArcAppId(
             old_item.app_launch_id.app_id());
 
     ash::AppLaunchId app_launch_id(shelf_app_id,
                                    old_item.app_launch_id.launch_id());
     ash::launcher::RemovePinPosition(profile(), app_launch_id);
   }
 }
 
-void ChromeLauncherControllerImpl::OnSetShelfItemDelegate(
-    ash::ShelfID id,
-    ash::ShelfItemDelegate* item_delegate) {
-  // TODO(skuhne): This fixes crbug.com/429870, but it does not answer why we
-  // get into this state in the first place.
-  IDToItemControllerMap::iterator iter = id_to_item_controller_map_.find(id);
-  if (iter == id_to_item_controller_map_.end() || item_delegate == iter->second)
-    return;
-  LOG(ERROR) << "Unexpected change of shelf item delegate, id: " << id;
-  id_to_item_controller_map_.erase(iter);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // ash::WindowTreeHostManager::Observer:
 
 void ChromeLauncherControllerImpl::OnDisplayConfigurationChanged() {
   // In BOTTOM_LOCKED state, ignore the call of SetShelfBehaviorsFromPrefs.
   // Because it might be called by some operations, like crbug.com/627040
   // rotating screen.
   ash::WmShelf* shelf =
       ash::WmShelf::ForWindow(ash::WmShell::Get()->GetPrimaryRootWindow());
   if (shelf->alignment() != ash::SHELF_ALIGNMENT_BOTTOM_LOCKED)
@@ skipping 16 matching lines @@
   if (item.title != title) {
     item.title = title;
     model_->Set(app_list_index, item);
   }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // AppIconLoaderDelegate:
 
 void ChromeLauncherControllerImpl::OnAppImageUpdated(
-    const std::string& id,
+    const std::string& app_id,
     const gfx::ImageSkia& image) {
   // TODO: need to get this working for shortcuts.
   for (int index = 0; index < model_->item_count(); ++index) {
     ash::ShelfItem item = model_->items()[index];
-    if (GetAppIDForShelfID(item.id) != id)
+    ash::ShelfItemDelegate* delegate = model_->GetShelfItemDelegate(item.id);
+    if (item.type == ash::TYPE_APP_PANEL || !delegate ||
+        delegate->image_set_by_controller() ||
+        item.app_launch_id.app_id() != app_id) {
       continue;
-    ash::ShelfItemDelegate* delegate = GetShelfItemDelegate(item.id);
-    if (!delegate || delegate->image_set_by_controller())
-      continue;
+    }
     item.image = image;
     if (arc_deferred_launcher_)
-      arc_deferred_launcher_->MaybeApplySpinningEffect(id, &item.image);
+      arc_deferred_launcher_->MaybeApplySpinningEffect(app_id, &item.image);
     model_->Set(index, item);
     // It's possible we're waiting on more than one item, so don't break.
   }
 }