Reduced the lifetime of DownloadCreateInfo.

chrome/browser/download/download_item.cc

Index: chrome/browser/download/download_item.cc
diff --git a/chrome/browser/download/download_item.cc b/chrome/browser/download/download_item.cc
index 59126a86d43769586c96fbf06678a0fd2ce9b69c..56d985225c28206f5c7c845a7e05bce4026278e7 100644
--- a/chrome/browser/download/download_item.cc
+++ b/chrome/browser/download/download_item.cc
@@ -14,13 +14,13 @@
 #include "base/timer.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/net_util.h"
+#include "chrome/browser/download/download_create_info.h"
 #include "chrome/browser/download/download_extensions.h"
 #include "chrome/browser/download/download_file_manager.h"
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_util.h"
-#include "chrome/browser/history/download_create_info.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -112,37 +112,67 @@ DownloadItem::DangerType GetDangerType(bool dangerous_file,
 
 }  // namespace
 
+DownloadItem::DownloadStateInfo::DownloadStateInfo()
+  : path_uniquifier(0),

[Paweł Hajdan Jr., 2011/05/20 09:04:42]

nit: Shouldn't this be indented +4?

[ahendrickson, 2011/05/20 18:31:25]

Done.

+    has_user_gesture(false),
+    prompt_user_for_save_location(false),
+    is_dangerous_file(false),
+    is_dangerous_url(false),
+    is_extension_install(false) {
+}
+
+DownloadItem::DownloadStateInfo::DownloadStateInfo(
+    bool has_user_gesture,
+    bool prompt_user_for_save_location)
+  : path_uniquifier(0),
+    has_user_gesture(has_user_gesture),
+    prompt_user_for_save_location(prompt_user_for_save_location),
+    is_dangerous_file(false),
+    is_dangerous_url(false),
+    is_extension_install(false) {
+}
+
+DownloadItem::DownloadStateInfo::DownloadStateInfo(
+    const FilePath& target,
+    const FilePath& forced_name,
+    bool has_user_gesture,
+    bool prompt_user_for_save_location,
+    int uniquifier,
+    bool dangerous_file,
+    bool dangerous_url,
+    bool extension_install)
+  : target_name(target),
+    path_uniquifier(uniquifier),
+    has_user_gesture(has_user_gesture),
+    prompt_user_for_save_location(prompt_user_for_save_location),
+    is_dangerous_file(dangerous_file),
+    is_dangerous_url(dangerous_url),
+    is_extension_install(extension_install),
+    force_file_name(forced_name) {
+}
+
+bool DownloadItem::DownloadStateInfo::IsDangerous() const {
+  return is_dangerous_url || is_dangerous_file;
+}
+
 // Constructor for reading from the history service.
 DownloadItem::DownloadItem(DownloadManager* download_manager,
-                           const DownloadCreateInfo& info)
-    : id_(-1),
-      full_path_(info.path),
-      path_uniquifier_(0),
-      url_chain_(info.url_chain),
-      referrer_url_(info.referrer_url),
-      mime_type_(info.mime_type),
-      original_mime_type_(info.original_mime_type),
-      total_bytes_(info.total_bytes),
-      received_bytes_(info.received_bytes),
+                           const DownloadHistoryInfo& info)
+    : history_info_(info.path, info.url_chain, info.referrer_url,
+                    info.start_time, info.received_bytes, info.total_bytes,
+                    info.state, info.db_handle, info.download_id),
       start_tick_(base::TimeTicks()),
-      state_(static_cast<DownloadState>(info.state)),
-      start_time_(info.start_time),
-      db_handle_(info.db_handle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
       safety_state_(SAFE),
-      danger_type_(NOT_DANGEROUS),
       auto_opened_(false),
-      target_name_(info.original_name),
-      save_as_(false),
       is_otr_(false),
-      is_extension_install_(info.is_extension_install),
       is_temporary_(false),
       all_data_saved_(false),
       opened_(false) {
   if (IsInProgress())
-    state_ = CANCELLED;
+    history_info_.state = CANCELLED;
   if (IsComplete())
     all_data_saved_ = true;
   Init(false /* don't start progress timer */);
@@ -152,33 +182,28 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
 DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const DownloadCreateInfo& info,
                            bool is_otr)
-    : id_(info.download_id),
-      full_path_(info.path),
-      path_uniquifier_(info.path_uniquifier),
-      url_chain_(info.url_chain),
-      referrer_url_(info.referrer_url),
+    : history_info_(info.path, info.url_chain, info.referrer_url,
+                    info.start_time, info.received_bytes, info.total_bytes,
+                    IN_PROGRESS, DownloadHistory::kUninitializedHandle,
+                    info.download_id),
+      manager_state_(info.original_name, info.save_info.file_path,
+             info.has_user_gesture, info.prompt_user_for_save_location,
+             info.path_uniquifier, info.is_dangerous_file,
+             info.is_dangerous_url, info.is_extension_install),
+      process_handle_(info.process_handle),
+      content_disposition_(info.content_disposition),
       mime_type_(info.mime_type),
       original_mime_type_(info.original_mime_type),
-      total_bytes_(info.total_bytes),
-      received_bytes_(0),
+      referrer_charset_(info.referrer_charset),
       last_os_error_(0),
       start_tick_(base::TimeTicks::Now()),
-      state_(IN_PROGRESS),
-      start_time_(info.start_time),
-      db_handle_(DownloadHistory::kUninitializedHandle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
       safety_state_(GetSafetyState(info.is_dangerous_file,
                                    info.is_dangerous_url)),
-      danger_type_(GetDangerType(info.is_dangerous_file,
-                                 info.is_dangerous_url)),
       auto_opened_(false),
-      target_name_(info.original_name),
-      process_handle_(info.process_handle),
-      save_as_(info.prompt_user_for_save_location),
       is_otr_(is_otr),
-      is_extension_install_(info.is_extension_install),
       is_temporary_(!info.save_info.file_path.empty()),
       all_data_saved_(false),
       opened_(false) {
@@ -190,29 +215,15 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const FilePath& path,
                            const GURL& url,
                            bool is_otr)
-    : id_(1),
-      full_path_(path),
-      path_uniquifier_(0),
-      url_chain_(1, url),
-      referrer_url_(GURL()),
-      mime_type_(std::string()),
-      original_mime_type_(std::string()),
-      total_bytes_(0),
-      received_bytes_(0),
+    : history_info_(path, url, base::Time::Now(), 0, 0, IN_PROGRESS),
       last_os_error_(0),
       start_tick_(base::TimeTicks::Now()),
-      state_(IN_PROGRESS),
-      start_time_(base::Time::Now()),
-      db_handle_(DownloadHistory::kUninitializedHandle),
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
       safety_state_(SAFE),
-      danger_type_(NOT_DANGEROUS),
       auto_opened_(false),
-      save_as_(false),
       is_otr_(is_otr),
-      is_extension_install_(false),
       is_temporary_(false),
       all_data_saved_(false),
       opened_(false) {
@@ -220,7 +231,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
 }
 
 DownloadItem::~DownloadItem() {
-  state_ = REMOVING;
+  history_info_.state = REMOVING;
   UpdateObservers();
 }
 
@@ -237,7 +248,7 @@ void DownloadItem::UpdateObservers() {
 }
 
 bool DownloadItem::CanOpenDownload() {
-  return !Extension::IsExtension(target_name_);
+  return !Extension::IsExtension(manager_state_.target_name);
 }
 
 bool DownloadItem::ShouldOpenFileBasedOnExtension() {
@@ -290,18 +301,18 @@ void DownloadItem::ShowDownloadInShell() {
 
 void DownloadItem::DangerousDownloadValidated() {
   UMA_HISTOGRAM_ENUMERATION("Download.DangerousDownloadValidated",
-                            danger_type_,
+                            CurrentDangerType(),
                             DANGEROUS_TYPE_MAX);
   download_manager_->DangerousDownloadValidated(this);
 }
 
 void DownloadItem::UpdateSize(int64 bytes_so_far) {
-  received_bytes_ = bytes_so_far;
+  history_info_.received_bytes = bytes_so_far;
 
   // If we've received more data than we were expecting (bad server info?),
   // revert to 'unknown size mode'.
-  if (received_bytes_ > total_bytes_)
-    total_bytes_ = 0;
+  if (history_info_.received_bytes > history_info_.total_bytes)
+    history_info_.total_bytes = 0;
 }
 
 void DownloadItem::StartProgressTimer() {
@@ -336,16 +347,16 @@ void DownloadItem::Cancel(bool update_history) {
 
   download_util::RecordDownloadCount(download_util::CANCELLED_COUNT);
 
-  state_ = CANCELLED;
+  history_info_.state = CANCELLED;
   UpdateObservers();
   StopProgressTimer();
   if (update_history)
-    download_manager_->DownloadCancelled(id_);
+    download_manager_->DownloadCancelled(history_info_.download_id);
 }
 
 void DownloadItem::MarkAsComplete() {
   DCHECK(all_data_saved_);
-  state_ = COMPLETE;
+  history_info_.state = COMPLETE;
   UpdateObservers();
 }
 
@@ -380,7 +391,7 @@ void DownloadItem::Completed() {
   }
 
   DCHECK(all_data_saved_);
-  state_ = COMPLETE;
+  history_info_.state = COMPLETE;
   UpdateObservers();
   download_manager_->DownloadCompleted(id());
 }
@@ -388,7 +399,7 @@ void DownloadItem::Completed() {
 void DownloadItem::Interrupted(int64 size, int os_error) {
   if (!IsInProgress())
     return;
-  state_ = INTERRUPTED;
+  history_info_.state = INTERRUPTED;
   last_os_error_ = os_error;
   UpdateSize(size);
   StopProgressTimer();
@@ -398,11 +409,11 @@ void DownloadItem::Interrupted(int64 size, int os_error) {
 void DownloadItem::Delete(DeleteReason reason) {
   switch (reason) {
     case DELETE_DUE_TO_USER_DISCARD:
-      UMA_HISTOGRAM_ENUMERATION("Download.UserDiscard", danger_type_,
+      UMA_HISTOGRAM_ENUMERATION("Download.UserDiscard", CurrentDangerType(),
                                 DANGEROUS_TYPE_MAX);
       break;
     case DELETE_DUE_TO_BROWSER_SHUTDOWN:
-      UMA_HISTOGRAM_ENUMERATION("Download.Discard", danger_type_,
+      UMA_HISTOGRAM_ENUMERATION("Download.Discard", CurrentDangerType(),
                                 DANGEROUS_TYPE_MAX);
       break;
     default:
@@ -410,28 +421,28 @@ void DownloadItem::Delete(DeleteReason reason) {
   }
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableFunction(&DeleteDownloadedFile, full_path_));
+      NewRunnableFunction(&DeleteDownloadedFile, history_info_.path));
   Remove();
   // We have now been deleted.
 }
 
 void DownloadItem::Remove() {
   Cancel(true);
-  state_ = REMOVING;
-  download_manager_->RemoveDownload(db_handle_);
+  history_info_.state = REMOVING;
+  download_manager_->RemoveDownload(history_info_.db_handle);
   // We have now been deleted.
 }
 
 bool DownloadItem::TimeRemaining(base::TimeDelta* remaining) const {
-  if (total_bytes_ <= 0)
+  if (history_info_.total_bytes <= 0)
     return false;  // We never received the content_length for this download.
 
   int64 speed = CurrentSpeed();
   if (speed == 0)
     return false;
 
-  *remaining =
-      base::TimeDelta::FromSeconds((total_bytes_ - received_bytes_) / speed);
+  *remaining = base::TimeDelta::FromSeconds(
+      (history_info_.total_bytes - history_info_.received_bytes) / speed);
   return true;
 }
 
@@ -440,12 +451,14 @@ int64 DownloadItem::CurrentSpeed() const {
     return 0;
   base::TimeDelta diff = base::TimeTicks::Now() - start_tick_;
   int64 diff_ms = diff.InMilliseconds();
-  return diff_ms == 0 ? 0 : received_bytes_ * 1000 / diff_ms;
+  return diff_ms == 0 ? 0 : history_info_.received_bytes * 1000 / diff_ms;
 }
 
 int DownloadItem::PercentComplete() const {
-  return (total_bytes_ > 0) ?
-      static_cast<int>(received_bytes_ * 100.0 / total_bytes_) : -1;
+  return (history_info_.total_bytes > 0) ?
+      static_cast<int>(history_info_.received_bytes * 100.0 /
+          history_info_.total_bytes) :
+      -1;
 }
 
 void DownloadItem::Rename(const FilePath& full_path) {
@@ -453,12 +466,12 @@ void DownloadItem::Rename(const FilePath& full_path) {
            << " full_path = \"" << full_path.value() << "\""
            << " " << DebugString(true);
   DCHECK(!full_path.empty());
-  full_path_ = full_path;
+  history_info_.path = full_path;
 }
 
 void DownloadItem::TogglePause() {
   DCHECK(IsInProgress());
-  download_manager_->PauseDownload(id_, !is_paused_);
+  download_manager_->PauseDownload(history_info_.download_id, !is_paused_);
   is_paused_ = !is_paused_;
   UpdateObservers();
 }
@@ -487,7 +500,7 @@ void DownloadItem::OnDownloadCompleting(DownloadFileManager* file_manager) {
 
 void DownloadItem::OnDownloadRenamedToFinalName(const FilePath& full_path) {
   VLOG(20) << __FUNCTION__ << "()"
-           << " full_path = " << full_path.value()
+           << " full_path = \"" << full_path.value() << "\""
            << " needed rename = " << NeedsRename()
            << " " << DebugString(false);
   DCHECK(NeedsRename());
@@ -503,7 +516,7 @@ bool DownloadItem::MatchesQuery(const string16& query) const {
 
   DCHECK_EQ(query, base::i18n::ToLower(query));
 
-  string16 url_raw(base::i18n::ToLower(UTF8ToUTF16(url().spec())));
+  string16 url_raw(base::i18n::ToLower(UTF8ToUTF16(GetUrl().spec())));
   if (url_raw.find(query) != string16::npos)
     return true;
 
@@ -514,7 +527,8 @@ bool DownloadItem::MatchesQuery(const string16& query) const {
   //   "/%E4%BD%A0%E5%A5%BD%E4%BD%A0%E5%A5%BD"
   PrefService* prefs = download_manager_->profile()->GetPrefs();
   std::string languages(prefs->GetString(prefs::kAcceptLanguages));
-  string16 url_formatted(base::i18n::ToLower(net::FormatUrl(url(), languages)));
+  string16 url_formatted(
+      base::i18n::ToLower(net::FormatUrl(GetUrl(), languages)));
   if (url_formatted.find(query) != string16::npos)
     return true;
 
@@ -526,60 +540,53 @@ bool DownloadItem::MatchesQuery(const string16& query) const {
   return (path.find(query) != string16::npos);
 }
 
-void DownloadItem::SetFileCheckResults(const FilePath& path,
-                                       bool is_dangerous_file,
-                                       bool is_dangerous_url,
-                                       int path_uniquifier,
-                                       bool prompt,
-                                       bool is_extension_install,
-                                       const FilePath& original_name) {
-  VLOG(20) << " " << __FUNCTION__ << "()"
-           << " path = \"" << path.value() << "\""
-           << " is_dangerous_file = " << is_dangerous_file
-           << " is_dangerous_url = " << is_dangerous_url
-           << " path_uniquifier = " << path_uniquifier
-           << " prompt = " << prompt
-           << " is_extension_install = " << is_extension_install
-           << " path = \"" << path.value() << "\""
-           << " original_name = \"" << original_name.value() << "\""
-           << " " << DebugString(true);
-  // Make sure the initial file name is set only once.
-  DCHECK(full_path_.empty());
-  DCHECK(!path.empty());
+void DownloadItem::SetFileCheckResults(const DownloadStateInfo& state) {
+  VLOG(20) << " " << __FUNCTION__ << "()" << " this = " << DebugString(true);
+  manager_state_ = state;
+  VLOG(20) << " " << __FUNCTION__ << "()" << " this = " << DebugString(true);
+
+  safety_state_ = GetSafetyState(manager_state_.is_dangerous_file,
+                                 manager_state_.is_dangerous_url);
+}
+
+void DownloadItem::UpdateTarget() {
+  if (manager_state_.target_name.value().empty())
+    manager_state_.target_name = history_info_.path.BaseName();
+}
+
+DownloadItem::DangerType DownloadItem::CurrentDangerType() const {
+  return GetDangerType(manager_state_.is_dangerous_file,
+                       manager_state_.is_dangerous_url);
+}
 
-  full_path_ = path;
-  safety_state_ = GetSafetyState(is_dangerous_file, is_dangerous_url);
-  danger_type_ = GetDangerType(is_dangerous_file, is_dangerous_url);
-  path_uniquifier_ = path_uniquifier;
-  save_as_ = prompt;
-  is_extension_install_ = is_extension_install;
-  target_name_ = original_name;
+bool DownloadItem::IsDangerous() const {
+  return CurrentDangerType() != DownloadItem::NOT_DANGEROUS;
+}
 
-  if (target_name_.value().empty())
-    target_name_ = full_path_.BaseName();
+void DownloadItem::MarkUrlDangerous() {
+  manager_state_.is_dangerous_url = true;
 }
 
 FilePath DownloadItem::GetTargetFilePath() const {
-  return full_path_.DirName().Append(target_name_);
+  return history_info_.path.DirName().Append(manager_state_.target_name);
 }
 
 FilePath DownloadItem::GetFileNameToReportUser() const {
-  if (path_uniquifier_ > 0) {
-    FilePath name(target_name_);
-    download_util::AppendNumberToPath(&name, path_uniquifier_);
+  if (manager_state_.path_uniquifier > 0) {
+    FilePath name(manager_state_.target_name);
+    download_util::AppendNumberToPath(&name, manager_state_.path_uniquifier);
     return name;
   }
-  return target_name_;
+  return manager_state_.target_name;
 }
 
 FilePath DownloadItem::GetUserVerifiedFilePath() const {
   return (safety_state_ == DownloadItem::SAFE) ?
-      GetTargetFilePath() : full_path_;
+      GetTargetFilePath() : history_info_.path;
 }
 
 void DownloadItem::Init(bool start_timer) {
-  if (target_name_.value().empty())
-    target_name_ = full_path_.BaseName();
+  UpdateTarget();
   if (start_timer)
     StartProgressTimer();
   VLOG(20) << __FUNCTION__ << "() " << DebugString(true);
@@ -588,38 +595,59 @@ void DownloadItem::Init(bool start_timer) {
 // TODO(ahendrickson) -- Move |INTERRUPTED| from |IsCancelled()| to
 // |IsPartialDownload()|, when resuming interrupted downloads is implemented.
 bool DownloadItem::IsPartialDownload() const {
-  return (state_ == IN_PROGRESS);
+  return (history_info_.state == IN_PROGRESS);
 }
 
 bool DownloadItem::IsInProgress() const {
-  return (state_ == IN_PROGRESS);
+  return (history_info_.state == IN_PROGRESS);
 }
 
 bool DownloadItem::IsCancelled() const {
-  return (state_ == CANCELLED) || (state_ == INTERRUPTED);
+  return (history_info_.state == CANCELLED) ||
+         (history_info_.state == INTERRUPTED);
 }
 
 bool DownloadItem::IsInterrupted() const {
-  return (state_ == INTERRUPTED);
+  return (history_info_.state == INTERRUPTED);
 }
 
 bool DownloadItem::IsComplete() const {
-  return (state_ == COMPLETE);
+  return (history_info_.state == COMPLETE);
+}
+
+// This function converts history_info_.state (which is an int32) to a
+// DownloadItem::DownloadState enum, and returns it.
+// A DCHECK() will be fire if it has an illegal value.

[Paweł Hajdan Jr., 2011/05/20 09:04:42]

nit: This is an implementation comment, please remove.

[ahendrickson, 2011/05/20 18:31:25]

Done.

+DownloadItem::DownloadState DownloadItem::state() const {
+  if ((history_info_.state < 0) ||

[Paweł Hajdan Jr., 2011/05/20 09:04:42]

To make sure it works, could you explicitly initialize the first entry of the
enum to 0?

[ahendrickson, 2011/05/20 18:31:25]

Done.

+      (history_info_.state > MAX_DOWNLOAD_STATE)) {

[Paweł Hajdan Jr., 2011/05/20 09:04:42]

nit: Shouldn't it be >= MAX_DOWNLOAD_STATE? I think MAX_DOWNLOAD_STATE is an
invalid state, right?

[ahendrickson, 2011/05/20 18:31:25]

Done.

+    NOTREACHED() << " state = " << history_info_.state;
+    return IN_PROGRESS;
+  }
+  return static_cast<DownloadState>(history_info_.state);
+}
+
+const GURL& DownloadItem::GetUrl() const {
+  return history_info_.url_chain.empty() ?
+             GURL::EmptyGURL() : history_info_.url_chain.back();
 }
 
 std::string DownloadItem::DebugString(bool verbose) const {
-  std::string description = base::StringPrintf(
-      "{ id_ = %d state = %s", id_, DebugDownloadStateString(state()));
+  std::string description =
+      base::StringPrintf("{ id = %d"
+                         " state = %s",
+                         history_info_.download_id,
+                         DebugDownloadStateString(state()));
 
   // Construct a string of the URL chain.
   std::string url_list("<none>");
-  if (!url_chain_.empty()) {
-    std::vector<GURL>::const_iterator iter = url_chain_.begin();
-    std::vector<GURL>::const_iterator last = url_chain_.end();
+  if (!history_info_.url_chain.empty()) {
+    std::vector<GURL>::const_iterator iter = history_info_.url_chain.begin();
+    std::vector<GURL>::const_iterator last = history_info_.url_chain.end();
     url_list = (*iter).spec();
     ++iter;
     for ( ; verbose && (iter != last); ++iter) {
-      url_list += " -> ";
+      url_list += " ->\n\t";
       const GURL& next_url = *iter;
       url_list += next_url.spec();
     }
@@ -629,12 +657,12 @@ std::string DownloadItem::DebugString(bool verbose) const {
     description += base::StringPrintf(
         " db_handle = %" PRId64
         " total_bytes = %" PRId64
-        " is_paused = " "%c"
-        " is_extension_install = " "%c"
-        " is_otr = " "%c"
-        " safety_state = " "%s"
-        " url_chain = " "\"%s\""
-        " target_name_ = \"%" PRFilePath "\""
+        " is_paused = %c"
+        " is_extension_install = %c"
+        " is_otr = %c"
+        " safety_state = %s"
+        " url_chain = \n\t\"%s\"\n\t"
+        " target_name = \"%" PRFilePath "\""
         " full_path = \"%" PRFilePath "\"",
         db_handle(),
         total_bytes(),
@@ -643,10 +671,13 @@ std::string DownloadItem::DebugString(bool verbose) const {
         is_otr() ? 'T' : 'F',
         DebugSafetyStateString(safety_state()),
         url_list.c_str(),
-        target_name_.value().c_str(),
+        manager_state_.target_name.value().c_str(),
         full_path().value().c_str());
   } else {
     description += base::StringPrintf(" url = \"%s\"", url_list.c_str());
   }
+
+  description += " }";
+
   return description;
 }