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

components/history/core/browser/thumbnail_database.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_HISTORY_CORE_BROWSER_THUMBNAIL_DATABASE_H_
 #define COMPONENTS_HISTORY_CORE_BROWSER_THUMBNAIL_DATABASE_H_
 
 #include <vector>
 
 #include "base/gtest_prod_util.h"
@@ skipping 69 matching lines @@
                          std::vector<FaviconBitmap>* favicon_bitmaps);
 
   // Gets the last updated time, bitmap data, and pixel size of the favicon
   // bitmap at |bitmap_id|. Returns true if successful.
   bool GetFaviconBitmap(FaviconBitmapID bitmap_id,
                         base::Time* last_updated,
                         base::Time* last_requested,
                         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);
 
   // Favicons ------------------------------------------------------------------
 
   // Sets the the favicon as out of date. This will set |last_updated| for all
   // 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.
   favicon_base::FaviconID GetFaviconIDForFaviconURL(
       const GURL& icon_url,
       favicon_base::IconType icon_type);
 
   // Gets the icon_url, icon_type and sizes for the specified |icon_id|.
   bool GetFaviconHeader(favicon_base::FaviconID icon_id,
                         GURL* icon_url,
                         favicon_base::IconType* icon_type);
 
   // Adds favicon with |icon_url|, |icon_type| and |favicon_sizes| to the
   // favicon db, returning its id.
   favicon_base::FaviconID AddFavicon(const GURL& icon_url,
                                      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,
       const scoped_refptr<base::RefCountedMemory>& icon_data,
       base::Time time,
       const gfx::Size& pixel_size);
 
   // Delete the favicon with the provided id. Returns false on failure
   bool DeleteFavicon(favicon_base::FaviconID id);
 
@@ skipping 103 matching lines @@
 
   sql::Connection db_;
   sql::MetaTable meta_table_;
 
   HistoryBackendClient* backend_client_;
 };
 
 }  // namespace history
 
 #endif  // COMPONENTS_HISTORY_CORE_BROWSER_THUMBNAIL_DATABASE_H_