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

components/history/core/browser/thumbnail_database.cc

 // 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.
 
 #include "components/history/core/browser/thumbnail_database.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <algorithm>
 #include <string>
@@ skipping 56 matching lines @@
 //                    redownloaded from the web.
 //   image_data       PNG encoded data of the favicon.
 //   width            Pixel width of |image_data|.
 //   height           Pixel height of |image_data|.
 //   last_requested   The time at which this bitmap was last requested. This is
 //                    used to determine the priority with which the bitmap
 //                    should be retained on cleanup.
 
 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
 // older).  Databases containing deprecated versions will be cleared
 // at startup.  Since this database is a cache, losing old data is not
 // fatal (in fact, very old data may be expired immediately at startup
 // anyhow).
 
 // Version 8: 982ef2c1/r323176 by rogerm@chromium.org on 2015-03-31
 // Version 7: 911a634d/r209424 by qsr@chromium.org on 2013-07-01
@@ skipping 173 matching lines @@
     // test-expectation framework that the error was handled.
     ignore_result(sql::Connection::IsExpectedSqliteError(extended_error));
     return;
   }
 
   // The default handling is to assert on debug and to ignore on release.
   if (!sql::Connection::IsExpectedSqliteError(extended_error))
     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() {
 }
 
 ThumbnailDatabase::IconMappingEnumerator::~IconMappingEnumerator() {
 }
 
 bool ThumbnailDatabase::IconMappingEnumerator::GetNextIconMapping(
     IconMapping* icon_mapping) {
@@ skipping 121 matching lines @@
 void ThumbnailDatabase::Vacuum() {
   DCHECK(db_.transaction_nesting() == 0) <<
       "Can not have a transaction when vacuuming.";
   ignore_result(db_.Execute("VACUUM"));
 }
 
 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) {
   DCHECK(icon_id);
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT id, width, height FROM favicon_bitmaps WHERE icon_id=?"));
   statement.BindInt64(0, icon_id);
 
   bool result = false;
   while (statement.Step()) {
@@ skipping 39 matching lines @@
                                           statement.ColumnInt(4));
     favicon_bitmap.last_requested =
         base::Time::FromInternalValue(statement.ColumnInt64(5));
     favicon_bitmaps->push_back(favicon_bitmap);
   }
   return result;
 }
 
 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);
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT last_updated, image_data, width, height, last_requested "
       "FROM favicon_bitmaps WHERE id=?"));
   statement.BindInt64(0, bitmap_id);
 
   if (!statement.Step())
     return false;
 
   if (last_updated)
     *last_updated = base::Time::FromInternalValue(statement.ColumnInt64(0));
 
   if (png_icon_data && statement.ColumnByteLength(1) > 0) {
     scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes());
     statement.ColumnBlobAsVector(1, &data->data());
     *png_icon_data = data;
   }
 
   if (pixel_size) {
     *pixel_size = gfx::Size(statement.ColumnInt(2),
                             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;
 }
 
 FaviconBitmapID ThumbnailDatabase::AddFaviconBitmap(
     favicon_base::FaviconID icon_id,
     const scoped_refptr<base::RefCountedMemory>& icon_data,
     base::Time time,
     const gfx::Size& pixel_size) {
   DCHECK(icon_id);
@@ skipping 39 matching lines @@
     FaviconBitmapID bitmap_id,
     base::Time time) {
   DCHECK(bitmap_id);
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "UPDATE favicon_bitmaps SET last_updated=? WHERE id=?"));
   statement.BindInt64(0, time.ToInternalValue());
   statement.BindInt64(1, bitmap_id);
   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=?"));
   statement.BindInt64(0, bitmap_id);
   return statement.Run();
 }
 
 bool ThumbnailDatabase::SetFaviconOutOfDate(favicon_base::FaviconID icon_id) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "UPDATE favicon_bitmaps SET last_updated=? WHERE icon_id=?"));
@@ skipping 484 matching lines @@
   meta_table_.SetVersionNumber(8);
   meta_table_.SetCompatibleVersionNumber(std::min(8, kCompatibleVersionNumber));
   return true;
 }
 
 bool ThumbnailDatabase::IsFaviconDBStructureIncorrect() {
   return !db_.IsSQLValid("SELECT id, url, icon_type FROM favicons");
 }
 
 }  // namespace history