Add last_requested field to the favicon_bitmaps table of the favicons database.

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 eefeca047f3a6b0e2419b4c768cd1bba05ce17a1..9bea81ae5476036a9a725a5a0b79f8c7bc1215ca 100644
--- a/components/history/core/browser/thumbnail_database.cc
+++ b/components/history/core/browser/thumbnail_database.cc
@@ -66,6 +66,16 @@ namespace history {
 //  image_data        PNG encoded data of the favicon.
 //  width             Pixel width of |image_data|.
 //  height            Pixel height of |image_data|.
+//
+// favicon_bitmap_usage

[huangs, 2015/03/14 02:15:26]

I think |bitmap_usage| would be cleaner.  Although the filename is
thumbnail_database, the DB name is 'Favicon'.

[Roger McFarlane (Chromium), 2015/03/17 20:55:53]

Done.

+//                    This table contains a record of the last time a given
+//                    favicon bitmap was requested via the various icon

[huangs, 2015/03/14 02:15:26]

s/favicon/icon/ (|favicon| is overloaded, but it's good to blur if possible).

[Roger McFarlane (Chromium), 2015/03/17 20:55:53]

Done.

+//                    services.
+//
+//   bitmap_id        The ID of the favicon_bitmaps entry for this usage record.
+//   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 {
 
@@ -77,6 +87,7 @@ namespace {
 // fatal (in fact, very old data may be expired immediately at startup
 // anyhow).
 
+// Version 8: ????????/r?????? by rogerm@chromium on 2015-??-??
 // Version 7: 911a634d/r209424 by qsr@chromium.org on 2013-07-01
 // Version 6: 610f923b/r152367 by pkotwicz@chromium.org on 2012-08-20
 // Version 5: e2ee8ae9/r105004 by groby@chromium.org on 2011-10-12
@@ -86,7 +97,7 @@ namespace {
 // Version number of the database.
 // NOTE(shess): When changing the version, add a new golden file for
 // the new version and a test to verify that Init() works with it.
-const int kCurrentVersionNumber = 7;
+const int kCurrentVersionNumber = 8;
 const int kCompatibleVersionNumber = 7;
 const int kDeprecatedVersionNumber = 4;  // and earlier.
 
@@ -315,6 +326,14 @@ bool InitTables(sql::Connection* db) {
   if (!db->Execute(kFaviconBitmapsSql))
     return false;
 
+  const char kFaviconBitmapUsageSql[] =
+      "CREATE TABLE IF NOT EXISTS favicon_bitmap_usage"
+      "("
+      "bitmap_id INTEGER PRIMARY KEY,"
+      "last_requested INTEGER)";
+  if (!db->Execute(kFaviconBitmapUsageSql))
+    return false;
+
   return true;
 }
 
@@ -396,7 +415,7 @@ void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) {
   // NOTE(shess): This code is currently specific to the version
   // number.  I am working on simplifying things to loosen the
   // dependency, meanwhile contact me if you need to bump the version.
-  DCHECK_EQ(7, kCurrentVersionNumber);
+  DCHECK_EQ(8, kCurrentVersionNumber);
 
   // TODO(shess): Reset back after?
   db->reset_error_callback();
@@ -450,7 +469,7 @@ void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) {
 
   // Earlier versions have been handled or deprecated, later versions should be
   // impossible.
-  if (version != 7) {
+  if (version != 8) {
     sql::Recovery::Unrecoverable(recovery.Pass());
     RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION);
     return;
@@ -727,6 +746,22 @@ bool ThumbnailDatabase::GetFaviconBitmap(
   return true;
 }
 
+bool ThumbnailDatabase::GetFaviconBitmapLastRequestedTime(
+    FaviconBitmapID bitmap_id,
+    base::Time* last_requested) {
+  DCHECK(bitmap_id);
+  DCHECK(last_requested);
+  sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
+      "SELECT last_requested FROM favicon_bitmap_usage WHERE bitmap_id=?"));
+  statement.BindInt64(0, bitmap_id);
+
+  if (!statement.Step())
+    return false;
+
+  *last_requested = base::Time::FromInternalValue(statement.ColumnInt64(0));
+  return true;
+}
+
 FaviconBitmapID ThumbnailDatabase::AddFaviconBitmap(
     favicon_base::FaviconID icon_id,
     const scoped_refptr<base::RefCountedMemory>& icon_data,
@@ -782,10 +817,28 @@ 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,
+      "INSERT OR REPLACE INTO favicon_bitmap_usage (bitmap_id, last_requested)"
+      " VALUES (?, ?)"));
+  statement.BindInt64(0, bitmap_id);
+  statement.BindInt64(1, time.ToInternalValue());
+  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);
+  if (!statement.Run())
+    return false;
+
+  statement.Assign(db_.GetCachedStatement(SQL_FROM_HERE,
+      "DELETE FROM favicon_bitmap_usage WHERE bitmap_id=?"));
+  statement.BindInt64(0, bitmap_id);
   return statement.Run();
 }
 
@@ -866,13 +919,20 @@ favicon_base::FaviconID ThumbnailDatabase::AddFavicon(
 bool ThumbnailDatabase::DeleteFavicon(favicon_base::FaviconID id) {
   sql::Statement statement;
   statement.Assign(db_.GetCachedStatement(SQL_FROM_HERE,
-      "DELETE FROM favicons WHERE id = ?"));
+      "DELETE FROM favicons WHERE id=?"));
   statement.BindInt64(0, id);
   if (!statement.Run())
     return false;
 
   statement.Assign(db_.GetCachedStatement(SQL_FROM_HERE,
-      "DELETE FROM favicon_bitmaps WHERE icon_id = ?"));
+      "DELETE FROM favicon_bitmap_usage WHERE bitmap_id IN"
+      " (SELECT id from favicon_bitmaps where icon_id=?)"));
+  statement.BindInt64(0, id);
+  if (!statement.Run())
+    return false;
+
+  statement.Assign(db_.GetCachedStatement(SQL_FROM_HERE,
+      "DELETE FROM favicon_bitmaps WHERE icon_id=?"));
   statement.BindInt64(0, id);
   return statement.Run();
 }
@@ -1235,6 +1295,12 @@ sql::InitStatus ThumbnailDatabase::InitImpl(const base::FilePath& db_name) {
       return CantUpgradeToVersion(cur_version);
   }
 
+  if (cur_version == 7) {
+    ++cur_version;
+    if (!UpgradeToVersion8())
+      return CantUpgradeToVersion(cur_version);
+  }
+
   LOG_IF(WARNING, cur_version < kCurrentVersionNumber) <<
       "Thumbnail database version " << cur_version << " is too old to handle.";
 
@@ -1315,6 +1381,12 @@ bool ThumbnailDatabase::UpgradeToVersion7() {
   return true;
 }
 
+bool ThumbnailDatabase::UpgradeToVersion8() {
+  meta_table_.SetVersionNumber(8);
+  meta_table_.SetCompatibleVersionNumber(std::min(8, kCompatibleVersionNumber));
+  return true;

[huangs, 2015/03/14 02:15:26]

Would you need to add the table by executing |kFaviconBitmapUsageSql| ?

[Roger McFarlane (Chromium), 2015/03/17 20:55:53]

No, that's already done by this point.

The tables get initialized in InitTables() and InitIndices() (called around line
1254) so I only have to do any lingering alterations here.

+}
+
 bool ThumbnailDatabase::IsFaviconDBStructureIncorrect() {
   return !db_.IsSQLValid("SELECT id, url, icon_type FROM favicons");
 }