Do we get any kind of event on this failure, or did it just not show up? (If the latter, I don't know how we can add UMA for this event.)

I believe this happens if the network request for the icon fails for some reason, which is easy to track.

You said it's "easy to track"; can you point at where in the code you would track this.

Then you can assign it back to me and I can add UMA.

https://cs.chromium.org/chromium/src/chrome/browser/extensions/bookmark_app_helper.cc?rcl=646431cd2652536a6b1e5556bbeb53704bb75225&l=285

You just need to log something in the !success case.

You may also want to consider adding a stat for figuring out how often icons are generated in general, and why. This would help to debug the different flavors of icon failure. Cases I can think of are:
- No icon urls specified on sync
- Icon download failure on sync
- Insufficient icon sizes on install

I think it's possible that case 2 creates an app with no icon urls, which perpetuates as case 1, ruining icons for sync clients thereafter.

It's also possible that my old bookmark apps don't sync their icon URLs, so they're borked in the long-term (but I don't think this needs to be fixed).

67 has branched, moving bugs over to 68.

Yoink.

Looks like icons that get generated (and don't have a URL) are added to the WebApplicationInfo in BookmarkAppHelper::UpdateWebAppIconsWithoutChangingLinks() (despite the name). This could be a source of invalid URLs that get synced.
The sync push logic in ExtensionSyncData::ToAppSpecifics() doesn't sanitise the linked_icon.urls.

mgiuca has examples of bookmark apps that fail to sync their icons though this is likely due to  issue 601646  which was fixed by https://chromium-review.googlesource.com/c/chromium/src/+/656867. We confirmed this by checking the chrome:sync-internals data and that the installation timestamp was prior to the bug fix.

I wonder if we can just reload the site's manifest instead of caching it ourselves.

Looks like in ConvertWebAppToExtension() we strip out any linked icon urls that aren't valid so they shouldn't get synced, will add a DCHECK for this in the sync proto creation.

One problem with our current setup even if everything goes smoothly on Chrome's end is that the site itself may change its icon URLs. We should log the status codes of the icon downloads to monitor the extent of this problem.

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

commit ddace756be8170a976bdf1b6c8779dfe2d5c47b6
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue May 15 08:34:10 2018

Use range based for loops in bookmark_app_helper.cc

This is a small code clean up to use ranged based for loops instead of
manual iterator logic. This reduces the line wrapping and makes
the type of the iterated values more obvious in some cases.

Bug:  809816 
Change-Id: I4f6e856a4dcd052d3d7ec5befd5792b9c1f09602
Reviewed-on: https://chromium-review.googlesource.com/1058987
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558641}
[modify] https://crrev.com/ddace756be8170a976bdf1b6c8779dfe2d5c47b6/chrome/browser/extensions/bookmark_app_helper.cc

#12 I don't really think we need to know the extent of this problem. It'll be fixed with the automatic update (which we already know we need to do).

Given that the code change is small, and that the end-to-end system is complex, I'd prefer having monitoring here anyway. If there's an uptick in graceful failures, we will have stats that capture when that happened. Is there any strong reason not to log this?

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

commit 7fe2485bf413c0cc864752cd2b91970d8852e5f0
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed May 16 06:41:18 2018

Assert that bookmark icon URLs are valid before syncing

Bug:  809816 

Change-Id: I4993e87ccc76ccc77e523412e82d07a662092044
Reviewed-on: https://chromium-review.googlesource.com/1058996
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558997}
[modify] https://crrev.com/7fe2485bf413c0cc864752cd2b91970d8852e5f0/chrome/browser/extensions/extension_sync_data.cc

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

commit 411ccdbc93c1911fbc68cdd3930a652b1a9d86f1
Author: Alan Cutter <alancutter@chromium.org>
Date: Fri May 18 07:23:08 2018

Log HTTP status code classes when downloading icons for bookmark apps

This CL adds a counter for the class of HTTP status codes returned by
attempts to download bookmark app icons for new vs synced bookmark apps.

Bookmark apps currently don't have a reliable way of synchronising updates
to a site's manifest icon configuration with the user's profile.
Having these get out of sync can result in obsolete icon URLs being used
for bookmark app icons due to the old URLs getting synced across devices.

Bug:  809816 
Change-Id: I55522a782b28aee436272b3d1b2a5598ea9df8c6
Reviewed-on: https://chromium-review.googlesource.com/1059981
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559825}
[modify] https://crrev.com/411ccdbc93c1911fbc68cdd3930a652b1a9d86f1/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/411ccdbc93c1911fbc68cdd3930a652b1a9d86f1/chrome/browser/extensions/favicon_downloader.cc
[modify] https://crrev.com/411ccdbc93c1911fbc68cdd3930a652b1a9d86f1/chrome/browser/extensions/favicon_downloader.h
[modify] https://crrev.com/411ccdbc93c1911fbc68cdd3930a652b1a9d86f1/chrome/browser/extensions/favicon_downloader_unittest.cc
[modify] https://crrev.com/411ccdbc93c1911fbc68cdd3930a652b1a9d86f1/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/411ccdbc93c1911fbc68cdd3930a652b1a9d86f1/tools/metrics/histograms/histograms.xml