[Thumbnails DB] Allow setting last_requested time when accessing favicons.

components/history/core/browser/thumbnail_database.h

Index: components/history/core/browser/thumbnail_database.h
diff --git a/components/history/core/browser/thumbnail_database.h b/components/history/core/browser/thumbnail_database.h
index ec21369d43830d0025471da7bfc3c42dd1803a0d..7c6f1bab0949b9bcd3134f667c6e31696d0174fc 100644
--- a/components/history/core/browser/thumbnail_database.h
+++ b/components/history/core/browser/thumbnail_database.h
@@ -87,37 +87,36 @@ class ThumbnailDatabase {
                         scoped_refptr<base::RefCountedMemory>* png_icon_data,
                         gfx::Size* pixel_size);
 
-  // Adds a bitmap component at |pixel_size| for the favicon with |icon_id|.
-  // Only favicons representing a .ico file should have multiple favicon bitmaps
-  // per favicon.
+  // Adds a bitmap component of |type| at |pixel_size| for the favicon with
+  // |icon_id|. Only favicons representing a .ico file should have multiple
+  // favicon bitmaps per favicon.
   // |icon_data| is the png encoded data.
-  // The |time| indicates the access time, and is used to detect when the
-  // favicon should be refreshed.
+  // The |type| indicates how the lifetime of this icon should be managed.
+  // The |time| is used for lifetime management of the bitmap (should be Now()).
   // |pixel_size| is the pixel dimensions of |icon_data|.
   // Returns the id of the added bitmap or 0 if unsuccessful.
   FaviconBitmapID AddFaviconBitmap(
       favicon_base::FaviconID icon_id,
       const scoped_refptr<base::RefCountedMemory>& icon_data,
+      FaviconBitmapType type,
       base::Time time,
       const gfx::Size& pixel_size);
 
   // Sets the bitmap data and the last updated time for the favicon bitmap at
-  // |bitmap_id|.
+  // |bitmap_id|. Should not be called for bitmaps of type ON_DEMAND as they
+  // should never get updated (the call silently changes the type to ON_VISIT).
   // Returns true if successful.
   bool SetFaviconBitmap(FaviconBitmapID bitmap_id,
                         scoped_refptr<base::RefCountedMemory> bitmap_data,
                         base::Time time);
 
-  // Sets the last updated time for the favicon bitmap at |bitmap_id|.
-  // Returns true if successful.
+  // Sets the last_updated time for the favicon bitmap at |bitmap_id|. Should
+  // not be called for bitmaps of type ON_DEMAND as last_updated time is only
+  // tracked for ON_VISIT bitmaps (the call silently changes the type to
+  // ON_VISIT). Returns true if successful.
   bool SetFaviconBitmapLastUpdateTime(FaviconBitmapID bitmap_id,
                                       base::Time time);
 
-  // Sets the last requested time for the favicon bitmap at |bitmap_id|.
-  // Returns true if successful.
-  bool SetFaviconBitmapLastRequestedTime(FaviconBitmapID bitmap_id,
-                                         base::Time time);
-
   // Deletes the favicon bitmap with |bitmap_id|.
   // Returns true if successful.
   bool DeleteFaviconBitmap(FaviconBitmapID bitmap_id);
@@ -128,6 +127,16 @@ class ThumbnailDatabase {
   // of the bitmaps for |icon_id| to be out of date.
   bool SetFaviconOutOfDate(favicon_base::FaviconID icon_id);
 
+  // Mark all bitmaps of type ON_DEMAND at |icon_url| as requested at |time|.
+  // This postpones their automatic eviction from the database. Not all calls
+  // end up in a write into the DB:
+  // - it is no-op if the bitmaps are not of type ON_DEMAND;
+  // - the updates of the last requested time have limited frequency for each

[pkotwicz, 2017/06/07 17:40:53]

Nit: Add quotes around 'last requested time'

[jkrcal, 2017/06/09 16:38:38]

Done.

+  //   particular bitmap (e.g. once per week). This limits the overhead of
+  //   cache management for on-demand favicons.
+  // Returns true if successful.
+  bool TouchOnDemandFavicon(const GURL& icon_url, base::Time time);
+
   // Returns the id of the entry in the favicon database with the specified url
   // and icon type.
   // Returns 0 if no entry exists for the specified url.
@@ -146,7 +155,7 @@ class ThumbnailDatabase {
                                      favicon_base::IconType icon_type);
 
   // Adds a favicon with a single bitmap. This call is equivalent to calling
-  // AddFavicon and AddFaviconBitmap.
+  // AddFavicon and AddFaviconBitmap (of type FaviconBitmapType::ON_VISIT).

[pkotwicz, 2017/06/07 17:40:53]

Can you please add the type as a parameter?

[jkrcal, 2017/06/09 16:38:38]

Hm, I wanted to avoid it because it has many call sites. Anyway, I agree that
this way it is clearer.

   favicon_base::FaviconID AddFavicon(
       const GURL& icon_url,
       favicon_base::IconType icon_type,