SPM: Reduce the number of pixels sent to Ash for window icons.

There are two icon properties, kAppIcon and kWindowIcon. For the
purposes of Ash, these are used interchangeably (kAppIcon is preferred
and kWindowIcon is a fallback). Thus we can skip sending kWindowIcon.
This should cut down on memory usage by half.

Further, the only place that the icon is used for single process
mash is in overview mode, which displays the icon at 24dip. We can
downscale the icon before sending it over. For tabbed browser windows,
that's 24dip*24dip instead of 192px * 192px.

Since the icons are set on the content window as well as the root
window, they are each copied to the window service twice. We do need
to transport the icons that are set on the root window, but not on
the content window. PropertyConverter doesn't distinguish on the window
type, so new keys are introduced for the root window: kAppIconSmallKey
and kAppIconLargeKey. These are the only ones that are sent to the WS.
They are only used in Mash. kAppIconKey and kWindowIconKey are retained
for use in the browser, e.g. by AppWindowLauncherItemController.
kAppIconLargeKey is only set by the task manager and used by
ShelfWindowWatcher for the shelf icon.

The net effect is that every browser window goes from allocating
192x192px four times, to 24x24dip once. According to chrome://tracing
this lops about 4MiB off malloc'd memory in the browser process when
there are 10 browser windows open.

Bug: 938105

Change-Id: I7bc04784ff3fb1d0857219eae2184542b1f14fd4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504164Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Auto-Submit: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638622}

ash/mus_property_mirror_ash_unittest.cc

@@ -59,18 +59,6 @@ TEST_F(MusPropertyMirrorAshTest, OwnedProperties) {
             *window_2->GetProperty(aura::client::kTitleKey));
   EXPECT_NE(window_1->GetProperty(aura::client::kTitleKey),
             window_2->GetProperty(aura::client::kTitleKey));
-
-  window_1->ClearProperty(aura::client::kWindowIconKey);
-  EXPECT_EQ(nullptr, window_1->GetProperty(aura::client::kWindowIconKey));
-  window_2->ClearProperty(aura::client::kWindowIconKey);
-  EXPECT_EQ(nullptr, window_2->GetProperty(aura::client::kWindowIconKey));
-  window_1->SetProperty(aura::client::kWindowIconKey, new gfx::ImageSkia());
-  EXPECT_NE(nullptr, window_1->GetProperty(aura::client::kWindowIconKey));
-  mus_property_mirror_ash.MirrorPropertyFromWidgetWindowToRootWindow(
-      window_1.get(), window_2.get(), aura::client::kWindowIconKey);
-  EXPECT_NE(nullptr, window_2->GetProperty(aura::client::kWindowIconKey));
-  EXPECT_NE(window_1->GetProperty(aura::client::kWindowIconKey),
-            window_2->GetProperty(aura::client::kWindowIconKey));
 }
 
 }  // namespace ash

ash/public/cpp/mus_property_mirror_ash.cc

@@ -50,8 +50,6 @@ void MusPropertyMirrorAsh::MirrorPropertyFromWidgetWindowToRootWindow(
   } else if (key == kWindowPinTypeKey) {
     ash::mojom::WindowPinType value = window->GetProperty(kWindowPinTypeKey);
     root_window->SetProperty(kWindowPinTypeKey, value);
-  } else if (key == aura::client::kAppIconKey) {
-    MirrorOwnedProperty(window, root_window, aura::client::kAppIconKey);
   } else if (key == kRestoreBoundsOverrideKey) {
     MirrorOwnedProperty(window, root_window, kRestoreBoundsOverrideKey);
   } else if (key == kRestoreWindowStateTypeOverrideKey) {
@@ -73,9 +71,6 @@ void MusPropertyMirrorAsh::MirrorPropertyFromWidgetWindowToRootWindow(
   } else if (key == aura::client::kTopViewInset) {
     root_window->SetProperty(aura::client::kTopViewInset,
                              window->GetProperty(aura::client::kTopViewInset));
-
-  } else if (key == aura::client::kWindowIconKey) {
-    MirrorOwnedProperty(window, root_window, aura::client::kWindowIconKey);
   } else if (key == kFrameActiveColorKey) {
     root_window->SetProperty(kFrameActiveColorKey,
                              window->GetProperty(kFrameActiveColorKey));

ash/shelf/shelf_window_watcher.cc

@@ -65,7 +65,9 @@ void UpdateShelfItemForWindow(ShelfItem* item, aura::Window* window) {
   }
 
   // Prefer app icons over window icons, they're typically larger.
-  gfx::ImageSkia* image = window->GetProperty(aura::client::kAppIconKey);
+  gfx::ImageSkia* image = window->GetProperty(aura::client::kAppIconLargeKey);
+  if (!image || image->isNull())
+    image = window->GetProperty(aura::client::kAppIconKey);
   if (!image || image->isNull())
     image = window->GetProperty(aura::client::kWindowIconKey);
   if (!image || image->isNull()) {

ash/wm/overview/caption_container_view.cc

@@ -243,7 +243,11 @@ CaptionContainerView::CaptionContainerView(views::ButtonListener* listener,
           kHorizontalLabelPaddingDp));
   AddChildWithLayer(listener_button_, header_view_);
 
-  gfx::ImageSkia* icon = window->GetProperty(aura::client::kAppIconKey);
+  // Prefer kAppIconSmallKey (set by the client in Mash), then kAppIconKey and
+  // kWindowIconKey (set for client windows in classic Ash but not Mash).
+  gfx::ImageSkia* icon = window->GetProperty(aura::client::kAppIconSmallKey);
+  if (!icon || icon->size().IsEmpty())
+    icon = window->GetProperty(aura::client::kAppIconKey);
   if (!icon || icon->size().IsEmpty())
     icon = window->GetProperty(aura::client::kWindowIconKey);
   if (icon && !icon->size().IsEmpty()) {

chrome/browser/ui/views/task_manager_view.cc

@@ -39,6 +39,7 @@
 #include "chrome/grit/theme_resources.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/gfx/image/image_skia.h"
 #endif  // defined(OS_CHROMEOS)
 
@@ -102,10 +103,16 @@ task_manager::TaskManagerTableModel* TaskManagerView::Show(Browser* browser) {
   const ash::ShelfID shelf_id(kTaskManagerId);
   window->SetProperty(ash::kShelfIDKey, new std::string(shelf_id.Serialize()));
   window->SetProperty<int>(ash::kShelfItemTypeKey, ash::TYPE_DIALOG);
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  gfx::ImageSkia* icon = rb.GetImageSkiaNamed(IDR_ASH_SHELF_ICON_TASK_MANAGER);
-  // The new gfx::ImageSkia instance is owned by the window itself.
-  window->SetProperty(aura::client::kWindowIconKey, new gfx::ImageSkia(*icon));
+  // For classic Ash, GetWindowIcon() is sufficient. For Mash, the task manager
+  // specifically needs to set a large app icon for the benefit of
+  // ShelfWindowWatcher.
+  if (features::IsUsingWindowService()) {
+    window->GetRootWindow()->SetProperty(
+        aura::client::kAppIconLargeKey,
+        new gfx::ImageSkia(
+            *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+                IDR_ASH_SHELF_ICON_TASK_MANAGER)));
+  }
 #endif
   return g_task_manager_view->table_model_.get();
 }
@@ -183,6 +190,15 @@ base::string16 TaskManagerView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_TASK_MANAGER_TITLE);
 }
 
+gfx::ImageSkia TaskManagerView::GetWindowIcon() {
+#if defined(OS_CHROMEOS)
+  return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+      IDR_ASH_SHELF_ICON_TASK_MANAGER);
+#else
+  return views::DialogDelegateView::GetWindowIcon();
+#endif
+}
+
 std::string TaskManagerView::GetWindowName() const {
   return prefs::kTaskManagerWindowPlacement;
 }

chrome/browser/ui/views/task_manager_view.h

@@ -60,6 +60,7 @@ class TaskManagerView : public TableViewDelegate,
   bool CanMinimize() const override;
   bool ExecuteWindowsCommand(int command_id) override;
   base::string16 GetWindowTitle() const override;
+  gfx::ImageSkia GetWindowIcon() override;
   std::string GetWindowName() const override;
   bool Accept() override;
   bool Close() override;

services/ws/public/mojom/window_manager.mojom

@@ -63,8 +63,11 @@ interface WindowManager {
   // A boolean determining whether animations are disabled for the window.
   const string kAnimationsDisabled_Property = "prop:animations-disabled";
 
-  // The application icon; typically larger for shelf icons, etc. Type: SkBitmap
-  const string kAppIcon_Property = "prop:app-icon";
+  // A large version of the application icon. Type: SkBitmap
+  const string kAppIconLarge_Property = "prop:app-icon-large";
+
+  // A small version of the application icon. Type: SkBitmap
+  const string kAppIconSmall_Property = "prop:app-icon-small";
 
   // The Android Java-style package name for an ARC++ window, such as
   // "com.google.Photos". Type: mojom::String.
@@ -153,9 +156,6 @@ interface WindowManager {
   // aura::client::kWindowCornerRadiusKey. Type: int.
   const string kWindowCornerRadius_Property = "prop:window-corner-radius";
 
-  // The window icon; typically 16x16 for titlebars. Type: SkBitmap
-  const string kWindowIcon_Property = "prop:window-icon";
-
   // The window's title. Maps to aura::client::kTitleKey. Type: mojom::String
   const string kWindowTitle_Property = "prop:window-title";
 

ui/aura/client/aura_constants.cc

@@ -44,6 +44,8 @@ DEFINE_UI_CLASS_PROPERTY_KEY(bool, kActivateOnPointerKey, true)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kAlwaysOnTopKey, false)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kAnimationsDisabledKey, false)
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::ImageSkia, kAppIconKey, nullptr)
+DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::ImageSkia, kAppIconLargeKey, nullptr)
+DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::ImageSkia, kAppIconSmallKey, nullptr)
 DEFINE_UI_CLASS_PROPERTY_KEY(int, kAppType, 0)
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::SizeF, kAspectRatio, nullptr)
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::ImageSkia, kAvatarIconKey, nullptr)

ui/aura/client/aura_constants.h

@@ -49,8 +49,19 @@ AURA_EXPORT extern const WindowProperty<bool>* const kAlwaysOnTopKey;
 AURA_EXPORT extern const WindowProperty<bool>* const kAnimationsDisabledKey;
 
 // A property key to store the app icon, typically larger for shelf icons, etc.
+// This is not transported to the window service.
 AURA_EXPORT extern const WindowProperty<gfx::ImageSkia*>* const kAppIconKey;
 
+// A property key to store a large version of the app icon, which is
+// transported to the window service.
+AURA_EXPORT extern const WindowProperty<gfx::ImageSkia*>* const
+    kAppIconLargeKey;
+
+// A property key to store a smaller version of the app icon, which is
+// transported to the window service.
+AURA_EXPORT extern const WindowProperty<gfx::ImageSkia*>* const
+    kAppIconSmallKey;
+
 // A property key to store the type of window that will be used to record
 // pointer metrics. See AppType in ash/public/cpp/app_types.h for more details.
 AURA_EXPORT extern const WindowProperty<int>* const kAppType;

ui/aura/mus/property_converter.cc

@@ -72,10 +72,10 @@ PropertyConverter::CreateAcceptAnyValueCallback() {
 
 PropertyConverter::PropertyConverter() {
   // Add known aura properties with associated mus properties.
-  RegisterImageSkiaProperty(client::kAppIconKey,
-                            ws::mojom::WindowManager::kAppIcon_Property);
-  RegisterImageSkiaProperty(client::kWindowIconKey,
-                            ws::mojom::WindowManager::kWindowIcon_Property);
+  RegisterImageSkiaProperty(client::kAppIconLargeKey,
+                            ws::mojom::WindowManager::kAppIconLarge_Property);
+  RegisterImageSkiaProperty(client::kAppIconSmallKey,
+                            ws::mojom::WindowManager::kAppIconSmall_Property);
   RegisterPrimitiveProperty(client::kAlwaysOnTopKey,
                             ws::mojom::WindowManager::kAlwaysOnTop_Property,
                             CreateAcceptAnyValueCallback());

ui/views/mus/desktop_window_tree_host_mus.cc

@@ -25,6 +25,7 @@
 #include "ui/events/gestures/gesture_recognizer_observer.h"
 #include "ui/gfx/geometry/dip_util.h"
 #include "ui/gfx/geometry/vector2d_conversions.h"
+#include "ui/gfx/image/image_skia_operations.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/corewm/tooltip_aura.h"
 #include "ui/views/mus/cursor_manager_owner.h"
@@ -897,7 +898,20 @@ void DesktopWindowTreeHostMus::SetAspectRatio(const gfx::SizeF& aspect_ratio) {
 
 void DesktopWindowTreeHostMus::SetWindowIcons(const gfx::ImageSkia& window_icon,
                                               const gfx::ImageSkia& app_icon) {
-  NativeWidgetAura::AssignIconToAuraWindow(window(), window_icon, app_icon);
+  // In Ash, the app icon is always used in preference to the window icon, so
+  // ignore the window icon. The max size that ash needs is 24dip, which is
+  // kIconSize in caption_container_view.cc.
+  constexpr gfx::Size kMaxUsefulAppIconSize(24, 24);
+  DCHECK_EQ(app_icon.width(), app_icon.height());
+  gfx::ImageSkia app_icon_resized =
+      app_icon.width() > kMaxUsefulAppIconSize.width()
+          ? gfx::ImageSkiaOperations::CreateResizedImage(
+                app_icon, skia::ImageOperations::RESIZE_BEST,
+                kMaxUsefulAppIconSize)
+          : app_icon;
+
+  window()->GetRootWindow()->SetProperty(aura::client::kAppIconSmallKey,
+                                         new gfx::ImageSkia(app_icon_resized));
 }
 
 void DesktopWindowTreeHostMus::InitModalType(ui::ModalType modal_type) {

ui/views/mus/mus_client.cc

@@ -246,22 +246,6 @@ MusClient::ConfigurePropertiesFromParams(
           mojo::ConvertTo<TransportType>(
               init_params.delegate->GetWindowTitle());
     }
-
-    // TODO(crbug.com/667566): Support additional scales or gfx::Image[Skia].
-    gfx::ImageSkia app_icon = init_params.delegate->GetWindowAppIcon();
-    SkBitmap app_bitmap = app_icon.GetRepresentation(1.f).GetBitmap();
-    if (!app_bitmap.isNull()) {
-      properties[WindowManager::kAppIcon_Property] =
-          mojo::ConvertTo<TransportType>(app_bitmap);
-    }
-
-    // TODO(crbug.com/667566): Support additional scales or gfx::Image[Skia].
-    gfx::ImageSkia window_icon = init_params.delegate->GetWindowIcon();
-    SkBitmap window_bitmap = window_icon.GetRepresentation(1.f).GetBitmap();
-    if (!window_bitmap.isNull()) {
-      properties[WindowManager::kWindowIcon_Property] =
-          mojo::ConvertTo<TransportType>(window_bitmap);
-    }
   }
 
   return properties;