So far, on-demand favicons are stored only if there is no existing icon for given page_url. This does not work for bookmarks that mostly have 16x16 icons from sync. We should allow writing these on-demand icons even for bookmarks.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3bacaaf183589f9664670c5492e3ad28948bb7eb commit 3bacaaf183589f9664670c5492e3ad28948bb7eb Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Feb 20 08:37:24 2018 [Favicons] Writing on-demand favicons along with expired icons This CL enables writing on-demand favicons if all other icons are expired (and have a different type). The motivation is to be able to fetch on-demand favicons for bookmarked URLs that have an (expired) favicon from sync. Fetching higher-res icons for bookmarks from the Google favicon server is currently being implemented for iOS bookmarks UI. For the approach to work, the CL changes the MergeFavicon function to set all merged icons as expired, notably all favicons from sync. A follow-up CL will make LargeIconService:: GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache() check CanSetOnDemandFavicons() before issuing the request (and also rename the function to GetLargeIconFromGoogleServer()). Bug: 799447 Change-Id: I59f4e16c1e6e8a8bd754e122c98de93cd258bfea Reviewed-on: https://chromium-review.googlesource.com/852256 Commit-Queue: Jan Krcal <jkrcal@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Cr-Commit-Position: refs/heads/master@{#537737} [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/favicon/core/favicon_service.h [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/favicon/core/favicon_service_impl.cc [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/favicon/core/favicon_service_impl.h [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/favicon/core/large_icon_service_unittest.cc [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/favicon/core/test/mock_favicon_service.h [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/history_backend.cc [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/history_backend.h [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/history_backend_unittest.cc [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/history_service.cc [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/history_service.h [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/thumbnail_database.cc [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/thumbnail_database.h [modify] https://crrev.com/3bacaaf183589f9664670c5492e3ad28948bb7eb/components/history/core/browser/thumbnail_database_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c46882f1ee9b861130c86408aeebf946438e4ebe commit c46882f1ee9b861130c86408aeebf946438e4ebe Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Mar 12 12:45:32 2018 [LargeIconService] Call CanSetOnDemandFavicons() before the fetch This CL makes use of the new API function of FaviconService and thus avoids an unnecessary network request if the icon cannot be stored. Bug: 799447 Change-Id: I187ff46c3b56f895b168eaae04023280e261f1a9 Reviewed-on: https://chromium-review.googlesource.com/951763 Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Reviewed-by: Gayane Petrosyan <gayane@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#542466} [modify] https://crrev.com/c46882f1ee9b861130c86408aeebf946438e4ebe/components/favicon/core/large_icon_service.cc [modify] https://crrev.com/c46882f1ee9b861130c86408aeebf946438e4ebe/components/favicon/core/large_icon_service.h [modify] https://crrev.com/c46882f1ee9b861130c86408aeebf946438e4ebe/components/favicon/core/large_icon_service_unittest.cc [modify] https://crrev.com/c46882f1ee9b861130c86408aeebf946438e4ebe/components/favicon_base/favicon_types.h [modify] https://crrev.com/c46882f1ee9b861130c86408aeebf946438e4ebe/components/ntp_tiles/icon_cacher_impl_unittest.cc [modify] https://crrev.com/c46882f1ee9b861130c86408aeebf946438e4ebe/tools/metrics/histograms/enums.xml
Stepan, this blocker of crbug.com/697329 is now fixed. If you need more context, feel free to reach out to me!
Comment 1 by martiw@chromium.org
, Jan 19 2018