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

chrome/browser/download/download_manager.cc

Index: chrome/browser/download/download_manager.cc
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index fc42b17ceb82a7dac1fc8fffc8b93c65787c748a..84e2852aab4aef2d6190e6ba43cf93897df6ee5a 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/download/download_manager.h"
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
@@ -12,6 +13,7 @@
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
+#include "base/synchronization/lock.h"
 #include "base/sys_string_conversions.h"
 #include "base/task.h"
 #include "base/utf_string_conversions.h"
@@ -52,6 +54,7 @@
 
 DownloadManager::DownloadManager(DownloadStatusUpdater* status_updater)
     : shutdown_needed_(false),
+      next_id_(0),
       profile_(NULL),
       file_manager_(NULL),
       status_updater_(status_updater->AsWeakPtr()) {
@@ -222,6 +225,26 @@ void DownloadManager::SearchDownloads(const string16& query,
     original_profile->GetDownloadManager()->SearchDownloads(query, result);
 }
 
+void DownloadManager::OnHistoryGetNextId(int next_id) {
+  DVLOG(1) << __FUNCTION__ << " " << next_id;
+  base::AutoLock _(next_id_lock_);

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

I puzzled over this for a while before I got it.  It's a fine idiom, but it's
not one I've seen before in Chrome, and if you're not used to it I think it'll
confused people.  I'd use "autolock" instead of "_".

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

Done.

+  // TODO(benjhayden) Delay Profile initialization until here, and set next_id_
+  // = next_id.

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

A bit more of a comment as to why this actually makes sense would make sense, I
think (i.e. explain that we just need to make sure assigned ids are unique per
session for right now, and this does it in the face of the download/getting
first id race).

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

Done.

+  next_id_ += next_id;
+}
+
+DownloadId DownloadManager::GetNextId() {
+  // May be called on any thread via the GetNextIdThunk.
+  // TODO(benjhayden) If otr, forward to parent DM.
+  base::AutoLock _(next_id_lock_);
+  return DownloadId(this, next_id_++);
+}
+
+DownloadManager::GetNextIdThunkType DownloadManager::GetNextIdThunk() {
+  // TODO(benjhayden) If otr, forward to parent DM.
+  return base::Bind(&DownloadManager::GetNextId, this);
+}
+
 // Query the history service for information about all persisted downloads.
 bool DownloadManager::Init(Profile* profile) {
   DCHECK(profile);
@@ -230,6 +253,8 @@ bool DownloadManager::Init(Profile* profile) {
 
   profile_ = profile;
   download_history_.reset(new DownloadHistory(profile));
+  download_history_->GetNextId(NewCallback(
+      this, &DownloadManager::OnHistoryGetNextId));
   download_history_->Load(
       NewCallback(this, &DownloadManager::OnQueryDownloadEntriesComplete));
 
@@ -606,7 +631,7 @@ void DownloadManager::ContinueDownloadWithPath(DownloadItem* download,
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
           file_manager_, &DownloadFileManager::RenameInProgressDownloadFile,
-          download->id(), download_path));
+          download->gid(), download_path));
 
   download->Rename(download_path);
 
@@ -813,7 +838,7 @@ void DownloadManager::OnDownloadRenamedToFinalName(int download_id,
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
-          file_manager_, &DownloadFileManager::CompleteDownload, download_id));
+          file_manager_, &DownloadFileManager::CompleteDownload, item->gid()));
 
   if (uniquifier)
     item->set_path_uniquifier(uniquifier);
@@ -849,10 +874,9 @@ void DownloadManager::DownloadCancelledInternal(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   request_handle.CancelRequest();
 
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::CancelDownload, download_id));
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, NewRunnableMethod(
+      file_manager_, &DownloadFileManager::CancelDownload,
+      DownloadId(this, download_id)));
 }
 
 void DownloadManager::OnDownloadError(int32 download_id,
@@ -885,10 +909,8 @@ void DownloadManager::OnDownloadError(int32 download_id,
     download_history_->UpdateEntry(download);
   }
 
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::CancelDownload, download_id));
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, NewRunnableMethod(
+      file_manager_, &DownloadFileManager::CancelDownload, download->gid()));
 }
 
 void DownloadManager::UpdateAppIcon() {