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

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

Issue 10823406: Remove DownloadFileManager in favor of direct ownership of DownloadFiles. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Incorporated comments and merged to LKGR. Created 8 years, 4 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/save_package.cc
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 0ef878efa2581dca506cbbea68a4b196f860784f..c043bc161187c2977756b55251a591d1f49b2142 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -18,7 +18,6 @@
#include "base/sys_string_conversions.h"
#include "base/threading/thread.h"
#include "base/utf_string_conversions.h"
-#include "content/browser/download/download_file_manager.h"
#include "content/browser/download/download_item_impl.h"
#include "content/browser/download/download_manager_impl.h"
#include "content/browser/download/download_stats.h"
@@ -35,6 +34,8 @@
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/download_manager_delegate.h"
#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
#include "content/public/browser/resource_context.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
@@ -333,12 +334,19 @@ void SavePackage::OnMHTMLGenerated(const FilePath& path, int64 size) {
return;
}
wrote_to_completed_file_ = true;
- download_->SetTotalBytes(size);
- download_->UpdateProgress(size, size, DownloadItem::kEmptyFileHash);
- // Must call OnAllDataSaved here in order for
- // GDataDownloadObserver::ShouldUpload() to return true.
- // ShouldCompleteDownload() may depend on the gdata uploader to finish.
- download_->OnAllDataSaved(size, DownloadItem::kEmptyFileHash);
+
+ // Hack to avoid touching download_ after user cancel.
+ // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem
+ // with SavePackage flow.
+ if (download_->IsInProgress()) {
+ download_->SetTotalBytes(size);
+ download_->UpdateProgress(size, size, DownloadItem::kEmptyFileHash);
+ // Must call OnAllDataSaved here in order for
+ // GDataDownloadObserver::ShouldUpload() to return true.
+ // ShouldCompleteDownload() may depend on the gdata uploader to finish.
+ download_->OnAllDataSaved(DownloadItem::kEmptyFileHash);
+ }
+
if (!download_manager_->GetDelegate() ||
download_manager_->GetDelegate()->ShouldCompleteDownload(
download_, base::Bind(&SavePackage::Finish, this))) {
@@ -740,13 +748,17 @@ void SavePackage::Finish() {
file_manager_,
save_ids));
- if (download_) {
- if (save_type_ != content::SAVE_PAGE_TYPE_AS_MHTML)
- download_->OnAllDataSaved(all_save_items_count_,
- DownloadItem::kEmptyFileHash);
+ // Hack to avoid touching download_ after user cancel.
+ // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem
+ // with SavePackage flow.
+ if (download_ && download_->IsInProgress()) {
+ if (save_type_ != content::SAVE_PAGE_TYPE_AS_MHTML) {
+ download_->UpdateProgress(all_save_items_count_, CurrentSpeed(), "");
+ download_->OnAllDataSaved(DownloadItem::kEmptyFileHash);
+ }
download_->MarkAsComplete();
- FinalizeDownloadEntry();
}
+ FinalizeDownloadEntry();
}
// Called for updating end state.
@@ -766,7 +778,10 @@ void SavePackage::SaveFinished(int32 save_id, int64 size, bool is_success) {
// Inform the DownloadItem to update UI.
// We use the received bytes as number of saved files.
- if (download_)
+ // Hack to avoid touching download_ after user cancel.
+ // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem
+ // with SavePackage flow.
+ if (download_ && download_->IsInProgress())
download_->UpdateProgress(completed_count(), CurrentSpeed(), "");
if (save_item->save_source() == SaveFileCreateInfo::SAVE_FILE_FROM_DOM &&
@@ -808,7 +823,10 @@ void SavePackage::SaveFailed(const GURL& save_url) {
// Inform the DownloadItem to update UI.
// We use the received bytes as number of saved files.
- if (download_)
+ // Hack to avoid touching download_ after user cancel.
+ // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem
+ // with SavePackage flow.
+ if (download_ && download_->IsInProgress())
download_->UpdateProgress(completed_count(), CurrentSpeed(), "");
if ((save_type_ == content::SAVE_PAGE_TYPE_AS_ONLY_HTML) ||
@@ -1096,7 +1114,10 @@ void SavePackage::OnReceivedSavableResourceLinksForCurrentPage(
static_cast<int>(frames_list.size());
// We use total bytes as the total number of files we want to save.
- if (download_)
+ // Hack to avoid touching download_ after user cancel.
+ // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem
+ // with SavePackage flow.
+ if (download_ && download_->IsInProgress())
download_->SetTotalBytes(all_save_items_count_);
if (all_save_items_count_) {
@@ -1365,6 +1386,16 @@ void SavePackage::FinalizeDownloadEntry() {
DCHECK(download_);
DCHECK(download_manager_);
+ content::NotificationService::current()->Notify(
+ content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
+ // We use the DownloadManager as the source as that's a
+ // central SavePackage related location that observers can
+ // get to if they want to wait for notifications for a
+ // particular BrowserContext. Alternatively, we could make
+ // it come from the WebContents, which would be more specific
+ // but less useful to (current) customers.
+ content::Source<content::DownloadManager>(download_manager_),
+ content::Details<content::DownloadItem>(download_));
download_manager_->SavePageDownloadFinished(download_);
StopObservation();
}
« no previous file with comments | « content/browser/download/mock_download_file.cc ('k') | content/browser/renderer_host/resource_dispatcher_host_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698