Persist download ids across sessions by merging them with db_handles.
Remove DownloadId from the content api. It made sense in a world with DownloadFileManager, but that world is no more.
Remove meta_table.next_download_id_. {HistoryService,HistoryBackend,DownloadDatabase}::GetNextDownloadId() returns `select max(id) from downloads` + 1. GetNextDownloadId() is called once when ChromeDownloadManagerDelegate is created, before DownloadDatabase::QueryDownloads() is called.
{DownloadManager,DownloadManagerDelegate}::GetNextId() is now *a*synchronous. If GetNextId() is called before History*::GetNextDownloadId() finishes, then the callback arguments go into a queue that is flushed when History*::GetNextDownloadId() calls back to CDMD.
Even if the `select max(id) from downloads` statement takes a long time, browser/profile initialization is not delayed. The only operations that are delayed in this case are downloads.
We cannot simply allow sqlite to automatically assign the id because some DownloadItems are never added to the history.
If all items were added to the history, then we could allow sqlite to automatically assign the id by merging DownloadDatabase::CreateDownload() with DownloadDatabase::GetNextDownloadId(). But some items are never added to the history, so we don't want to create records for them. Also, that would delay surfacing *every* download to the UI by a DB thread bounce, whereas this CL only delays the downloads that are started before DownloadDatabase::GetNextDownloadId() returns.
DownloadHistory now uses loading_id_ instead of loading_db_handle_. This way is safer.
DownloadHistoryData uses a tri-state enum {NOT_PERSISTED, PERSISTING, PERSISTED}.
The invalid id constant is moved to content::DownloadItem::kInvalidId.
BUG=98253
Review URL: https://chromiumcodereview.appspot.com/15584002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211414 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 08e32c6..0582385a 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -48,7 +48,7 @@
namespace {
void BeginDownload(scoped_ptr<DownloadUrlParameters> params,
- DownloadId download_id) {
+ uint32 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
@@ -170,7 +170,7 @@
virtual DownloadItemImpl* CreatePersistedItem(
DownloadItemImplDelegate* delegate,
- DownloadId download_id,
+ uint32 download_id,
const base::FilePath& current_path,
const base::FilePath& target_path,
const std::vector<GURL>& url_chain,
@@ -204,7 +204,7 @@
virtual DownloadItemImpl* CreateActiveItem(
DownloadItemImplDelegate* delegate,
- DownloadId download_id,
+ uint32 download_id,
const DownloadCreateInfo& info,
const net::BoundNetLog& bound_net_log) OVERRIDE {
return new DownloadItemImpl(delegate, download_id, info, bound_net_log);
@@ -212,7 +212,7 @@
virtual DownloadItemImpl* CreateSavePageItem(
DownloadItemImplDelegate* delegate,
- DownloadId download_id,
+ uint32 download_id,
const base::FilePath& path,
const GURL& url,
const std::string& mime_type,
@@ -245,25 +245,25 @@
}
DownloadItemImpl* DownloadManagerImpl::CreateActiveItem(
- DownloadId id, const DownloadCreateInfo& info) {
+ uint32 id, const DownloadCreateInfo& info) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(!ContainsKey(downloads_, id));
net::BoundNetLog bound_net_log =
net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
DownloadItemImpl* download =
item_factory_->CreateActiveItem(this, id, info, bound_net_log);
- downloads_[id.local()] = download;
+ downloads_[id] = download;
return download;
}
-DownloadId DownloadManagerImpl::GetNextId() {
- DownloadId id;
- if (delegate_)
- id = delegate_->GetNextId();
- if (!id.IsValid()) {
- static int next_id;
- id = DownloadId(browser_context_, ++next_id);
+void DownloadManagerImpl::GetNextId(const DownloadIdCallback& callback) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (delegate_) {
+ delegate_->GetNextId(callback);
+ return;
}
-
- return id;
+ static uint32 next_id = content::DownloadItem::kInvalidId + 1;
+ callback.Run(next_id++);
}
void DownloadManagerImpl::DetermineDownloadTarget(
@@ -352,20 +352,41 @@
delegate_ = NULL;
}
-DownloadItem* DownloadManagerImpl::StartDownload(
+void DownloadManagerImpl::StartDownload(
scoped_ptr<DownloadCreateInfo> info,
- scoped_ptr<ByteStreamReader> stream) {
+ scoped_ptr<ByteStreamReader> stream,
+ const DownloadUrlParameters::OnStartedCallback& on_started) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- DownloadId id(info->download_id);
- const bool new_download = !id.IsValid();
- DownloadItemImpl* download = NULL;
-
+ DCHECK(info);
+ uint32 download_id = info->download_id;
+ const bool new_download = (download_id == content::DownloadItem::kInvalidId);
+ base::Callback<void(uint32)> got_id(base::Bind(
+ &DownloadManagerImpl::StartDownloadWithId,
+ weak_factory_.GetWeakPtr(),
+ base::Passed(info.Pass()),
+ base::Passed(stream.Pass()),
+ on_started,
+ new_download));
if (new_download) {
- id = GetNextId();
+ GetNextId(got_id);
+ } else {
+ got_id.Run(download_id);
+ }
+}
+
+void DownloadManagerImpl::StartDownloadWithId(
+ scoped_ptr<DownloadCreateInfo> info,
+ scoped_ptr<ByteStreamReader> stream,
+ const DownloadUrlParameters::OnStartedCallback& on_started,
+ bool new_download,
+ uint32 id) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK_NE(content::DownloadItem::kInvalidId, id);
+ DownloadItemImpl* download = NULL;
+ if (new_download) {
download = CreateActiveItem(id, *info);
} else {
- DownloadMap::iterator item_iterator = downloads_.find(id.local());
+ DownloadMap::iterator item_iterator = downloads_.find(id);
// Trying to resume an interrupted download.
if (item_iterator == downloads_.end() ||
(item_iterator->second->GetState() == DownloadItem::CANCELLED)) {
@@ -373,7 +394,9 @@
// removed after it was resumed. Ignore. If the download is cancelled
// while resuming, then also ignore the request.
info->request_handle.CancelRequest();
- return NULL;
+ if (!on_started.is_null())
+ on_started.Run(NULL, net::ERR_ABORTED);
+ return;
}
download = item_iterator->second;
DCHECK_EQ(DownloadItem::INTERRUPTED, download->GetState());
@@ -407,7 +430,8 @@
if (new_download)
FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, download));
- return download;
+ if (!on_started.is_null())
+ on_started.Run(download, net::OK);
}
void DownloadManagerImpl::CheckForHistoryFilesRemoval() {
@@ -431,7 +455,7 @@
}
}
-void DownloadManagerImpl::OnFileExistenceChecked(int32 download_id,
+void DownloadManagerImpl::OnFileExistenceChecked(uint32 download_id,
bool result) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!result) { // File does not exist.
@@ -444,30 +468,48 @@
return browser_context_;
}
-DownloadItemImpl* DownloadManagerImpl::CreateSavePackageDownloadItem(
+void DownloadManagerImpl::CreateSavePackageDownloadItem(
const base::FilePath& main_file_path,
const GURL& page_url,
const std::string& mime_type,
scoped_ptr<DownloadRequestHandleInterface> request_handle,
- DownloadItem::Observer* observer) {
+ const DownloadItemImplCreated& item_created) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ GetNextId(base::Bind(
+ &DownloadManagerImpl::CreateSavePackageDownloadItemWithId,
+ weak_factory_.GetWeakPtr(),
+ main_file_path,
+ page_url,
+ mime_type,
+ base::Passed(request_handle.Pass()),
+ item_created));
+}
+
+void DownloadManagerImpl::CreateSavePackageDownloadItemWithId(
+ const base::FilePath& main_file_path,
+ const GURL& page_url,
+ const std::string& mime_type,
+ scoped_ptr<DownloadRequestHandleInterface> request_handle,
+ const DownloadItemImplCreated& item_created,
+ uint32 id) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK_NE(content::DownloadItem::kInvalidId, id);
+ DCHECK(!ContainsKey(downloads_, id));
net::BoundNetLog bound_net_log =
net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
DownloadItemImpl* download_item = item_factory_->CreateSavePageItem(
this,
- GetNextId(),
+ id,
main_file_path,
page_url,
mime_type,
request_handle.Pass(),
bound_net_log);
-
- download_item->AddObserver(observer);
- DCHECK(!ContainsKey(downloads_, download_item->GetId()));
downloads_[download_item->GetId()] = download_item;
FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(
this, download_item));
-
- return download_item;
+ if (!item_created.is_null())
+ item_created.Run(download_item);
}
void DownloadManagerImpl::OnSavePackageSuccessfullyFinished(
@@ -481,7 +523,7 @@
// download.
void DownloadManagerImpl::ResumeInterruptedDownload(
scoped_ptr<content::DownloadUrlParameters> params,
- content::DownloadId id) {
+ uint32 id) {
BrowserThread::PostTask(
BrowserThread::IO,
FROM_HERE,
@@ -506,7 +548,7 @@
if (!download)
return;
- int32 download_id = download->GetId();
+ uint32 download_id = download->GetId();
if (downloads_.find(download_id) == downloads_.end())
return;
@@ -554,7 +596,8 @@
DCHECK(params->method() == "POST");
}
BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
- &BeginDownload, base::Passed(¶ms), DownloadId()));
+ &BeginDownload, base::Passed(¶ms),
+ content::DownloadItem::kInvalidId));
}
void DownloadManagerImpl::AddObserver(Observer* observer) {
@@ -566,6 +609,7 @@
}
DownloadItem* DownloadManagerImpl::CreateDownloadItem(
+ uint32 id,
const base::FilePath& current_path,
const base::FilePath& target_path,
const std::vector<GURL>& url_chain,
@@ -578,9 +622,12 @@
DownloadDangerType danger_type,
DownloadInterruptReason interrupt_reason,
bool opened) {
+ DCHECK(!ContainsKey(downloads_, id));
+ if (ContainsKey(downloads_, id))
+ return NULL;
DownloadItemImpl* item = item_factory_->CreatePersistedItem(
this,
- GetNextId(),
+ id,
current_path,
target_path,
url_chain,
@@ -594,8 +641,7 @@
interrupt_reason,
opened,
net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD));
- DCHECK(!ContainsKey(downloads_, item->GetId()));
- downloads_[item->GetId()] = item;
+ downloads_[id] = item;
FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, item));
VLOG(20) << __FUNCTION__ << "() download = " << item->DebugString(true);
return item;
@@ -611,7 +657,7 @@
return count;
}
-DownloadItem* DownloadManagerImpl::GetDownload(int download_id) {
+DownloadItem* DownloadManagerImpl::GetDownload(uint32 download_id) {
return ContainsKey(downloads_, download_id) ? downloads_[download_id] : NULL;
}
