Fix history importing by delaying DownloadManager creation.

content/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:
 //   DownloadItem::Observer:
 //     - allows observers to receive notifications about one download from start
 //       to completion
 // Use AddObserver() / RemoveObserver() on the appropriate download object to
 // receive state updates.
 
 #ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_H_
 #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_H_
 #pragma once
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/observer_list.h"
 #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"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_errors.h"
 
 class DownloadFileManager;
 class DownloadId;
 class DownloadManager;
 struct DownloadCreateInfo;
@@ skipping 68 matching lines @@
 
     // Called when a downloaded file has been opened.
     virtual void OnDownloadOpened(DownloadItem* download) = 0;
 
    protected:
     virtual ~Observer() {}
   };
 
   // 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,
                const DownloadCreateInfo& info,
                const DownloadRequestHandle& request_handle,
                bool is_otr);
 
   // Constructing for the "Save Page As..." feature:
   DownloadItem(DownloadManager* download_manager,
                const FilePath& path,
@@ skipping 142 matching lines @@
   std::string suggested_filename() const { return suggested_filename_; }
   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 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; }
   int64 db_handle() const { return db_handle_; }
   DownloadManager* download_manager() { return download_manager_; }
   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_; }
@@ skipping 80 matching lines @@
   void UpdateTarget();
 
   // State information used by the download manager.
   DownloadStateInfo state_info_;
 
   // The handle to the request information.  Used for operations outside the
   // download system.
   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_;
 
   // A number that should be appended to the path to make it unique, or 0 if the
   // path should be used as is.
   int path_uniquifier_;
 
   // The chain of redirects that leading up to and including the final URL.
   std::vector<GURL> url_chain_;
@@ skipping 91 matching lines @@
   // only.
   bool open_enabled_;
 
   // Did the delegate delay calling Complete on this download?
   bool delegate_delayed_complete_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_H_