Make a new integer field in sql::MetaTable (a per-profile db) containing a counter for the next download id, so that this id is unique across sessions. This id will allow us to merge download id with db_handle and merge most/all of the maps in DownloadManager in future CLs.

Make DownloadManager read this field to initialize its next_id_ counter in Init().
Put a fine-grained mutex in DownloadManager::GetNextId() so that it can be called directly from any thread.
Define a thunk wrapping DM::GNI() to be passed around between threads to guard against other threads calling any other DM methods.
This thunk owns a scoped_refptr<DM> to manage life-time issues. This pattern is implemented for DM elsewhere.
Store this thunk in ResourceContext to be called by ResourceDispatchHost/DownloadThrottlingResourceHandler on the IO thread. Pass the returned DownloadId into DownloadResourceHandler.
The alternative way to obtain ids on the IO thread is to jump over to the UI thread and back. This way would add significant latency to a critical path. GetNextId() should be fast and easily accessible from any thread.
Now that ids are per-profile, define a class DownloadId containing a per-profile id and an indication of which profile, currently the DownloadManager*. DownloadIds are hashable, comparable, globally unique, not persistent, and are used by DownloadFileManager.
When the download is added to the history, MetaTable.next_download_id will be set to the new download's id +1 if that number is greater than MT.next_download_id. Increasing this counter at the same time as the download is added to the db prevents the counter from desyncing from the db, which was the primary concern re storing the counter in the BrowserPrefs.
See also http://codereview.chromium.org/7192016
LMK what to write a test for if anything.

Review URL: http://codereview.chromium.org/7237034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98656 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 11 insertions, 16 deletions

diff --git a/content/browser/download/download_file_manager.h b/content/browser/download/download_file_manager.h
index 0c14dca..d7e62f0 100644
--- a/content/browser/download/download_file_manager.h
+++ b/content/browser/download/download_file_manager.h
@@ -49,6 +49,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/timer.h"
 #include "content/browser/download/download_request_handle.h"
+#include "content/browser/download/download_id.h"
 #include "ui/gfx/native_widget_types.h"
 
 struct DownloadBuffer;
@@ -73,21 +74,18 @@ class DownloadFileManager
   // Called on shutdown on the UI thread.
   void Shutdown();
 
-  // Called on the IO or UI threads.
-  int GetNextId();
-
   // Called on UI thread to make DownloadFileManager start the download.
   void StartDownload(DownloadCreateInfo* info);
 
   // Handlers for notifications sent from the IO thread and run on the
   // FILE thread.
-  void UpdateDownload(int id, DownloadBuffer* buffer);
+  void UpdateDownload(DownloadId id, DownloadBuffer* buffer);
   // |os_error| is 0 for normal completions, and non-0 for errors.
   // |security_info| contains SSL information (cert_id, cert_status,
   // security_bits, ssl_connection_status), which can be used to
   // fine-tune the error message.  It is empty if the transaction
   // was not performed securely.
-  void OnResponseCompleted(int id,
+  void OnResponseCompleted(DownloadId id,
                            DownloadBuffer* buffer,
                            int os_error,
                            const std::string& security_info);
@@ -97,22 +95,22 @@ class DownloadFileManager
   // download file, as far as the DownloadFileManager is concerned -- if
   // anything happens to the download file after they are called, it will
   // be ignored.
-  void CancelDownload(int id);
-  void CompleteDownload(int id);
+  void CancelDownload(DownloadId id);
+  void CompleteDownload(DownloadId id);
 
   // Called on FILE thread by DownloadManager at the beginning of its shutdown.
   void OnDownloadManagerShutdown(DownloadManager* manager);
 
   // The DownloadManager in the UI thread has provided an intermediate
   // .crdownload name for the download specified by |id|.
-  void RenameInProgressDownloadFile(int id, const FilePath& full_path);
+  void RenameInProgressDownloadFile(DownloadId id, const FilePath& full_path);
 
   // The DownloadManager in the UI thread has provided a final name for the
   // download specified by |id|.
   // |overwrite_existing_file| prevents uniquification, and is used for SAFE
   // downloads, as the user may have decided to overwrite the file.
   // Sent from the UI thread and run on the FILE thread.
-  void RenameCompletingDownloadFile(int id,
+  void RenameCompletingDownloadFile(DownloadId id,
                                     const FilePath& full_path,
                                     bool overwrite_existing_file);
 
@@ -144,20 +142,17 @@ class DownloadFileManager
                           bool hash_needed);
 
   // Called only on the download thread.
-  DownloadFile* GetDownloadFile(int id);
+  DownloadFile* GetDownloadFile(DownloadId id);
 
   // Called only from RenameInProgressDownloadFile and
   // RenameCompletingDownloadFile on the FILE thread.
-  void CancelDownloadOnRename(int id);
+  void CancelDownloadOnRename(DownloadId id);
 
   // Erases the download file with the given the download |id| and removes
   // it from the maps.
-  void EraseDownload(int id);
-
-  // Unique ID for each DownloadItem.
-  base::AtomicSequenceNumber next_id_;
+  void EraseDownload(DownloadId id);
 
-  typedef base::hash_map<int, DownloadFile*> DownloadFileMap;
+  typedef base::hash_map<DownloadId, DownloadFile*> DownloadFileMap;
 
   // A map of all in progress downloads.  It owns the download files.
   DownloadFileMap downloads_;