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

components/history/core/browser/thumbnail_database.cc

Index: components/history/core/browser/thumbnail_database.cc
diff --git a/components/history/core/browser/thumbnail_database.cc b/components/history/core/browser/thumbnail_database.cc
index d9bc22defb1f9eefac4d2f0c6154dd757ffc920a..3c772ada4f0e82b11c01ccf59537d71a3f5a0b89 100644
--- a/components/history/core/browser/thumbnail_database.cc
+++ b/components/history/core/browser/thumbnail_database.cc
@@ -74,6 +74,8 @@ namespace history {
 
 namespace {
 
+const int kFaviconUpdateLastRequestedAfterDays = 14;
+
 // For this database, schema migrations are deprecated after two
 // years.  This means that the oldest non-deprecated version should be
 // two years old or greater (thus the migrations to get there are
@@ -267,6 +269,33 @@ void DatabaseErrorCallback(sql::Connection* db,
     DLOG(FATAL) << db->GetErrorMessage();
 }
 
+bool SetFaviconBitmapLastRequestedTime(sql::Connection* db,
+                                       FaviconBitmapID bitmap_id,
+                                       base::Time access_time) {
+  DCHECK(bitmap_id);
+  sql::Statement statement(db->GetCachedStatement(
+      SQL_FROM_HERE, "UPDATE favicon_bitmaps SET last_requested=? WHERE id=?"));
+  statement.BindInt64(0, access_time.ToInternalValue());
+  statement.BindInt64(1, bitmap_id);
+  return statement.Run();
+}
+
+void DeleteOrphanagedFavicons(sql::Connection* db) {
+  sql::Statement favicons(db->GetCachedStatement(
+      SQL_FROM_HERE,
+      "DELETE FROM favicons WHERE NOT EXISTS (SELECT id FROM favicon_bitmaps "
+      "WHERE favicon_bitmaps.icon_id = favicons.id)"));
+  favicons.Run();
+}
+
+void DeleteOrphanagedMappings(sql::Connection* db) {
+  sql::Statement mappings(db->GetCachedStatement(
+      SQL_FROM_HERE,
+      "DELETE FROM icon_mapping WHERE NOT EXISTS (SELECT id FROM favicons "
+      "WHERE favicons.id = icon_mapping.icon_id)"));
+  mappings.Run();
+}
+
 }  // namespace
 
 ThumbnailDatabase::IconMappingEnumerator::IconMappingEnumerator() {
@@ -408,6 +437,17 @@ void ThumbnailDatabase::TrimMemory(bool aggressively) {
   db_.TrimMemory(aggressively);
 }
 
+void ThumbnailDatabase::CleanUnusedFavicons(base::Time expiration_threshold) {
+  // Delete bitmaps that have not been accessed for a while.
+  sql::Statement bitmaps(db_.GetCachedStatement(
+      SQL_FROM_HERE, "DELETE FROM favicon_bitmaps WHERE last_requested<?"));
+  bitmaps.BindInt64(0, expiration_threshold.ToInternalValue());
+  bitmaps.Run();
+
+  DeleteOrphanagedFavicons(&db_);
+  DeleteOrphanagedMappings(&db_);

[pkotwicz, 2017/05/04 18:34:19]

Drive by: Is it possible that this will end up deleting bookmark favicons on
Android? In particular, this might end up deleting the nice large bookmark
favicons and keep the version which was stored in the database via
MergeFavicon()

[jkrcal, 2017/05/05 16:36:46]

Huh, correct! I've forgotten about that. I will address this problem in the next
patch set. Thanks for spotting this!

[jkrcal, 2017/05/15 14:26:57]

I've reimplemented the code in order to filter out bookmarks. The favicons are
now deleted one-by-one. I am wondering how to improve worst-case performance for
deleting a set of favicons by id. Do you think that
 - setting up a transaction or
 - deleting in batches by "WHERE id IN (?,?,?,?,?,?,?,..,?)" with e.g. up to 100
ids per query
would be worth it?
 

+}
+
 bool ThumbnailDatabase::GetFaviconBitmapIDSizes(
     favicon_base::FaviconID icon_id,
     std::vector<FaviconBitmapIDSize>* bitmap_id_sizes) {
@@ -467,8 +507,8 @@ bool ThumbnailDatabase::GetFaviconBitmaps(
 
 bool ThumbnailDatabase::GetFaviconBitmap(
     FaviconBitmapID bitmap_id,
+    base::Time access_time,
     base::Time* last_updated,
-    base::Time* last_requested,
     scoped_refptr<base::RefCountedMemory>* png_icon_data,
     gfx::Size* pixel_size) {
   DCHECK(bitmap_id);
@@ -494,8 +534,12 @@ bool ThumbnailDatabase::GetFaviconBitmap(
                             statement.ColumnInt(3));
   }
 
-  if (last_requested)
-    *last_requested = base::Time::FromInternalValue(statement.ColumnInt64(4));
+  base::Time last_requested =
+      base::Time::FromInternalValue(statement.ColumnInt64(4));
+  if (access_time - last_requested >
+      base::TimeDelta::FromDays(kFaviconUpdateLastRequestedAfterDays)) {
+    SetFaviconBitmapLastRequestedTime(&db_, bitmap_id, access_time);
+  }
 
   return true;
 }
@@ -555,17 +599,6 @@ bool ThumbnailDatabase::SetFaviconBitmapLastUpdateTime(
   return statement.Run();
 }
 
-bool ThumbnailDatabase::SetFaviconBitmapLastRequestedTime(
-    FaviconBitmapID bitmap_id,
-    base::Time time) {
-  DCHECK(bitmap_id);
-  sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
-      "UPDATE favicon_bitmaps SET last_requested=? WHERE id=?"));
-  statement.BindInt64(0, time.ToInternalValue());
-  statement.BindInt64(1, bitmap_id);
-  return statement.Run();
-}
-
 bool ThumbnailDatabase::DeleteFaviconBitmap(FaviconBitmapID bitmap_id) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM favicon_bitmaps WHERE id=?"));