[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 b8838abc90cf67059151f618c39facd8402d3d6d..49543b43547aa3d4d5b6c34a2ba8d00fda98ff6b 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 DeleteOrphanagedFaviconBitmaps(sql::Connection* db) {
+  sql::Statement favicons(db->GetCachedStatement(
+      SQL_FROM_HERE,
+      "DELETE FROM favicon_bitmaps WHERE NOT EXISTS (SELECT id FROM favicons "
+      "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,47 @@ void ThumbnailDatabase::TrimMemory(bool aggressively) {
   db_.TrimMemory(aggressively);
 }
 
+void ThumbnailDatabase::CleanUnusedFavicons(base::Time expiration_threshold) {
+  // Select all bitmaps (and their page URLs) that have not been accessed for a
+  // while.
+  sql::Statement delete_candidates(db_.GetCachedStatement(
+      SQL_FROM_HERE,
+      "SELECT favicon_bitmaps.icon_id, icon_mapping.page_url FROM "
+      "favicon_bitmaps, icon_mapping WHERE favicon_bitmaps.icon_id = "
+      "icon_mapping.icon_id AND favicon_bitmaps.last_requested<?;"));

[pkotwicz, 2017/05/17 14:56:49]

It we end up setting last_requested for only some rows, you will need to change
the WHERE statement to

favicon_bitmaps.last_requested < ? AND favicon_bitmaps.last_updated < ?

The SQL looks reasonable. I would compare the runtime of |delete_candidates| as
one SQL statement vs as multiple SQL statements in order to determine which is
better. My guess is that the single SQL statement is better

We will likely need to check whether there is a visit associated with the page
URL and avoid deleting the favicon if there is one

+  delete_candidates.BindInt64(0, expiration_threshold.ToInternalValue());
+
+  std::set<FaviconBitmapID> ids_of_bookmarked_favicons;
+  std::set<FaviconBitmapID> ids_to_delete;
+  // Multiple page URLs may map to the same favicon. We omit the favicon_bitmap
+  // from cleaning if at least one of its associated page URLs is bookmarked.
+  while (delete_candidates.Step()) {
+    FaviconBitmapID bitmap_id = delete_candidates.ColumnInt64(0);
+    if (ids_of_bookmarked_favicons.count(bitmap_id))
+      continue;
+
+    if (backend_client_ && backend_client_->IsBookmarked(
+                               GURL(delete_candidates.ColumnString(1)))) {
+      ids_of_bookmarked_favicons.insert(bitmap_id);
+      ids_to_delete.erase(bitmap_id);
+      continue;
+    }
+
+    ids_to_delete.insert(bitmap_id);
+  }
+
+  for (FaviconBitmapID bitmap_id : ids_to_delete) {
+    sql::Statement statement(db_.GetCachedStatement(
+        SQL_FROM_HERE, "DELETE FROM favicons WHERE id=?"));
+    statement.BindInt64(0, bitmap_id);
+    statement.Run();
+  }
+
+  // The bitmaps and mappings for all deleted favicons are deleted at once.
+  DeleteOrphanagedFaviconBitmaps(&db_);
+  DeleteOrphanagedMappings(&db_);
+}
+
 bool ThumbnailDatabase::GetFaviconBitmapIDSizes(
     favicon_base::FaviconID icon_id,
     std::vector<FaviconBitmapIDSize>* bitmap_id_sizes) {
@@ -467,8 +537,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 +564,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 +629,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=?"));