[Thumbnails DB] Add functionality to clear unused on-demand 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 255 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();
 }
 
+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();
+}
+
 void FillStatementForAddFaviconBitmap(
     favicon_base::FaviconID icon_id,
     const scoped_refptr<base::RefCountedMemory>& icon_data,
     base::Time time,
     const gfx::Size& pixel_size,
     sql::Statement* statement) {
   DCHECK(statement);
   DCHECK(icon_id);
   statement->BindInt64(0, icon_id);
   if (icon_data.get() && icon_data->size()) {
@@ skipping 141 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::ClearOldOnDemandFavicons(
+    base::Time expiration_threshold) {
+  // Select all bitmaps (and their page URLs) that have not been accessed for a
+  // while. Restrict to on-demand bitmaps (i.e. with last_requested != 0).
+  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>0 AND "
+      "favicon_bitmaps.last_requested<?;"));

[pkotwicz, 2017/05/24 17:05:03]

If:
1) SetOnDemandFavicons() is called
2) SetFavicons() with the same page URL (as a result of a user visiting the
page)

Won't this function incorrectly delete the favicon?

Don't you also need to check that favicon_bitmaps.last_updated is expired?

[jkrcal, 2017/06/21 17:19:20]

This has been fixed in the preceding CL: it holds that 
  last_requested > 0 iff last_updated == 0.

On the other hand, I am wondering about the transition to this system: 
 - Before this work is complete, all on demand favicons are stored with both
timestamps equal to 0 and I'd like to tidy up.
 - Thus, I'd like to delete here both old on demand favicons as well as expired
on-visit favicons. 
 - This could be only for a transitional phase and after one milestone, get back
to deleting only old on-demand bitmaps.

Do you see a problem in deleting expired icons?

[jkrcal, 2017/06/22 11:22:11]

Pinging this question.

[pkotwicz, 2017/06/22 20:32:22]

I don't see a problem in expiring the icons

[jkrcal, 2017/07/06 17:01:48]

Done.

+  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);

[pkotwicz, 2017/05/24 17:05:03]

The "id" column in the favicon_bitmaps table is the
favicon_base::FaviconBitmapID. The "icon_id" column in the favicon_bitmaps table
is the favicon_base::FaviconID 

[jkrcal, 2017/06/21 17:19:20]

Ah, thanks! Done (incl. variable names).

+    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);

[pkotwicz, 2017/05/24 17:05:03]

Is it possible for |bitmap_id| to have been added to |ids_to_delete|?

[jkrcal, 2017/06/21 17:19:20]

Yes, if first some non-bookmarked page with this icon appeared in the while
loop.

I reworded the variables, I hope it is clearer now.

[pkotwicz, 2017/06/21 19:50:17]

Ahhh. I see now. Thank you for the explanation

+      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 665 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