Works fine on 71.0.3558/11098.0.0, so must be recent regression.

estade@,jamescook@ are there any recent changes to shelf icon stuff for mash?
kaznacheev@, I believe you were also working on shelf/applist icon stuff. lmk if you have an idea.

I have done some changes to icons recently, but these were specific to ARC++ and Chrome apps.
Just out of curiosity, does that repro on a newly created profile?

msw@, any recent shelf icon changes?

I haven't made any or reviewed any.

Er, really +msw

No, I'm not aware of any recent changes to shelf icons, sorry.
Is there any way to repro this (run Crostini) on linux-chromeos development builds?

I don't see any obvious suspects in the revision log between those versions, but there's one CL I might suggest looking at:
https://chromium.googlesource.com/chromium/src/+log/71.0.3558.0..71.0.3565.0?pretty=fuller&n=10000
https://chromium.googlesource.com/chromium/src/+/eaa9ecac07cfaab0eec81072334043830e886256

Maybe https://chromium-review.googlesource.com/c/chromium/src/+/1251961 which fixed icons not showing up in the suggestion chips and search. Caching the wrong icon size maybe?

yeah, that looks suspicious, thanks.

xiyuan@ can you take a look?

My CL changes how CrostiniAppIconLoader work and should not be on this code path. CrostiniAppItem uses CrostiniAppIcon to load and decode the icon. It should load a 64x64 icon. There is no icon cache AFAICT.

We cache resized icons based on their scale factor at profiledir/crostini.icons/icon_100p.png. Previously we always used the same size (relatedly the actual request to the container is still hard-coded to a fixed size: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/crostini/crostini_registry_service.cc?type=cs&q=crostini_registry&g=0&l=824), but of course if we use different icon sizes, a single file per scale factor won't suffice.

Tentatively assigning to jkardatzke...

Reverting (and re-enabling crostini) did fix it.

For shelf/app icon, we should just use largest icon, and for small one (in suggestion) it should use the closest.

btw crostini/nocturne is very unreliable on tot. (it didn't startup, crash during install etc, it didn't reboot properly etc).

Screenshots of what the icons look like as requested with the fix that is pending right now.

Deleted: Screenshot 2018-10-04 at 11.40.50 AM.png 649 KB

	Screenshot 2018-10-04 at 11.40.50 AM.png 649 KB View Download

Deleted: Screenshot 2018-10-04 at 11.41.03 AM.png 620 KB

	Screenshot 2018-10-04 at 11.41.03 AM.png 620 KB View Download

Deleted: Screenshot 2018-10-04 at 11.41.11 AM.png 648 KB

	Screenshot 2018-10-04 at 11.41.11 AM.png 648 KB View Download

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

commit ef7e0ad38e5ccd69b8a2521add71bb86d9104d79
Author: Jeffrey Kardatzke <jkardatzke@google.com>
Date: Thu Oct 04 19:07:43 2018

Don't cache scaled Crostini icons

Icons are now being rendered at more than one size, and previously we
were keeping one cached copy per scale factor at the requested icon
size. If a smaller variant of an icon was rendered, we would then update
the cached version to be that and lose resolution when that was loaded
and upscaled later. Now we just keep the one original size and scale on
the fly when needed.

BUG= chromium:891588 
TEST=Verified on eve icons render at multiple sizes correctly

Change-Id: I42ce8eeb774389115bccb7578988305521d98731
Reviewed-on: https://chromium-review.googlesource.com/c/1262396
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Cr-Commit-Position: refs/heads/master@{#596790}
[modify] https://crrev.com/ef7e0ad38e5ccd69b8a2521add71bb86d9104d79/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc

FWIW ARC++ recently went from single icon size to multiple sizes (https://chromium-review.googlesource.com/c/chromium/src/+/1154136).
The new implementation still caches the icons but encodes the dip size into the file name (along with the scale factor).

Users will probably need manual action if they were on an affected version - moving/renaming the /usr/share/applications folder and then restoring it after (say) 10 seconds should probably fix it. This would have the effect of deleting all the app entries (incl. cached icons) on the Chrome side and then re-adding them.

@jkardatzke/timloh is there anything we can do to auto-reset for affected users? I'm not sure how many folks will discover this and we still have a sizable dev channel population.

https://www.reddit.com/r/Crostini/comments/9lxwz0/extremely_pixelated_linux_app_icons_after_force/

I thought of a clean way to do this now (couldn't think of one last week when we were debating this). I'll try it out today and see if it works (it should) and then put in a new CL with that fix.

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

commit ad944ddf0de18374204af0ab8503c32629eaa886
Author: Jeffrey Kardatzke <jkardatzke@google.com>
Date: Mon Oct 08 22:37:43 2018

Fixed bad Crostini icons cached on some systems

This deals with an earlier bug we have already fixed that left some
users with low resolution icons cached for Crostini. This detects if the
icons in the cache are lower resolution than what we want to display and
then requests them from Linux again. It remember if it did this, because
there can be cases where only lower resolution icons exist and this will
prevent infinitely retrying.

Bug:  chromium:891588 
Test: Manually verified with eve
Change-Id: Ie786459af254db2ae5f876d0b72014420af88afe
Reviewed-on: https://chromium-review.googlesource.com/c/1269340
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Cr-Commit-Position: refs/heads/master@{#597713}
[modify] https://crrev.com/ad944ddf0de18374204af0ab8503c32629eaa886/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc
[modify] https://crrev.com/ad944ddf0de18374204af0ab8503c32629eaa886/chrome/browser/ui/app_list/crostini/crostini_app_icon.h

 Issue 893578  has been merged into this issue.