Database usage adjustment for .../history

chrome/browser/history/thumbnail_database.cc

Index: chrome/browser/history/thumbnail_database.cc
diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc
index 68270777e6361c14f43170a3f7018ab7d5c72dfa..86413d1cd6565c522c50cd95b885bc2b5eee1654 100644
--- a/chrome/browser/history/thumbnail_database.cc
+++ b/chrome/browser/history/thumbnail_database.cc
@@ -176,7 +176,6 @@ bool ThumbnailDatabase::UpgradeToVersion3() {
 
   for (int i = 0; alterations[i] != NULL; ++i) {
     if (!db_.Execute(alterations[i])) {
-      NOTREACHED();
       return false;
     }
   }
@@ -239,7 +238,7 @@ void ThumbnailDatabase::Vacuum() {
   ignore_result(db_.Execute("VACUUM"));
 }
 
-void ThumbnailDatabase::SetPageThumbnail(
+bool ThumbnailDatabase::SetPageThumbnail(
     const GURL& url,
     URLID id,
     const gfx::Image* thumbnail,
@@ -247,48 +246,47 @@ void ThumbnailDatabase::SetPageThumbnail(
     base::Time time) {
   if (use_top_sites_) {
     LOG(WARNING) << "Use TopSites instead.";
-    return;  // Not possible after migration to TopSites.
+    return false;  // Not possible after migration to TopSites.
   }
 
-  if (thumbnail) {
-    bool add_thumbnail = true;
-    ThumbnailScore current_score;
-    if (ThumbnailScoreForId(id, &current_score)) {
-      add_thumbnail = ShouldReplaceThumbnailWith(current_score, score);
-    }
+  if (!thumbnail)
+    return DeleteThumbnail(id);
 
-    if (add_thumbnail) {
-      sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
-          "INSERT OR REPLACE INTO thumbnails "
-          "(url_id, boring_score, good_clipping, at_top, last_updated, data) "
-          "VALUES (?,?,?,?,?,?)"));
-      if (!statement)
-        return;
-
-      std::vector<unsigned char> jpeg_data;
-      bool encoded = gfx::JPEGEncodedDataFromImage(*thumbnail, kImageQuality,
-                                                   &jpeg_data);
-      if (encoded) {
-        statement.BindInt64(0, id);
-        statement.BindDouble(1, score.boring_score);
-        statement.BindBool(2, score.good_clipping);
-        statement.BindBool(3, score.at_top);
-        statement.BindInt64(4, score.time_at_snapshot.ToTimeT());
-        statement.BindBlob(5, &jpeg_data[0],
-                           static_cast<int>(jpeg_data.size()));
-        if (!statement.Run())
-          NOTREACHED() << db_.GetErrorMessage();
-      }
-
-      // Publish the thumbnail to any indexers listening to us.
-      // The tests may send an invalid url. Hence avoid publishing those.
-      if (url.is_valid() && history_publisher_ != NULL)
-        history_publisher_->PublishPageThumbnail(jpeg_data, url, time);
-    }
-  } else {
-    if (!DeleteThumbnail(id) )
-      DLOG(WARNING) << "Unable to delete thumbnail";
+  bool add_thumbnail = true;
+  ThumbnailScore current_score;
+  if (ThumbnailScoreForId(id, &current_score)) {
+    add_thumbnail = ShouldReplaceThumbnailWith(current_score, score);
+  }
+
+  if (!add_thumbnail)
+    return true;
+
+  std::vector<unsigned char> jpeg_data;
+  bool encoded = gfx::JPEGEncodedDataFromImage(
+      *thumbnail, kImageQuality, &jpeg_data);
+  if (encoded) {
+    sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
+        "INSERT OR REPLACE INTO thumbnails "
+        "(url_id, boring_score, good_clipping, at_top, last_updated, data) "
+        "VALUES (?,?,?,?,?,?)"));
+    statement.BindInt64(0, id);
+    statement.BindDouble(1, score.boring_score);
+    statement.BindBool(2, score.good_clipping);
+    statement.BindBool(3, score.at_top);
+    statement.BindInt64(4, score.time_at_snapshot.ToTimeT());
+    statement.BindBlob(5, &jpeg_data[0],
+                       static_cast<int>(jpeg_data.size()));
+
+    if (!statement.Run())
+      return false;
   }
+
+  // Publish the thumbnail to any indexers listening to us.
+  // The tests may send an invalid url. Hence avoid publishing those.
+  if (url.is_valid() && history_publisher_ != NULL)
+    history_publisher_->PublishPageThumbnail(jpeg_data, url, time);
+
+  return true;
 }
 
 bool ThumbnailDatabase::GetPageThumbnail(URLID id,
@@ -300,10 +298,8 @@ bool ThumbnailDatabase::GetPageThumbnail(URLID id,
 
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT data FROM thumbnails WHERE url_id=?"));
-  if (!statement)
-    return false;
-
   statement.BindInt64(0, id);
+
   if (!statement.Step())
     return false;  // don't have a thumbnail for this ID
 
@@ -318,15 +314,14 @@ bool ThumbnailDatabase::DeleteThumbnail(URLID id) {
 
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM thumbnails WHERE url_id = ?"));
-  if (!statement)
-    return false;
-
   statement.BindInt64(0, id);
+
   return statement.Run();
 }
 
 bool ThumbnailDatabase::ThumbnailScoreForId(URLID id,
                                             ThumbnailScore* score) {
+  DCHECK(score);
   if (use_top_sites_) {
     LOG(WARNING) << "Use TopSites instead.";
     return false;  // Not possible after migration to TopSites.
@@ -337,61 +332,46 @@ bool ThumbnailDatabase::ThumbnailScoreForId(URLID id,
   sql::Statement select_statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT boring_score, good_clipping, at_top, last_updated "
       "FROM thumbnails WHERE url_id=?"));
-  if (!select_statement) {
-    NOTREACHED() << "Couldn't build select statement!";
-  } else {
-    select_statement.BindInt64(0, id);
-    if (select_statement.Step()) {
-      double current_boring_score = select_statement.ColumnDouble(0);
-      bool current_clipping = select_statement.ColumnBool(1);
-      bool current_at_top = select_statement.ColumnBool(2);
-      base::Time last_updated =
-          base::Time::FromTimeT(select_statement.ColumnInt64(3));
-      *score = ThumbnailScore(current_boring_score, current_clipping,
-                              current_at_top, last_updated);
-      return true;
-    }
-  }
+  select_statement.BindInt64(0, id);
 
-  return false;
+  if (!select_statement.Step())
+    return false;
+
+  double current_boring_score = select_statement.ColumnDouble(0);
+  bool current_clipping = select_statement.ColumnBool(1);
+  bool current_at_top = select_statement.ColumnBool(2);
+  base::Time last_updated =
+      base::Time::FromTimeT(select_statement.ColumnInt64(3));
+  *score = ThumbnailScore(current_boring_score, current_clipping,
+                          current_at_top, last_updated);
+  return true;
 }
 
 bool ThumbnailDatabase::SetFavicon(URLID icon_id,
                                    scoped_refptr<RefCountedMemory> icon_data,
                                    base::Time time) {
   DCHECK(icon_id);
+  sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
+      "UPDATE favicons SET image_data=?, last_updated=? WHERE id=?"));
   if (icon_data->size()) {
-    sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
-        "UPDATE favicons SET image_data=?, last_updated=? WHERE id=?"));
-    if (!statement)
-      return 0;
-
     statement.BindBlob(0, icon_data->front(),
                        static_cast<int>(icon_data->size()));
-    statement.BindInt64(1, time.ToTimeT());
-    statement.BindInt64(2, icon_id);
-    return statement.Run();
   } else {
-    sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
-        "UPDATE favicons SET image_data=NULL, last_updated=? WHERE id=?"));
-    if (!statement)
-      return 0;
-
-    statement.BindInt64(0, time.ToTimeT());
-    statement.BindInt64(1, icon_id);
-    return statement.Run();
+    statement.BindNull(0);
   }
+  statement.BindInt64(1, time.ToTimeT());
+  statement.BindInt64(2, icon_id);

[Scott Hess - ex-Googler, 2012/01/10 22:29:49]

+100!

[Greg Billock, 2012/01/11 23:42:12]

winning! :-)

+
+  return statement.Run();
 }
 
 bool ThumbnailDatabase::SetFaviconLastUpdateTime(FaviconID icon_id,
                                                  base::Time time) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "UPDATE favicons SET last_updated=? WHERE id=?"));
-  if (!statement)
-    return 0;
-
   statement.BindInt64(0, time.ToTimeT());
   statement.BindInt64(1, icon_id);
+
   return statement.Run();
 }
 
@@ -401,11 +381,9 @@ FaviconID ThumbnailDatabase::GetFaviconIDForFaviconURL(const GURL& icon_url,
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT id, icon_type FROM favicons WHERE url=? AND (icon_type & ? > 0) "
       "ORDER BY icon_type DESC"));
-  if (!statement)
-    return 0;
-
   statement.BindString(0, URLDatabase::GURLToDatabaseURL(icon_url));
   statement.BindInt(1, required_icon_type);
+
   if (!statement.Step())
     return 0;  // not cached
 
@@ -423,9 +401,6 @@ bool ThumbnailDatabase::GetFavicon(
 
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT last_updated, image_data, url FROM favicons WHERE id=?"));
-  if (!statement)
-    return 0;
-
   statement.BindInt64(0, icon_id);
 
   if (!statement.Step())
@@ -445,11 +420,9 @@ FaviconID ThumbnailDatabase::AddFavicon(const GURL& icon_url,
 
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "INSERT INTO favicons (url, icon_type) VALUES (?, ?)"));
-  if (!statement)
-    return 0;
-
   statement.BindString(0, URLDatabase::GURLToDatabaseURL(icon_url));
   statement.BindInt(1, icon_type);
+
   if (!statement.Run())
     return 0;
   return db_.GetLastInsertRowId();
@@ -458,10 +431,8 @@ FaviconID ThumbnailDatabase::AddFavicon(const GURL& icon_url,
 bool ThumbnailDatabase::DeleteFavicon(FaviconID id) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM favicons WHERE id = ?"));
-  if (!statement)
-    return false;
-
   statement.BindInt64(0, id);
+
   return statement.Run();
 }
 
@@ -494,9 +465,6 @@ bool ThumbnailDatabase::GetIconMappingsForPageURL(
       "ON icon_mapping.icon_id = favicons.id "
       "WHERE icon_mapping.page_url=? "
       "ORDER BY favicons.icon_type DESC"));
-  if (!statement)
-    return false;
-
   statement.BindString(0, URLDatabase::GURLToDatabaseURL(page_url));
 
   bool result = false;
@@ -521,21 +489,17 @@ bool ThumbnailDatabase::UpdateIconMapping(IconMappingID mapping_id,
                                           FaviconID icon_id) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "UPDATE icon_mapping SET icon_id=? WHERE id=?"));
-  if (!statement)
-    return 0;
-
   statement.BindInt64(0, icon_id);
   statement.BindInt64(1, mapping_id);
+
   return statement.Run();
 }
 
 bool ThumbnailDatabase::DeleteIconMappings(const GURL& page_url) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM icon_mapping WHERE page_url = ?"));
-  if (!statement)
-    return false;
-
   statement.BindString(0, URLDatabase::GURLToDatabaseURL(page_url));
+
   return statement.Run();
 }
 
@@ -543,10 +507,8 @@ bool ThumbnailDatabase::HasMappingFor(FaviconID id) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT id FROM icon_mapping "
       "WHERE icon_id=?"));
-  if (!statement)
-    return false;
-
   statement.BindInt64(0, id);
+
   return statement.Step();
 }
 
@@ -555,7 +517,7 @@ bool ThumbnailDatabase::CloneIconMapping(const GURL& old_page_url,
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT icon_id FROM icon_mapping "
       "WHERE page_url=?"));
-  if (!statement)
+  if (!statement.is_valid())
     return false;
 
   // Do nothing if there are existing bindings
@@ -567,8 +529,6 @@ bool ThumbnailDatabase::CloneIconMapping(const GURL& old_page_url,
       "INSERT INTO icon_mapping (page_url, icon_id) "
         "SELECT ?, icon_id FROM icon_mapping "
         "WHERE page_url = ?"));
-  if (!statement)
-    return false;
 
   statement.BindString(0, URLDatabase::GURLToDatabaseURL(new_page_url));
   statement.BindString(1, URLDatabase::GURLToDatabaseURL(old_page_url));
@@ -613,9 +573,8 @@ FaviconID ThumbnailDatabase::CopyToTemporaryFaviconTable(FaviconID source) {
       "INSERT INTO temp_favicons (url, last_updated, image_data, icon_type)"
       "SELECT url, last_updated, image_data, icon_type "
       "FROM favicons WHERE id = ?"));
-  if (!statement)
-    return 0;
   statement.BindInt64(0, source);
+
   if (!statement.Run())
     return 0;
 
@@ -649,7 +608,6 @@ bool ThumbnailDatabase::RenameAndDropThumbnails(const FilePath& old_db_file,
 
   if (!InitFaviconsTable(&favicons, false) ||
       !InitIconMappingTable(&favicons, false)) {
-    NOTREACHED() << "Couldn't init favicons and icon-mapping table.";
     favicons.Close();
     return false;
   }
@@ -664,8 +622,7 @@ bool ThumbnailDatabase::RenameAndDropThumbnails(const FilePath& old_db_file,
     // This block is needed because otherwise the attach statement is
     // never cleared from cache and we can't close the DB :P
     sql::Statement attach(db_.GetUniqueStatement("ATTACH ? AS new_favicons"));
-    if (!attach) {
-      NOTREACHED() << "Unable to attach database.";
+    if (!attach.is_valid()) {
       // Keep the transaction open, even though we failed.
       BeginTransaction();
       return false;
@@ -678,7 +635,6 @@ bool ThumbnailDatabase::RenameAndDropThumbnails(const FilePath& old_db_file,
 #endif
 
     if (!attach.Run()) {
-      NOTREACHED() << db_.GetErrorMessage();
       BeginTransaction();
       return false;
     }
@@ -687,13 +643,13 @@ bool ThumbnailDatabase::RenameAndDropThumbnails(const FilePath& old_db_file,
   // Move favicons to the new DB.
   if (!db_.Execute("INSERT OR REPLACE INTO new_favicons.favicons "
                    "SELECT * FROM favicons")) {
-    NOTREACHED() << "Unable to copy favicons.";
+    DLOG(FATAL) << "Unable to copy favicons.";
     BeginTransaction();
     return false;
   }
 
   if (!db_.Execute("DETACH new_favicons")) {
-    NOTREACHED() << "Unable to detach database.";
+    DLOG(FATAL) << "Unable to detach database.";
     BeginTransaction();
     return false;
   }
@@ -759,9 +715,6 @@ IconMappingID ThumbnailDatabase::AddIconMapping(const GURL& page_url,
 
   sql::Statement statement(
       db_.GetCachedStatement(sql::StatementID(statement_name), sql.c_str()));
-  if (!statement)
-    return 0;
-
   statement.BindString(0, URLDatabase::GURLToDatabaseURL(page_url));
   statement.BindInt64(1, icon_id);
 
@@ -779,7 +732,6 @@ bool ThumbnailDatabase::UpgradeToVersion4() {
   // Set the default icon type as favicon, so the current data are set
   // correctly.
   if (!db_.Execute("ALTER TABLE favicons ADD icon_type INTEGER DEFAULT 1")) {
-    NOTREACHED();
     return false;
   }
   meta_table_.SetVersionNumber(4);
@@ -789,7 +741,6 @@ bool ThumbnailDatabase::UpgradeToVersion4() {
 
 bool ThumbnailDatabase::UpgradeToVersion5() {
   if (!db_.Execute("ALTER TABLE favicons ADD sizes LONGVARCHAR")) {
-    NOTREACHED();
     return false;
   }
   meta_table_.SetVersionNumber(5);