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

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

Issue 10689093: Move Rename functionality from DownloadFileManager to DownloadFileImple. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Upload fater merging past revert to figure out if I still have a patch. Created 8 years, 5 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.cc
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index 3c9521d314180cf38803328d6e5e1da5d7dda4e0..1912cce88b283dea91def38489559dbdd450e6be 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -489,8 +489,19 @@ void DownloadItemImpl::Cancel(bool user_cancel) {
// An error occurred somewhere.
void DownloadItemImpl::Interrupt(content::DownloadInterruptReason reason) {
- // It should not be possible both to have an error and complete.
- DCHECK(IsInProgress());
+ // Somewhat counter-intuitively, it is possible for us to receive an
+ // interrupt after we've already been interrupted. The generation of
+ // interrupts from the file thread Renames and the generation of
+ // interrupts from disk writes go through two different mechanisms (driven
+ // by rename requests from UI thread and by write requests from IO thread,
+ // respectively), and since we choose not to keep state on the File thread,
+ // this is the place where the races collide. It's also possible for
+ // interrupts to race with cancels.
+
+ // Whatever happens, the first one to hit the UI thread wins.
+ if (!IsInProgress())
+ return;
+
last_reason_ = reason;
TransitionTo(INTERRUPTED);
download_stats::RecordDownloadInterrupted(
@@ -739,6 +750,7 @@ void DownloadItemImpl::OnDownloadCompleting(DownloadFileManager* file_manager) {
void DownloadItemImpl::OnDownloadRenamedToFinalName(
DownloadFileManager* file_manager,
+ content::DownloadInterruptReason reason,
const FilePath& full_path) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -748,12 +760,13 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
<< " " << DebugString(false);
DCHECK(NeedsRename());
- if (full_path.empty())
- // Indicates error; also reported
- // by DownloadManagerImpl::OnDownloadInterrupted.
+ if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
+ Interrupt(reason);
return;
+ }
// full_path is now the current and target file path.
+ DCHECK(!full_path.empty());
target_path_ = full_path;
SetFullPath(full_path);
delegate_->DownloadRenamedToFinalName(this);
@@ -775,12 +788,16 @@ void DownloadItemImpl::OnDownloadFileReleased() {
}
void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
+ content::DownloadInterruptReason reason,
const FilePath& full_path) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- if (!full_path.empty()) {
+ if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
+ Interrupt(reason);
+ } else {
SetFullPath(full_path);
UpdateObservers();
}
+
delegate_->DownloadRenamedToIntermediateName(this);
}
« no previous file with comments | « content/browser/download/download_item_impl.h ('k') | content/browser/download/download_item_impl_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698