Adding last_forced column to thumbnails database.

chrome/browser/history/top_sites_database.cc

Index: chrome/browser/history/top_sites_database.cc
diff --git a/chrome/browser/history/top_sites_database.cc b/chrome/browser/history/top_sites_database.cc
index 5c1939fbf6be059e23958c1ec0f3a806bc27cc26..77d4c03f87bbbbcd1212f7efcd11f01b2a823345 100644
--- a/chrome/browser/history/top_sites_database.cc
+++ b/chrome/browser/history/top_sites_database.cc
@@ -9,19 +9,44 @@
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/history/top_sites_database.h"
+#include "chrome/common/thumbnail_score.h"
 #include "sql/connection.h"
 #include "sql/transaction.h"
 
+// Description of database table:
+//
+// thumbnails
+//   url              URL of the sites for which we have a thumbnail.
+//   url_rank         Index of the URL in that thumbnail, 0-based. The thumbnail
+//                    with the highest rank will be the next one evicted. Forced
+//                    thumbnails have a rank of -1.
+//   title            The title to display under that thumbnail.
+//   redirects        A space separated list of URLs that are known to redirect
+//                    to this url.
+//   boring_score     How "boring" that thumbnail is. See ThumbnailScore.
+//   good_clipping    True if the thumbnail was clipped from the bottom, keeping
+//                    the entire width of the window. See ThumbnailScore.
+//   at_top           True if the thumbnail was captured at the top of the
+//                    website.
+//   last_updated     The time at which this thumbnail was last updated.
+//   load_completed   True if the thumbnail was captured after the page load was
+//                    completed.
+//   last_forced      If this is a forced thumbnail, records the last time it
+//                    was forced. If it's not a forced thumbnail, 0.
+
 namespace history {
 
+// TODO(beaudoin): Fill revision/date details of Version 3 after landing.
+// Version 3:  by beaudoin@chromium.org
 // Version 2: eb0b24e6/r87284 by satorux@chromium.org on 2011-05-31
 // Version 1: 809cc4d8/r64072 by sky@chromium.org on 2010-10-27
 
 // From the version 1 to 2, one column was added. Old versions of Chrome
-// should be able to read version 2 files just fine.
+// should be able to read version 2 files just fine. Same thing for version 2
+// to 3.
 // 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.
-static const int kVersionNumber = 2;
+static const int kVersionNumber = 3;
 
 TopSitesDatabase::TopSitesDatabase() {
 }
@@ -69,6 +94,13 @@ bool TopSitesDatabase::Init(const base::FilePath& db_name) {
     }
   }
 
+  if (meta_table_.GetVersionNumber() == 2) {
+    if (!UpgradeToVersion3()) {
+      LOG(WARNING) << "Unable to upgrade top sites database to version 3.";
+      return false;
+    }
+  }
+
   // Version check.
   if (meta_table_.GetVersionNumber() != kVersionNumber)
     return false;
@@ -84,15 +116,16 @@ bool TopSitesDatabase::InitThumbnailTable() {
   if (!db_->DoesTableExist("thumbnails")) {
     if (!db_->Execute("CREATE TABLE thumbnails ("
                       "url LONGVARCHAR PRIMARY KEY,"
-                      "url_rank INTEGER ,"
+                      "url_rank INTEGER,"
                       "title LONGVARCHAR,"
                       "thumbnail BLOB,"
                       "redirects LONGVARCHAR,"
-                      "boring_score DOUBLE DEFAULT 1.0, "
-                      "good_clipping INTEGER DEFAULT 0, "
-                      "at_top INTEGER DEFAULT 0, "
-                      "last_updated INTEGER DEFAULT 0, "
-                      "load_completed INTEGER DEFAULT 0) ")) {
+                      "boring_score DOUBLE DEFAULT 1.0,"
+                      "good_clipping INTEGER DEFAULT 0,"
+                      "at_top INTEGER DEFAULT 0,"
+                      "last_updated INTEGER DEFAULT 0,"
+                      "load_completed INTEGER DEFAULT 0,"
+                      "last_forced INTEGER DEFAULT 0)")) {
       LOG(WARNING) << db_->GetErrorMessage();
       return false;
     }
@@ -111,13 +144,24 @@ bool TopSitesDatabase::UpgradeToVersion2() {
   return true;
 }
 
+bool TopSitesDatabase::UpgradeToVersion3() {
+  // Add 'last_forced' column.
+  if (!db_->Execute(
+          "ALTER TABLE thumbnails ADD last_forced INTEGER DEFAULT 0")) {
+    NOTREACHED();
+    return false;
+  }
+  meta_table_.SetVersionNumber(3);
+  return true;
+}
+
 void TopSitesDatabase::GetPageThumbnails(MostVisitedURLList* urls,
                                          URLToImagesMap* thumbnails) {
   sql::Statement statement(db_->GetCachedStatement(
       SQL_FROM_HERE,
       "SELECT url, url_rank, title, thumbnail, redirects, "
-      "boring_score, good_clipping, at_top, last_updated, load_completed "
-      "FROM thumbnails ORDER BY url_rank "));
+      "boring_score, good_clipping, at_top, last_updated, load_completed, "
+      "last_forced FROM thumbnails ORDER BY url_rank, last_forced"));
 
   if (!statement.is_valid()) {
     LOG(WARNING) << db_->GetErrorMessage();
@@ -128,11 +172,14 @@ void TopSitesDatabase::GetPageThumbnails(MostVisitedURLList* urls,
   thumbnails->clear();
 
   while (statement.Step()) {
-    // Results are sorted by url_rank.
+    // Results are sorted by url_rank. For forced thumbnails with url_rank = -1,
+    // thumbnails are sorted by last_forced.
     MostVisitedURL url;
     GURL gurl(statement.ColumnString(0));
     url.url = gurl;
     url.title = statement.ColumnString16(2);
+    url.last_forced_time =
+        base::Time::FromInternalValue(statement.ColumnInt64(10));
     std::string redirects = statement.ColumnString(4);
     SetRedirects(redirects, &url);
     urls->push_back(url);
@@ -148,7 +195,6 @@ void TopSitesDatabase::GetPageThumbnails(MostVisitedURLList* urls,
     thumbnail.thumbnail_score.time_at_snapshot =
         base::Time::FromInternalValue(statement.ColumnInt64(8));
     thumbnail.thumbnail_score.load_completed = statement.ColumnBool(9);
-
     (*thumbnails)[gurl] = thumbnail;
   }
 }
@@ -171,13 +217,13 @@ void TopSitesDatabase::SetRedirects(const std::string& redirects,
 }
 
 void TopSitesDatabase::SetPageThumbnail(const MostVisitedURL& url,
-                                            int new_rank,
-                                            const Images& thumbnail) {
+                                        int new_rank,
+                                        const Images& thumbnail) {
   sql::Transaction transaction(db_.get());
   transaction.Begin();
 
   int rank = GetURLRank(url);
-  if (rank == -1) {
+  if (rank == kRankOfNonExistingURL) {
     AddPageThumbnail(url, new_rank, thumbnail);
   } else {
     UpdatePageRankNoTransaction(url, new_rank);
@@ -194,7 +240,7 @@ bool TopSitesDatabase::UpdatePageThumbnail(
       "UPDATE thumbnails SET "
       "title = ?, thumbnail = ?, redirects = ?, "
       "boring_score = ?, good_clipping = ?, at_top = ?, last_updated = ?, "
-      "load_completed = ? "
+      "load_completed = ?, last_forced = ?"
       "WHERE url = ? "));
   statement.BindString16(0, url.title);
   if (thumbnail.thumbnail.get() && thumbnail.thumbnail->front()) {
@@ -208,24 +254,23 @@ bool TopSitesDatabase::UpdatePageThumbnail(
   statement.BindBool(5, score.at_top);
   statement.BindInt64(6, score.time_at_snapshot.ToInternalValue());
   statement.BindBool(7, score.load_completed);
-  statement.BindString(8, url.url.spec());
+  statement.BindInt64(8, url.last_forced_time.ToInternalValue());
+  statement.BindString(9, url.url.spec());
 
   return statement.Run();
 }
 
 void TopSitesDatabase::AddPageThumbnail(const MostVisitedURL& url,
-                                            int new_rank,
-                                            const Images& thumbnail) {
-  int count = GetRowCount();
-
+                                        int new_rank,
+                                        const Images& thumbnail) {
   sql::Statement statement(db_->GetCachedStatement(
       SQL_FROM_HERE,
       "INSERT OR REPLACE INTO thumbnails "
       "(url, url_rank, title, thumbnail, redirects, "
-      "boring_score, good_clipping, at_top, last_updated, load_completed) "
-      "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
+      "boring_score, good_clipping, at_top, last_updated, load_completed, "
+      "last_forced) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
   statement.BindString(0, url.url.spec());
-  statement.BindInt(1, count);  // Make it the last url.
+  statement.BindInt(1, kRankOfForcedURL);  // Fist make it a forced thumbnail.
   statement.BindString16(2, url.title);
   if (thumbnail.thumbnail.get() && thumbnail.thumbnail->front()) {
     statement.BindBlob(3, thumbnail.thumbnail->front(),
@@ -238,14 +283,25 @@ void TopSitesDatabase::AddPageThumbnail(const MostVisitedURL& url,
   statement.BindBool(7, score.at_top);
   statement.BindInt64(8, score.time_at_snapshot.ToInternalValue());
   statement.BindBool(9, score.load_completed);
+  int64 last_forced = url.last_forced_time.ToInternalValue();
+  DCHECK((last_forced == 0) == (new_rank != kRankOfForcedURL))
+    << "Thumbnail without a forced time stamp has a forced rank, or the "

[brettw, 2013/10/28 22:22:27]

Nit: intend 2 more spaces.

[beaudoin, 2013/10/29 15:27:50]

Done.

+    << "opposite.";
+  statement.BindInt64(10, last_forced);
   if (!statement.Run())
     return;
 
-  UpdatePageRankNoTransaction(url, new_rank);
+  // Update rank if this is not a forced thumbnail.
+  if (new_rank != kRankOfForcedURL)
+    UpdatePageRankNoTransaction(url, new_rank);
 }
 
 void TopSitesDatabase::UpdatePageRank(const MostVisitedURL& url,
-                                          int new_rank) {
+                                      int new_rank) {
+  DCHECK((url.last_forced_time.ToInternalValue() == 0) ==
+         (new_rank != kRankOfForcedURL))
+    << "Thumbnail without a forced time stamp has a forced rank, or the "

[brettw, 2013/10/28 22:22:27]

ditto

[beaudoin, 2013/10/29 15:27:50]

Done.

+    << "opposite.";
   sql::Transaction transaction(db_.get());
   transaction.Begin();
   UpdatePageRankNoTransaction(url, new_rank);
@@ -256,43 +312,76 @@ void TopSitesDatabase::UpdatePageRank(const MostVisitedURL& url,
 void TopSitesDatabase::UpdatePageRankNoTransaction(
     const MostVisitedURL& url, int new_rank) {
   DCHECK_GT(db_->transaction_nesting(), 0);
+
   int prev_rank = GetURLRank(url);
-  if (prev_rank == -1) {
+  if (prev_rank == kRankOfNonExistingURL) {
     LOG(WARNING) << "Updating rank of an unknown URL: " << url.url.spec();
     return;
   }
 
   // Shift the ranks.
   if (prev_rank > new_rank) {
-    // Shift up
-    sql::Statement shift_statement(db_->GetCachedStatement(
-        SQL_FROM_HERE,
-        "UPDATE thumbnails "
-        "SET url_rank = url_rank + 1 "
-        "WHERE url_rank >= ? AND url_rank < ?"));
-    shift_statement.BindInt(0, new_rank);
-    shift_statement.BindInt(1, prev_rank);
-    shift_statement.Run();
+    if (new_rank == kRankOfForcedURL) {
+      // From non-forced to forced, shift down.
+      // Example: 2 -> -1
+      // -1, -1, -1, 0, 1, [2 -> -1], [3 -> 2], [4 -> 3]
+      sql::Statement shift_statement(db_->GetCachedStatement(
+          SQL_FROM_HERE,
+          "UPDATE thumbnails "
+          "SET url_rank = url_rank - 1 "
+          "WHERE url_rank > ?"));
+      shift_statement.BindInt(0, prev_rank);
+      shift_statement.Run();
+    } else {
+      // From non-forced to non-forced, shift up.
+      // Example: 3 -> 1
+      // -1, -1, -1, 0, [1 -> 2], [2 -> 3], [3 -> 1], 4
+      sql::Statement shift_statement(db_->GetCachedStatement(
+          SQL_FROM_HERE,
+          "UPDATE thumbnails "
+          "SET url_rank = url_rank + 1 "
+          "WHERE url_rank >= ? AND url_rank < ?"));
+      shift_statement.BindInt(0, new_rank);
+      shift_statement.BindInt(1, prev_rank);
+      shift_statement.Run();
+    }
   } else if (prev_rank < new_rank) {
-    // Shift down
-    sql::Statement shift_statement(db_->GetCachedStatement(
-        SQL_FROM_HERE,
-        "UPDATE thumbnails "
-        "SET url_rank = url_rank - 1 "
-        "WHERE url_rank > ? AND url_rank <= ?"));
-    shift_statement.BindInt(0, prev_rank);
-    shift_statement.BindInt(1, new_rank);
-    shift_statement.Run();
+    if (prev_rank == kRankOfForcedURL) {
+      // From non-forced to forced, shift up.
+      // Example: -1 -> 2
+      // -1, [-1 -> 2], -1, 0, 1, [2 -> 3], [3 -> 4], [4 -> 5]
+      sql::Statement shift_statement(db_->GetCachedStatement(
+          SQL_FROM_HERE,
+          "UPDATE thumbnails "
+          "SET url_rank = url_rank + 1 "
+          "WHERE url_rank >= ?"));
+      shift_statement.BindInt(0, new_rank);
+      shift_statement.Run();
+    } else {
+      // From non-forced to non-forced, shift down.
+      // Example: 1 -> 3.
+      // -1, -1, -1, 0, [1 -> 3], [2 -> 1], [3 -> 2], 4
+      sql::Statement shift_statement(db_->GetCachedStatement(
+          SQL_FROM_HERE,
+          "UPDATE thumbnails "
+          "SET url_rank = url_rank - 1 "
+          "WHERE url_rank > ? AND url_rank <= ?"));
+      shift_statement.BindInt(0, prev_rank);
+      shift_statement.BindInt(1, new_rank);
+      shift_statement.Run();
+    }
   }
 
-  // Set the url's rank.
+  // Set the url's rank and last_forced, since the latter changes when a URL
+  // goes from forced to non-forced and vice-versa.
   sql::Statement set_statement(db_->GetCachedStatement(
       SQL_FROM_HERE,
       "UPDATE thumbnails "
-      "SET url_rank = ? "
+      "SET url_rank = ?, last_forced = ? "
       "WHERE url == ?"));
   set_statement.BindInt(0, new_rank);
-  set_statement.BindString(1, url.url.spec());
+  set_statement.BindInt64(1, url.last_forced_time.ToInternalValue());
+  set_statement.BindString(2, url.url.spec());
   set_statement.Run();
 }
 
@@ -317,16 +406,6 @@ bool TopSitesDatabase::GetPageThumbnail(const GURL& url,
   return true;
 }
 
-int TopSitesDatabase::GetRowCount() {
-  sql::Statement select_statement(db_->GetCachedStatement(
-      SQL_FROM_HERE,
-      "SELECT COUNT (url) FROM thumbnails"));
-  if (select_statement.Step())
-    return select_statement.ColumnInt(0);
-
-  return 0;
-}
-
 int TopSitesDatabase::GetURLRank(const MostVisitedURL& url) {
   sql::Statement select_statement(db_->GetCachedStatement(
       SQL_FROM_HERE,
@@ -336,27 +415,29 @@ int TopSitesDatabase::GetURLRank(const MostVisitedURL& url) {
   if (select_statement.Step())
     return select_statement.ColumnInt(0);
 
-  return -1;
+  return kRankOfNonExistingURL;
 }
 
 // Remove the record for this URL. Returns true iff removed successfully.
 bool TopSitesDatabase::RemoveURL(const MostVisitedURL& url) {
   int old_rank = GetURLRank(url);
-  if (old_rank < 0)
+  if (old_rank == kRankOfNonExistingURL)
     return false;
 
   sql::Transaction transaction(db_.get());
   transaction.Begin();
-  // Decrement all following ranks.
-  sql::Statement shift_statement(db_->GetCachedStatement(
-      SQL_FROM_HERE,
-      "UPDATE thumbnails "
-      "SET url_rank = url_rank - 1 "
-      "WHERE url_rank > ?"));
-  shift_statement.BindInt(0, old_rank);
+  if (old_rank != kRankOfForcedURL) {
+    // Decrement all following ranks.
+    sql::Statement shift_statement(db_->GetCachedStatement(
+        SQL_FROM_HERE,
+        "UPDATE thumbnails "
+        "SET url_rank = url_rank - 1 "
+        "WHERE url_rank > ?"));
+    shift_statement.BindInt(0, old_rank);
 
-  if (!shift_statement.Run())
-    return false;
+    if (!shift_statement.Run())
+      return false;
+  }
 
   sql::Statement delete_statement(
       db_->GetCachedStatement(SQL_FROM_HERE,