New issue
Advanced search Search tips

Issue 836044 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature



Sign in to add a comment

Add icons to App Registry

Project Member Reported by tbuck...@chromium.org, Apr 23 2018

Issue description

The App Registry should store Linux app icons for use in Launcher, Shelf, etc.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/43947d3f697e470c870ff0bbcdf06c921b3fe64c

commit 43947d3f697e470c870ff0bbcdf06c921b3fe64c
Author: Jeffrey Kardatzke <jkardatzke@google.com>
Date: Thu Apr 26 03:16:57 2018

vm_tools: Enhance garcon icon searching when scale factor set

If garcon gets a request for an icon with a scale factor, but it can't
find one, then it will search for an icon with no scale factor and
return that instead. Currently I don't see any icons getting installed
with scale factors, and by default (at least on eve) we have a scale
factor of 2 and doing the icon scaling ourselves is better than no icons
at all.

BUG= chromium:836044 
TEST=Verified garcon returns alternate icons

Change-Id: Ie674e97189cd94a3d69dd2126be7bec6911efabe
Reviewed-on: https://chromium-review.googlesource.com/1028585
Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com>
Tested-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/43947d3f697e470c870ff0bbcdf06c921b3fe64c/vm_tools/garcon/icon_finder.h
[modify] https://crrev.com/43947d3f697e470c870ff0bbcdf06c921b3fe64c/vm_tools/garcon/icon_finder.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 26 2018

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

commit 339390d872fe1cb26b77b17c7c55439b84429903
Author: Jeffrey Kardatzke <jkardatzke@google.com>
Date: Thu Apr 26 22:26:57 2018

Add launcher icons for Crostini applications

This updates the Crostini launcher integration so that it includes the
icons. Icons are stored in the user's storage directory and the file path
for the icons is generated based on the app_id. We use the
CrostiniManager for requesting the icons from the Linux container.
Resizing of returned icons is performed if we do not get the size/scale
factor we requested from Linux.

Bug:  836044 
Test: Verified icons load, handles scaling and app removal/addition
Change-Id: I37c4297af6219ddadc200598cec52d7c40140758
Reviewed-on: https://chromium-review.googlesource.com/1028588
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554172}
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc
[add] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_icon.h
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_icon_loader.cc
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_icon_loader.h
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_item.cc
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_item.h
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc
[modify] https://crrev.com/339390d872fe1cb26b77b17c7c55439b84429903/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.h

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/328a8d758ab4aedf6d1803a4ded2e226305528c0

commit 328a8d758ab4aedf6d1803a4ded2e226305528c0
Author: Jeffrey Kardatzke <jkardatzke@google.com>
Date: Fri Apr 27 19:22:39 2018

vm_tools: Fixed icon path searching and size limit

These changes are required for VSCode's icon to show up. We were not
also searching /usr/share/pixmaps as a last resort which we should
according to the spec. We also had a very low limit set for max icon
size of 64k, that's now 1MB which is more reasonable.

BUG= chromium:836044 
TEST=Verified VSCode icon shows up properly

Change-Id: I992459bbe2a79a8b41aab8f31840cd88458d6db0
Reviewed-on: https://chromium-review.googlesource.com/1031378
Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com>
Tested-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Tim Zheng <timzheng@chromium.org>

[modify] https://crrev.com/328a8d758ab4aedf6d1803a4ded2e226305528c0/vm_tools/garcon/service_impl.cc
[modify] https://crrev.com/328a8d758ab4aedf6d1803a4ded2e226305528c0/vm_tools/garcon/icon_finder.cc

Comment 5 by vapier@chromium.org, May 17 2018

Labels: -Restrict-View-Google

Sign in to add a comment