mash: Remove shelf app menu item objects.

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

Index: chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc
index c783d8f4da1c00a2a3abd0d6c3fa5b3cdbdb7481..8a11c51d25c00613f1c3ada9c2b5ebcf6560037d 100644
--- a/chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc
+++ b/chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc
@@ -8,45 +8,21 @@
 #include "ash/wm/window_state_aura.h"
 #include "ash/wm/window_util.h"
 #include "base/memory/ptr_util.h"
-#include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item_v2app.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/launcher_context_menu.h"
 #include "chrome/browser/ui/ash/launcher/launcher_item_controller.h"
 #include "components/favicon/content/content_favicon_driver.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/app_window/native_app_window.h"
-#include "skia/ext/image_operations.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
 #include "ui/events/event.h"
-#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image.h"
 #include "ui/wm/core/window_animations.h"
 
-namespace {
-
-// Size of the icon in the shelf launcher in display-independent pixels.
-const int kAppListIconSize = 24;
-
-// This will return a slightly smaller icon than the app icon to be used in
-// the application list menu.
-gfx::Image GetAppListIcon(extensions::AppWindow* app_window) {
-  // TODO(skuhne): We instead might want to use LoadImages in
-  // AppWindow::UpdateExtensionAppIcon() to let the extension give us
-  // pre-defined icons in the launcher and the launcher list sizes. Since there
-  // is no mock yet, doing this now seems a bit premature and we scale for the
-  // time being.
-  if (app_window->app_icon().IsEmpty())
-    return gfx::Image();
-
-  SkBitmap bmp = skia::ImageOperations::Resize(

[James Cook, 2017/03/01 19:50:48]

So this stuff isn't necessary any more? Are there highdpi vs. lowdpi issues?

[msw, 2017/03/02 02:59:20]

AFAICT, MenuItemView correctly clamps the icon's preferred height here:
https://cs.chromium.org/chromium/src/ui/views/controls/menu/menu_item_view.cc...
Those values are either 32 (size of kMenuCheckIcon) or 16 (kMenuCheckSize); I
believe we are using 32 in practice. So, this code seems to be downscaling the
app icon smaller than necessary. The menu icons look fine (unchanged) for the
apps I tested (Files, Chrome Web Store, Chrome Remote Desktop), albeit that's on
linux-desktop over remote desktop; I didn't do manual testing on a high-dpi
device. I suspect this scaling is superfluous or even detrimental, but it's
tangential to my CL and I can leave it intact, if you prefer.

-      *app_window->app_icon().ToSkBitmap(), skia::ImageOperations::RESIZE_BEST,
-      kAppListIconSize, kAppListIconSize);
-  return gfx::Image(gfx::ImageSkia::CreateFrom1xBitmap(bmp));
-}
-
-}  // namespace
-
 ExtensionAppWindowLauncherItemController::
     ExtensionAppWindowLauncherItemController(
         const std::string& app_id,
@@ -60,45 +36,40 @@ ExtensionAppWindowLauncherItemController::
 void ExtensionAppWindowLauncherItemController::AddAppWindow(
     extensions::AppWindow* app_window) {
   DCHECK(!app_window->window_type_is_panel());
-  DCHECK(window_to_app_window_.find(app_window->GetBaseWindow()) ==
-         window_to_app_window_.end());
   AddWindow(app_window->GetBaseWindow());
-  window_to_app_window_[app_window->GetBaseWindow()] = app_window;
 }
 
 ash::ShelfAppMenuItemList
 ExtensionAppWindowLauncherItemController::GetAppMenuItems(int event_flags) {
   ash::ShelfAppMenuItemList items;
-  int index = 0;
-  for (const auto* window : windows()) {
-    extensions::AppWindow* app_window = window_to_app_window_[window];
+  CHECK_LT(windows().size(), std::numeric_limits<uint32_t>::max());
+  extensions::AppWindowRegistry* app_window_registry =
+      extensions::AppWindowRegistry::Get(launcher_controller()->profile());
+
+  uint32_t window_index = 0;
+  for (const ui::BaseWindow* window : windows()) {
+    extensions::AppWindow* app_window =
+        app_window_registry->GetAppWindowForNativeWindow(
+            window->GetNativeWindow());
     DCHECK(app_window);
 
-    // If the app's web contents provides a favicon, use it. Otherwise, use a
-    // scaled down app icon.
+    // Use the app's web contents favicon, or the app window's icon.
     favicon::FaviconDriver* favicon_driver =
         favicon::ContentFaviconDriver::FromWebContents(
             app_window->web_contents());
-    gfx::Image result = favicon_driver->GetFavicon();
-    if (result.IsEmpty())
-      result = GetAppListIcon(app_window);
+    gfx::Image icon = favicon_driver->GetFavicon();
+    if (icon.IsEmpty())
+      icon = app_window->app_icon();
 
-    items.push_back(base::MakeUnique<ChromeLauncherAppMenuItemV2App>(
-        app_window->GetTitle(),
-        &result,  // Will be copied
-        app_id(), launcher_controller(), index));
-    ++index;
+    items.push_back(base::MakeUnique<ash::ShelfApplicationMenuItem>(
+        window_index, app_window->GetTitle(), &icon));
+    ++window_index;
   }
   return items;
 }
 
-void ExtensionAppWindowLauncherItemController::OnWindowRemoved(
-    ui::BaseWindow* window) {
-  WindowToAppWindow::iterator it = window_to_app_window_.find(window);
-  if (it == window_to_app_window_.end()) {
-    NOTREACHED();
-    return;
-  }
-
-  window_to_app_window_.erase(it);
+void ExtensionAppWindowLauncherItemController::ExecuteCommand(
+    uint32_t command_id,
+    int32_t event_flags) {
+  launcher_controller()->ActivateShellApp(app_id(), command_id);
 }