New issue
Advanced search Search tips

Issue 724292 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Refactor custom icon for extension and ARC apps.

Project Member Reported by khmel@chromium.org, May 18 2017

Issue description

Currently we have dedicated functionality to set custom icon for extension and ARC apps. It would be beneficial to implement unified way via aura::Window property and discard dup code from both item controllers. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ddcada88969fcf84e51511060588094ddbd54c8

commit 1ddcada88969fcf84e51511060588094ddbd54c8
Author: khmel <khmel@chromium.org>
Date: Fri Jun 02 19:47:09 2017

Handle app custom icon via aura::Window property.

This does refactoring of custom icon functionality for Chrome extension
app and for ARC app by unifying applying a custom icon. Before each app
class has own logic in window controller that applies the custom icon
for the shelf item. With this new CL custom icon is merged to concept
of window app icon property which is set by widget delegate. Base
AppWindowLauncherController observes changes of native window related
to app icon and applies the new window app icon as shelf icon.
This CL also moves loading of Chrome App icon from extensions
sub-project to chrome/browser (AppWindow->ChromeNativeAppWindowViews).
ChromeNativeAppWindowViews is only one consumer of loaded App icon.
From other side, chrome/browser has reach functionality to handle
Chrome app icons, including ChromeAppIcon.
With this CL ExtensionAppLauncher is de-facto deprecated in
ChromeLauncherController (to be removed in next CLs).

BUG= 724292 
TEST=Tests + Manually on device, including test apps to validate custom
     icons for Extension and ARC based apps.

Review-Url: https://codereview.chromium.org/2900783003
Cr-Commit-Position: refs/heads/master@{#476761}

[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/extensions/chrome_app_icon.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/apps/chrome_app_delegate.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/apps/chrome_app_delegate.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_deferred_launcher_controller.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_deferred_launcher_controller.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_window.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_window.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_window_launcher_item_controller.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/arc_app_window_launcher_item_controller.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc
[add] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/test/test_app_window_icon_observer.cc
[add] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/test/test_app_window_icon_observer.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/views/apps/chrome_native_app_window_views.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/browser/ui/views/apps/chrome_native_app_window_views.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/test/BUILD.gn
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/chrome/test/data/extensions/platform_apps/app_icon/test.js
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/components/exo/shell_surface.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/components/exo/shell_surface.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/browser/api/app_window/app_window_apitest.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/browser/app_window/app_delegate.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/browser/app_window/app_window.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/browser/app_window/app_window_registry.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/browser/app_window/app_window_registry.h
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/common/manifest_handlers/icons_handler.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/shell/browser/shell_app_delegate.cc
[modify] https://crrev.com/1ddcada88969fcf84e51511060588094ddbd54c8/extensions/shell/browser/shell_app_delegate.h

Comment 2 by khmel@chromium.org, Jun 2 2017

Labels: OS-Chrome
Status: Fixed (was: Untriaged)

Comment 3 by khmel@chromium.org, Jun 2 2017

Labels: M-61

Comment 4 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment