Database usage adjustment for .../history

chrome/browser/history/history_database.cc

 // Copyright (c) 2011 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 "chrome/browser/history/history_database.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 
@@ skipping 30 matching lines @@
   if (base::RandInt(1, 100) != 50)
     return;  // Only do this computation sometimes since it can be expensive.
 
   int64 file_size = 0;
   if (!file_util::GetFileSize(history_name, &file_size))
     return;
   int file_mb = static_cast<int>(file_size / (1024 * 1024));
   UMA_HISTOGRAM_MEMORY_MB("History.DatabaseFileMB", file_mb);
 
   sql::Statement url_count(db.GetUniqueStatement("SELECT count(*) FROM urls"));
-  if (!url_count || !url_count.Step())
+  if (!url_count.Step())
     return;
   UMA_HISTOGRAM_COUNTS("History.URLTableCount", url_count.ColumnInt(0));
 
   sql::Statement visit_count(db.GetUniqueStatement(
       "SELECT count(*) FROM visits"));
-  if (!visit_count || !visit_count.Step())
+  if (!visit_count.Step())
     return;
   UMA_HISTOGRAM_COUNTS("History.VisitTableCount", visit_count.ColumnInt(0));
 }
 
 }  // namespace
 
 HistoryDatabase::HistoryDatabase()
     : needs_version_17_migration_(false) {
 }
 
@@ skipping 113 matching lines @@
 
 bool HistoryDatabase::GetNeedsThumbnailMigration() {
   int value = 0;
   return (meta_table_.GetValue(kNeedsThumbnailMigrationKey, &value) &&
           value != 0);
 }
 
 bool HistoryDatabase::SetSegmentID(VisitID visit_id, SegmentID segment_id) {
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "UPDATE visits SET segment_id = ? WHERE id = ?"));
-  if (!s) {
-    NOTREACHED() << db_.GetErrorMessage();
-    return false;
-  }
   s.BindInt64(0, segment_id);
   s.BindInt64(1, visit_id);
   DCHECK(db_.GetLastChangeCount() == 1);

[Scott Hess - ex-Googler, 2012/01/03 23:41:57]

I'm having troubles seeing how this line could possibly be making the assertions
it seems to think it is asserting.

[Greg Billock, 2012/01/04 19:20:20]

I read it as saying "ASSERT(number of rows changed by the previous UPDATE
statement was exactly 1)"

Meaning that the caller must ensure that visit_id is present and unique in the
visits table.

Of course, by that time it's too late and the DB is corrupted... :-/

On 2012/01/03 23:41:57, shess wrote:

[Scott Hess - ex-Googler, 2012/01/10 22:04:56]

Do you mean an update statement which ran sometime prior to the call to
SetSegmentID()?  Because the UPDATE statement a couple lines above here has not
run, yet.

+
   return s.Run();
 }
 
 SegmentID HistoryDatabase::GetSegmentID(VisitID visit_id) {
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT segment_id FROM visits WHERE id = ?"));
-  if (!s) {
-    NOTREACHED() << db_.GetErrorMessage();
-    return 0;
-  }
+  s.BindInt64(0, visit_id);
 
-  s.BindInt64(0, visit_id);
   if (s.Step()) {
     if (s.ColumnType(0) == sql::COLUMN_TYPE_NULL)
       return 0;
     else
       return s.ColumnInt64(0);

[Scott Hess - ex-Googler, 2012/01/03 23:41:57]

This line would give 0 if the column type is NULL.

[Greg Billock, 2012/01/04 19:20:20]

So we can just return s.ColumnInt64(0)?

[Scott Hess - ex-Googler, 2012/01/10 22:04:56]

Yes, NULL is interpreted as zero for integer requests.

   }
   return 0;
 }
 
 base::Time HistoryDatabase::GetEarlyExpirationThreshold() {
   if (!cached_early_expiration_threshold_.is_null())
     return cached_early_expiration_threshold_;
 
   int64 threshold;
   if (!meta_table_.GetValue(kEarlyExpirationThresholdKey, &threshold)) {
@@ skipping 112 matching lines @@
       "WHERE id IN (SELECT id FROM segment_usage WHERE time_slot > 0);"));
 
   // Erase all the full text index files. These will take a while to update and
   // are less important, so we just blow them away. Same with the archived
   // database.
   needs_version_17_migration_ = true;
 }
 #endif
 
 }  // namespace history