Database usage adjustment for .../history

chrome/browser/history/download_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/download_database.h"
 
 #include <limits>
 #include <string>
 #include <vector>
 
@@ skipping 97 matching lines @@
 void DownloadDatabase::QueryDownloads(
     std::vector<DownloadPersistentStoreInfo>* results) {
   CheckThread();
   results->clear();
 
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "SELECT id, full_path, url, start_time, received_bytes, "
         "total_bytes, state, end_time, opened "
       "FROM downloads "
       "ORDER BY start_time"));
-  if (!statement)
-    return;
 
   while (statement.Step()) {
     DownloadPersistentStoreInfo info;
     info.db_handle = statement.ColumnInt64(0);
 
     info.path = ColumnFilePath(statement, 1);
     info.url = GURL(statement.ColumnString(2));
     info.start_time = base::Time::FromTimeT(statement.ColumnInt64(3));
     info.received_bytes = statement.ColumnInt64(4);
     info.total_bytes = statement.ColumnInt64(5);
@@ skipping 11 matching lines @@
       returned_ids_.insert(info.db_handle);
   }
 }
 
 bool DownloadDatabase::UpdateDownload(const DownloadPersistentStoreInfo& data) {
   CheckThread();
   DCHECK(data.db_handle > 0);
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads "
       "SET received_bytes=?, state=?, end_time=?, opened=? WHERE id=?"));
-  if (!statement)
-    return false;
-
   statement.BindInt64(0, data.received_bytes);
   statement.BindInt(1, data.state);
   statement.BindInt64(2, data.end_time.ToTimeT());
   statement.BindInt(3, (data.opened ? 1 : 0));
   statement.BindInt64(4, data.db_handle);
+
   return statement.Run();
 }
 
 bool DownloadDatabase::UpdateDownloadPath(const FilePath& path,
                                           DownloadID db_handle) {
   CheckThread();
   DCHECK(db_handle > 0);
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads SET full_path=? WHERE id=?"));
-  if (!statement)
-    return false;
-
   BindFilePath(statement, path, 0);
   statement.BindInt64(1, db_handle);
+
   return statement.Run();
 }
 
 bool DownloadDatabase::CleanUpInProgressEntries() {
   CheckThread();
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "UPDATE downloads SET state=? WHERE state=?"));
-  if (!statement)
-    return false;
   statement.BindInt(0, DownloadItem::CANCELLED);
   statement.BindInt(1, DownloadItem::IN_PROGRESS);
+
   return statement.Run();
 }
 
 int64 DownloadDatabase::CreateDownload(
     const DownloadPersistentStoreInfo& info) {
   CheckThread();
 
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "INSERT INTO downloads "
       "(full_path, url, start_time, received_bytes, total_bytes, state, "
       "end_time, opened) "
       "VALUES (?, ?, ?, ?, ?, ?, ?, ?)"));
-  if (!statement)
-    return 0;
-
   BindFilePath(statement, info.path, 0);
   statement.BindString(1, info.url.spec());
   statement.BindInt64(2, info.start_time.ToTimeT());
   statement.BindInt64(3, info.received_bytes);
   statement.BindInt64(4, info.total_bytes);
   statement.BindInt(5, info.state);
   statement.BindInt64(6, info.end_time.ToTimeT());
   statement.BindInt(7, info.opened ? 1 : 0);
 
   if (statement.Run()) {
     int64 id = GetDB().GetLastInsertRowId();
 
     // TODO(rdsmith): Remove when http://crbug.com/96627 is resolved.
     if (returned_ids_.count(id) != 0) {
       // We have an invariant violation and we're going to crash.  Take a
       // moment more before crashing to figure out if it's a returned_ids_/DB
       // inconsistency, or an inconsistency inside the DB.
       sql::Statement dbg_statement(GetDB().GetCachedStatement(
           SQL_FROM_HERE,
           "SELECT id FROM downloads;"));
-      CHECK(dbg_statement);
+      CHECK(dbg_statement.is_valid());
 
       std::set<int64> database_ids;
       while (dbg_statement.Step()) {
         bool success = database_ids.insert(dbg_statement.ColumnInt64(0)).second;
         CHECK(success);
       }
       CHECK(false);
     }
 
     returned_ids_.insert(id);
 
     // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
     meta_table_.SetValue(kNextDownloadId, ++next_id_);
 
     return id;
   }
   return 0;
 }
 
 void DownloadDatabase::RemoveDownload(DownloadID db_handle) {
   CheckThread();
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM downloads WHERE id=?"));
-  if (!statement)
+  statement.BindInt64(0, db_handle);
+
+  // TODO(rdsmith): Remove when http://crbug.com/96627 is resolved.

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

The target of this TODO seems ambiguous, now.  Maybe should loop in rdsmith.

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

I think I misplaced it. Should be by the erase I think.

[Randy Smith (Not in Mondays), 2012/01/04 19:34:18]

Yes, it should.  

+  if (!statement.Run())
     return;
 
-  statement.BindInt64(0, db_handle);
-  if (statement.Run())
-    // TODO(rdsmith): Remove when http://crbug.com/96627 is resolved.
-    returned_ids_.erase(db_handle);
+  returned_ids_.erase(db_handle);
 }
 
-void DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin,
+bool DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin,
                                               base::Time delete_end) {
   CheckThread();
   time_t start_time = delete_begin.ToTimeT();
   time_t end_time = delete_end.ToTimeT();
 
   // TODO(rdsmith): Remove when http://crbug.com/96627 is resolved.
   {
     sql::Statement dbg_statement(GetDB().GetCachedStatement(
         SQL_FROM_HERE,
         "SELECT id FROM downloads WHERE start_time >= ? AND start_time < ? "
         "AND (State = ? OR State = ? OR State = ?)"));
-    if (!dbg_statement)
-      return;
     dbg_statement.BindInt64(0, start_time);
     dbg_statement.BindInt64(
         1,
         end_time ? end_time : std::numeric_limits<int64>::max());
     dbg_statement.BindInt(2, DownloadItem::COMPLETE);
     dbg_statement.BindInt(3, DownloadItem::CANCELLED);
     dbg_statement.BindInt(4, DownloadItem::INTERRUPTED);
+
+    if (!dbg_statement.is_valid())
+      return false;
+
     while (dbg_statement.Step()) {
       int64 id_to_delete = dbg_statement.ColumnInt64(0);
       returned_ids_.erase(id_to_delete);
     }
     CHECK(dbg_statement.Succeeded());
   }
 
   // This does not use an index. We currently aren't likely to have enough
   // downloads where an index by time will give us a lot of benefit.
   sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM downloads WHERE start_time >= ? AND start_time < ? "
       "AND (State = ? OR State = ? OR State = ?)"));
-  if (!statement)
-    return;
-
   statement.BindInt64(0, start_time);
   statement.BindInt64(
       1,
       end_time ? end_time : std::numeric_limits<int64>::max());
   statement.BindInt(2, DownloadItem::COMPLETE);
   statement.BindInt(3, DownloadItem::CANCELLED);
   statement.BindInt(4, DownloadItem::INTERRUPTED);
-  statement.Run();
+
+  return statement.Run();
 }
 
 }  // namespace history