move InProgressCache and some methods into InProgressDownloadManager
Currently the InProgressCache is owned by ChromeDownloadManagerDelegate.
In order for download to work as a service, the InProgressDownloadManager
should be able to manage InProgressCache. It should read from the cache
and recreate the download before resuming it.
This CL moves the InProgressCache from ChromeDownloadManagerDelegate into
InProgressDownloadmanager. And it also moves StartDownload() method as
this is needed when resuming the download without DownloadManagerImpl.
DownloadManagerImpl now becomes a delegate of InProgressDownloadManager.
It will provide InProgressDownloadManager the information to create a new
download. When resuming a download after launching download service,
DownloadManagerImpl is not needed.
BUG=803135
Change-Id: Ie7a22213e935638dac10dd82d2e079326e8083f2
Reviewed-on: https://chromium-review.googlesource.com/1024723
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553359}diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 76c41d2..e0387f9 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -272,11 +272,6 @@
{base::MayBlock(), base::TaskPriority::BACKGROUND,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})),
weak_ptr_factory_(this) {
- DCHECK(!profile_->GetPath().empty());
- base::FilePath metadata_cache_file =
- profile_->GetPath().Append(chrome::kDownloadMetadataStoreFilename);
- download_metadata_cache_.reset(new download::InProgressCacheImpl(
- metadata_cache_file, disk_access_task_runner_));
#if defined(OS_ANDROID)
location_dialog_bridge_.reset(new DownloadLocationDialogBridgeImpl);
#endif
@@ -572,11 +567,6 @@
callback);
}
-download::InProgressCache* ChromeDownloadManagerDelegate::GetInProgressCache() {
- DCHECK(download_metadata_cache_ != nullptr);
- return download_metadata_cache_.get();
-}
-
void ChromeDownloadManagerDelegate::SanitizeSavePackageResourceName(
base::FilePath* filename) {
safe_browsing::FileTypePolicies* file_type_policies =
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index 4db0502..ed2db7c 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -97,7 +97,6 @@
const base::FilePath::StringType& default_extension,
bool can_save_as_complete,
const content::SavePackagePathPickedCallback& callback) override;
- download::InProgressCache* GetInProgressCache() override;
void SanitizeSavePackageResourceName(base::FilePath* filename) override;
void OpenDownload(download::DownloadItem* download) override;
bool IsMostRecentDownloadItemAtFilePath(
@@ -215,8 +214,6 @@
Profile* profile_;
- std::unique_ptr<download::InProgressCache> download_metadata_cache_;
-
#if defined(OS_ANDROID)
std::unique_ptr<DownloadLocationDialogBridge> location_dialog_bridge_;
#endif
@@ -231,9 +228,9 @@
IdCallbackVector id_callbacks_;
std::unique_ptr<DownloadPrefs> download_prefs_;
- // SequencedTaskRunner to check for file existence and read/write metadata
- // cache. A sequence is used so that a large download history doesn't cause a
- // large number of concurrent disk operations.
+ // SequencedTaskRunner to check for file existence. A sequence is used so
+ // that a large download history doesn't cause a large number of concurrent
+ // disk operations.
const scoped_refptr<base::SequencedTaskRunner> disk_access_task_runner_;
#if BUILDFLAG(ENABLE_EXTENSIONS)
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index f32ffad..d4e0823a 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -139,8 +139,6 @@
FPL("Certificate Revocation Lists");
const base::FilePath::CharType kCustomDictionaryFileName[] =
FPL("Custom Dictionary.txt");
-const base::FilePath::CharType kDownloadMetadataStoreFilename[] =
- FPL("in_progress_download_metadata_store");
const base::FilePath::CharType kDownloadServiceStorageDirname[] =
FPL("Download Service");
const base::FilePath::CharType kExtensionActivityLogFilename[] =
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index 202a01b..6b072cc 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -47,7 +47,6 @@
extern const base::FilePath::CharType kCookieFilename[];
extern const base::FilePath::CharType kCRLSetFilename[];
extern const base::FilePath::CharType kCustomDictionaryFileName[];
-extern const base::FilePath::CharType kDownloadMetadataStoreFilename[];
extern const base::FilePath::CharType kDownloadServiceStorageDirname[];
extern const base::FilePath::CharType kExtensionActivityLogFilename[];
extern const base::FilePath::CharType kExtensionsCookieFilename[];
diff --git a/components/download/downloader/in_progress/download_entry.h b/components/download/downloader/in_progress/download_entry.h
index 4881ae8..c06ac01e 100644
--- a/components/download/downloader/in_progress/download_entry.h
+++ b/components/download/downloader/in_progress/download_entry.h
@@ -7,6 +7,7 @@
#include <string>
+#include "components/download/public/common/download_item.h"
#include "components/download/public/common/download_source.h"
#include "components/download/public/common/download_url_parameters.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
diff --git a/components/download/downloader/in_progress/in_progress_cache.h b/components/download/downloader/in_progress/in_progress_cache.h
index af2c859..6812c13 100644
--- a/components/download/downloader/in_progress/in_progress_cache.h
+++ b/components/download/downloader/in_progress/in_progress_cache.h
@@ -12,6 +12,8 @@
namespace download {
+extern const base::FilePath::CharType kDownloadMetadataStoreFilename[];
+
// InProgressCache provides a write-through cache that persists
// information related to an in-progress download such as request origin, retry
// count, resumption parameters etc to the disk. The entries are written to disk
diff --git a/components/download/downloader/in_progress/in_progress_cache_impl.cc b/components/download/downloader/in_progress/in_progress_cache_impl.cc
index 5640de2..6aa9ec0 100644
--- a/components/download/downloader/in_progress/in_progress_cache_impl.cc
+++ b/components/download/downloader/in_progress/in_progress_cache_impl.cc
@@ -14,6 +14,9 @@
namespace download {
+const base::FilePath::CharType kDownloadMetadataStoreFilename[] =
+ FILE_PATH_LITERAL("in_progress_download_metadata_store");
+
namespace {
// Helper functions for |entries_| related operations.
diff --git a/components/download/internal/common/download_utils.cc b/components/download/internal/common/download_utils.cc
index 2248949..cc9f500 100644
--- a/components/download/internal/common/download_utils.cc
+++ b/components/download/internal/common/download_utils.cc
@@ -5,6 +5,7 @@
#include "components/download/public/common/download_utils.h"
#include "base/format_macros.h"
+#include "base/rand_util.h"
#include "base/strings/stringprintf.h"
#include "components/download/public/common/download_create_info.h"
#include "components/download/public/common/download_interrupt_reasons_utils.h"
@@ -347,4 +348,24 @@
return headers;
}
+DownloadEntry CreateDownloadEntryFromItem(
+ const DownloadItem& item,
+ const std::string& request_origin,
+ DownloadSource download_source,
+ bool fetch_error_body,
+ const DownloadUrlParameters::RequestHeadersType& request_headers) {
+ return DownloadEntry(item.GetGuid(), request_origin, download_source,
+ fetch_error_body, request_headers,
+ GetUniqueDownloadId());
+}
+
+uint64_t GetUniqueDownloadId() {
+ // Get a new UKM download_id that is not 0.
+ uint64_t download_id = 0;
+ do {
+ download_id = base::RandUint64();
+ } while (download_id == 0);
+ return download_id;
+}
+
} // namespace download
diff --git a/components/download/internal/common/in_progress_download_manager.cc b/components/download/internal/common/in_progress_download_manager.cc
index bde18424..c5b3dea 100644
--- a/components/download/internal/common/in_progress_download_manager.cc
+++ b/components/download/internal/common/in_progress_download_manager.cc
@@ -5,8 +5,13 @@
#include "components/download/public/common/in_progress_download_manager.h"
#include "base/optional.h"
+#include "base/task_scheduler/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
+#include "components/download/downloader/in_progress/in_progress_cache_impl.h"
#include "components/download/internal/common/resource_downloader.h"
+#include "components/download/public/common/download_file.h"
+#include "components/download/public/common/download_item_impl.h"
+#include "components/download/public/common/download_stats.h"
#include "components/download/public/common/download_task_runner.h"
#include "components/download/public/common/download_url_loader_factory_getter.h"
#include "components/download/public/common/download_url_parameters.h"
@@ -68,8 +73,8 @@
scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) {
DCHECK(GetIOTaskRunner()->BelongsToCurrentThread());
- download::UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader(
- download::ResourceDownloader::InterceptNavigationResponse(
+ UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader(
+ ResourceDownloader::InterceptNavigationResponse(
download_manager, std::move(resource_request), render_process_id,
render_frame_id, site_url, tab_url, tab_referrer_url,
std::move(url_chain), suggested_filename, std::move(response),
@@ -82,11 +87,84 @@
main_task_runner);
}
+// Responsible for persisting the in-progress metadata associated with a
+// download.
+class InProgressDownloadObserver : public DownloadItem::Observer {
+ public:
+ explicit InProgressDownloadObserver(InProgressCache* in_progress_cache);
+ ~InProgressDownloadObserver() override;
+
+ private:
+ // DownloadItem::Observer
+ void OnDownloadUpdated(DownloadItem* download) override;
+ void OnDownloadRemoved(DownloadItem* download) override;
+
+ // The persistent cache to store in-progress metadata.
+ InProgressCache* in_progress_cache_;
+
+ DISALLOW_COPY_AND_ASSIGN(InProgressDownloadObserver);
+};
+
+InProgressDownloadObserver::InProgressDownloadObserver(
+ InProgressCache* in_progress_cache)
+ : in_progress_cache_(in_progress_cache) {}
+
+InProgressDownloadObserver::~InProgressDownloadObserver() = default;
+
+void InProgressDownloadObserver::OnDownloadUpdated(DownloadItem* download) {
+ // TODO(crbug.com/778425): Properly handle fail/resume/retry for downloads
+ // that are in the INTERRUPTED state for a long time.
+ if (!in_progress_cache_)
+ return;
+
+ switch (download->GetState()) {
+ case DownloadItem::DownloadState::COMPLETE:
+ // Intentional fallthrough.
+ case DownloadItem::DownloadState::CANCELLED:
+ if (in_progress_cache_)
+ in_progress_cache_->RemoveEntry(download->GetGuid());
+ break;
+
+ case DownloadItem::DownloadState::INTERRUPTED:
+ // Intentional fallthrough.
+ case DownloadItem::DownloadState::IN_PROGRESS: {
+ // Make sure the entry exists in the cache.
+ base::Optional<DownloadEntry> entry_opt =
+ in_progress_cache_->RetrieveEntry(download->GetGuid());
+ DownloadEntry entry;
+ if (!entry_opt.has_value()) {
+ entry = CreateDownloadEntryFromItem(
+ *download, std::string(), /* request_origin */
+ DownloadSource::UNKNOWN, false, /* fetch_error_body */
+ DownloadUrlParameters::RequestHeadersType());
+ in_progress_cache_->AddOrReplaceEntry(entry);
+ break;
+ }
+ entry = entry_opt.value();
+ break;
+ }
+
+ default:
+ break;
+ }
+}
+
+void InProgressDownloadObserver::OnDownloadRemoved(DownloadItem* download) {
+ if (!in_progress_cache_)
+ return;
+
+ in_progress_cache_->RemoveEntry(download->GetGuid());
+}
+
} // namespace
InProgressDownloadManager::InProgressDownloadManager(
- UrlDownloadHandler::Delegate* delegate)
- : delegate_(delegate), weak_factory_(this) {}
+ Delegate* delegate,
+ const IsOriginSecureCallback& is_origin_secure_cb)
+ : delegate_(delegate),
+ file_factory_(new DownloadFileFactory()),
+ is_origin_secure_cb_(is_origin_secure_cb),
+ weak_factory_(this) {}
InProgressDownloadManager::~InProgressDownloadManager() = default;
@@ -95,11 +173,8 @@
std::unique_ptr<InputStream> input_stream,
scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
const DownloadUrlParameters::OnStartedCallback& callback) {
- if (delegate_) {
- delegate_->OnUrlDownloadStarted(
- std::move(download_create_info), std::move(input_stream),
- std::move(url_loader_factory_getter), callback);
- }
+ StartDownload(std::move(download_create_info), std::move(input_stream),
+ std::move(url_loader_factory_getter), callback);
}
void InProgressDownloadManager::OnUrlDownloadStopped(
@@ -119,7 +194,7 @@
url_download_handlers_.push_back(std::move(downloader));
}
-void InProgressDownloadManager::StartDownload(
+void InProgressDownloadManager::BeginDownload(
std::unique_ptr<DownloadUrlParameters> params,
scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
uint32_t download_id,
@@ -161,6 +236,21 @@
base::ThreadTaskRunnerHandle::Get()));
}
+void InProgressDownloadManager::Initialize(
+ const base::FilePath& metadata_cache_dir,
+ const base::RepeatingClosure& callback) {
+ DCHECK(!metadata_cache_dir.empty());
+ base::FilePath metadata_cache_file =
+ metadata_cache_dir.Append(kDownloadMetadataStoreFilename);
+ download_metadata_cache_ = std::make_unique<InProgressCacheImpl>(
+ metadata_cache_file,
+ base::CreateSequencedTaskRunnerWithTraits(
+ {base::MayBlock(), base::TaskPriority::BACKGROUND,
+ base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}));
+
+ download_metadata_cache_->Initialize(callback);
+}
+
void InProgressDownloadManager::ShutDown() {
url_download_handlers_.clear();
}
@@ -170,4 +260,147 @@
uint32_t id,
const GURL& site_url) {}
+base::Optional<DownloadEntry> InProgressDownloadManager::GetInProgressEntry(
+ DownloadItemImpl* download) {
+ if (!download || !download_metadata_cache_)
+ return base::Optional<DownloadEntry>();
+
+ return download_metadata_cache_->RetrieveEntry(download->GetGuid());
+}
+
+void InProgressDownloadManager::ReportBytesWasted(DownloadItemImpl* download) {
+ if (!download_metadata_cache_)
+ return;
+ base::Optional<DownloadEntry> entry_opt =
+ download_metadata_cache_->RetrieveEntry(download->GetGuid());
+ if (entry_opt.has_value()) {
+ DownloadEntry entry = entry_opt.value();
+ entry.bytes_wasted = download->GetBytesWasted();
+ download_metadata_cache_->AddOrReplaceEntry(entry);
+ }
+}
+
+void InProgressDownloadManager::StartDownload(
+ std::unique_ptr<DownloadCreateInfo> info,
+ std::unique_ptr<InputStream> stream,
+ scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
+ const DownloadUrlParameters::OnStartedCallback& on_started) {
+ DCHECK(info);
+
+ uint32_t download_id = info->download_id;
+ bool new_download = (download_id == DownloadItem::kInvalidId);
+ if (new_download && info->result == DOWNLOAD_INTERRUPT_REASON_NONE) {
+ if (delegate_ && delegate_->InterceptDownload(*info)) {
+ GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release());
+ return;
+ }
+ }
+
+ // |stream| is only non-null if the download request was successful.
+ DCHECK(
+ (info->result == DOWNLOAD_INTERRUPT_REASON_NONE && !stream->IsEmpty()) ||
+ (info->result != DOWNLOAD_INTERRUPT_REASON_NONE && stream->IsEmpty()));
+ DVLOG(20) << __func__
+ << "() result=" << DownloadInterruptReasonToString(info->result);
+
+ GURL url = info->url();
+ std::vector<GURL> url_chain = info->url_chain;
+ std::string mime_type = info->mime_type;
+
+ if (new_download) {
+ RecordDownloadConnectionSecurity(info->url(), info->url_chain);
+ RecordDownloadContentTypeSecurity(info->url(), info->url_chain,
+ info->mime_type, is_origin_secure_cb_);
+ }
+
+ base::RepeatingCallback<void(uint32_t)> got_id(base::BindRepeating(
+ &InProgressDownloadManager::StartDownloadWithId,
+ weak_factory_.GetWeakPtr(), base::Passed(&info), base::Passed(&stream),
+ std::move(url_loader_factory_getter), on_started, new_download));
+ if (new_download) {
+ // TODO(qinmin): use GUID as the key for downloads history table so we don't
+ // rely on the delegate to provide the next download ID.
+ if (delegate_)
+ delegate_->GetNextId(std::move(got_id));
+ } else {
+ std::move(got_id).Run(download_id);
+ }
+}
+
+void InProgressDownloadManager::StartDownloadWithId(
+ std::unique_ptr<DownloadCreateInfo> info,
+ std::unique_ptr<InputStream> stream,
+ scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
+ const DownloadUrlParameters::OnStartedCallback& on_started,
+ bool new_download,
+ uint32_t id) {
+ DCHECK_NE(DownloadItem::kInvalidId, id);
+ DownloadItemImpl* download =
+ delegate_ ? delegate_->GetDownloadItem(id, new_download, *info) : nullptr;
+
+ if (!download) {
+ // If the download is no longer known to the DownloadManager, then it was
+ // removed after it was resumed. Ignore. If the download is cancelled
+ // while resuming, then also ignore the request.
+ if (info->request_handle)
+ info->request_handle->CancelRequest(true);
+ if (!on_started.is_null())
+ on_started.Run(nullptr, DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
+ // The ByteStreamReader lives and dies on the download sequence.
+ if (info->result == DOWNLOAD_INTERRUPT_REASON_NONE)
+ GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release());
+ return;
+ }
+
+ base::FilePath default_download_directory;
+ if (delegate_)
+ default_download_directory = delegate_->GetDefaultDownloadDirectory();
+
+ if (download_metadata_cache_) {
+ base::Optional<DownloadEntry> entry_opt =
+ download_metadata_cache_->RetrieveEntry(download->GetGuid());
+ if (!entry_opt.has_value()) {
+ download_metadata_cache_->AddOrReplaceEntry(CreateDownloadEntryFromItem(
+ *download, info->request_origin, info->download_source,
+ info->fetch_error_body, info->request_headers));
+ }
+ }
+
+ if (!in_progress_download_observer_) {
+ in_progress_download_observer_ =
+ std::make_unique<InProgressDownloadObserver>(
+ download_metadata_cache_.get());
+ }
+ // May already observe this item, remove observer first.
+ download->RemoveObserver(in_progress_download_observer_.get());
+ download->AddObserver(in_progress_download_observer_.get());
+
+ std::unique_ptr<DownloadFile> download_file;
+ if (info->result == DOWNLOAD_INTERRUPT_REASON_NONE) {
+ DCHECK(stream.get());
+ download_file.reset(file_factory_->CreateFile(
+ std::move(info->save_info), default_download_directory,
+ std::move(stream), id, download->DestinationObserverAsWeakPtr()));
+ }
+ // It is important to leave info->save_info intact in the case of an interrupt
+ // so that the DownloadItem can salvage what it can out of a failed
+ // resumption attempt.
+
+ download->Start(
+ std::move(download_file), std::move(info->request_handle), *info,
+ std::move(url_loader_factory_getter),
+ delegate_ ? delegate_->GetURLRequestContextGetter(*info) : nullptr);
+
+ // For interrupted downloads, Start() will transition the state to
+ // IN_PROGRESS and consumers will be notified via OnDownloadUpdated().
+ // For new downloads, we notify here, rather than earlier, so that
+ // the download_file is bound to download and all the usual
+ // setters (e.g. Cancel) work.
+ if (new_download && delegate_)
+ delegate_->OnNewDownloadStarted(download);
+
+ if (!on_started.is_null())
+ on_started.Run(download, DOWNLOAD_INTERRUPT_REASON_NONE);
+}
+
} // namespace download
diff --git a/components/download/public/common/download_utils.h b/components/download/public/common/download_utils.h
index 9fc9c5a..c3e4b9f2 100644
--- a/components/download/public/common/download_utils.h
+++ b/components/download/public/common/download_utils.h
@@ -5,8 +5,11 @@
#ifndef COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_UTILS_H_
#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_UTILS_H_
+#include "components/download/downloader/in_progress/download_entry.h"
#include "components/download/public/common/download_export.h"
#include "components/download/public/common/download_interrupt_reasons.h"
+#include "components/download/public/common/download_item.h"
+#include "components/download/public/common/download_source.h"
#include "net/base/net_errors.h"
#include "net/cert/cert_status_flags.h"
#include "net/http/http_response_headers.h"
@@ -57,6 +60,16 @@
COMPONENTS_DOWNLOAD_EXPORT std::unique_ptr<net::HttpRequestHeaders>
GetAdditionalRequestHeaders(DownloadUrlParameters* params);
+// Helper functions for DownloadItem -> DownloadEntry for InProgressCache.
+COMPONENTS_DOWNLOAD_EXPORT DownloadEntry CreateDownloadEntryFromItem(
+ const DownloadItem& item,
+ const std::string& request_origin,
+ DownloadSource download_source,
+ bool fetch_error_body,
+ const DownloadUrlParameters::RequestHeadersType& request_headers);
+
+COMPONENTS_DOWNLOAD_EXPORT uint64_t GetUniqueDownloadId();
+
} // namespace download
#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_UTILS_H_
diff --git a/components/download/public/common/in_progress_download_manager.h b/components/download/public/common/in_progress_download_manager.h
index dff70ab3..5e53c82 100644
--- a/components/download/public/common/in_progress_download_manager.h
+++ b/components/download/public/common/in_progress_download_manager.h
@@ -12,10 +12,15 @@
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "components/download/public/common/download_export.h"
+#include "components/download/public/common/download_file_factory.h"
#include "components/download/public/common/download_item_impl_delegate.h"
#include "components/download/public/common/url_download_handler.h"
#include "url/gurl.h"
+namespace net {
+class URLRequestContextGetter;
+}
+
namespace network {
struct ResourceResponse;
}
@@ -24,17 +29,53 @@
class DownloadURLLoaderFactoryGetter;
class DownloadUrlParameters;
+class InProgressCache;
// Manager for handling all active downloads.
class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager
: public UrlDownloadHandler::Delegate,
public DownloadItemImplDelegate {
public:
- InProgressDownloadManager(UrlDownloadHandler::Delegate* delegate);
- ~InProgressDownloadManager() override;
+ using DownloadIdCallback = base::RepeatingCallback<void(uint32_t)>;
+ // Class to be notified when download starts/stops.
+ class COMPONENTS_DOWNLOAD_EXPORT Delegate {
+ public:
+ // Intercepts the download to another system if applicable. Returns true if
+ // the download was intercepted.
+ virtual bool InterceptDownload(
+ const DownloadCreateInfo& download_create_info) = 0;
+
+ // Called to get an ID for a new download. |callback| may be called
+ // synchronously.
+ virtual void GetNextId(const DownloadIdCallback& callback) = 0;
+
+ // Gets the default download directory.
+ virtual base::FilePath GetDefaultDownloadDirectory() = 0;
+
+ // Gets the download item for the given id.
+ // TODO(qinmin): remove this method and let InProgressDownloadManager
+ // create the DownloadItem from in-progress cache.
+ virtual DownloadItemImpl* GetDownloadItem(
+ uint32_t id,
+ bool new_download,
+ const DownloadCreateInfo& download_create_info) = 0;
+
+ // Gets the URLRequestContextGetter for sending requests.
+ // TODO(qinmin): remove this once network service is fully enabled.
+ virtual net::URLRequestContextGetter* GetURLRequestContextGetter(
+ const DownloadCreateInfo& download_create_info) = 0;
+
+ // Called when a new download is started.
+ virtual void OnNewDownloadStarted(DownloadItem* download) = 0;
+ };
+
+ using IsOriginSecureCallback = base::RepeatingCallback<bool(const GURL&)>;
+ InProgressDownloadManager(Delegate* delegate,
+ const IsOriginSecureCallback& is_origin_secure_cb);
+ ~InProgressDownloadManager() override;
// Called to start a download.
- void StartDownload(
+ void BeginDownload(
std::unique_ptr<DownloadUrlParameters> params,
scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
uint32_t download_id,
@@ -57,9 +98,32 @@
network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter);
+ // TODO(qinmin): change the |callback| to be an OnceClosure.
+ void Initialize(const base::FilePath& metadata_cache_dir,
+ const base::RepeatingClosure& callback);
+
+ void StartDownload(
+ std::unique_ptr<DownloadCreateInfo> info,
+ std::unique_ptr<InputStream> stream,
+ scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
+ const DownloadUrlParameters::OnStartedCallback& on_started);
+
// Shutting down the manager and stop all downloads.
void ShutDown();
+ // DownloadItemImplDelegate implementations.
+ void ResumeInterruptedDownload(std::unique_ptr<DownloadUrlParameters> params,
+ uint32_t id,
+ const GURL& site_url) override;
+ base::Optional<DownloadEntry> GetInProgressEntry(
+ DownloadItemImpl* download) override;
+ void ReportBytesWasted(DownloadItemImpl* download) override;
+
+ void set_file_factory(std::unique_ptr<DownloadFileFactory> file_factory) {
+ file_factory_ = std::move(file_factory);
+ }
+ DownloadFileFactory* file_factory() { return file_factory_.get(); }
+
private:
// UrlDownloadHandler::Delegate implementations.
void OnUrlDownloadStarted(
@@ -71,20 +135,33 @@
void OnUrlDownloadHandlerCreated(
UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader) override;
- // Overridden from DownloadItemImplDelegate.
- void ResumeInterruptedDownload(std::unique_ptr<DownloadUrlParameters> params,
- uint32_t id,
- const GURL& site_url) override;
+ // Start a download with given ID.
+ void StartDownloadWithId(
+ std::unique_ptr<DownloadCreateInfo> info,
+ std::unique_ptr<InputStream> stream,
+ scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter,
+ const DownloadUrlParameters::OnStartedCallback& on_started,
+ bool new_download,
+ uint32_t id);
// Active download handlers.
std::vector<UrlDownloadHandler::UniqueUrlDownloadHandlerPtr>
url_download_handlers_;
- // Delegate to call when download starts. Only OnUrlDownloadStarted() is being
- // passed through for now as we need content layer to provide default download
- // dir.
- // TODO(qinmin): remove this once this class drives all active download.
- UrlDownloadHandler::Delegate* delegate_;
+ // Delegate to provide information to create a new download. Can be null.
+ Delegate* delegate_;
+
+ // Factory for the creation of download files.
+ std::unique_ptr<DownloadFileFactory> file_factory_;
+
+ // Cache for storing metadata about in progress downloads.
+ std::unique_ptr<InProgressCache> download_metadata_cache_;
+
+ // listens to information about in-progress download items.
+ std::unique_ptr<DownloadItem::Observer> in_progress_download_observer_;
+
+ // callback to check if an origin is secure.
+ IsOriginSecureCallback is_origin_secure_cb_;
base::WeakPtrFactory<InProgressDownloadManager> weak_factory_;
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 946f764c..73135a67 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -16,7 +16,6 @@
#include "base/memory/weak_ptr.h"
#include "base/message_loop/message_loop.h"
#include "base/metrics/histogram_macros.h"
-#include "base/rand_util.h"
#include "base/stl_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/sys_string_conversions.h"
@@ -27,7 +26,6 @@
#include "components/download/downloader/in_progress/in_progress_cache_impl.h"
#include "components/download/public/common/download_create_info.h"
#include "components/download/public/common/download_file.h"
-#include "components/download/public/common/download_file_factory.h"
#include "components/download/public/common/download_interrupt_reasons.h"
#include "components/download/public/common/download_item_factory.h"
#include "components/download/public/common/download_item_impl.h"
@@ -37,7 +35,6 @@
#include "components/download/public/common/download_url_loader_factory_getter.h"
#include "components/download/public/common/download_url_parameters.h"
#include "components/download/public/common/download_utils.h"
-#include "components/download/public/common/in_progress_download_manager.h"
#include "components/download/public/common/url_download_handler_factory.h"
#include "content/browser/byte_stream.h"
#include "content/browser/child_process_security_policy_impl.h"
@@ -136,20 +133,6 @@
nullptr, params->callback()));
}
-// Helper functions for DownloadItem -> DownloadEntry for InProgressCache.
-
-download::DownloadEntry CreateDownloadEntryFromItem(
- const download::DownloadItem& item,
- const std::string& request_origin,
- download::DownloadSource download_source,
- bool fetch_error_body,
- const download::DownloadUrlParameters::RequestHeadersType&
- request_headers) {
- return download::DownloadEntry(item.GetGuid(), request_origin,
- download_source, fetch_error_body,
- request_headers, GetUniqueDownloadId());
-}
-
void BeginDownload(std::unique_ptr<download::DownloadUrlParameters> params,
std::unique_ptr<storage::BlobDataHandle> blob_data_handle,
content::ResourceContext* resource_context,
@@ -290,94 +273,27 @@
} // namespace
-// Responsible for persisting the in-progress metadata associated with a
-// download.
-class InProgressDownloadObserver : public download::DownloadItem::Observer {
- public:
- explicit InProgressDownloadObserver(
- download::InProgressCache* in_progress_cache);
- ~InProgressDownloadObserver() override;
-
- private:
- // download::DownloadItem::Observer
- void OnDownloadUpdated(download::DownloadItem* download) override;
- void OnDownloadRemoved(download::DownloadItem* download) override;
-
- // The persistent cache to store in-progress metadata.
- download::InProgressCache* in_progress_cache_;
-
- DISALLOW_COPY_AND_ASSIGN(InProgressDownloadObserver);
-};
-
-InProgressDownloadObserver::InProgressDownloadObserver(
- download::InProgressCache* in_progress_cache)
- : in_progress_cache_(in_progress_cache) {}
-
-InProgressDownloadObserver::~InProgressDownloadObserver() = default;
-
-void InProgressDownloadObserver::OnDownloadUpdated(
- download::DownloadItem* download) {
- // TODO(crbug.com/778425): Properly handle fail/resume/retry for downloads
- // that are in the INTERRUPTED state for a long time.
- if (!in_progress_cache_)
- return;
-
- switch (download->GetState()) {
- case download::DownloadItem::DownloadState::COMPLETE:
- // Intentional fallthrough.
- case download::DownloadItem::DownloadState::CANCELLED:
- if (in_progress_cache_)
- in_progress_cache_->RemoveEntry(download->GetGuid());
- break;
-
- case download::DownloadItem::DownloadState::INTERRUPTED:
- // Intentional fallthrough.
- case download::DownloadItem::DownloadState::IN_PROGRESS: {
- // Make sure the entry exists in the cache.
- base::Optional<download::DownloadEntry> entry_opt =
- in_progress_cache_->RetrieveEntry(download->GetGuid());
- download::DownloadEntry entry;
- if (!entry_opt.has_value()) {
- entry = CreateDownloadEntryFromItem(
- *download, std::string(), /* request_origin */
- download::DownloadSource::UNKNOWN, false, /* fetch_error_body */
- download::DownloadUrlParameters::RequestHeadersType());
- in_progress_cache_->AddOrReplaceEntry(entry);
- break;
- }
- entry = entry_opt.value();
- break;
- }
-
- default:
- break;
- }
-}
-
-void InProgressDownloadObserver::OnDownloadRemoved(
- download::DownloadItem* download) {
- if (!in_progress_cache_)
- return;
-
- in_progress_cache_->RemoveEntry(download->GetGuid());
-}
-
DownloadManagerImpl::DownloadManagerImpl(BrowserContext* browser_context)
: item_factory_(new DownloadItemFactoryImpl()),
- file_factory_(new download::DownloadFileFactory()),
shutdown_needed_(true),
initialized_(false),
history_db_initialized_(false),
in_progress_cache_initialized_(false),
browser_context_(browser_context),
delegate_(nullptr),
- in_progress_manager_(new download::InProgressDownloadManager(this)),
weak_factory_(this) {
DCHECK(browser_context);
download::SetIOTaskRunner(
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
download::UrlDownloadHandlerFactory::Install(new UrlDownloaderFactory());
+ in_progress_manager_ = std::make_unique<download::InProgressDownloadManager>(
+ this, base::BindRepeating(&IsOriginSecure));
+ in_progress_manager_->Initialize(
+ browser_context_->GetPath(),
+ base::BindRepeating(
+ &DownloadManagerImpl::PostInitialization, weak_factory_.GetWeakPtr(),
+ DOWNLOAD_INITIALIZATION_DEPENDENCY_IN_PROGRESS_CACHE));
}
DownloadManagerImpl::~DownloadManagerImpl() {
@@ -460,29 +376,6 @@
void DownloadManagerImpl::SetDelegate(DownloadManagerDelegate* delegate) {
delegate_ = delegate;
-
- // If initialization has not occurred and there's a delegate and cache,
- // initialize and indicate initialization is complete. Else, just indicate it
- // is ok to continue.
- // TODO(qinmin): if |delegate_| changes, the logic here is very prone to
- // errors. DownloadManagerImpl should own the in-progress cache.
- base::RepeatingClosure post_init_callback = base::BindRepeating(
- &DownloadManagerImpl::PostInitialization, weak_factory_.GetWeakPtr(),
- DOWNLOAD_INITIALIZATION_DEPENDENCY_IN_PROGRESS_CACHE);
-
- if (delegate_) {
- download::InProgressCache* in_progress_cache =
- delegate_->GetInProgressCache();
- if (in_progress_cache) {
- in_progress_download_observer_ =
- std::make_unique<InProgressDownloadObserver>(in_progress_cache);
- in_progress_cache->Initialize(std::move(post_init_callback));
- return;
- }
- }
-
- BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
- std::move(post_init_callback));
}
DownloadManagerDelegate* DownloadManagerImpl::GetDelegate() const {
@@ -536,88 +429,7 @@
return true;
}
-void DownloadManagerImpl::StartDownload(
- std::unique_ptr<download::DownloadCreateInfo> info,
- std::unique_ptr<download::InputStream> stream,
- scoped_refptr<download::DownloadURLLoaderFactoryGetter>
- url_loader_factory_getter,
- const download::DownloadUrlParameters::OnStartedCallback& on_started) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- DCHECK(info);
-
- uint32_t download_id = info->download_id;
- bool new_download = (download_id == download::DownloadItem::kInvalidId);
- if (new_download &&
- info->result == download::DOWNLOAD_INTERRUPT_REASON_NONE &&
- InterceptDownload(*info)) {
- download::GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release());
- return;
- }
-
- // |stream| is only non-nil if the download request was successful.
- DCHECK((info->result == download::DOWNLOAD_INTERRUPT_REASON_NONE &&
- !stream->IsEmpty()) ||
- (info->result != download::DOWNLOAD_INTERRUPT_REASON_NONE &&
- stream->IsEmpty()));
- DVLOG(20) << __func__ << "() result="
- << download::DownloadInterruptReasonToString(info->result);
- if (new_download) {
- download::RecordDownloadConnectionSecurity(info->url(), info->url_chain);
- download::RecordDownloadContentTypeSecurity(
- info->url(), info->url_chain, info->mime_type,
- base::BindRepeating(&IsOriginSecure));
- }
- base::Callback<void(uint32_t)> got_id(base::Bind(
- &DownloadManagerImpl::StartDownloadWithId, weak_factory_.GetWeakPtr(),
- base::Passed(&info), base::Passed(&stream),
- std::move(url_loader_factory_getter), on_started, new_download));
- if (new_download) {
- GetNextId(std::move(got_id));
- } else {
- std::move(got_id).Run(download_id);
- }
-}
-
-void DownloadManagerImpl::StartDownloadWithId(
- std::unique_ptr<download::DownloadCreateInfo> info,
- std::unique_ptr<download::InputStream> stream,
- scoped_refptr<download::DownloadURLLoaderFactoryGetter>
- url_loader_factory_getter,
- const download::DownloadUrlParameters::OnStartedCallback& on_started,
- bool new_download,
- uint32_t id) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- DCHECK_NE(download::DownloadItem::kInvalidId, id);
- download::DownloadItemImpl* download = nullptr;
- if (new_download) {
- download = CreateActiveItem(id, *info);
- } else {
- auto item_iterator = downloads_.find(id);
- // Trying to resume an interrupted download.
- if (item_iterator == downloads_.end() ||
- (item_iterator->second->GetState() ==
- download::DownloadItem::CANCELLED)) {
- // If the download is no longer known to the DownloadManager, then it was
- // removed after it was resumed. Ignore. If the download is cancelled
- // while resuming, then also ignore the request.
- if (info->request_handle)
- info->request_handle->CancelRequest(true);
- if (!on_started.is_null())
- on_started.Run(nullptr,
- download::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
- // The ByteStreamReader lives and dies on the download sequence.
- if (info->result == download::DOWNLOAD_INTERRUPT_REASON_NONE)
- download::GetDownloadTaskRunner()->DeleteSoon(FROM_HERE,
- stream.release());
- return;
- }
- download = item_iterator->second.get();
- }
- DownloadItemUtils::AttachInfo(
- download, GetBrowserContext(),
- WebContentsImpl::FromRenderFrameHostID(info->render_process_id,
- info->render_frame_id));
-
+base::FilePath DownloadManagerImpl::GetDefaultDownloadDirectory() {
base::FilePath default_download_directory;
#if defined(USE_X11)
// TODO(thomasanderson,crbug.com/784010): Remove this when all Linux
@@ -640,56 +452,61 @@
GetContentClient()->browser()->GetDefaultDownloadDirectory();
}
- if (delegate_) {
- download::InProgressCache* in_progress_cache =
- delegate_->GetInProgressCache();
- if (in_progress_cache) {
- base::Optional<download::DownloadEntry> entry_opt =
- in_progress_cache->RetrieveEntry(download->GetGuid());
- if (!entry_opt.has_value()) {
- in_progress_cache->AddOrReplaceEntry(CreateDownloadEntryFromItem(
- *download, info->request_origin, info->download_source,
- info->fetch_error_body, info->request_headers));
- }
+ return default_download_directory;
+}
- // May already observe this item, remove observer first.
- download->RemoveObserver(in_progress_download_observer_.get());
- download->AddObserver(in_progress_download_observer_.get());
- }
- }
+void DownloadManagerImpl::OnNewDownloadStarted(
+ download::DownloadItem* download) {
+ for (auto& observer : observers_)
+ observer.OnDownloadCreated(this, download);
+}
- std::unique_ptr<download::DownloadFile> download_file;
+download::DownloadItemImpl* DownloadManagerImpl::GetDownloadItem(
+ uint32_t id,
+ bool new_download,
+ const download::DownloadCreateInfo& info) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ DCHECK_NE(download::DownloadItem::kInvalidId, id);
- if (info->result == download::DOWNLOAD_INTERRUPT_REASON_NONE) {
- DCHECK(stream.get());
- download_file.reset(file_factory_->CreateFile(
- std::move(info->save_info), default_download_directory,
- std::move(stream), id, download->DestinationObserverAsWeakPtr()));
- }
- // It is important to leave info->save_info intact in the case of an interrupt
- // so that the download::DownloadItem can salvage what it can out of a failed
- // resumption attempt.
-
- StoragePartition* storage_partition = GetStoragePartition(
- browser_context_, info->render_process_id, info->render_frame_id);
-
- download->Start(
- std::move(download_file), std::move(info->request_handle), *info,
- std::move(url_loader_factory_getter),
- storage_partition ? storage_partition->GetURLRequestContext() : nullptr);
-
- // For interrupted downloads, Start() will transition the state to
- // IN_PROGRESS and consumers will be notified via OnDownloadUpdated().
- // For new downloads, we notify here, rather than earlier, so that
- // the download_file is bound to download and all the usual
- // setters (e.g. Cancel) work.
+ download::DownloadItemImpl* download = nullptr;
if (new_download) {
- for (auto& observer : observers_)
- observer.OnDownloadCreated(this, download);
+ download = CreateActiveItem(id, info);
+ } else {
+ auto item_iterator = downloads_.find(id);
+ // Trying to resume an interrupted download.
+ if (item_iterator == downloads_.end() ||
+ (item_iterator->second->GetState() ==
+ download::DownloadItem::CANCELLED)) {
+ return nullptr;
+ }
+ download = item_iterator->second.get();
}
+ DownloadItemUtils::AttachInfo(
+ download, GetBrowserContext(),
+ WebContentsImpl::FromRenderFrameHostID(info.render_process_id,
+ info.render_frame_id));
+ return download;
+}
- if (!on_started.is_null())
- on_started.Run(download, download::DOWNLOAD_INTERRUPT_REASON_NONE);
+net::URLRequestContextGetter* DownloadManagerImpl::GetURLRequestContextGetter(
+ const download::DownloadCreateInfo& info) {
+ StoragePartition* storage_partition = GetStoragePartition(
+ browser_context_, info.render_process_id, info.render_frame_id);
+ return storage_partition ? storage_partition->GetURLRequestContext()
+ : nullptr;
+}
+
+void DownloadManagerImpl::StartDownload(
+ std::unique_ptr<download::DownloadCreateInfo> info,
+ std::unique_ptr<download::InputStream> stream,
+ scoped_refptr<download::DownloadURLLoaderFactoryGetter>
+ url_loader_factory_getter,
+ const download::DownloadUrlParameters::OnStartedCallback& on_started) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ DCHECK(info);
+ in_progress_manager_->StartDownload(std::move(info), std::move(stream),
+ std::move(url_loader_factory_getter),
+ on_started);
}
void DownloadManagerImpl::CheckForHistoryFilesRemoval() {
@@ -794,12 +611,12 @@
void DownloadManagerImpl::SetDownloadFileFactoryForTesting(
std::unique_ptr<download::DownloadFileFactory> file_factory) {
- file_factory_ = std::move(file_factory);
+ in_progress_manager_->set_file_factory(std::move(file_factory));
}
download::DownloadFileFactory*
DownloadManagerImpl::GetDownloadFileFactoryForTesting() {
- return file_factory_.get();
+ return in_progress_manager_->file_factory();
}
void DownloadManagerImpl::DownloadRemoved(
@@ -822,14 +639,7 @@
base::Optional<download::DownloadEntry> DownloadManagerImpl::GetInProgressEntry(
download::DownloadItemImpl* download) {
- if (!download || !delegate_)
- return base::Optional<download::DownloadEntry>();
-
- download::InProgressCache* in_progress_cache =
- delegate_->GetInProgressCache();
- if (in_progress_cache)
- return in_progress_cache->RetrieveEntry(download->GetGuid());
- return base::Optional<download::DownloadEntry>();
+ return in_progress_manager_->GetInProgressEntry(download);
}
bool DownloadManagerImpl::IsOffTheRecord() const {
@@ -838,16 +648,7 @@
void DownloadManagerImpl::ReportBytesWasted(
download::DownloadItemImpl* download) {
- if (!delegate_)
- return;
- auto entry_opt = GetInProgressEntry(download);
- if (entry_opt.has_value()) {
- download::DownloadEntry entry = entry_opt.value();
- entry.bytes_wasted = download->GetBytesWasted();
- download::InProgressCache* in_progress_cache =
- delegate_->GetInProgressCache();
- in_progress_cache->AddOrReplaceEntry(entry);
- }
+ in_progress_manager_->ReportBytesWasted(download);
}
void DownloadManagerImpl::OnUrlDownloadHandlerCreated(
@@ -1252,7 +1053,7 @@
storage_partition, rfh, !params->suggested_name().empty());
}
- in_progress_manager_->StartDownload(
+ in_progress_manager_->BeginDownload(
std::move(params), std::move(url_loader_factory_getter), id, site_url,
tab_url, tab_referrer_url);
} else {
diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h
index 0dd52f74..e7aa6ce 100644
--- a/content/browser/download/download_manager_impl.h
+++ b/content/browser/download/download_manager_impl.h
@@ -22,6 +22,7 @@
#include "base/synchronization/lock.h"
#include "components/download/public/common/download_item_impl_delegate.h"
#include "components/download/public/common/download_url_parameters.h"
+#include "components/download/public/common/in_progress_download_manager.h"
#include "components/download/public/common/url_download_handler.h"
#include "content/browser/loader/navigation_url_loader.h"
#include "content/common/content_export.h"
@@ -37,7 +38,6 @@
class DownloadItemFactory;
class DownloadItemImpl;
class DownloadRequestHandleInterface;
-class InProgressDownloadManager;
}
namespace content {
@@ -47,6 +47,7 @@
class CONTENT_EXPORT DownloadManagerImpl
: public DownloadManager,
public download::UrlDownloadHandler::Delegate,
+ public download::InProgressDownloadManager::Delegate,
private download::DownloadItemImplDelegate {
public:
using DownloadItemImplCreated =
@@ -181,15 +182,6 @@
friend class DownloadManagerTest;
friend class DownloadTest;
- void StartDownloadWithId(
- std::unique_ptr<download::DownloadCreateInfo> info,
- std::unique_ptr<download::InputStream> stream,
- scoped_refptr<download::DownloadURLLoaderFactoryGetter>
- url_loader_factory_getter,
- const download::DownloadUrlParameters::OnStartedCallback& on_started,
- bool new_download,
- uint32_t id);
-
void CreateSavePackageDownloadItemWithId(
const base::FilePath& main_file_path,
const GURL& page_url,
@@ -200,9 +192,17 @@
const DownloadItemImplCreated& on_started,
uint32_t id);
- // Intercepts the download to another system if applicable. Returns true if
- // the download was intercepted.
- bool InterceptDownload(const download::DownloadCreateInfo& info);
+ // InProgressDownloadManager::Delegate implementations.
+ bool InterceptDownload(const download::DownloadCreateInfo& info) override;
+ base::FilePath GetDefaultDownloadDirectory() override;
+ download::DownloadItemImpl* GetDownloadItem(
+ uint32_t id,
+ bool new_download,
+ const download::DownloadCreateInfo& info) override;
+ net::URLRequestContextGetter* GetURLRequestContextGetter(
+ const download::DownloadCreateInfo& info) override;
+ void OnNewDownloadStarted(download::DownloadItem* download) override;
+ void GetNextId(const DownloadIdCallback& callback) override;
// Create a new active item based on the info. Separate from
// StartDownload() for testing.
@@ -210,9 +210,6 @@
uint32_t id,
const download::DownloadCreateInfo& info);
- // Get next download id. |callback| is called on the UI thread and may
- // be called synchronously.
- void GetNextId(const DownloadIdCallback& callback);
// Called with the result of DownloadManagerDelegate::CheckForFileExistence.
// Updates the state of the file and then notifies this update to the file's
@@ -264,9 +261,6 @@
// Factory for creation of downloads items.
std::unique_ptr<download::DownloadItemFactory> item_factory_;
- // Factory for the creation of download files.
- std::unique_ptr<download::DownloadFileFactory> file_factory_;
-
// |downloads_| is the owning set for all downloads known to the
// DownloadManager. This includes downloads started by the user in
// this session, downloads initialized from the history system, and
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index 29969a8..70e55bf 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -161,14 +161,20 @@
std::unique_ptr<download::DownloadRequestHandleInterface> request_handle)
override;
+ void set_is_download_started(bool is_download_started) {
+ is_download_started_ = is_download_started;
+ }
+
private:
std::map<uint32_t, download::MockDownloadItemImpl*> items_;
download::DownloadItemImplDelegate item_delegate_;
+ bool is_download_started_;
DISALLOW_COPY_AND_ASSIGN(MockDownloadItemFactory);
};
-MockDownloadItemFactory::MockDownloadItemFactory() {}
+MockDownloadItemFactory::MockDownloadItemFactory()
+ : is_download_started_(false) {}
MockDownloadItemFactory::~MockDownloadItemFactory() {}
@@ -243,6 +249,10 @@
.WillRepeatedly(Return(download_id));
EXPECT_CALL(*result, GetGuid())
.WillRepeatedly(ReturnRefOfCopy(base::GenerateGUID()));
+ if (is_download_started_) {
+ EXPECT_CALL(*result, RemoveObserver(_));
+ EXPECT_CALL(*result, AddObserver(_));
+ }
items_[download_id] = result;
// Active items are created and then immediately are called to start
@@ -444,6 +454,7 @@
// Key test variable; we'll keep it available to sub-classes.
std::unique_ptr<DownloadManagerImpl> download_manager_;
base::WeakPtr<MockDownloadFileFactory> mock_download_file_factory_;
+ base::WeakPtr<MockDownloadItemFactory> mock_download_item_factory_;
// Target detetermined callback.
bool callback_called_;
@@ -457,7 +468,6 @@
private:
TestBrowserThreadBundle thread_bundle_;
- base::WeakPtr<MockDownloadItemFactory> mock_download_item_factory_;
std::unique_ptr<MockDownloadManagerDelegate> mock_download_manager_delegate_;
std::unique_ptr<MockDownloadManagerObserver> observer_;
std::unique_ptr<TestBrowserContext> browser_context_;
@@ -495,6 +505,7 @@
MockCreateFile(Ref(*info->save_info.get()), input_stream.get()))
.WillOnce(Return(mock_file));
+ mock_download_item_factory_->set_is_download_started(true);
download_manager_->StartDownload(
std::move(info), std::move(input_stream), nullptr,
download::DownloadUrlParameters::OnStartedCallback());
diff --git a/content/browser/download/download_utils.cc b/content/browser/download/download_utils.cc
index df4bdc3..c9e72c3 100644
--- a/content/browser/download/download_utils.cc
+++ b/content/browser/download/download_utils.cc
@@ -6,7 +6,6 @@
#include "base/format_macros.h"
#include "base/process/process_handle.h"
-#include "base/rand_util.h"
#include "base/strings/stringprintf.h"
#include "base/task_scheduler/post_task.h"
#include "components/download/downloader/in_progress/download_entry.h"
@@ -99,14 +98,4 @@
return request;
}
-// Gets the unique download id for ukm reporting.
-uint64_t GetUniqueDownloadId() {
- // Get a new UKM download_id that is not 0.
- uint64_t download_id = 0;
- do {
- download_id = base::RandUint64();
- } while (download_id == 0);
- return download_id;
-}
-
} // namespace content
diff --git a/content/browser/download/download_utils.h b/content/browser/download/download_utils.h
index 3ff8026..df86cad 100644
--- a/content/browser/download/download_utils.h
+++ b/content/browser/download/download_utils.h
@@ -33,8 +33,6 @@
storage::BlobStorageContext* BlobStorageContextGetter(
ResourceContext* resource_context);
-uint64_t GetUniqueDownloadId();
-
} // namespace content
#endif // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_UTILS_H_
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index eaa07b3..0f395a3 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -30,11 +30,11 @@
#include "components/download/public/common/download_stats.h"
#include "components/download/public/common/download_task_runner.h"
#include "components/download/public/common/download_ukm_helper.h"
+#include "components/download/public/common/download_utils.h"
#include "components/filename_generation/filename_generation.h"
#include "components/url_formatter/url_formatter.h"
#include "content/browser/bad_message.h"
#include "content/browser/download/download_manager_impl.h"
-#include "content/browser/download/download_utils.h"
#include "content/browser/download/save_file.h"
#include "content/browser/download/save_file_manager.h"
#include "content/browser/download/save_item.h"
@@ -259,7 +259,7 @@
download::RecordSavePackageEvent(download::SAVE_PACKAGE_STARTED);
ukm_source_id_ = ukm::UkmRecorder::GetNewSourceID();
- ukm_download_id_ = GetUniqueDownloadId();
+ ukm_download_id_ = download::GetUniqueDownloadId();
download::DownloadUkmHelper::RecordDownloadStarted(
ukm_download_id_, ukm_source_id_, download::DownloadContent::TEXT,
download::DownloadSource::UNKNOWN);
diff --git a/content/public/browser/download_manager_delegate.cc b/content/public/browser/download_manager_delegate.cc
index 8eac05f..b5963aa 100644
--- a/content/public/browser/download_manager_delegate.cc
+++ b/content/public/browser/download_manager_delegate.cc
@@ -53,10 +53,6 @@
return false;
}
-download::InProgressCache* DownloadManagerDelegate::GetInProgressCache() {
- return nullptr;
-}
-
std::string
DownloadManagerDelegate::ApplicationClientIdForFileScanning() const {
return std::string();
diff --git a/content/public/browser/download_manager_delegate.h b/content/public/browser/download_manager_delegate.h
index 579ea35..ac3055c2 100644
--- a/content/public/browser/download_manager_delegate.h
+++ b/content/public/browser/download_manager_delegate.h
@@ -17,9 +17,6 @@
#include "content/public/browser/resource_request_info.h"
#include "content/public/browser/save_page_type.h"
-namespace download {
-class InProgressCache;
-} // namespace download
namespace content {
@@ -138,9 +135,6 @@
base::FilePath* download_save_dir,
bool* skip_dir_check) {}
- // Returns the metadata cache for in-progress downloads.
- virtual download::InProgressCache* GetInProgressCache();
-
// Asks the user for the path to save a page. The delegate calls the callback
// to give the answer.
virtual void ChooseSavePath(
