Eventually expire favicons stored via LargeIconService::GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache()
Assigning this to Jan just so that it is on your team's radar
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/97b537bf1aba5ca6e106920faa65f140dd80df32 commit 97b537bf1aba5ca6e106920faa65f140dd80df32 Author: jkrcal <jkrcal@chromium.org> Date: Wed Jun 21 17:27:26 2017 [Thumbnails DB] Allow setting last_requested time when accessing favicons. This CL deals with favicons that are added without the user visiting the corresponding page. The CL introduces the notion of "on-demand" favicons for them: 1) The CL reuses (and renames) an existing function in FaviconService for storing such on-demand favicons and pushes this concept further to ThumbnailDatabase. 2) The CL adds a new function to touch (i.e. update last_requested time-stamp) for "on-deman" favicons. The functionality to clear unused on-demand favicons is left for a further CL (https://codereview.chromium.org/2903573002/). Similarly, calling TouchOnDemandFavicon from LargeIconService is also left for a later CL. BUG= 709471 Review-Url: https://codereview.chromium.org/2856873002 Cr-Commit-Position: refs/heads/master@{#481232} [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/favicon/core/favicon_service.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/favicon/core/favicon_service_impl.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/favicon/core/favicon_service_impl.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/favicon/core/large_icon_service.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/favicon/core/large_icon_service_unittest.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/favicon/core/test/mock_favicon_service.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/android/favicon_sql_handler.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/history_backend.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/history_backend.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/history_backend_unittest.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/history_service.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/history_service.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/history_types.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/thumbnail_database.cc [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/thumbnail_database.h [modify] https://crrev.com/97b537bf1aba5ca6e106920faa65f140dd80df32/components/history/core/browser/thumbnail_database_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ade20c97acc5082652e22657e22b781a3bebcc5 commit 5ade20c97acc5082652e22657e22b781a3bebcc5 Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Jul 11 13:03:00 2017 [LargeIconService] Touch icons from google server upon reading This CL marks on-demand favicons from the Google server as accessed upon reading from them favicon database. In order to do so: - it adds LargeIconService::TouchIconFromGoogleServer() so that the clients of this service do not have to keep the handle to FaviconService as well; - it reveals icon_url info into the LargeIconImageResult struct. This is the last CL in the series for removing old on-demand favicons. Bug: 709471 Change-Id: I92d51be4b1bed14b6e6e06636e9aa7996b1fc060 Reviewed-on: https://chromium-review.googlesource.com/563402 Commit-Queue: Jan Krcal <jkrcal@chromium.org> Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Reviewed-by: Friedrich Horschig <fhorschig@chromium.org> Cr-Commit-Position: refs/heads/master@{#485612} [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/favicon/core/large_icon_service.cc [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/favicon/core/large_icon_service.h [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/favicon/core/large_icon_service_unittest.cc [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/favicon_base/favicon_types.cc [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/favicon_base/favicon_types.h [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/ntp_snippets/content_suggestions_service.cc [modify] https://crrev.com/5ade20c97acc5082652e22657e22b781a3bebcc5/components/ntp_tiles/icon_cacher_impl.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9946c3ce2b116a954f25a63a19be1ba414b3be53 commit 9946c3ce2b116a954f25a63a19be1ba414b3be53 Author: jkrcal <jkrcal@chromium.org> Date: Tue Jul 25 15:23:07 2017 [Thumbnails DB] Add functionality to clear unused on-demand favicons. The preceding CL 2856873002 adds a concept of on-demand favicons into ThumbnailsDatabase that should get evicted based on last_requested timestamp. This CL adds the functionality to clear on-demand favicons that have not been used for long. BUG= 709471 Review-Url: https://codereview.chromium.org/2903573002 Cr-Commit-Position: refs/heads/master@{#489307} [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/expire_history_backend.cc [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/expire_history_backend.h [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/expire_history_backend_unittest.cc [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/history_types.cc [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/history_types.h [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/thumbnail_database.cc [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/thumbnail_database.h [modify] https://crrev.com/9946c3ce2b116a954f25a63a19be1ba414b3be53/components/history/core/browser/thumbnail_database_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/961edb38b6dc0099f7921e5ae612447852316898 commit 961edb38b6dc0099f7921e5ae612447852316898 Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Aug 14 12:01:42 2017 [Favicons] Automatic eviction of on-demand favicons in bot config This CL adds to fieldtrial_testing_config.json a feature related to a technical launch https://crbug.com/752413. Bug: 709471 Change-Id: I64ac78d1c8ced4e481c9854c47d7127a32bba302 Reviewed-on: https://chromium-review.googlesource.com/602127 Reviewed-by: Steven Holte <holte@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#494036} [modify] https://crrev.com/961edb38b6dc0099f7921e5ae612447852316898/testing/variations/fieldtrial_testing_config.json
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8390b2bbbc0d8705cc73a0308479456dc039afb commit d8390b2bbbc0d8705cc73a0308479456dc039afb Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Aug 14 16:03:09 2017 [Remote suggestions] Mark favicons stored by FaviconHelper as on-demand This CL updates the old code path for fetching and storing favicons for content suggestions so that it uses a new mechanism for automatic removal of old (on-demand) favicons. In detail, the CL: - updates the code that caches such downloaded icons to mark them as on-demand icons (see https://codereview.chromium.org/2903573002/); - removes the isTemporary parameter from the function because with the new removal mechanism, all icons downloaded by this method are by definition on-demand and temporary; - adds a new method in FaviconHelper that touches the downloaded icon and thus postpones its automatic eviction. Before this CL, these icons were never cleared (unless you visit the home page of the suggested article causing the icons to be re-downloaded and later removed by the standard history expiration). Bug: 709471 Change-Id: I307e2c39c3b29af98270f1e9e2a6686a843489df Reviewed-on: https://chromium-review.googlesource.com/597857 Commit-Queue: Jan Krcal <jkrcal@chromium.org> Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Cr-Commit-Position: refs/heads/master@{#494059} [modify] https://crrev.com/d8390b2bbbc0d8705cc73a0308479456dc039afb/chrome/android/java/src/org/chromium/chrome/browser/favicon/FaviconHelper.java [modify] https://crrev.com/d8390b2bbbc0d8705cc73a0308479456dc039afb/chrome/android/java/src/org/chromium/chrome/browser/suggestions/ImageFetcher.java [modify] https://crrev.com/d8390b2bbbc0d8705cc73a0308479456dc039afb/chrome/browser/android/favicon_helper.cc [modify] https://crrev.com/d8390b2bbbc0d8705cc73a0308479456dc039afb/chrome/browser/android/favicon_helper.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3af4ceafc931897600e24f3609cb9c40e1ea8813 commit 3af4ceafc931897600e24f3609cb9c40e1ea8813 Author: Jan Krcal <jkrcal@chromium.org> Date: Wed Sep 27 16:43:59 2017 [On demand icons] Regularly clean-up only strictly on-demand icons. This CL changes the deletion procedure to actually delete only icons that have been explicitly stored as on-demand icons. One reason is that the cleanup procedure is a potential culprit for http://crbug.com/758272, where some icons disappear too aggressively. Another reason is a performace regressions (https://bugs.chromium.org/p/chromium/issues/detail?id=756101#c3). The clean-up of previously stored icons would be better as an on-off procedure (mastiz@ is looking into that). Bug: 709471 Change-Id: If46fb9f8ef912e8103d78209c86cacde02eae966 Reviewed-on: https://chromium-review.googlesource.com/670666 Commit-Queue: Jan Krcal <jkrcal@chromium.org> Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#504703} [modify] https://crrev.com/3af4ceafc931897600e24f3609cb9c40e1ea8813/components/history/core/browser/thumbnail_database.cc [modify] https://crrev.com/3af4ceafc931897600e24f3609cb9c40e1ea8813/components/history/core/browser/thumbnail_database_unittest.cc
This is in the process of being launched.
Comment 1 by pkotw...@chromium.org
, Apr 7 2017Status: Assigned (was: Untriaged)