Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1761)

Unified Diff: chrome/browser/download/download_item.h

Issue 6969009: Reduced the lifetime of DownloadCreateInfo. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Merged with trunk Created 9 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/download/download_item.h
diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h
index 3a6bbd7f649545e25039b9d3762ef3483cd06244..053bc5fc3564462ab216b7113908e332804c5094 100644
--- a/chrome/browser/download/download_item.h
+++ b/chrome/browser/download/download_item.h
@@ -26,11 +26,12 @@
#include "base/time.h"
#include "base/timer.h"
#include "chrome/browser/download/download_process_handle.h"
+#include "chrome/browser/history/download_history_info.h"
#include "googleurl/src/gurl.h"
+struct DownloadCreateInfo;
class DownloadFileManager;
class DownloadManager;
-struct DownloadCreateInfo;
// 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
@@ -86,6 +87,56 @@ class DownloadItem {
DELETE_DUE_TO_USER_DISCARD
};
+ // Contains information relating to the process of determining what to do with
+ // the download.
+ struct DownloadStateInfo {
Paweł Hajdan Jr. 2011/05/19 16:18:25 Why is it declared inside DownloadInfo? Is it poss
ahendrickson 2011/05/19 20:16:49 I've been having problems with creating new files
Paweł Hajdan Jr. 2011/05/20 09:04:42 I'm not sure whether it's some local problem or so
ahendrickson 2011/05/20 18:31:24 Extracted.
+ DownloadStateInfo();
+ explicit DownloadStateInfo(const DownloadItem& item);
+ DownloadStateInfo(bool has_user_gesture,
+ bool prompt_user_for_save_location);
+ 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);
+
+ // Indicates if the download is dangerous.
+ bool IsDangerous() const;
+
+ // The original name for a dangerous download, specified by the request.
+ FilePath target_name;
+
+ FilePath suggested_path;
+
+ // A number that should be added to the suggested path to make it unique.
+ // 0 means no number should be appended. It is eventually incorporated
+ // into the final file name.
+ int path_uniquifier;
+
+ bool has_user_gesture;
+
+ // True if we should display the 'save as...' UI and prompt the user
+ // for the download location.
+ // False if the UI should be suppressed and the download performed to the
+ // default location.
+ bool prompt_user_for_save_location;
+
+ // Whether this download file is potentially dangerous (ex: exe, dll, ...).
+ bool is_dangerous_file;
+
+ // If safebrowsing believes this URL leads to malware.
+ bool is_dangerous_url;
+
+ // Whether this download is for extension install or not.
+ bool is_extension_install;
+
+ // Whether this download's file name was specified initially.
+ FilePath force_file_name;
+ };
+
// Interface that observers of a particular download must implement in order
// to receive updates to the download's status.
class Observer {
@@ -101,7 +152,7 @@ class DownloadItem {
// Constructing from persistent store:
DownloadItem(DownloadManager* download_manager,
- const DownloadCreateInfo& info);
+ const DownloadHistoryInfo& info);
// Constructing for a regular download:
DownloadItem(DownloadManager* download_manager,
@@ -194,16 +245,13 @@ class DownloadItem {
// Whether or not this download has saved all of its data.
bool all_data_saved() const { return all_data_saved_; }
- // Update the fields that may have changed in DownloadCreateInfo as a
+ // Update the fields that may have changed in DownloadStateInfo as a
// result of analyzing the file and figuring out its type, location, etc.
// May only be called once.
- void 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);
+ void SetFileCheckResults(const DownloadStateInfo& state);
+
+ // Updates the target file.
+ void UpdateTarget();
// Update the download's path, the actual file is renamed on the download
// thread.
@@ -239,22 +287,28 @@ class DownloadItem {
bool IsComplete() const;
// Accessors
- DownloadState state() const { return state_; }
- FilePath full_path() const { return full_path_; }
- void set_path_uniquifier(int uniquifier) { path_uniquifier_ = uniquifier; }
- const GURL& url() const { return url_chain_.back(); }
- const std::vector<GURL>& url_chain() const { return url_chain_; }
- const GURL& original_url() const { return url_chain_.front(); }
- const GURL& referrer_url() const { return referrer_url_; }
+ DownloadState state() const;
+ FilePath full_path() const { return history_info_.path; }
+ void set_path_uniquifier(int uniquifier) {
+ manager_state_.path_uniquifier = uniquifier;
+ }
+ const GURL& url() const;
+ const std::vector<GURL>& url_chain() const { return history_info_.url_chain; }
+ const GURL& original_url() const { return history_info_.url_chain.front(); }
+ const GURL& referrer_url() const { return history_info_.referrer_url; }
+ 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_; }
- 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 id_; }
- base::Time start_time() const { return start_time_; }
- void set_db_handle(int64 handle) { db_handle_ = handle; }
- int64 db_handle() const { return db_handle_; }
+ std::string referrer_charset() const { return referrer_charset_; }
+ int64 total_bytes() const { return history_info_.total_bytes; }
+ void set_total_bytes(int64 total_bytes) {
+ history_info_.total_bytes = total_bytes;
+ }
+ int64 received_bytes() const { return history_info_.received_bytes; }
+ int32 id() const { return history_info_.download_id; }
+ base::Time start_time() const { return history_info_.start_time; }
+ void set_db_handle(int64 handle) { history_info_.db_handle = handle; }
+ int64 db_handle() const { return history_info_.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; }
@@ -264,14 +318,27 @@ class DownloadItem {
}
DangerType danger_type() { return danger_type_;}
bool auto_opened() { return auto_opened_; }
- FilePath target_name() const { return target_name_; }
- bool save_as() const { return save_as_; }
+ FilePath target_name() const { return manager_state_.target_name; }
+ bool save_as() const { return manager_state_.prompt_user_for_save_location; }
bool is_otr() const { return is_otr_; }
- bool is_extension_install() const { return is_extension_install_; }
+ bool is_extension_install() const {
+ return manager_state_.is_extension_install;
+ }
bool is_temporary() const { return is_temporary_; }
void set_opened(bool opened) { opened_ = opened; }
bool opened() const { return opened_; }
+ // History state mutators.
+ void set_path(const FilePath& path) { history_info_.path = path; }
Randy Smith (Not in Mondays) 2011/05/19 17:05:27 I get nervous when we create new setters, specific
ahendrickson 2011/05/19 20:16:49 Done.
+
+ // Manager state accessors.
+ bool IsDangerous() const { return manager_state_.IsDangerous(); }
Randy Smith (Not in Mondays) 2011/05/19 17:05:27 Why isn't this redundant with danger_type() == NOT
ahendrickson 2011/05/19 20:16:49 Done.
+
+ // Manager state mutators.
+ void set_dangerous_url(bool is_dangerous_url) {
Randy Smith (Not in Mondays) 2011/05/19 17:05:27 I'm uncomfortable about this setter for the same r
ahendrickson 2011/05/19 20:16:49 Done.
+ manager_state_.is_dangerous_url = is_dangerous_url;
+ }
+
const DownloadProcessHandle& process_handle() const {
return process_handle_;
}
@@ -280,7 +347,7 @@ class DownloadItem {
FilePath GetTargetFilePath() const;
// Returns the file-name that should be reported to the user, which is
- // target_name_ possibly with the uniquifier number.
+ // target_name possibly with the uniquifier number.
FilePath GetFileNameToReportUser() const;
// Returns the user-verified target file path for the download.
@@ -290,7 +357,7 @@ class DownloadItem {
// Returns true if the current file name is not the final target name yet.
bool NeedsRename() const {
- return target_name_ != full_path_.BaseName();
+ return manager_state_.target_name != history_info_.path.BaseName();
}
std::string DebugString(bool verbose) const;
@@ -309,34 +376,31 @@ class DownloadItem {
void StartProgressTimer();
void StopProgressTimer();
- // Request ID assigned by the ResourceDispatcherHost.
- int32 id_;
-
- // Full path to the downloaded or downloading file.
- FilePath full_path_;
+ // Information that is saved to the history DB.
+ DownloadHistoryInfo history_info_;
- // 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_;
+ // State information used by the download manager.
+ DownloadStateInfo manager_state_;
Paweł Hajdan Jr. 2011/05/19 16:18:25 nit: How about just state_info for consistency? I
ahendrickson 2011/05/19 20:16:49 I called it |manager_state_| because it's informat
Paweł Hajdan Jr. 2011/05/20 09:04:42 I think that consistency is still more important.
ahendrickson 2011/05/20 18:31:24 Done.
- // The chain of redirects that leading up to and including the final URL.
- std::vector<GURL> url_chain_;
+ // The handle to the process information. Used for operations outside the
+ // download system.
+ DownloadProcessHandle process_handle_;
- // The URL of the page that initiated the download.
- GURL referrer_url_;
+ // Information from the request.
+ // Content-disposition field from the header.
+ std::string content_disposition_;
- // The mimetype of the download
+ // Mime-type from the header. Subject to change.
std::string mime_type_;
- // The value of the content type header received when downloading
- // this item. |mime_type_| may be different because of type sniffing.
+ // The value of the content type header sent with the downloaded item. It
+ // may be different from |mime_type_|, which may be set based on heuristics
+ // which may look at the file extension and first few bytes of the file.
std::string original_mime_type_;
- // Total bytes expected
- int64 total_bytes_;
-
- // Current received bytes
- int64 received_bytes_;
+ // The charset of the referring page where the download request comes from.
+ // It's used to construct a suggested filename.
+ std::string referrer_charset_;
// Last error.
int last_os_error_;
@@ -344,18 +408,9 @@ class DownloadItem {
// Start time for calculating remaining time
base::TimeTicks start_tick_;
- // The current state of this download
- DownloadState state_;
-
// The views of this item in the download shelf and download tab
ObserverList<Observer> observers_;
- // Time the download was started
- base::Time start_time_;
-
- // Our persistent store handle
- int64 db_handle_;
-
// Timer for regularly updating our observers
base::RepeatingTimer<DownloadItem> update_timer_;
@@ -380,24 +435,9 @@ class DownloadItem {
// before the observer is added.
bool auto_opened_;
- // Dangerous downloads or ongoing downloads are given temporary names until
- // the user approves them or the downloads finish.
- // This stores their final target name.
- FilePath target_name_;
-
- // The handle to the process information. Used for operations outside the
- // download system.
- DownloadProcessHandle process_handle_;
-
- // True if the item was downloaded as a result of 'save as...'
- bool save_as_;
-
// True if the download was initiated in an incognito window.
bool is_otr_;
- // True if the item was downloaded for an extension installation.
- bool is_extension_install_;
-
// True if the item was downloaded temporarily.
bool is_temporary_;

Powered by Google App Engine
This is Rietveld 408576698