Clean up entries left by crashes in the DownloadDB.

chrome/browser/history/download_database.h

Index: chrome/browser/history/download_database.h
diff --git a/chrome/browser/history/download_database.h b/chrome/browser/history/download_database.h
index d8ceda9c3e014dcae6eae2969c2c9f44cea99cd1..e70341bc164c48595d208aaac556eb691d7a3882 100644
--- a/chrome/browser/history/download_database.h
+++ b/chrome/browser/history/download_database.h
@@ -22,6 +22,24 @@ struct DownloadRow;
 // Maintains a table of downloads.
 class DownloadDatabase {
  public:
+  // Visible outside of download_database.cc only for testing.

[brettw, 2013/04/01 23:52:51]

Can this be private and the test made a friend? Also, I think these two sections
need a comment about what they mean.

[Randy Smith (Not in Mondays), 2013/04/02 18:58:52]

Done, but I'm not certain I think it was worth it, as it resulted in extra
include files in .h files (because of moving static functions into the class so
they had access to the constants and that requiring including a content include
file in the .h file).  I've isolated the changes for this comment to a single
patch set; willing to take a quick look and decide if you think it was worth it?
 If so, I'm good--I could just imagine that generic goodness would not equal
specific goodness in your mind in this case.
Done.

+  static const int kStateInvalid;
+  static const int kStateInProgress;
+  static const int kStateComplete;
+  static const int kStateCancelled;
+  static const int kStateBug140687;
+  static const int kStateInterrupted;
+
+  static const int kDangerTypeInvalid;
+  static const int kDangerTypeNotDangerous;
+  static const int kDangerTypeDangerousFile;
+  static const int kDangerTypeDangerousUrl;
+  static const int kDangerTypeDangerousContent;
+  static const int kDangerTypeMaybeDangerousContent;
+  static const int kDangerTypeUncommonContent;
+  static const int kDangerTypeUserValidated;
+  static const int kDangerTypeDangerousHost;
+
   // The value of |db_handle| indicating that the associated DownloadItem is not
   // yet persisted.
   static const int64 kUninitializedHandle;
@@ -41,12 +59,6 @@ class DownloadDatabase {
   // to select the row in the database table to update.
   bool UpdateDownload(const DownloadRow& data);
 
-  // Fixes state of the download entries. Sometimes entries with IN_PROGRESS
-  // state are not updated during browser shutdown (particularly when crashing).
-  // On the next start such entries are considered canceled. This functions
-  // fixes such entries.
-  bool CleanUpInProgressEntries();
-
   // Create a new database entry for one download and return its primary db id.
   int64 CreateDownload(const DownloadRow& info);
 
@@ -79,6 +91,13 @@ class DownloadDatabase {
   bool DropDownloadTable();
 
  private:
+  // Fixes state of the download entries. Sometimes entries with IN_PROGRESS
+  // state are not updated during browser shutdown (particularly when crashing).
+  // On the next start such entries are considered interrupted with
+  // interrupt reason |DOWNLOAD_INTERRUPT_REASON_CRASH|.  This functions
+  // fixes such entries.
+  void CleanUpInProgressEntries();
+
   bool EnsureColumnExists(const std::string& name, const std::string& type);
 
   bool owning_thread_set_;
@@ -87,6 +106,13 @@ class DownloadDatabase {
   int next_id_;
   int next_db_handle_;
 
+  // Initialized to false on construction, and checked in all functional
+  // routines post-migration in the database for a possible call to
+  // CleanUpInProgressEntries().  This allows us to avoid
+  // doing the cleanup until after any DB migration and unless we are
+  // actually use the downloads database.
+  bool in_progress_entry_cleanup_completed_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadDatabase);
 };