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

Unified Diff: content/browser/download/download_item_impl.h

Issue 1751603002: [Downloads] Rework how hashes are calculated for download files. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address first round of comments Created 4 years, 9 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: content/browser/download/download_item_impl.h
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index 18ba4495dca85e21b6688d232db26cb617acf8e2..fa6bda473b9cadc1a9a4b9393ab3f731e7d7cf6a 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -16,10 +16,10 @@
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "base/time/time.h"
+#include "content/browser/download/download_destination_observer.h"
#include "content/browser/download/download_net_log_parameters.h"
#include "content/browser/download/download_request_handle.h"
#include "content/common/content_export.h"
-#include "content/public/browser/download_destination_observer.h"
#include "content/public/browser/download_interrupt_reasons.h"
#include "content/public/browser/download_item.h"
#include "net/log/net_log.h"
@@ -65,6 +65,7 @@ class CONTENT_EXPORT DownloadItemImpl
const std::string& last_modified,
int64_t received_bytes,
int64_t total_bytes,
+ const std::string& hash,
DownloadItem::DownloadState state,
DownloadDangerType danger_type,
DownloadInterruptReason interrupt_reason,
@@ -133,7 +134,6 @@ class CONTENT_EXPORT DownloadItemImpl
base::FilePath GetFileNameToReportUser() const override;
TargetDisposition GetTargetDisposition() const override;
const std::string& GetHash() const override;
- const std::string& GetHashState() const override;
bool GetFileExternallyRemoved() const override;
void DeleteFile(const base::Callback<void(bool)>& callback) override;
bool IsDangerous() const override;
@@ -205,18 +205,20 @@ class CONTENT_EXPORT DownloadItemImpl
// Called by SavePackage to set the total number of bytes on the item.
virtual void SetTotalBytes(int64_t total_bytes);
- virtual void OnAllDataSaved(const std::string& final_hash);
+ virtual void OnAllDataSaved(int64_t total_bytes,
+ scoped_ptr<crypto::SecureHash> hash_state);
// Called by SavePackage to display progress when the DownloadItem
// should be considered complete.
virtual void MarkAsComplete();
// DownloadDestinationObserver
- void DestinationUpdate(int64_t bytes_so_far,
- int64_t bytes_per_sec,
- const std::string& hash_state) override;
- void DestinationError(DownloadInterruptReason reason) override;
- void DestinationCompleted(const std::string& final_hash) override;
+ void DestinationUpdate(int64_t bytes_so_far, int64_t bytes_per_sec) override;
+ void DestinationError(DownloadInterruptReason reason,
+ int64_t bytes_so_far,
+ scoped_ptr<crypto::SecureHash> hash_state) override;
+ void DestinationCompleted(int64_t total_bytes,
+ scoped_ptr<crypto::SecureHash> hash_state) override;
private:
// Fine grained states of a download.
@@ -426,8 +428,24 @@ class CONTENT_EXPORT DownloadItemImpl
// Helper routines -----------------------------------------------------------
- // Indicate that an error has occurred on the download.
- void Interrupt(DownloadInterruptReason reason);
+ // Indicate that an error has occurred on the download. Discards partial
+ // state. The interrupted download will not be considered continuable, but may
+ // be restarted.
+ void InterruptAndDiscardPartialState(DownloadInterruptReason reason);
+
+ // Indiates that an error has occurred on the download. The |bytes_so_far| and
+ // |hash_state| should correspond to the state of the DownloadFile. If the
+ // interrupt reason allows, this partial state may be allowed to continue the
+ // interrupted download upon resumption.
+ void InterruptWithPartialState(int64_t bytes_so_far,
+ scoped_ptr<crypto::SecureHash> hash_state,
+ DownloadInterruptReason reason);
+
+ void UpdateProgress(int64_t bytes_so_far, int64_t bytes_per_sec);
+
+ // Set |hash_| based on the assumption that |hash_state_| represents the
+ // partial hash state.
+ void UpdatePrefixHash();
// Destroy the DownloadFile object. If |destroy_file| is true, the file is
// destroyed with it. Otherwise, DownloadFile::Detach() is called before
@@ -486,12 +504,6 @@ class CONTENT_EXPORT DownloadItemImpl
// considered to be |target_path_.BaseName()|.
base::FilePath display_name_;
- // Full path to the downloaded or downloading file. This is the path to the
- // physical file, if one exists. The final target path is specified by
- // |target_path_|. |current_path_| can be empty if the in-progress path hasn't
- // been determined.
- base::FilePath current_path_;
-
// Target path of an in-progress download. We may be downloading to a
// temporary or intermediate file (specified by |current_path_|. Once the
// download completes, we will rename the file to |target_path_|.
@@ -546,27 +558,6 @@ class CONTENT_EXPORT DownloadItemImpl
// Total bytes expected.
int64_t total_bytes_ = 0;
- // Current received bytes.
- int64_t received_bytes_ = 0;
-
- // Current speed. Calculated by the DownloadFile.
- int64_t bytes_per_sec_ = 0;
-
- // Sha256 hash of the content. This might be empty either because
- // the download isn't done yet or because the hash isn't needed
- // (ChromeDownloadManagerDelegate::GenerateFileHash() returned false).
- std::string hash_;
-
- // A blob containing the state of the hash algorithm. Only valid while the
- // download is in progress.
- std::string hash_state_;
-
- // Server's time stamp for the file.
- std::string last_modified_time_;
-
- // Server's ETAG for the file.
- std::string etag_;
-
// Last reason.
DownloadInterruptReason last_reason_ = DOWNLOAD_INTERRUPT_REASON_NONE;
@@ -594,9 +585,6 @@ class CONTENT_EXPORT DownloadItemImpl
// In progress downloads may be paused by the user, we note it here.
bool is_paused_ = false;
- // The number of times this download has been resumed automatically.
- int auto_resume_count_ = 0;
-
// A flag for indicating if the download should be opened at completion.
bool open_when_complete_ = false;
@@ -611,14 +599,6 @@ class CONTENT_EXPORT DownloadItemImpl
// True if the item was downloaded temporarily.
bool is_temporary_ = false;
- // True if we've saved all the data for the download.
- bool all_data_saved_ = false;
-
- // Error return from DestinationError. Stored separately from
- // last_reason_ so that we can avoid handling destination errors until
- // after file name determination has occurred.
- DownloadInterruptReason destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
-
// Did the user open the item either directly or indirectly (such as by
// setting always open files of this type)? The shelf also sets this field
// when the user closes the shelf before the item has been opened but should
@@ -628,12 +608,56 @@ class CONTENT_EXPORT DownloadItemImpl
// Did the delegate delay calling Complete on this download?
bool delegate_delayed_complete_ = false;
+ // Error return from DestinationError. Stored separately from
+ // last_reason_ so that we can avoid handling destination errors until
+ // after file name determination has occurred.
+ DownloadInterruptReason destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
+
+ // The following fields describe the current state of the download file.
+
// DownloadFile associated with this download. Note that this
// pointer may only be used or destroyed on the FILE thread.
// This pointer will be non-null only while the DownloadItem is in
// the IN_PROGRESS state.
scoped_ptr<DownloadFile> download_file_;
+ // Full path to the downloaded or downloading file. This is the path to the
+ // physical file, if one exists. The final target path is specified by
+ // |target_path_|. |current_path_| can be empty if the in-progress path hasn't
+ // been determined.
+ base::FilePath current_path_;
+
+ // Current received bytes.
+ int64_t received_bytes_ = 0;
+
+ // Current speed. Calculated by the DownloadFile.
+ int64_t bytes_per_sec_ = 0;
+
+ // True if we've saved all the data for the download. If true, then the file
+ // at |current_path_| contains |received_bytes_|, which constitute the
+ // entirety of what we expect to save there. A digest of its contents can be
+ // found at |hash_|.
+ bool all_data_saved_ = false;
+
+ // The number of times this download has been resumed automatically. Will be
+ // reset to 0 if a resumption is performed in response to a Resume() call.
+ int auto_resume_count_ = 0;
+
+ // SHA256 hash of the possibly partial content. The hash is updated each time
+ // the download is interrupted, or when the all the data has been transferred.
+ std::string hash_;
+
+ // In the event of an interruption, the DownloadDestinationObserver interface
+ // exposes the partial hash state. This state can be held by the download item
+ // in case it's needed for resumption.
+ scoped_ptr<crypto::SecureHash> hash_state_;
+
+ // Contents of the Last-Modified header for the most recent server response.
+ std::string last_modified_time_;
+
+ // Server's ETAG for the file.
+ std::string etag_;
+
// Net log to use for this download.
const net::BoundNetLog bound_net_log_;

Powered by Google App Engine
This is Rietveld 408576698