diff options
34 files changed, 229 insertions, 161 deletions
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc index 0d7bf68..690ae50 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/chrome/browser/download/chrome_download_manager_delegate.cc @@ -63,9 +63,6 @@ void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) { download_history_->Load( base::Bind(&DownloadManager::OnPersistentStoreQueryComplete, base::Unretained(dm))); - download_history_->GetNextId( - base::Bind(&DownloadManager::OnPersistentStoreGetNextId, - base::Unretained(dm))); } void ChromeDownloadManagerDelegate::Shutdown() { diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc index 58b905f..00223ad 100644 --- a/chrome/browser/download/download_manager_unittest.cc +++ b/chrome/browser/download/download_manager_unittest.cc @@ -26,6 +26,7 @@ #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file.h" #include "content/browser/download/download_file_manager.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_item.h" #include "content/browser/download/download_manager.h" #include "content/browser/download/download_request_handle.h" @@ -42,6 +43,8 @@ #include "ui/base/l10n/l10n_util.h" #include "ui/base/text/bytes_formatting.h" +DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain"; + class DownloadManagerTest : public testing::Test { public: static const char* kTestData; @@ -51,8 +54,11 @@ class DownloadManagerTest : public testing::Test { : profile_(new TestingProfile()), download_manager_delegate_(new ChromeDownloadManagerDelegate( profile_.get())), + id_factory_(new DownloadIdFactory(kValidIdDomain)), download_manager_(new MockDownloadManager( - download_manager_delegate_, &download_status_updater_)), + download_manager_delegate_, + id_factory_, + &download_status_updater_)), ui_thread_(BrowserThread::UI, &message_loop_), file_thread_(BrowserThread::FILE, &message_loop_), download_buffer_(new content::DownloadBuffer) { @@ -71,7 +77,7 @@ class DownloadManagerTest : public testing::Test { } void AddDownloadToFileManager(int id, DownloadFile* download_file) { - file_manager()->downloads_[DownloadId(download_manager_.get(), id)] = + file_manager()->downloads_[DownloadId(kValidIdDomain, id)] = download_file; } @@ -100,7 +106,7 @@ class DownloadManagerTest : public testing::Test { BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, base::Bind(&DownloadFileManager::UpdateDownload, file_manager_.get(), - DownloadId(download_manager_.get(), id), download_buffer_)); + DownloadId(kValidIdDomain, id), download_buffer_)); message_loop_.RunAllPending(); } @@ -121,6 +127,7 @@ class DownloadManagerTest : public testing::Test { DownloadStatusUpdater download_status_updater_; scoped_ptr<TestingProfile> profile_; scoped_refptr<ChromeDownloadManagerDelegate> download_manager_delegate_; + scoped_refptr<DownloadIdFactory> id_factory_; scoped_refptr<DownloadManager> download_manager_; scoped_refptr<DownloadFileManager> file_manager_; MessageLoopForUI message_loop_; @@ -383,7 +390,7 @@ TEST_F(DownloadManagerTest, StartDownload) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - info->download_id = static_cast<int>(i); + info->download_id = DownloadId(kValidIdDomain, static_cast<int>(i)); info->prompt_user_for_save_location = kStartDownloadCases[i].save_as; info->url_chain.push_back(GURL(kStartDownloadCases[i].url)); info->mime_type = kStartDownloadCases[i].mime_type; @@ -392,9 +399,9 @@ TEST_F(DownloadManagerTest, StartDownload) { DownloadFile* download_file( new DownloadFile(info.get(), DownloadRequestHandle(), download_manager_)); - AddDownloadToFileManager(info->download_id, download_file); + AddDownloadToFileManager(info->download_id.local(), download_file); download_file->Initialize(false); - download_manager_->StartDownload(info->download_id); + download_manager_->StartDownload(info->download_id.local()); message_loop_.RunAllPending(); // SelectFileObserver will have recorded any attempt to open the @@ -416,14 +423,14 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - info->download_id = static_cast<int>(i); + info->download_id = DownloadId(kValidIdDomain, static_cast<int>(i)); info->prompt_user_for_save_location = false; info->url_chain.push_back(GURL()); const FilePath new_path(kDownloadRenameCases[i].suggested_path); MockDownloadFile* download_file( new MockDownloadFile(info.get(), download_manager_)); - AddDownloadToFileManager(info->download_id, download_file); + AddDownloadToFileManager(info->download_id.local(), download_file); // |download_file| is owned by DownloadFileManager. ::testing::Mock::AllowLeak(download_file); @@ -479,7 +486,7 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - info->download_id = static_cast<int>(0); + info->download_id = DownloadId(kValidIdDomain, 0); info->prompt_user_for_save_location = false; info->url_chain.push_back(GURL()); info->total_bytes = static_cast<int64>(kTestDataLen); @@ -488,7 +495,7 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) { MockDownloadFile* download_file( new MockDownloadFile(info.get(), download_manager_)); - AddDownloadToFileManager(info->download_id, download_file); + AddDownloadToFileManager(info->download_id.local(), download_file); // |download_file| is owned by DownloadFileManager. ::testing::Mock::AllowLeak(download_file); @@ -569,8 +576,8 @@ TEST_F(DownloadManagerTest, DownloadFileErrorTest) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - int32 id = 0; - info->download_id = id; + static const int32 local_id = 0; + info->download_id = DownloadId(kValidIdDomain, local_id); info->prompt_user_for_save_location = false; info->url_chain.push_back(GURL()); info->total_bytes = static_cast<int64>(kTestDataLen * 3); @@ -579,7 +586,7 @@ TEST_F(DownloadManagerTest, DownloadFileErrorTest) { // Create a download file that we can insert errors into. DownloadFileWithMockStream* download_file(new DownloadFileWithMockStream( info.get(), download_manager_, mock_stream)); - AddDownloadToFileManager(id, download_file); + AddDownloadToFileManager(local_id, download_file); // |download_file| is owned by DownloadFileManager. download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle()); @@ -594,7 +601,7 @@ TEST_F(DownloadManagerTest, DownloadFileErrorTest) { scoped_ptr<ItemObserver> observer(new ItemObserver(download)); // Add some data before finalizing the file name. - UpdateData(id, kTestData, kTestDataLen); + UpdateData(local_id, kTestData, kTestDataLen); // Finalize the file name. ContinueDownloadWithPath(download, path); @@ -602,11 +609,11 @@ TEST_F(DownloadManagerTest, DownloadFileErrorTest) { EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); // Add more data. - UpdateData(id, kTestData, kTestDataLen); + UpdateData(local_id, kTestData, kTestDataLen); // Add more data, but an error occurs. download_file->SetForcedError(net::ERR_FAILED); - UpdateData(id, kTestData, kTestDataLen); + UpdateData(local_id, kTestData, kTestDataLen); // Check the state. The download should have been interrupted. EXPECT_TRUE(GetActiveDownloadItem(0) == NULL); @@ -648,7 +655,7 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - info->download_id = static_cast<int>(0); + info->download_id = DownloadId(kValidIdDomain, 0); info->prompt_user_for_save_location = false; info->url_chain.push_back(GURL()); const FilePath new_path(FILE_PATH_LITERAL("foo.zip")); @@ -656,7 +663,7 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) { MockDownloadFile* download_file( new MockDownloadFile(info.get(), download_manager_)); - AddDownloadToFileManager(info->download_id, download_file); + AddDownloadToFileManager(info->download_id.local(), download_file); // |download_file| is owned by DownloadFileManager. ::testing::Mock::AllowLeak(download_file); @@ -732,7 +739,7 @@ TEST_F(DownloadManagerTest, DownloadOverwriteTest) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - info->download_id = static_cast<int>(0); + info->download_id = DownloadId(kValidIdDomain, 0); info->prompt_user_for_save_location = true; info->url_chain.push_back(GURL()); @@ -756,7 +763,7 @@ TEST_F(DownloadManagerTest, DownloadOverwriteTest) { // This creates the .crdownload version of the file. download_file->Initialize(false); // |download_file| is owned by DownloadFileManager. - AddDownloadToFileManager(info->download_id, download_file); + AddDownloadToFileManager(info->download_id.local(), download_file); ContinueDownloadWithPath(download, new_path); message_loop_.RunAllPending(); @@ -808,7 +815,7 @@ TEST_F(DownloadManagerTest, DownloadRemoveTest) { // responsible for deleting it. In these unit tests, however, we // don't call the function that deletes it, so we do so ourselves. scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo); - info->download_id = static_cast<int>(0); + info->download_id = DownloadId(kValidIdDomain, 0); info->prompt_user_for_save_location = true; info->url_chain.push_back(GURL()); @@ -832,7 +839,7 @@ TEST_F(DownloadManagerTest, DownloadRemoveTest) { // This creates the .crdownload version of the file. download_file->Initialize(false); // |download_file| is owned by DownloadFileManager. - AddDownloadToFileManager(info->download_id, download_file); + AddDownloadToFileManager(info->download_id.local(), download_file); ContinueDownloadWithPath(download, new_path); message_loop_.RunAllPending(); diff --git a/chrome/browser/download/download_service.cc b/chrome/browser/download/download_service.cc index 6b5fcb2..c5263be 100644 --- a/chrome/browser/download/download_service.cc +++ b/chrome/browser/download/download_service.cc @@ -9,14 +9,26 @@ #include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_manager.h" DownloadService::DownloadService(Profile* profile) : download_manager_created_(false), - profile_(profile) {} + profile_(profile) { + if (profile_->IsOffTheRecord()) { + id_factory_ = DownloadServiceFactory::GetForProfile( + profile_->GetOriginalProfile())->GetDownloadIdFactory(); + } else { + id_factory_ = new DownloadIdFactory(this); + } +} DownloadService::~DownloadService() {} +DownloadIdFactory* DownloadService::GetDownloadIdFactory() const { + return id_factory_.get(); +} + DownloadManager* DownloadService::GetDownloadManager() { if (!download_manager_created_) { // In case the delegate has already been set by @@ -24,7 +36,9 @@ DownloadManager* DownloadService::GetDownloadManager() { if (!manager_delegate_.get()) manager_delegate_ = new ChromeDownloadManagerDelegate(profile_); manager_ = new DownloadManager( - manager_delegate_.get(), g_browser_process->download_status_updater()); + manager_delegate_.get(), + id_factory_.get(), + g_browser_process->download_status_updater()); manager_->Init(profile_); manager_delegate_->SetDownloadManager(manager_); download_manager_created_ = true; diff --git a/chrome/browser/download/download_service.h b/chrome/browser/download/download_service.h index 2cc6570..2317de1 100644 --- a/chrome/browser/download/download_service.h +++ b/chrome/browser/download/download_service.h @@ -14,6 +14,7 @@ class ChromeDownloadManagerDelegate; class DownloadManager; class Profile; +class DownloadIdFactory; // Owning class for DownloadManager (content) and // ChromeDownloadManagerDelegate (chrome) @@ -22,6 +23,8 @@ class DownloadService : public ProfileKeyedService { explicit DownloadService(Profile* profile); virtual ~DownloadService(); + DownloadIdFactory* GetDownloadIdFactory() const; + // Get the download manager. Creates the download manager if // it does not already exist. DownloadManager* GetDownloadManager(); @@ -46,6 +49,8 @@ class DownloadService : public ProfileKeyedService { virtual void Shutdown() OVERRIDE; private: + scoped_refptr<DownloadIdFactory> id_factory_; + bool download_manager_created_; Profile* profile_; diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc index 181e87f..985983d 100644 --- a/chrome/browser/profiles/profile_io_data.cc +++ b/chrome/browser/profiles/profile_io_data.cc @@ -18,6 +18,8 @@ #include "chrome/browser/content_settings/cookie_settings.h" #include "chrome/browser/content_settings/host_content_settings_map.h" #include "chrome/browser/custom_handlers/protocol_handler_registry.h" +#include "chrome/browser/download/download_service.h" +#include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/extensions/extension_info_map.h" #include "chrome/browser/extensions/extension_protocols.h" #include "chrome/browser/io_thread.h" @@ -42,6 +44,7 @@ #include "chrome/common/url_constants.h" #include "content/browser/appcache/chrome_appcache_service.h" #include "content/browser/chrome_blob_storage_context.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/host_zoom_map.h" #include "content/browser/renderer_host/media/media_stream_manager.h" #include "content/browser/renderer_host/resource_dispatcher_host.h" @@ -187,7 +190,8 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); PrefService* pref_service = profile->GetPrefs(); - next_download_id_thunk_ = profile->GetDownloadManager()->GetNextIdThunk(); + download_id_factory_ = DownloadServiceFactory::GetForProfile(profile)-> + GetDownloadIdFactory(); scoped_ptr<ProfileParams> params(new ProfileParams); params->path = profile->GetPath(); @@ -515,7 +519,7 @@ void ProfileIOData::LazyInitialize() const { resource_context_.SetUserData(NULL, const_cast<ProfileIOData*>(this)); resource_context_.set_media_observer( io_thread_globals->media.media_internals.get()); - resource_context_.set_next_download_id_thunk(next_download_id_thunk_); + resource_context_.set_download_id_factory(download_id_factory_); resource_context_.set_media_stream_manager(media_stream_manager_.get()); LazyInitializeInternal(profile_params_.get()); diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h index c5a0f59..f991ca7 100644 --- a/chrome/browser/profiles/profile_io_data.h +++ b/chrome/browser/profiles/profile_io_data.h @@ -7,6 +7,7 @@ #pragma once #include <set> + #include "base/basictypes.h" #include "base/callback.h" #include "base/file_path.h" @@ -16,7 +17,6 @@ #include "base/synchronization/lock.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/prefs/pref_member.h" -#include "content/browser/download/download_manager.h" #include "content/browser/resource_context.h" #include "net/base/cookie_monster.h" @@ -25,6 +25,7 @@ class ChromeAppCacheService; class ChromeBlobStorageContext; class CookieSettings; class DesktopNotificationService; +class DownloadIdFactory; class ExtensionInfoMap; class HostContentSettingsMap; class HostZoomMap; @@ -297,7 +298,7 @@ class ProfileIOData { mutable scoped_refptr<fileapi::FileSystemContext> file_system_context_; mutable scoped_refptr<quota::QuotaManager> quota_manager_; mutable scoped_refptr<HostZoomMap> host_zoom_map_; - mutable DownloadManager::GetNextIdThunkType next_download_id_thunk_; + mutable scoped_refptr<DownloadIdFactory> download_id_factory_; mutable scoped_ptr<media_stream::MediaStreamManager> media_stream_manager_; // TODO(willchan): Remove from ResourceContext. diff --git a/content/browser/download/download_create_info.cc b/content/browser/download/download_create_info.cc index 72bb323..afd95a7 100644 --- a/content/browser/download/download_create_info.cc +++ b/content/browser/download/download_create_info.cc @@ -15,7 +15,7 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path, int64 received_bytes, int64 total_bytes, int32 state, - int32 download_id, + const DownloadId& download_id, bool has_user_gesture, content::PageTransition transition_type) : path(path), @@ -37,7 +37,7 @@ DownloadCreateInfo::DownloadCreateInfo() received_bytes(0), total_bytes(0), state(-1), - download_id(-1), + download_id(DownloadId::Invalid()), has_user_gesture(false), transition_type(content::PAGE_TRANSITION_LINK), db_handle(0), @@ -49,14 +49,14 @@ DownloadCreateInfo::~DownloadCreateInfo() { std::string DownloadCreateInfo::DebugString() const { return base::StringPrintf("{" - " download_id = %d" + " download_id = %s" " url = \"%s\"" " path = \"%" PRFilePath "\"" " received_bytes = %" PRId64 " total_bytes = %" PRId64 " prompt_user_for_save_location = %c" " }", - download_id, + download_id.DebugString().c_str(), url().spec().c_str(), path.value().c_str(), received_bytes, diff --git a/content/browser/download/download_create_info.h b/content/browser/download/download_create_info.h index 1608891..ed05294 100644 --- a/content/browser/download/download_create_info.h +++ b/content/browser/download/download_create_info.h @@ -13,6 +13,7 @@ #include "base/file_path.h" #include "base/time.h" #include "content/browser/download/download_file.h" +#include "content/browser/download/download_id.h" #include "content/common/content_export.h" #include "content/public/common/page_transition_types.h" #include "googleurl/src/gurl.h" @@ -26,7 +27,7 @@ struct CONTENT_EXPORT DownloadCreateInfo { int64 received_bytes, int64 total_bytes, int32 state, - int32 download_id, + const DownloadId& download_id, bool has_user_gesture, content::PageTransition transition_type); DownloadCreateInfo(); @@ -65,7 +66,7 @@ struct CONTENT_EXPORT DownloadCreateInfo { int32 state; // The (per-session) ID of the download. - int32 download_id; + DownloadId download_id; // True if the download was initiated by user action. bool has_user_gesture; diff --git a/content/browser/download/download_file.cc b/content/browser/download/download_file.cc index a2bfb34..06f643e 100644 --- a/content/browser/download/download_file.cc +++ b/content/browser/download/download_file.cc @@ -57,7 +57,7 @@ std::string DownloadFile::DebugString() const { " request_handle = %s" " Base File = %s" " }", - id_, + id_.local(), request_handle_.DebugString().c_str(), BaseFile::DebugString().c_str()); } diff --git a/content/browser/download/download_file.h b/content/browser/download/download_file.h index 3424054..d7795c716 100644 --- a/content/browser/download/download_file.h +++ b/content/browser/download/download_file.h @@ -11,6 +11,7 @@ #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "content/browser/download/base_file.h" +#include "content/browser/download/download_id.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_types.h" #include "content/common/content_export.h" @@ -33,8 +34,9 @@ class CONTENT_EXPORT DownloadFile : public BaseFile { // Cancels the download request associated with this file. void CancelDownloadRequest(); - int id() const { return id_; } + int id() const { return id_.local(); } DownloadManager* GetDownloadManager(); + const DownloadId& global_id() const { return id_; } virtual std::string DebugString() const; @@ -61,7 +63,7 @@ class CONTENT_EXPORT DownloadFile : public BaseFile { private: // The unique identifier for this download, assigned at creation by // the DownloadFileManager for its internal record keeping. - int id_; + DownloadId id_; // The handle to the request information. Used for operations outside the // download system, specifically canceling a download. diff --git a/content/browser/download/download_file_manager.cc b/content/browser/download/download_file_manager.cc index 80e6f17..829cab9 100644 --- a/content/browser/download/download_file_manager.cc +++ b/content/browser/download/download_file_manager.cc @@ -70,9 +70,8 @@ void DownloadFileManager::CreateDownloadFile( return; } - DownloadId global_id(download_manager, info->download_id); - DCHECK(GetDownloadFile(global_id) == NULL); - downloads_[global_id] = download_file.release(); + DCHECK(GetDownloadFile(info->download_id) == NULL); + downloads_[info->download_id] = download_file.release(); // The file is now ready, we can un-pause the request and start saving data. request_handle.ResumeRequest(); @@ -82,7 +81,7 @@ void DownloadFileManager::CreateDownloadFile( BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, base::Bind(&DownloadManager::StartDownload, download_manager, - info->download_id)); + info->download_id.local())); } DownloadFile* DownloadFileManager::GetDownloadFile(DownloadId global_id) { @@ -280,7 +279,7 @@ void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) { for (std::set<DownloadFile*>::iterator i = to_remove.begin(); i != to_remove.end(); ++i) { - downloads_.erase(DownloadId((*i)->GetDownloadManager(), (*i)->id())); + downloads_.erase((*i)->global_id()); delete *i; } } diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc index 44fb740..4d42171 100644 --- a/content/browser/download/download_file_unittest.cc +++ b/content/browser/download/download_file_unittest.cc @@ -7,6 +7,8 @@ #include "base/string_number_conversions.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file.h" +#include "content/browser/download/download_id.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_manager.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_status_updater.h" @@ -17,6 +19,8 @@ #include "net/base/net_errors.h" #include "testing/gtest/include/gtest/gtest.h" +DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain"; + class DownloadFileTest : public testing::Test { public: @@ -33,6 +37,7 @@ class DownloadFileTest : public testing::Test { // calling Release() on |download_manager_| won't ever result in its // destructor being called and we get a leak. DownloadFileTest() : + id_factory_(new DownloadIdFactory(kValidIdDomain)), ui_thread_(BrowserThread::UI, &loop_), file_thread_(BrowserThread::FILE, &loop_) { } @@ -43,7 +48,9 @@ class DownloadFileTest : public testing::Test { virtual void SetUp() { download_manager_delegate_.reset(new MockDownloadManagerDelegate()); download_manager_ = new MockDownloadManager( - download_manager_delegate_.get(), &download_status_updater_); + download_manager_delegate_.get(), + id_factory_, + &download_status_updater_); } virtual void TearDown() { @@ -58,7 +65,7 @@ class DownloadFileTest : public testing::Test { virtual void CreateDownloadFile(scoped_ptr<DownloadFile>* file, int offset) { DownloadCreateInfo info; - info.download_id = kDummyDownloadId + offset; + info.download_id = DownloadId(kValidIdDomain, kDummyDownloadId + offset); // info.request_handle default constructed to null. info.save_info.file_stream = file_stream_; file->reset( @@ -104,6 +111,7 @@ class DownloadFileTest : public testing::Test { private: MessageLoop loop_; + scoped_refptr<DownloadIdFactory> id_factory_; // UI thread. content::TestBrowserThread ui_thread_; // File thread to satisfy debug checks in DownloadFile. diff --git a/content/browser/download/download_id.cc b/content/browser/download/download_id.cc index bef4964..8cd7b0e 100644 --- a/content/browser/download/download_id.cc +++ b/content/browser/download/download_id.cc @@ -2,8 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <string> + +#include "base/stringprintf.h" #include "content/browser/download/download_id.h" +std::string DownloadId::DebugString() const { + return base::StringPrintf("%p:%d", domain_, local_id_); +} + std::ostream& operator<<(std::ostream& out, const DownloadId& global_id) { - return out << global_id.manager_ << ":" << global_id.local(); + return out << global_id.DebugString(); } diff --git a/content/browser/download/download_id.h b/content/browser/download/download_id.h index 187a79c..d9726bf 100644 --- a/content/browser/download/download_id.h +++ b/content/browser/download/download_id.h @@ -2,17 +2,16 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_ -#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_ +#ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_ +#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_ #pragma once #include <iosfwd> +#include <string> #include "base/hash_tables.h" #include "content/common/content_export.h" -class DownloadManager; - // DownloadId combines per-profile Download ids with an indication of which // profile in order to be globally unique. DownloadIds are not persistent across // sessions, but their local() field is. @@ -20,8 +19,11 @@ class DownloadId { public: static DownloadId Invalid() { return DownloadId(NULL, -1); } - DownloadId(const DownloadManager* manager, int32 local_id) - : manager_(manager), + // Domain separates spaces of local ids. + typedef const void* Domain; + + DownloadId(Domain domain, int32 local_id) + : domain_(domain), local_id_(local_id) { } @@ -30,43 +32,40 @@ class DownloadId { // Returns true if this DownloadId has been allocated and could possibly refer // to a DownloadItem that exists. - bool IsValid() const { return ((manager_ != NULL) && (local_id_ >= 0)); } + bool IsValid() const { return ((domain_ != NULL) && (local_id_ >= 0)); } // The following methods (operator==, hash(), copy, and assign) provide // support for STL containers such as hash_map. bool operator==(const DownloadId& that) const { return ((that.local_id_ == local_id_) && - (that.manager_ == manager_)); + (that.domain_ == domain_)); } bool operator<(const DownloadId& that) const { - // Even though DownloadManager* < DownloadManager* is not well defined and + // Even though Domain::operator< is not well defined and // GCC does not require it for hash_map, MSVC requires operator< for // hash_map. We don't ifdef it out here because we will probably make a // set<DownloadId> at some point, when GCC will require it. - return ((manager_ < that.manager_) || - ((manager_ == that.manager_) && (local_id_ < that.local_id_))); + return ((domain_ < that.domain_) || + ((domain_ == that.domain_) && (local_id_ < that.local_id_))); } size_t hash() const { - // The top half of manager is unlikely to be distinct, and the user is + // The top half of domain_ is unlikely to be distinct, and the user is // unlikely to have >64K downloads. If these assumptions are incorrect, then // DownloadFileManager's hash_map might have a few collisions, but it will // use operator== to safely disambiguate. - return reinterpret_cast<size_t>(manager_) + + return reinterpret_cast<size_t>(domain_) + (static_cast<size_t>(local_id_) << (4 * sizeof(size_t))); } + std::string DebugString() const; + private: - // DownloadId is used mostly off the UI thread, so manager's methods can't be - // called, but the pointer can be compared. - const DownloadManager* manager_; + Domain domain_; int32 local_id_; - friend CONTENT_EXPORT std::ostream& operator<<(std::ostream& out, - const DownloadId& global_id); - // Allow copy and assign. }; @@ -88,4 +87,4 @@ inline size_t hash_value(const DownloadId& id) { } #endif // COMPILER } -#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_ +#endif // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ID_H_ diff --git a/content/browser/download/download_id_factory.cc b/content/browser/download/download_id_factory.cc new file mode 100644 index 0000000..6b0797f --- /dev/null +++ b/content/browser/download/download_id_factory.cc @@ -0,0 +1,17 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/browser/download/download_id_factory.h" + +#include "content/browser/download/download_id.h" + +DownloadIdFactory::DownloadIdFactory(DownloadId::Domain domain) + : domain_(domain), + next_id_(0) { +} + +DownloadId DownloadIdFactory::GetNextId() { + base::AutoLock lock(lock_); + return DownloadId(domain_, next_id_++); +} diff --git a/content/browser/download/download_id_factory.h b/content/browser/download/download_id_factory.h new file mode 100644 index 0000000..9c1a7ed --- /dev/null +++ b/content/browser/download/download_id_factory.h @@ -0,0 +1,33 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ID_FACTORY_H_ +#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ID_FACTORY_H_ +#pragma once + +#include "base/memory/ref_counted.h" +#include "base/synchronization/lock.h" +#include "content/browser/download/download_id.h" +#include "content/public/browser/browser_thread.h" + +class DownloadManager; + +class CONTENT_EXPORT DownloadIdFactory + : public base::RefCountedThreadSafe<DownloadIdFactory> { + public: + // TODO(benjhayden): Instantiate with an explicit next id counter read from + // persistent storage. + DownloadIdFactory(DownloadId::Domain domain); + + DownloadId GetNextId(); + + private: + DownloadId::Domain domain_; + int next_id_; + base::Lock lock_; + + DISALLOW_COPY_AND_ASSIGN(DownloadIdFactory); +}; + +#endif // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ID_FACTORY_H_ diff --git a/content/browser/download/download_id_unittest.cc b/content/browser/download/download_id_unittest.cc index 2e09525..4caa9fe 100644 --- a/content/browser/download/download_id_unittest.cc +++ b/content/browser/download/download_id_unittest.cc @@ -26,7 +26,7 @@ class DownloadIdTest : public testing::Test { // Create the download managers. for (i = 0; i < num_managers_; ++i) { managers[i] = - new MockDownloadManager(download_manager_delegate_.get(), NULL); + new MockDownloadManager(download_manager_delegate_.get(), NULL, NULL); } // Sort by pointer value. std::sort(managers.begin(), managers.end()); diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc index bf64aae..0913569 100644 --- a/content/browser/download/download_item.cc +++ b/content/browser/download/download_item.cc @@ -124,7 +124,7 @@ const int DownloadItem::kUninitializedHandle = 0; // Constructor for reading from the history service. DownloadItem::DownloadItem(DownloadManager* download_manager, const DownloadPersistentStoreInfo& info) - : download_id_(-1), + : download_id_(download_manager->GetNextId()), full_path_(info.path), url_chain_(1, info.url), referrer_url_(info.referrer_url), @@ -201,7 +201,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, const GURL& url, bool is_otr, DownloadId download_id) - : download_id_(download_id.local()), + : download_id_(download_id), full_path_(path), url_chain_(1, url), referrer_url_(GURL()), @@ -235,10 +235,6 @@ DownloadItem::~DownloadItem() { download_manager_->AssertQueueStateConsistent(this); } -DownloadId DownloadItem::global_id() const { - return DownloadId(download_manager_, id()); -} - void DownloadItem::AddObserver(Observer* observer) { // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved. CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); @@ -777,7 +773,7 @@ std::string DownloadItem::DebugString(bool verbose) const { std::string description = base::StringPrintf("{ id = %d" " state = %s", - download_id_, + download_id_.local(), DebugDownloadStateString(state())); // Construct a string of the URL chain. diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h index 7a19ea0..9b7cb80 100644 --- a/content/browser/download/download_item.h +++ b/content/browser/download/download_item.h @@ -25,6 +25,7 @@ #include "base/observer_list.h" #include "base/time.h" #include "base/timer.h" +#include "content/browser/download/download_id.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_state_info.h" #include "content/browser/download/interrupt_reasons.h" @@ -277,8 +278,8 @@ class CONTENT_EXPORT DownloadItem { total_bytes_ = total_bytes; } int64 received_bytes() const { return received_bytes_; } - int32 id() const { return download_id_; } - DownloadId global_id() const; + int32 id() const { return download_id_.local(); } + DownloadId global_id() const { return download_id_; } base::Time start_time() const { return start_time_; } base::Time end_time() const { return end_time_; } void set_db_handle(int64 handle) { db_handle_ = handle; } @@ -379,7 +380,7 @@ class CONTENT_EXPORT DownloadItem { DownloadRequestHandle request_handle_; // Download ID assigned by DownloadResourceHandler. - int32 download_id_; + DownloadId download_id_; // Full path to the downloaded or downloading file. FilePath full_path_; diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc index b48da24..5cdf006 100644 --- a/content/browser/download/download_manager.cc +++ b/content/browser/download/download_manager.cc @@ -19,6 +19,7 @@ #include "content/browser/browser_context.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file_manager.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_item.h" #include "content/browser/download/download_persistent_store_info.h" #include "content/browser/download/download_stats.h" @@ -70,15 +71,16 @@ void BeginDownload(const URLParams& url_params, } // namespace DownloadManager::DownloadManager(content::DownloadManagerDelegate* delegate, + DownloadIdFactory* id_factory, DownloadStatusUpdater* status_updater) : shutdown_needed_(false), browser_context_(NULL), - next_id_(0), file_manager_(NULL), status_updater_((status_updater != NULL) ? status_updater->AsWeakPtr() : base::WeakPtr<DownloadStatusUpdater>()), delegate_(delegate), + id_factory_(id_factory), largest_db_handle_in_history_(DownloadItem::kUninitializedHandle) { // NOTE(benjhayden): status_updater may be NULL when using // TestingBrowserProcess. @@ -92,6 +94,10 @@ DownloadManager::~DownloadManager() { status_updater_->RemoveDelegate(this); } +DownloadId DownloadManager::GetNextId() { + return id_factory_->GetNextId(); +} + void DownloadManager::Shutdown() { VLOG(20) << __FUNCTION__ << "()" << " shutdown_needed_ = " << shutdown_needed_; @@ -205,30 +211,6 @@ void DownloadManager::SearchDownloads(const string16& query, } } -void DownloadManager::OnPersistentStoreGetNextId(int next_id) { - DVLOG(1) << __FUNCTION__ << " " << next_id; - base::AutoLock lock(next_id_lock_); - // TODO(benjhayden) Delay Profile initialization until here, and set next_id_ - // = next_id. The '+=' works for now because these ids are not yet persisted - // to the database. GetNextId() can allocate zero or more ids starting from 0, - // then this callback can increment next_id_, and the items with lower ids - // won't clash with any other items even though there may be items loaded from - // the history because items from the history don't have valid ids. - next_id_ += next_id; -} - -DownloadId DownloadManager::GetNextId() { - // May be called on any thread via the GetNextIdThunk. - // TODO(benjhayden) If otr, forward to parent DM. - base::AutoLock lock(next_id_lock_); - return DownloadId(this, next_id_++); -} - -DownloadManager::GetNextIdThunkType DownloadManager::GetNextIdThunk() { - // TODO(benjhayden) If otr, forward to parent DM. - return base::Bind(&DownloadManager::GetNextId, this); -} - // Query the history service for information about all persisted downloads. bool DownloadManager::Init(content::BrowserContext* browser_context) { DCHECK(browser_context); @@ -338,7 +320,7 @@ void DownloadManager::CreateDownloadItem( DownloadItem* download = new DownloadItem(this, *info, request_handle, browser_context_->IsOffTheRecord()); - int32 download_id = info->download_id; + int32 download_id = info->download_id.local(); DCHECK(!ContainsKey(in_progress_, download_id)); // TODO(rdsmith): Remove after http://crbug.com/85408 resolved. diff --git a/content/browser/download/download_manager.h b/content/browser/download/download_manager.h index 9255058..44157e6 100644 --- a/content/browser/download/download_manager.h +++ b/content/browser/download/download_manager.h @@ -44,6 +44,7 @@ #include "base/observer_list.h" #include "base/synchronization/lock.h" #include "base/time.h" +#include "content/browser/download/download_id.h" #include "content/browser/download/download_item.h" #include "content/browser/download/download_status_updater_delegate.h" #include "content/browser/download/interrupt_reasons.h" @@ -52,6 +53,7 @@ #include "net/base/net_errors.h" class DownloadFileManager; +class DownloadIdFactory; class DownloadRequestHandle; class DownloadStatusUpdater; class GURL; @@ -72,6 +74,7 @@ class CONTENT_EXPORT DownloadManager public DownloadStatusUpdaterDelegate { public: DownloadManager(content::DownloadManagerDelegate* delegate, + DownloadIdFactory* id_factory, DownloadStatusUpdater* status_updater); // Shutdown the download manager. Must be called before destruction. @@ -111,24 +114,6 @@ class CONTENT_EXPORT DownloadManager // everything. void SearchDownloads(const string16& query, DownloadVector* result); - // Returns the next download id in a DownloadId and increments the counter. - // May be called on any thread. The incremented counter is not persisted, but - // the base counter for this accessor is initialized from the largest id - // actually saved to the download history database. - DownloadId GetNextId(); - - // Instead of passing a DownloadManager* between threads and hoping users only - // call GetNextId(), you can pass this thunk around instead. Pass the thunk - // around by const ref and store it by copy per the base::Callback interface. - // The thunk may be copied, including between threads. If you change - // GetNextIdThunkType from base::Callback, then you should think about how - // you're changing the ref-count of DownloadManager. Use it like: - // const DownloadManager::GetNextIdThunkType& next_id_thunk = - // download_manager->GetNextIdThunk(); - // id = next_id_thunk.Run(); - typedef base::Callback<DownloadId(void)> GetNextIdThunkType; - GetNextIdThunkType GetNextIdThunk(); - // Returns true if initialized properly. bool Init(content::BrowserContext* browser_context); @@ -219,11 +204,6 @@ class CONTENT_EXPORT DownloadManager // Remove a download observer from ourself. void RemoveObserver(Observer* observer); - // Called by the embedder after creating the download manager to inform it of - // the next available download id. - // TODO(benjhayden): Separate this functionality out into a separate object. - void OnPersistentStoreGetNextId(int next_id); - // Called by the embedder, after creating the download manager, to let it know // about downloads from previous runs of the browser. void OnPersistentStoreQueryComplete( @@ -304,6 +284,8 @@ class CONTENT_EXPORT DownloadManager // other for testing only methods). void SetDownloadManagerDelegate(content::DownloadManagerDelegate* delegate); + DownloadId GetNextId(); + private: typedef std::set<DownloadItem*> DownloadSet; typedef base::hash_map<int64, DownloadItem*> DownloadMap; @@ -418,9 +400,6 @@ class CONTENT_EXPORT DownloadManager // The current active browser context. content::BrowserContext* browser_context_; - base::Lock next_id_lock_; - int next_id_; - // Non-owning pointer for handling file writing on the download_thread_. DownloadFileManager* file_manager_; @@ -434,6 +413,8 @@ class CONTENT_EXPORT DownloadManager // Allows an embedder to control behavior. Guaranteed to outlive this object. content::DownloadManagerDelegate* delegate_; + DownloadIdFactory* id_factory_; + // TODO(rdsmith): Remove when http://crbug.com/85408 is fixed. // For debugging only. int64 largest_db_handle_in_history_; diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc index f1783e4..ab22e56 100644 --- a/content/browser/download/download_resource_handler.cc +++ b/content/browser/download/download_resource_handler.cc @@ -89,7 +89,7 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id, // Deleted in DownloadManager. DownloadCreateInfo* info = new DownloadCreateInfo(FilePath(), GURL(), base::Time::Now(), 0, content_length_, DownloadItem::IN_PROGRESS, - download_id_.local(), request_info->has_user_gesture(), + download_id_, request_info->has_user_gesture(), request_info->transition_type()); info->url_chain = request_->url_chain(); info->referrer_url = GURL(request_->referrer()); @@ -97,7 +97,7 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id, info->received_bytes = 0; info->total_bytes = content_length_; info->state = DownloadItem::IN_PROGRESS; - info->download_id = download_id_.local(); + info->download_id = download_id_; info->has_user_gesture = request_info->has_user_gesture(); info->content_disposition = content_disposition_; info->mime_type = response->response_head.mime_type; diff --git a/content/browser/download/mock_download_manager.h b/content/browser/download/mock_download_manager.h index b9d1e30b..98f8480 100644 --- a/content/browser/download/mock_download_manager.h +++ b/content/browser/download/mock_download_manager.h @@ -14,8 +14,9 @@ class DownloadItem; class MockDownloadManager : public DownloadManager { public: explicit MockDownloadManager(content::DownloadManagerDelegate* delegate, + DownloadIdFactory* id_factory, DownloadStatusUpdater* updater) - : DownloadManager(delegate, updater) { + : DownloadManager(delegate, id_factory, updater) { } // Override some functions. diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc index f510219..03a8614 100644 --- a/content/browser/download/save_package.cc +++ b/content/browser/download/save_package.cc @@ -29,6 +29,7 @@ #include "content/browser/renderer_host/render_view_host.h" #include "content/browser/renderer_host/render_view_host_delegate.h" #include "content/browser/renderer_host/resource_dispatcher_host.h" +#include "content/browser/resource_context.h" #include "content/browser/tab_contents/tab_contents.h" #include "content/common/view_messages.h" #include "content/public/browser/browser_thread.h" diff --git a/content/browser/renderer_host/buffered_resource_handler.cc b/content/browser/renderer_host/buffered_resource_handler.cc index b4363d92..f639a03 100644 --- a/content/browser/renderer_host/buffered_resource_handler.cc +++ b/content/browser/renderer_host/buffered_resource_handler.cc @@ -10,6 +10,7 @@ #include "base/logging.h" #include "base/metrics/histogram.h" #include "base/string_util.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_resource_handler.h" #include "content/browser/plugin_service.h" #include "content/browser/renderer_host/resource_dispatcher_host.h" @@ -317,7 +318,7 @@ bool BufferedResourceHandler::CompleteResponseStarted(int request_id, info->set_is_download(true); - DownloadId dl_id = info->context()->next_download_id_thunk().Run(); + DownloadId dl_id = info->context()->download_id_factory()->GetNextId(); scoped_refptr<ResourceHandler> handler( new DownloadResourceHandler(host_, diff --git a/content/browser/renderer_host/resource_dispatcher_host.cc b/content/browser/renderer_host/resource_dispatcher_host.cc index 4a6a057..472c3fb 100644 --- a/content/browser/renderer_host/resource_dispatcher_host.cc +++ b/content/browser/renderer_host/resource_dispatcher_host.cc @@ -26,6 +26,7 @@ #include "content/browser/chrome_blob_storage_context.h" #include "content/browser/cross_site_request_manager.h" #include "content/browser/download/download_file_manager.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_manager.h" #include "content/browser/download/download_resource_handler.h" #include "content/browser/download/save_file_manager.h" @@ -50,10 +51,10 @@ #include "content/browser/ssl/ssl_client_auth_handler.h" #include "content/browser/ssl/ssl_manager.h" #include "content/browser/worker_host/worker_service.h" -#include "content/public/browser/notification_service.h" #include "content/common/resource_messages.h" #include "content/common/view_messages.h" #include "content/public/browser/content_browser_client.h" +#include "content/public/browser/notification_service.h" #include "content/public/browser/resource_dispatcher_host_delegate.h" #include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" @@ -830,7 +831,7 @@ void ResourceDispatcherHost::BeginDownload( request_id_--; - DownloadId dl_id = context.next_download_id_thunk().Run(); + DownloadId dl_id = context.download_id_factory()->GetNextId(); scoped_refptr<ResourceHandler> handler( new DownloadResourceHandler(this, diff --git a/content/browser/renderer_host/resource_dispatcher_host_unittest.cc b/content/browser/renderer_host/resource_dispatcher_host_unittest.cc index 919674f..12f335b 100644 --- a/content/browser/renderer_host/resource_dispatcher_host_unittest.cc +++ b/content/browser/renderer_host/resource_dispatcher_host_unittest.cc @@ -12,6 +12,7 @@ #include "base/process_util.h" #include "content/browser/child_process_security_policy.h" #include "content/browser/download/download_id.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/mock_resource_context.h" #include "content/browser/renderer_host/dummy_resource_handler.h" #include "content/browser/renderer_host/global_request_id.h" @@ -1161,8 +1162,10 @@ TEST_F(ResourceDispatcherHostTest, IgnoreCancelForDownloads) { HandleScheme("http"); MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah")); - content::MockResourceContext::GetInstance()->set_next_download_id_thunk( - base::Bind(&MockNextDownloadId)); + scoped_refptr<DownloadIdFactory> id_factory( + new DownloadIdFactory("valid DownloadId::Domain")); + content::MockResourceContext::GetInstance()->set_download_id_factory( + id_factory); // Return some data so that the request is identified as a download // and the proper resource handlers are created. EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage()); @@ -1198,8 +1201,10 @@ TEST_F(ResourceDispatcherHostTest, CancelRequestsForContext) { HandleScheme("http"); MakeTestRequest(render_view_id, request_id, GURL("http://example.com/blah")); - content::MockResourceContext::GetInstance()->set_next_download_id_thunk( - base::Bind(&MockNextDownloadId)); + scoped_refptr<DownloadIdFactory> id_factory( + new DownloadIdFactory("valid DownloadId::Domain")); + content::MockResourceContext::GetInstance()->set_download_id_factory( + id_factory); // Return some data so that the request is identified as a download // and the proper resource handlers are created. EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage()); diff --git a/content/browser/resource_context.cc b/content/browser/resource_context.cc index d59f6b1..d20b1e1 100644 --- a/content/browser/resource_context.cc +++ b/content/browser/resource_context.cc @@ -153,16 +153,15 @@ void ResourceContext::set_media_observer(MediaObserver* media_observer) { media_observer_ = media_observer; } -const DownloadManager::GetNextIdThunkType& -ResourceContext::next_download_id_thunk() const { +DownloadIdFactory* ResourceContext::download_id_factory() const { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); EnsureInitialized(); - return next_download_id_thunk_; + return download_id_factory_; } -void ResourceContext::set_next_download_id_thunk( - const DownloadManager::GetNextIdThunkType& thunk) { +void ResourceContext::set_download_id_factory( + DownloadIdFactory* download_id_factory) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - next_download_id_thunk_ = thunk; + download_id_factory_ = download_id_factory; } media_stream::MediaStreamManager* diff --git a/content/browser/resource_context.h b/content/browser/resource_context.h index 0e04d07..3879390 100644 --- a/content/browser/resource_context.h +++ b/content/browser/resource_context.h @@ -11,10 +11,10 @@ #include "base/callback.h" #include "base/memory/ref_counted.h" #include "content/common/content_export.h" -#include "content/browser/download/download_manager.h" class ChromeAppCacheService; class ChromeBlobStorageContext; +class DownloadIdFactory; class ExtensionInfoMap; class HostZoomMap; class MediaObserver; @@ -77,10 +77,8 @@ class CONTENT_EXPORT ResourceContext { MediaObserver* media_observer() const; void set_media_observer(MediaObserver* media_observer); - // TODO(benjhayden): Promote GetNextIdThunkType to a separate object. - const DownloadManager::GetNextIdThunkType& next_download_id_thunk() const; - void set_next_download_id_thunk( - const DownloadManager::GetNextIdThunkType& thunk); + DownloadIdFactory* download_id_factory() const; + void set_download_id_factory(DownloadIdFactory* download_id_factory); media_stream::MediaStreamManager* media_stream_manager() const; void set_media_stream_manager( @@ -101,7 +99,7 @@ class CONTENT_EXPORT ResourceContext { quota::QuotaManager* quota_manager_; HostZoomMap* host_zoom_map_; MediaObserver* media_observer_; - DownloadManager::GetNextIdThunkType next_download_id_thunk_; + DownloadIdFactory* download_id_factory_; media_stream::MediaStreamManager* media_stream_manager_; // Externally-defined data accessible by key. diff --git a/content/content_browser.gypi b/content/content_browser.gypi index f6667c3..9a90f2e 100644 --- a/content/content_browser.gypi +++ b/content/content_browser.gypi @@ -119,6 +119,8 @@ 'browser/download/download_file_manager.h', 'browser/download/download_id.cc', 'browser/download/download_id.h', + 'browser/download/download_id_factory.cc', + 'browser/download/download_id_factory.h', 'browser/download/download_item.cc', 'browser/download/download_item.h', 'browser/download/download_manager.cc', diff --git a/content/shell/shell_browser_context.cc b/content/shell/shell_browser_context.cc index 94ab2f1..78bef35 100644 --- a/content/shell/shell_browser_context.cc +++ b/content/shell/shell_browser_context.cc @@ -9,6 +9,7 @@ #include "base/path_service.h" #include "content/browser/appcache/chrome_appcache_service.h" #include "content/browser/chrome_blob_storage_context.h" +#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_manager.h" #include "content/browser/download/download_status_updater.h" #include "content/browser/file_system/browser_file_system_helper.h" @@ -80,7 +81,8 @@ namespace content { ShellBrowserContext::ShellBrowserContext( ShellBrowserMainParts* shell_main_parts) - : shell_main_parts_(shell_main_parts) { + : download_id_factory_(new DownloadIdFactory(this)), + shell_main_parts_(shell_main_parts) { } ShellBrowserContext::~ShellBrowserContext() { @@ -123,6 +125,7 @@ DownloadManager* ShellBrowserContext::GetDownloadManager() { download_manager_delegate_ = new ShellDownloadManagerDelegate(); download_manager_ = new DownloadManager(download_manager_delegate_, + download_id_factory_, download_status_updater_.get()); download_manager_delegate_->SetDownloadManager(download_manager_.get()); download_manager_->Init(this); @@ -156,7 +159,7 @@ const ResourceContext& ShellBrowserContext::GetResourceContext() { resource_context_.reset(new ShellResourceContext( static_cast<ShellURLRequestContextGetter*>(GetRequestContext()), GetBlobStorageContext(), - GetDownloadManager()->GetNextIdThunk())); + download_id_factory_)); } return *resource_context_.get(); } diff --git a/content/shell/shell_browser_context.h b/content/shell/shell_browser_context.h index c6471ee..55ee135 100644 --- a/content/shell/shell_browser_context.h +++ b/content/shell/shell_browser_context.h @@ -12,6 +12,7 @@ #include "base/memory/scoped_ptr.h" #include "content/browser/browser_context.h" +class DownloadIdFactory; class DownloadManager; class DownloadStatusUpdater; class GeolocationPermissionContext; @@ -60,6 +61,7 @@ class ShellBrowserContext : public BrowserContext { scoped_ptr<SSLHostState> ssl_host_state_; scoped_ptr<DownloadStatusUpdater> download_status_updater_; scoped_refptr<ShellDownloadManagerDelegate> download_manager_delegate_; + scoped_refptr<DownloadIdFactory> download_id_factory_; scoped_refptr<DownloadManager> download_manager_; scoped_refptr<net::URLRequestContextGetter> url_request_getter_; scoped_refptr<HostZoomMap> host_zoom_map_; diff --git a/content/shell/shell_resource_context.cc b/content/shell/shell_resource_context.cc index 5058992..21a696f 100644 --- a/content/shell/shell_resource_context.cc +++ b/content/shell/shell_resource_context.cc @@ -5,6 +5,7 @@ #include "content/shell/shell_resource_context.h" #include "content/browser/chrome_blob_storage_context.h" +#include "content/browser/download/download_id_factory.h" #include "content/shell/shell_url_request_context_getter.h" namespace content { @@ -12,10 +13,10 @@ namespace content { ShellResourceContext::ShellResourceContext( ShellURLRequestContextGetter* getter, ChromeBlobStorageContext* blob_storage_context, - DownloadManager::GetNextIdThunkType next_download_id_thunk) + DownloadIdFactory* download_id_factory) : getter_(getter), blob_storage_context_(blob_storage_context), - next_download_id_thunk_(next_download_id_thunk) { + download_id_factory_(download_id_factory) { } ShellResourceContext::~ShellResourceContext() { @@ -29,7 +30,7 @@ void ShellResourceContext::InitializeInternal() { set_request_context(getter_->GetURLRequestContext()); set_host_resolver(getter_->host_resolver()); set_blob_storage_context(blob_storage_context_); - set_next_download_id_thunk(next_download_id_thunk_); + set_download_id_factory(download_id_factory_); } } // namespace content diff --git a/content/shell/shell_resource_context.h b/content/shell/shell_resource_context.h index bafb05b..710778e 100644 --- a/content/shell/shell_resource_context.h +++ b/content/shell/shell_resource_context.h @@ -8,10 +8,10 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" -#include "content/browser/download/download_manager.h" #include "content/browser/resource_context.h" class ChromeBlobStorageContext; +class DownloadIdFactory; namespace content { @@ -22,7 +22,7 @@ class ShellResourceContext : public content::ResourceContext { ShellResourceContext( ShellURLRequestContextGetter* getter, ChromeBlobStorageContext* blob_storage_context, - DownloadManager::GetNextIdThunkType next_download_id_thunk); + DownloadIdFactory* download_id_factory); virtual ~ShellResourceContext(); private: @@ -32,7 +32,7 @@ class ShellResourceContext : public content::ResourceContext { scoped_refptr<ShellURLRequestContextGetter> getter_; scoped_refptr<ChromeBlobStorageContext> blob_storage_context_; - DownloadManager::GetNextIdThunkType next_download_id_thunk_; + scoped_refptr<DownloadIdFactory> download_id_factory_; DISALLOW_COPY_AND_ASSIGN(ShellResourceContext); }; |