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

chrome/browser/download/download_item.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Each download is represented by a DownloadItem, and all DownloadItems
 // are owned by the DownloadManager which maintains a global list of all
 // downloads. DownloadItems are created when a user initiates a download,
 // and exist for the duration of the browser life time.
 //
 // Download observers:
@@ skipping 15 matching lines @@
 #include "base/time.h"
 #include "base/timer.h"
 #include "chrome/browser/download/download_request_handle.h"
 #include "chrome/browser/download/download_state_info.h"
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 
 class CrxInstaller;
 class DownloadFileManager;
+class DownloadId;  // See the bottom of this file.
 class DownloadManager;
 struct DownloadCreateInfo;
 struct DownloadHistoryInfo;
 
 // One DownloadItem per download. This is the model class that stores all the
 // state for a download. Multiple views, such as a tab's download shelf and the
 // Destination tab's download view, may refer to a given DownloadItem.
 //
 // This is intended to be used only on the UI thread.
 class DownloadItem : public NotificationObserver {
@@ skipping 223 matching lines @@
   std::string content_disposition() const { return content_disposition_; }
   std::string mime_type() const { return mime_type_; }
   std::string original_mime_type() const { return original_mime_type_; }
   std::string referrer_charset() const { return referrer_charset_; }
   int64 total_bytes() const { return total_bytes_; }
   void set_total_bytes(int64 total_bytes) {
     total_bytes_ = total_bytes;
   }
   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_; }
   bool is_paused() const { return is_paused_; }
   bool open_when_complete() const { return open_when_complete_; }
   void set_open_when_complete(bool open) { open_when_complete_ = open; }
   bool file_externally_removed() const { return file_externally_removed_; }
   SafetyState safety_state() const { return safety_state_; }
   // Why |safety_state_| is not SAFE.
   DangerType GetDangerType() const;
@@ skipping 187 matching lines @@
   // Do we actual open downloads when requested?  For testing purposes
   // only.
   bool open_enabled_;
 
   // DownloadItem observes CRX installs it initiates.
   NotificationRegistrar registrar_;
 
   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_