Fix history importing by delaying DownloadManager creation.

content/browser/download/download_item.h

Index: content/browser/download/download_item.h
diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h
index 7a19ea014107f293a632e8e043d9a96cf8097beb..5d8d619ac67c71bb27b39cf751108716ff7eb02e 100644
--- a/content/browser/download/download_item.h
+++ b/content/browser/download/download_item.h
@@ -26,6 +26,7 @@
 #include "base/time.h"
 #include "base/timer.h"
 #include "content/browser/download/download_request_handle.h"
+#include "content/browser/download/download_id.h"

[Miranda Callahan, 2011/10/27 17:00:19]

nit: alphabetization

[benjhayden, 2011/10/27 19:04:41]

Done.

 #include "content/browser/download/download_state_info.h"
 #include "content/browser/download/interrupt_reasons.h"
 #include "content/common/content_export.h"
@@ -114,7 +115,8 @@ class CONTENT_EXPORT DownloadItem {
 
   // Constructing from persistent store:
   DownloadItem(DownloadManager* download_manager,
-               const DownloadPersistentStoreInfo& info);
+               const DownloadPersistentStoreInfo& info,
+               const DownloadId& download_id);

[Randy Smith (Not in Mondays), 2011/10/27 18:01:44]

Long term, when we construct from the persistent store, we're going to set the
local part of the id from that and the global part from the DownloadManager. 
Short term, we're just going to get the next download id from the
DownloadIdFactory (right?).  So can we just ask the download manager to give us
the next download id now, rather than adding something ot the construction
interface?  We'd just leave the GetNextId() function on the download manager and
have it delegate to the factory.

[benjhayden, 2011/10/27 19:04:41]

Done.

 
   // Constructing for a regular download:
   DownloadItem(DownloadManager* download_manager,
@@ -277,8 +279,8 @@ class CONTENT_EXPORT DownloadItem {
     total_bytes_ = total_bytes;
   }
   int64 received_bytes() const { return received_bytes_; }
-  int32 id() const { return download_id_; }
-  DownloadId global_id() const;
+  int32 id() const { return download_id_.local(); }
+  const DownloadId& global_id() const { return download_id_; }

[Randy Smith (Not in Mondays), 2011/10/27 18:01:44]

Hmmm.  I'm actually a litte uncomfortable about this, just because if you get
your lifetimes wrong you'll be holding onto a reference that goes nowhere, and
it's not obvious at the call site that that's what's happening.  This is almost
certainly ok in the large majority of call sites, but it's also not hard to copy
a DownloadId, so I'd leave it as a straight return type.  Just a suggestion.

[benjhayden, 2011/10/27 19:04:41]

Done.

   base::Time start_time() const { return start_time_; }
   base::Time end_time() const { return end_time_; }
   void set_db_handle(int64 handle) { db_handle_ = handle; }
@@ -379,7 +381,7 @@ class CONTENT_EXPORT DownloadItem {
   DownloadRequestHandle request_handle_;
 
   // Download ID assigned by DownloadResourceHandler.
-  int32 download_id_;
+  DownloadId download_id_;
 
   // Full path to the downloaded or downloading file.
   FilePath full_path_;