sql::MetaTable.next_download_id, DownloadManager::GetNextId()

chrome/browser/history/download_database.cc

Index: chrome/browser/history/download_database.cc
diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc
index 4f4bfac9e1d1b27fe02aacba4f03a3662d63b815..e485f2b5035bb875092e1cb8b23b872944de1d56 100644
--- a/chrome/browser/history/download_database.cc
+++ b/chrome/browser/history/download_database.cc
@@ -52,9 +52,14 @@ FilePath ColumnFilePath(sql::Statement& statement, int col) {
 
 #endif
 
+// Key in the meta_table containing the next id to use for a new download in
+// this profile.
+static const char kNextDownloadId[] = "next_download_id";
+
 }  // namespace
 
-DownloadDatabase::DownloadDatabase() {
+DownloadDatabase::DownloadDatabase()
+  : next_id_(0) {
 }
 
 DownloadDatabase::~DownloadDatabase() {
@@ -73,6 +78,8 @@ bool DownloadDatabase::InitDownloadTable() {
         "state INTEGER NOT NULL)"))
       return false;
   }
+  meta_table_.Init(&GetDB(), 0, 0);
+  meta_table_.GetValue(kNextDownloadId, &next_id_);
   return true;
 }
 
@@ -160,8 +167,12 @@ int64 DownloadDatabase::CreateDownload(const DownloadHistoryInfo& info) {
   statement.BindInt64(4, info.total_bytes);
   statement.BindInt(5, info.state);
 
-  if (statement.Run())
-    return GetDB().GetLastInsertRowId();
+  if (statement.Run()) {
+    int64 db_handle = GetDB().GetLastInsertRowId();
+    // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
+    meta_table_.SetValue(kNextDownloadId, ++next_id_);

[Randy Smith (Not in Mondays), 2011/07/25 20:20:12]

Is there a reason why it's safe not to implement the TODO?  The current behavior
looks like an obvious, dangerous race which the TODO is needed to fix.

[benjhayden, 2011/07/27 19:40:54]

This next_id_ is only communicated to DownloadManager once, and is not used to
allocate ids.
The todo cannot be implemented until id is added to DownloadHistoryInfo.

[Randy Smith (Not in Mondays), 2011/07/28 21:03:16]

Sorry, still having problems distinguishing non-persistent and persistent ids in
my head.  I think this is ok, since we're not actually storing anything in the
next_id_ namespace on a per-entry basis in the database.

[benjhayden, 2011/08/03 17:44:46]

Done.

+    return db_handle;
+  }
   return 0;
 }