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

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

Issue 10831302: Download resumption - Preliminary (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Fixed content unit tests. Created 8 years, 2 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_manager_impl.cc
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 77b4a20a0bfb1ebb9fbb821dcec9c46f1a96da7b..0d7507037f02a336fa46e2f076a1dd3dbdaf0603 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -39,6 +39,7 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/resource_context.h"
#include "content/public/browser/web_contents_delegate.h"
+#include "content/public/common/referrer.h"
#include "net/base/load_flags.h"
#include "net/base/upload_data.h"
#include "net/url_request/url_request_context.h"
@@ -75,7 +76,8 @@ class SavePageData : public base::SupportsUserData::Data {
const char SavePageData::kKey[] = "DownloadItem SavePageData";
-void BeginDownload(content::DownloadUrlParameters* params) {
+void BeginDownload(content::DownloadUrlParameters* params,
+ DownloadId download_id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
// ResourceDispatcherHost{Base} is-not-a URLRequest::Delegate, and
// DownloadUrlParameters can-not include resource_dispatcher_host_impl.h, so
@@ -117,6 +119,9 @@ void BeginDownload(content::DownloadUrlParameters* params) {
params->render_view_host_routing_id(),
params->prefer_cache(),
params->save_info(),
+ params->last_modified(),
+ params->etag(),
+ download_id,
params->callback());
}
@@ -177,10 +182,8 @@ class DownloadItemFactoryImpl : public content::DownloadItemFactory {
virtual DownloadItemImpl* CreateActiveItem(
DownloadItemImplDelegate* delegate,
const DownloadCreateInfo& info,
- scoped_ptr<DownloadRequestHandleInterface> request_handle,
const net::BoundNetLog& bound_net_log) OVERRIDE {
- return new DownloadItemImpl(delegate, info, request_handle.Pass(),
- bound_net_log);
+ return new DownloadItemImpl(delegate, info, bound_net_log);
}
virtual DownloadItemImpl* CreateSavePageItem(
@@ -353,6 +356,9 @@ DownloadItem* DownloadManagerImpl::StartDownload(
scoped_ptr<content::ByteStreamReader> stream) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ bool resuming =
+ (downloads_.find(info->download_id.local()) != downloads_.end());
+
// |bound_net_log| will be used for logging both the download item's and
// the download file's events.
net::BoundNetLog bound_net_log = CreateDownloadItem(info.get());
@@ -369,7 +375,9 @@ DownloadItem* DownloadManagerImpl::StartDownload(
}
DownloadFileManager::CreateDownloadFileCallback callback(
- base::Bind(&DownloadManagerImpl::OnDownloadFileCreated,
+ base::Bind(resuming ?
+ &DownloadManagerImpl::ContinueStartingDownload :
+ &DownloadManagerImpl::OnDownloadFileCreated,
this, download_id.local()));
BrowserThread::PostTask(
@@ -383,8 +391,32 @@ DownloadItem* DownloadManagerImpl::StartDownload(
return GetDownload(download_id.local());
}
+// We have received a message from DownloadFileManager about a new download.
+void DownloadManagerImpl::ContinueStartingDownload(
+ int32 download_id, content::DownloadInterruptReason reason) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ if (reason != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
+ OnDownloadInterrupted(download_id, reason);
+ // TODO(rdsmith): It makes no sense to continue along the
+ // regular download path after we've gotten an error. But it's
+ // the way the code has historically worked, and this allows us
+ // to get the download persisted and observers of the download manager
+ // notified, so tests work. When we execute all side effects of cancel
+ // (including queue removal) immedately rather than waiting for
+ // persistence we should replace this comment with a "return;".
+ }
+
+ DownloadItem* download = GetActiveDownload(download_id);
+ if (!download)
+ return;
+
+ MaybeCompleteDownload(download_id);
+}
+
void DownloadManagerImpl::OnDownloadFileCreated(
int32 download_id, content::DownloadInterruptReason reason) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (reason != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
OnDownloadInterrupted(download_id, reason);
// TODO(rdsmith): It makes no sense to continue along the
@@ -479,18 +511,44 @@ net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
DownloadCreateInfo* info) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- net::BoundNetLog bound_net_log =
- net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
+ // |bound_net_log| will be used for logging the both the download item's and
+ // the download file's events.
+ net::BoundNetLog bound_net_log;
+
if (!info->download_id.IsValid())
info->download_id = GetNextId();
- DownloadItemImpl* download = factory_->CreateActiveItem(
- this, *info,
- scoped_ptr<DownloadRequestHandleInterface>(
- new DownloadRequestHandle(info->request_handle)).Pass(),
- bound_net_log);
- DCHECK(!ContainsKey(downloads_, download->GetId()));
- downloads_[download->GetId()] = download;
+ int32 download_id = info->download_id.local();
+ DownloadItemImpl* download = NULL;
+ scoped_ptr<DownloadRequestHandleInterface> req_handle(
+ new DownloadRequestHandle(info->request_handle));
+ bool resuming = (downloads_.find(download_id) != downloads_.end());
+ if (resuming) {
+ download = downloads_[download_id];
+ DCHECK(download != NULL);
+ }
+
+ if (download) {
+ // Reuse an interrupted |DownloadItem|.
+ DCHECK(download->IsInterrupted());
+ bound_net_log = download->GetBoundNetLog(); // Connect to its net log.
+ download->Resume(req_handle.Pass());
+ } else {
+ // Create a new |DownloadItem|.
+ bound_net_log =
+ net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
+ download = factory_->CreateActiveItem(this, *info, bound_net_log);
+
+ download->SetRequest(req_handle.Pass());
+
+ DCHECK(!ContainsKey(downloads_, download->GetId()));
+ downloads_[download->GetId()] = download;
+ }
+
+ DVLOG(20) << __FUNCTION__ << "()"
+ << " info " << info->DebugString()
+ << " download " << download->DebugString(true);
+
DCHECK(!ContainsKey(active_downloads_, download->GetId()));
active_downloads_[download->GetId()] = download;
FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, download));
@@ -561,7 +619,7 @@ void DownloadManagerImpl::OnResponseCompleted(int32 download_id,
DownloadItemImpl* download = active_downloads_[download_id];
download->OnAllDataSaved(size, hash);
- MaybeCompleteDownload(download);
+ MaybeCompleteDownload(download_id);
}
void DownloadManagerImpl::AssertStateConsistent(
@@ -616,11 +674,18 @@ bool DownloadManagerImpl::IsDownloadReadyForCompletion(
// downloads. SavePackage always uses its own Finish() to mark downloads
// complete.
-void DownloadManagerImpl::MaybeCompleteDownload(
- DownloadItemImpl* download) {
+void DownloadManagerImpl::MaybeCompleteDownload(int32 download_id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ // This can be called from |DownloadItemImpl::OnDownloadedFileRemoved()|,
+ // without the download being active.
+ DCHECK(ContainsKey(downloads_, download_id));
+ if (!ContainsKey(active_downloads_, download_id))
+ return;
+ DownloadItemImpl* download = active_downloads_[download_id];
+
VLOG(20) << __FUNCTION__ << "()" << " download = "
- << download->DebugString(false);
+ << download->DebugString(true);
if (!IsDownloadReadyForCompletion(download))
return;
@@ -656,7 +721,7 @@ void DownloadManagerImpl::MaybeCompleteDownload(
void DownloadManagerImpl::MaybeCompleteDownloadById(int download_id) {
if (ContainsKey(active_downloads_, download_id))
- MaybeCompleteDownload(active_downloads_[download_id]);
+ MaybeCompleteDownload(download_id);
}
void DownloadManagerImpl::DownloadCompleted(DownloadItemImpl* download) {
@@ -669,10 +734,13 @@ void DownloadManagerImpl::DownloadCompleted(DownloadItemImpl* download) {
}
void DownloadManagerImpl::CancelDownload(int32 download_id) {
+ DownloadItem* download = GetActiveDownload(download_id);
// A cancel at the right time could remove the download from the
// |active_downloads_| map before we get here.
- if (ContainsKey(active_downloads_, download_id))
- active_downloads_[download_id]->Cancel(true);
+ if (!download)
+ return;
+
+ download->Cancel(true);
}
void DownloadManagerImpl::DownloadStopped(DownloadItemImpl* download) {
@@ -687,7 +755,11 @@ void DownloadManagerImpl::DownloadStopped(DownloadItemImpl* download) {
AssertStateConsistent(download);
DCHECK(file_manager_);
- download->OffThreadCancel();
+
+ if (download->IsInterrupted())
+ download->OffThreadInterrupt();
+ else
+ download->OffThreadCancel();
}
void DownloadManagerImpl::OnDownloadInterrupted(
@@ -697,9 +769,80 @@ void DownloadManagerImpl::OnDownloadInterrupted(
if (!ContainsKey(active_downloads_, download_id))
return;
+
+ VLOG(20) << __FUNCTION__ << "()"
+ << " reason " << InterruptReasonDebugString(reason)
+ << " download = "
+ << active_downloads_[download_id]->DebugString(true);
+
active_downloads_[download_id]->Interrupt(reason);
}
+// Resume a download of a specific URL. We send the request to the
+// ResourceDispatcherHost, and let it send us responses like a regular
+// download.
+void DownloadManagerImpl::RestartInterruptedDownload(
+ DownloadItemImpl* download,
+ const content::DownloadUrlParameters::OnStartedCallback& callback) {
Randy Smith (Not in Mondays) 2012/10/15 00:36:08 If we've already got a download item, let's not us
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ // Handle the case of clicking 'Resume' in the download shelf.
+ DCHECK(download);
+ DCHECK(download->IsInterrupted());
+
+ DVLOG(20) << __FUNCTION__ << "()"
+ << " download = " << download->DebugString(true);
+
+ // Restart the download.
+ WebContents* contents = GetWebContents(download);
+ if (!contents) {
+ if (!callback.is_null())
+ callback.Run(download, net::ERR_ACCESS_DENIED);
+ return;
+ }
+
+ content::DownloadSaveInfo save_info;
+ save_info.file_path = download->GetFullPath();
+ save_info.offset = download->GetReceivedBytes();
+ save_info.hash_state = download->GetHashState();
+
+ content::DownloadUrlParameters* download_params =
+ new content::DownloadUrlParameters(
+ download->GetOriginalUrl(),
+ contents->GetRenderProcessHost()->GetID(),
+ contents->GetRenderViewHost()->GetRoutingID(),
+ contents->GetBrowserContext()->GetResourceContext(),
+ save_info);
+
+ content::Referrer referrer(download->GetReferrerUrl(),
+ WebKit::WebReferrerPolicyDefault);
+ download_params->set_referrer(referrer);
+ download_params->set_last_modified(download->GetLastModifiedTime());
+ download_params->set_etag(download->GetETag());
+ download_params->set_callback(callback);
+
+ BrowserThread::PostTask(
+ BrowserThread::IO,
+ FROM_HERE,
+ base::Bind(
+ &BeginDownload,
+ base::Owned(download_params),
+ download->GetGlobalId()));
+}
+
+DownloadItem* DownloadManagerImpl::GetActiveDownload(int32 download_id) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DownloadMap::iterator it = active_downloads_.find(download_id);
+ if (it == active_downloads_.end())
+ return NULL;
+
+ DownloadItem* download = it->second;
+
+ DCHECK(download);
+ DCHECK_EQ(download_id, download->GetId());
+
+ return download;
+}
+
void DownloadManagerImpl::RemoveFromActiveList(DownloadItemImpl* download) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(download);
@@ -791,8 +934,12 @@ void DownloadManagerImpl::DownloadUrl(
DCHECK(params->prefer_cache());
DCHECK(params->method() == "POST");
}
- BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
- &BeginDownload, base::Owned(params.release())));
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(
+ &BeginDownload,
+ base::Owned(params.release()),
+ DownloadId()));
}
void DownloadManagerImpl::AddObserver(Observer* observer) {
@@ -889,9 +1036,10 @@ void DownloadManagerImpl::OnDownloadItemAddedToPersistentStore(
// completion status, and also inform any observers so that they get
// more than just the start notification.
if (item->IsInProgress()) {
- MaybeCompleteDownload(item);
+ MaybeCompleteDownload(item->GetId());
} else {
- DCHECK(item->IsCancelled());
+ DCHECK(item->IsCancelled() || item->IsInterrupted())
+ << " download = " << item->DebugString(true);
active_downloads_.erase(item->GetId());
if (delegate_)
delegate_->UpdateItemInPersistentStore(item);
@@ -902,12 +1050,7 @@ void DownloadManagerImpl::OnDownloadItemAddedToPersistentStore(
void DownloadManagerImpl::ShowDownloadInBrowser(DownloadItemImpl* download) {
// The 'contents' may no longer exist if the user closed the contents before
// we get this start completion event.
- WebContents* content = download->GetWebContents();
-
- // If the contents no longer exists, we ask the embedder to suggest another
- // contents.
- if (!content && delegate_)
- content = delegate_->GetAlternativeWebContentsToNotifyForDownload();
+ WebContents* content = GetWebContents(download);
if (content && content->GetDelegate())
content->GetDelegate()->OnStartDownload(content, download);
@@ -1046,3 +1189,14 @@ void DownloadManagerImpl::DownloadRenamedToFinalName(
download, download->GetFullPath());
}
}
+
+WebContents* DownloadManagerImpl::GetWebContents(const DownloadItem* download) {
+ WebContents* web_contents = download->GetWebContents();
+ // If the original download_id was created from the history DB, its
+ // |DownloadRequestHandle| will not be valid, and we won't get a proper
+ // |WebContents| pointer from it.
+ if (!web_contents)
+ web_contents = delegate_->GetAlternativeWebContentsToNotifyForDownload();
+
+ return web_contents;
+}

Powered by Google App Engine
This is Rietveld 408576698