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

chrome/browser/download/download_item.h

Index: chrome/browser/download/download_item.h
diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h
index 64704dd18debd44601dfb2ca18526e625d2b4170..a496f5323d43f2200e4949e65f442874a0b14286 100644
--- a/chrome/browser/download/download_item.h
+++ b/chrome/browser/download/download_item.h
@@ -33,6 +33,7 @@
 
 class CrxInstaller;
 class DownloadFileManager;
+class DownloadId;  // See the bottom of this file.
 class DownloadManager;
 struct DownloadCreateInfo;
 struct DownloadHistoryInfo;
@@ -276,6 +277,7 @@ class DownloadItem : public NotificationObserver {
   }
   int64 received_bytes() const { return received_bytes_; }
   int32 id() const { return download_id_; }
+  DownloadId gid() const;
   base::Time start_time() const { return start_time_; }
   void set_db_handle(int64 handle) { db_handle_ = handle; }
   int64 db_handle() const { return db_handle_; }
@@ -483,4 +485,54 @@ class DownloadItem : public NotificationObserver {
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
 
+// DownloadId combines per-profile Download ids with an indication of which
+// profile in order to be globally unique. DownloadIds are not persistent across
+// sessions.
+class DownloadId {

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

A download id seems like a "lower level" concept than a download item; for
instance, most of the places that need to know about DownloadIds (because
they're off the UI thread) don't need to know about DownloadItems (because
they're off the UI thread).  I think I'd suggest putting this in its own file.  

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

Done.

+ public:
+  explicit DownloadId(DownloadManager* m = NULL, int32 i = -1)
+    : manager_(m),
+      id_(i) {
+  }
+
+  // per-profile and persistent
+  int32 id() const { return id_; }
+
+  bool IsValid() const { return ((manager_ != NULL) && (id_ >= 0)); }
+
+  bool operator==(const DownloadId& that) const {

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

Chrome biases against creating full types (i.e. supporting ==, <, copy and
assign) so if you can put a comment in for why you need each of these (support
for STL containers mostly, I presume) that would be good.

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

Done.

+    return ((that.id_ == id_) &&
+            (that.manager_ == manager_));
+  }
+  bool operator<(const DownloadId& that) const {
+    return ((manager_ < that.manager_) ||
+            (id_ < that.id_));
+  }
+
+  size_t hash() const {
+    // The top half of manager is unlikely to be distinct, and the user is
+    // unlikely to have >64K downloads. If these assumptions are incorrect, then
+    // DownloadFileManager's hash_map might have a few collisions, but it will
+    // use operator== to safely disambiguate.
+    return reinterpret_cast<size_t>(manager_) +
+           (static_cast<size_t>(id_) << (4 * sizeof(size_t)));
+  }
+
+ private:
+  // DownloadId is used mostly off the UI thread, so manager's methods can't be
+  // called, but the pointer can be compared.
+  DownloadManager* manager_;
+
+  int32 id_;
+
+  // Allow copy and assign.
+};
+namespace __gnu_cxx {

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

This looks like black magic, and it's specifically scary to me since gcc isn't
the only compiler we're using.  Comment what's happening here?

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

Switched to BASE_HASH_NAMESPACE to afford the other compilers and commented, and
made a note to test this CL on the other platforms.

+template<> struct hash<DownloadId> {
+  size_t operator()(const DownloadId& did) const {
+    return did.hash();
+  }
+};
+}
+
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_H_