diff options
Diffstat (limited to 'content/browser/download')
23 files changed, 129 insertions, 241 deletions
diff --git a/content/browser/download/download_create_info.cc b/content/browser/download/download_create_info.cc index fbcd175..08999f7 100644 --- a/content/browser/download/download_create_info.cc +++ b/content/browser/download/download_create_info.cc @@ -9,22 +9,24 @@ #include "base/format_macros.h" #include "base/stringprintf.h" -DownloadCreateInfo::DownloadCreateInfo(const FilePath& path, - const GURL& url, - const base::Time& start_time, - int64 received_bytes, - int64 total_bytes, - int32 state, - const DownloadId& download_id, - bool has_user_gesture, - content::PageTransition transition_type) +using content::DownloadId; + +DownloadCreateInfo::DownloadCreateInfo( + const FilePath& path, + const GURL& url, + const base::Time& start_time, + int64 received_bytes, + int64 total_bytes, + int32 state, + bool has_user_gesture, + content::PageTransition transition_type) : path(path), url_chain(1, url), start_time(start_time), received_bytes(received_bytes), total_bytes(total_bytes), state(state), - download_id(download_id), + download_id(DownloadId::Invalid()), has_user_gesture(has_user_gesture), transition_type(transition_type), db_handle(0), diff --git a/content/browser/download/download_create_info.h b/content/browser/download/download_create_info.h index 9be00c1..dbfee7f 100644 --- a/content/browser/download/download_create_info.h +++ b/content/browser/download/download_create_info.h @@ -12,10 +12,10 @@ #include "base/basictypes.h" #include "base/file_path.h" #include "base/time.h" -#include "content/browser/download/download_id.h" #include "content/browser/download/download_types.h" #include "content/common/content_export.h" #include "content/public/browser/download_file.h" +#include "content/public/browser/download_id.h" #include "content/public/common/page_transition_types.h" #include "googleurl/src/gurl.h" @@ -28,7 +28,6 @@ struct CONTENT_EXPORT DownloadCreateInfo { int64 received_bytes, int64 total_bytes, int32 state, - const DownloadId& download_id, bool has_user_gesture, content::PageTransition transition_type); DownloadCreateInfo(); @@ -63,7 +62,7 @@ struct CONTENT_EXPORT DownloadCreateInfo { int32 state; // The (per-session) ID of the download. - DownloadId download_id; + content::DownloadId download_id; // True if the download was initiated by user action. bool has_user_gesture; diff --git a/content/browser/download/download_file_impl.cc b/content/browser/download/download_file_impl.cc index 4cff9e8..408081a 100644 --- a/content/browser/download/download_file_impl.cc +++ b/content/browser/download/download_file_impl.cc @@ -13,6 +13,7 @@ #include "content/public/browser/download_manager.h" using content::BrowserThread; +using content::DownloadId; namespace { diff --git a/content/browser/download/download_file_impl.h b/content/browser/download/download_file_impl.h index 69a7c5e..0521435 100644 --- a/content/browser/download/download_file_impl.h +++ b/content/browser/download/download_file_impl.h @@ -49,7 +49,7 @@ class CONTENT_EXPORT DownloadFileImpl : virtual public content::DownloadFile { virtual void CancelDownloadRequest() OVERRIDE; virtual int Id() const OVERRIDE; virtual content::DownloadManager* GetDownloadManager() OVERRIDE; - virtual const DownloadId& GlobalId() const OVERRIDE; + virtual const content::DownloadId& GlobalId() const OVERRIDE; virtual std::string DebugString() const OVERRIDE; private: @@ -58,7 +58,7 @@ class CONTENT_EXPORT DownloadFileImpl : virtual public content::DownloadFile { // The unique identifier for this download, assigned at creation by // the DownloadFileManager for its internal record keeping. - DownloadId id_; + content::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 4a1f771..a58cb78 100644 --- a/content/browser/download/download_file_manager.cc +++ b/content/browser/download/download_file_manager.cc @@ -29,6 +29,7 @@ using content::BrowserThread; using content::DownloadFile; +using content::DownloadId; namespace { diff --git a/content/browser/download/download_file_manager.h b/content/browser/download/download_file_manager.h index 4688835..1a3054ef 100644 --- a/content/browser/download/download_file_manager.h +++ b/content/browser/download/download_file_manager.h @@ -49,9 +49,9 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/timer.h" -#include "content/browser/download/download_id.h" #include "content/browser/download/interrupt_reasons.h" #include "content/common/content_export.h" +#include "content/public/browser/download_id.h" #include "net/base/net_errors.h" #include "ui/gfx/native_widget_types.h" @@ -96,14 +96,15 @@ class CONTENT_EXPORT DownloadFileManager // Handlers for notifications sent from the IO thread and run on the // FILE thread. - void UpdateDownload(DownloadId global_id, content::DownloadBuffer* buffer); + void UpdateDownload(content::DownloadId global_id, + content::DownloadBuffer* buffer); // |reason| is the reason for interruption, if one occurs. // |security_info| contains SSL information (cert_id, cert_status, // security_bits, ssl_connection_status), which can be used to // fine-tune the error message. It is empty if the transaction // was not performed securely. - void OnResponseCompleted(DownloadId global_id, + void OnResponseCompleted(content::DownloadId global_id, InterruptReason reason, const std::string& security_info); @@ -112,22 +113,23 @@ class CONTENT_EXPORT DownloadFileManager // download file, as far as the DownloadFileManager is concerned -- if // anything happens to the download file after they are called, it will // be ignored. - void CancelDownload(DownloadId id); - void CompleteDownload(DownloadId id); + void CancelDownload(content::DownloadId id); + void CompleteDownload(content::DownloadId id); // Called on FILE thread by DownloadManager at the beginning of its shutdown. void OnDownloadManagerShutdown(content::DownloadManager* manager); // The DownloadManager in the UI thread has provided an intermediate // .crdownload name for the download specified by |id|. - void RenameInProgressDownloadFile(DownloadId id, const FilePath& full_path); + void RenameInProgressDownloadFile(content::DownloadId id, + const FilePath& full_path); // The DownloadManager in the UI thread has provided a final name for the // download specified by |id|. // |overwrite_existing_file| prevents uniquification, and is used for SAFE // downloads, as the user may have decided to overwrite the file. // Sent from the UI thread and run on the FILE thread. - void RenameCompletingDownloadFile(DownloadId id, + void RenameCompletingDownloadFile(content::DownloadId id, const FilePath& full_path, bool overwrite_existing_file); @@ -161,18 +163,20 @@ class CONTENT_EXPORT DownloadFileManager bool hash_needed); // Called only on the download thread. - content::DownloadFile* GetDownloadFile(DownloadId global_id); + content::DownloadFile* GetDownloadFile(content::DownloadId global_id); // Called only from RenameInProgressDownloadFile and // RenameCompletingDownloadFile on the FILE thread. // |rename_error| indicates what error caused the cancel. - void CancelDownloadOnRename(DownloadId global_id, net::Error rename_error); + void CancelDownloadOnRename(content::DownloadId global_id, + net::Error rename_error); // Erases the download file with the given the download |id| and removes // it from the maps. - void EraseDownload(DownloadId global_id); + void EraseDownload(content::DownloadId global_id); - typedef base::hash_map<DownloadId, content::DownloadFile*> DownloadFileMap; + typedef base::hash_map<content::DownloadId, content::DownloadFile*> + DownloadFileMap; // A map of all in progress downloads. It owns the download files. DownloadFileMap downloads_; diff --git a/content/browser/download/download_file_manager_unittest.cc b/content/browser/download/download_file_manager_unittest.cc index ccbe34c..de710f2 100644 --- a/content/browser/download/download_file_manager_unittest.cc +++ b/content/browser/download/download_file_manager_unittest.cc @@ -11,13 +11,12 @@ #include "content/browser/browser_thread_impl.h" #include "content/browser/download/download_buffer.h" #include "content/browser/download/download_create_info.h" -#include "content/browser/download/download_id.h" -#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_status_updater.h" #include "content/browser/download/mock_download_file.h" #include "content/browser/download/mock_download_manager.h" #include "content/browser/download/mock_download_manager_delegate.h" +#include "content/public/browser/download_id.h" #include "net/base/io_buffer.h" #include "net/base/net_errors.h" #include "testing/gmock/include/gmock/gmock.h" @@ -25,6 +24,7 @@ using content::BrowserThread; using content::BrowserThreadImpl; +using content::DownloadId; using ::testing::_; using ::testing::AtLeast; @@ -34,8 +34,6 @@ using ::testing::StrEq; namespace { -DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain"; - class MockDownloadFileFactory : public DownloadFileManager::DownloadFileFactory { @@ -114,7 +112,6 @@ class DownloadFileManagerTest : public testing::Test { // destructor being called and we get a leak. DownloadFileManagerTest() : download_buffer_(new content::DownloadBuffer()), - id_factory_(new DownloadIdFactory(kValidIdDomain)), ui_thread_(BrowserThread::UI, &loop_), file_thread_(BrowserThread::FILE, &loop_) { } @@ -431,7 +428,6 @@ class DownloadFileManagerTest : public testing::Test { private: MessageLoop loop_; - scoped_refptr<DownloadIdFactory> id_factory_; // UI thread. BrowserThreadImpl ui_thread_; diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc index 4855d44..9ca77c5 100644 --- a/content/browser/download/download_file_unittest.cc +++ b/content/browser/download/download_file_unittest.cc @@ -8,8 +8,6 @@ #include "content/browser/browser_thread_impl.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file_impl.h" -#include "content/browser/download/download_id.h" -#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_status_updater.h" #include "content/browser/download/mock_download_manager.h" @@ -22,6 +20,7 @@ using content::BrowserThread; using content::BrowserThreadImpl; using content::DownloadFile; +using content::DownloadId; using content::DownloadManager; DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain"; @@ -42,7 +41,6 @@ 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_) { } @@ -113,7 +111,6 @@ class DownloadFileTest : public testing::Test { private: MessageLoop loop_; - scoped_refptr<DownloadIdFactory> id_factory_; // UI thread. BrowserThreadImpl 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 deleted file mode 100644 index 8cd7b0e..0000000 --- a/content/browser/download/download_id.cc +++ /dev/null @@ -1,16 +0,0 @@ -// 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 <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.DebugString(); -} diff --git a/content/browser/download/download_id.h b/content/browser/download/download_id.h deleted file mode 100644 index d9726bf..0000000 --- a/content/browser/download/download_id.h +++ /dev/null @@ -1,90 +0,0 @@ -// 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_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" - -// 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. -class DownloadId { - public: - static DownloadId Invalid() { return DownloadId(NULL, -1); } - - // Domain separates spaces of local ids. - typedef const void* Domain; - - DownloadId(Domain domain, int32 local_id) - : domain_(domain), - local_id_(local_id) { - } - - // Return the per-profile and persistent part of this DownloadId. - int32 local() const { return local_id_; } - - // Returns true if this DownloadId has been allocated and could possibly refer - // to a DownloadItem that exists. - 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.domain_ == domain_)); - } - bool operator<(const DownloadId& that) const { - // 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 ((domain_ < that.domain_) || - ((domain_ == that.domain_) && (local_id_ < that.local_id_))); - } - - size_t hash() const { - // 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>(domain_) + - (static_cast<size_t>(local_id_) << (4 * sizeof(size_t))); - } - - std::string DebugString() const; - - private: - Domain domain_; - - int32 local_id_; - - // Allow copy and assign. -}; - -// Allow logging DownloadIds. Looks like "0x01234567:42". -CONTENT_EXPORT std::ostream& operator<<(std::ostream& out, - const DownloadId& global_id); - -// Allow using DownloadIds as keys in hash_maps. -namespace BASE_HASH_NAMESPACE { -#if defined(COMPILER_GCC) -template<> struct hash<DownloadId> { - std::size_t operator()(const DownloadId& id) const { - return id.hash(); - } -}; -#elif defined(COMPILER_MSVC) -inline size_t hash_value(const DownloadId& id) { - return id.hash(); -} -#endif // COMPILER -} -#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 deleted file mode 100644 index 6b0797f..0000000 --- a/content/browser/download/download_id_factory.cc +++ /dev/null @@ -1,17 +0,0 @@ -// 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 deleted file mode 100644 index 729c752..0000000 --- a/content/browser/download/download_id_factory.h +++ /dev/null @@ -1,31 +0,0 @@ -// 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 CONTENT_EXPORT DownloadIdFactory - : public base::RefCountedThreadSafe<DownloadIdFactory> { - public: - // TODO(benjhayden): Instantiate with an explicit next id counter read from - // persistent storage. - explicit 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 18f157f..5a80a39 100644 --- a/content/browser/download/download_id_unittest.cc +++ b/content/browser/download/download_id_unittest.cc @@ -2,7 +2,7 @@ // 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.h" +#include "content/public/browser/download_id.h" #include <algorithm> #include <map> @@ -17,6 +17,7 @@ using content::BrowserThread; using content::BrowserThreadImpl; +using content::DownloadId; using content::DownloadManager; class DownloadIdTest : public testing::Test { diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc index cc48515..2d1ad6d 100644 --- a/content/browser/download/download_item_impl.cc +++ b/content/browser/download/download_item_impl.cc @@ -19,7 +19,6 @@ #include "base/utf_string_conversions.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file_manager.h" -#include "content/browser/download/download_id.h" #include "content/browser/download/download_persistent_store_info.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_stats.h" @@ -32,6 +31,7 @@ using content::BrowserThread; using content::DownloadFile; +using content::DownloadId; using content::DownloadItem; using content::DownloadManager; using content::WebContents; diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h index 87c7d31..c3d3119 100644 --- a/content/browser/download/download_item_impl.h +++ b/content/browser/download/download_item_impl.h @@ -14,9 +14,9 @@ #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/common/content_export.h" +#include "content/public/browser/download_id.h" #include "content/public/browser/download_item.h" #include "googleurl/src/gurl.h" #include "net/base/net_errors.h" @@ -82,7 +82,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem { // Constructing from persistent store: DownloadItemImpl(Delegate* delegate, - DownloadId download_id, + content::DownloadId download_id, const DownloadPersistentStoreInfo& info); // Constructing for a regular download. @@ -97,7 +97,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem { const FilePath& path, const GURL& url, bool is_otr, - DownloadId download_id); + content::DownloadId download_id); virtual ~DownloadItemImpl(); @@ -161,7 +161,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem { virtual int64 GetReceivedBytes() const OVERRIDE; virtual const std::string& GetHashState() const OVERRIDE; virtual int32 GetId() const OVERRIDE; - virtual DownloadId GetGlobalId() const OVERRIDE; + virtual content::DownloadId GetGlobalId() const OVERRIDE; virtual base::Time GetStartTime() const OVERRIDE; virtual base::Time GetEndTime() const OVERRIDE; virtual void SetDbHandle(int64 handle) OVERRIDE; @@ -240,7 +240,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem { scoped_ptr<DownloadRequestHandleInterface> request_handle_; // Download ID assigned by DownloadResourceHandler. - DownloadId download_id_; + content::DownloadId download_id_; // Full path to the downloaded or downloading file. FilePath full_path_; diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc index cc8d55c..2276afc 100644 --- a/content/browser/download/download_manager_impl.cc +++ b/content/browser/download/download_manager_impl.cc @@ -19,7 +19,6 @@ #include "build/build_config.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_impl.h" #include "content/browser/download/download_persistent_store_info.h" #include "content/browser/download/download_stats.h" @@ -49,6 +48,7 @@ #define CHECK_96627 CHECK using content::BrowserThread; +using content::DownloadId; using content::DownloadItem; using content::WebContents; @@ -116,16 +116,14 @@ namespace content { // static DownloadManager* DownloadManager::Create( content::DownloadManagerDelegate* delegate, - DownloadIdFactory* id_factory, DownloadStatusUpdater* status_updater) { - return new DownloadManagerImpl(delegate, id_factory, status_updater); + return new DownloadManagerImpl(delegate, status_updater); } } // namespace content DownloadManagerImpl::DownloadManagerImpl( content::DownloadManagerDelegate* delegate, - DownloadIdFactory* id_factory, DownloadStatusUpdater* status_updater) : shutdown_needed_(false), browser_context_(NULL), @@ -134,7 +132,6 @@ DownloadManagerImpl::DownloadManagerImpl( ? 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. @@ -147,7 +144,7 @@ DownloadManagerImpl::~DownloadManagerImpl() { } DownloadId DownloadManagerImpl::GetNextId() { - return id_factory_->GetNextId(); + return delegate_->GetNextId(); } bool DownloadManagerImpl::ShouldOpenDownload(DownloadItem* item) { diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h index 3a3253b..b69753d 100644 --- a/content/browser/download/download_manager_impl.h +++ b/content/browser/download/download_manager_impl.h @@ -22,7 +22,6 @@ #include "content/common/content_export.h" #include "content/public/browser/download_manager.h" -class DownloadIdFactory; class DownloadStatusUpdater; namespace content { @@ -35,7 +34,6 @@ class CONTENT_EXPORT DownloadManagerImpl public DownloadStatusUpdaterDelegate { public: DownloadManagerImpl(content::DownloadManagerDelegate* delegate, - DownloadIdFactory* id_factory, DownloadStatusUpdater* status_updater); // content::DownloadManager functions. @@ -153,8 +151,8 @@ class CONTENT_EXPORT DownloadManagerImpl // Show the download in the browser. void ShowDownloadInBrowser(content::DownloadItem* download); - // Get next download id from factory. - DownloadId GetNextId(); + // Get next download id. + content::DownloadId GetNextId(); // Called on the FILE thread to check the existence of a downloaded file. void CheckForFileRemovalOnFileThread(int64 db_handle, const FilePath& path); @@ -270,8 +268,6 @@ class CONTENT_EXPORT DownloadManagerImpl // 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 34d072d..43a2e17 100644 --- a/content/browser/download/download_resource_handler.cc +++ b/content/browser/download/download_resource_handler.cc @@ -14,6 +14,7 @@ #include "content/browser/download/download_buffer.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file_manager.h" +#include "content/browser/download/download_manager_impl.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_stats.h" #include "content/browser/download/interrupt_reasons.h" @@ -21,6 +22,7 @@ #include "content/browser/renderer_host/resource_dispatcher_host_request_info.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/download_item.h" +#include "content/public/browser/download_manager_delegate.h" #include "content/public/common/resource_response.h" #include "net/base/io_buffer.h" #include "net/base/net_errors.h" @@ -28,7 +30,9 @@ #include "net/url_request/url_request_context.h" using content::BrowserThread; +using content::DownloadId; using content::DownloadItem; +using content::DownloadManager; DownloadResourceHandler::DownloadResourceHandler( ResourceDispatcherHost* rdh, @@ -36,13 +40,12 @@ DownloadResourceHandler::DownloadResourceHandler( int render_view_id, int request_id, const GURL& url, - DownloadId dl_id, DownloadFileManager* download_file_manager, net::URLRequest* request, bool save_as, const DownloadResourceHandler::OnStartedCallback& started_cb, const DownloadSaveInfo& save_info) - : download_id_(dl_id), + : download_id_(DownloadId::Invalid()), global_id_(render_process_host_id, request_id), render_view_id_(render_view_id), content_length_(0), @@ -56,7 +59,6 @@ DownloadResourceHandler::DownloadResourceHandler( is_paused_(false), last_buffer_size_(0), bytes_read_(0) { - DCHECK(dl_id.IsValid()); download_stats::RecordDownloadCount(download_stats::UNTHROTTLED_COUNT); } @@ -79,7 +81,6 @@ bool DownloadResourceHandler::OnRequestRedirected( bool DownloadResourceHandler::OnResponseStarted( int request_id, content::ResourceResponse* response) { - DCHECK(download_id_.IsValid()); VLOG(20) << __FUNCTION__ << "()" << DebugString() << " request_id = " << request_id; download_start_time_ = base::TimeTicks::Now(); @@ -99,15 +100,13 @@ bool DownloadResourceHandler::OnResponseStarted( // Deleted in DownloadManager. DownloadCreateInfo* info = new DownloadCreateInfo(FilePath(), GURL(), base::Time::Now(), 0, content_length_, DownloadItem::IN_PROGRESS, - download_id_, request_info->has_user_gesture(), - request_info->transition_type()); + request_info->has_user_gesture(), request_info->transition_type()); info->url_chain = request_->url_chain(); info->referrer_url = GURL(request_->referrer()); info->start_time = base::Time::Now(); info->received_bytes = save_info_.offset; info->total_bytes = content_length_; info->state = DownloadItem::IN_PROGRESS; - info->download_id = download_id_; info->has_user_gesture = request_info->has_user_gesture(); info->content_disposition = content_disposition_; info->mime_type = response->mime_type; @@ -128,8 +127,6 @@ bool DownloadResourceHandler::OnResponseStarted( info->etag = etag; } - CallStartedCB(net::OK); - std::string content_type_header; if (!response->headers || !response->headers->GetMimeType(&content_type_header)) @@ -147,23 +144,26 @@ bool DownloadResourceHandler::OnResponseStarted( save_as_ && save_info_.file_path.empty(); info->referrer_charset = request_->context()->referrer_charset(); info->save_info = save_info_; + + BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, - base::Bind(&DownloadFileManager::StartDownload, - download_file_manager_, info, request_handle)); + base::Bind(&DownloadResourceHandler::StartOnUIThread, this, + info, request_handle)); - // We can't start saving the data before we create the file on disk. - // The request will be un-paused in DownloadFileManager::CreateDownloadFile. + // We can't start saving the data before we create the file on disk and have a + // download id. The request will be un-paused in + // DownloadFileManager::CreateDownloadFile. rdh_->PauseRequest(global_id_.child_id, global_id_.request_id, true); return true; } -void DownloadResourceHandler::CallStartedCB(net::Error error) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); +void DownloadResourceHandler::CallStartedCB(DownloadId id, net::Error error) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (started_cb_.is_null()) return; - started_cb_.Run(download_id_, error); + started_cb_.Run(id, error); started_cb_.Reset(); } @@ -233,6 +233,20 @@ bool DownloadResourceHandler::OnResponseCompleted( int request_id, const net::URLRequestStatus& status, const std::string& security_info) { + if (!download_id_.IsValid()) { + // We got cancelled before the task which sets the id ran on the IO thread. + // Wait for it. + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(base::IgnoreResult(&BrowserThread::PostTask), + BrowserThread::IO, FROM_HERE, + base::Bind(base::IgnoreResult( + &DownloadResourceHandler::OnResponseCompleted), this, + request_id, status, security_info))); + + return true; + } + VLOG(20) << __FUNCTION__ << "()" << DebugString() << " request_id = " << request_id << " status.status() = " << status.status() @@ -259,8 +273,12 @@ bool DownloadResourceHandler::OnResponseCompleted( download_stats::RecordAcceptsRanges(accept_ranges_, bytes_read_); - if (!download_id_.IsValid()) - CallStartedCB(error_code); + // If the callback was already run on the UI thread, this will be a noop. + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&DownloadResourceHandler::CallStartedCB, this, + download_id_, error_code)); + // We transfer ownership to |DownloadFileManager| to delete |buffer_|, // so that any functions queued up on the FILE thread are executed // before deletion. @@ -278,6 +296,29 @@ void DownloadResourceHandler::OnRequestClosed() { base::TimeTicks::Now() - download_start_time_); } +void DownloadResourceHandler::StartOnUIThread(DownloadCreateInfo* info, + DownloadRequestHandle handle) { + DownloadManager* download_manager = handle.GetDownloadManager(); + if (!download_manager) + return; // NULL in unittests + info->download_id = download_manager->delegate()->GetNextId(); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&DownloadResourceHandler::set_download_id, this, + info->download_id)); + CallStartedCB(info->download_id, net::OK); + // It's safe to continue on with download initiation before we have + // confirmation that that download_id_ has been set on the IO thread, as any + // messages generated by the UI thread that affect the IO thread will be + // behind the message posted above. + download_file_manager_->StartDownload(info, handle); +} + +void DownloadResourceHandler::set_download_id(content::DownloadId id) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + download_id_ = id; +} + // If the content-length header is not present (or contains something other // than numbers), the incoming content_length is -1 (unknown size). // Set the content length to 0 to indicate unknown size to DownloadManager. diff --git a/content/browser/download/download_resource_handler.h b/content/browser/download/download_resource_handler.h index 5425c51..b651c48 100644 --- a/content/browser/download/download_resource_handler.h +++ b/content/browser/download/download_resource_handler.h @@ -11,14 +11,16 @@ #include "base/callback.h" #include "base/memory/scoped_ptr.h" #include "base/timer.h" -#include "content/browser/download/download_id.h" #include "content/browser/download/download_types.h" #include "content/browser/renderer_host/resource_handler.h" +#include "content/public/browser/download_id.h" #include "content/public/browser/global_request_id.h" #include "net/base/net_errors.h" class DownloadFileManager; +class DownloadRequestHandle; class ResourceDispatcherHost; +struct DownloadCreateInfo; namespace content { class DownloadBuffer; @@ -31,18 +33,17 @@ class URLRequest; // Forwards data to the download thread. class DownloadResourceHandler : public ResourceHandler { public: - typedef base::Callback<void(DownloadId, net::Error)> - OnStartedCallback; + typedef base::Callback<void(content::DownloadId, net::Error)> + OnStartedCallback; static const size_t kLoadsToWrite = 100; // number of data buffers queued - // started_cb will be called exactly once. + // started_cb will be called exactly once on the UI thread. DownloadResourceHandler(ResourceDispatcherHost* rdh, int render_process_host_id, int render_view_id, int request_id, const GURL& url, - DownloadId dl_id, DownloadFileManager* download_file_manager, net::URLRequest* request, bool save_as, @@ -97,9 +98,14 @@ class DownloadResourceHandler : public ResourceHandler { virtual ~DownloadResourceHandler(); void StartPauseTimer(); - void CallStartedCB(net::Error error); + void CallStartedCB(content::DownloadId id, net::Error error); - DownloadId download_id_; + // Generates a DownloadId and calls DownloadFileManager. + void StartOnUIThread(DownloadCreateInfo* info, + DownloadRequestHandle handle); + void set_download_id(content::DownloadId id); + + content::DownloadId download_id_; content::GlobalRequestID global_id_; int render_view_id_; scoped_refptr<net::IOBuffer> read_buffer_; @@ -108,6 +114,7 @@ class DownloadResourceHandler : public ResourceHandler { DownloadFileManager* download_file_manager_; net::URLRequest* request_; bool save_as_; // Request was initiated via "Save As" by the user. + // This is used only on the UI thread. OnStartedCallback started_cb_; DownloadSaveInfo save_info_; scoped_refptr<content::DownloadBuffer> buffer_; diff --git a/content/browser/download/mock_download_file.h b/content/browser/download/mock_download_file.h index 2e97b04..6aff4da 100644 --- a/content/browser/download/mock_download_file.h +++ b/content/browser/download/mock_download_file.h @@ -11,9 +11,8 @@ #include "base/file_path.h" #include "base/memory/ref_counted.h" -#include "content/browser/download/download_id.h" -#include "content/browser/download/download_request_handle.h" #include "content/public/browser/download_file.h" +#include "content/public/browser/download_id.h" #include "content/public/browser/download_manager.h" #include "net/base/net_errors.h" #include "testing/gmock/include/gmock/gmock.h" @@ -43,7 +42,7 @@ class MockDownloadFile : virtual public content::DownloadFile { MOCK_METHOD0(CancelDownloadRequest, void()); MOCK_CONST_METHOD0(Id, int()); MOCK_METHOD0(GetDownloadManager, content::DownloadManager*()); - MOCK_CONST_METHOD0(GlobalId, const DownloadId&()); + MOCK_CONST_METHOD0(GlobalId, const content::DownloadId&()); MOCK_CONST_METHOD0(DebugString, std::string()); }; diff --git a/content/browser/download/mock_download_item.h b/content/browser/download/mock_download_item.h index 42d25b8..1212ee5 100644 --- a/content/browser/download/mock_download_item.h +++ b/content/browser/download/mock_download_item.h @@ -8,9 +8,9 @@ #include <string> #include <vector> -#include "content/browser/download/download_id.h" #include "content/browser/download/download_persistent_store_info.h" #include "content/browser/download/interrupt_reasons.h" +#include "content/public/browser/download_id.h" #include "content/public/browser/download_item.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -73,7 +73,7 @@ class MockDownloadItem : public content::DownloadItem { MOCK_CONST_METHOD0(GetHashState, const std::string&()); MOCK_CONST_METHOD0(GetHash, const std::string&()); MOCK_CONST_METHOD0(GetId, int32()); - MOCK_CONST_METHOD0(GetGlobalId, DownloadId()); + MOCK_CONST_METHOD0(GetGlobalId, content::DownloadId()); MOCK_CONST_METHOD0(GetStartTime, base::Time()); MOCK_CONST_METHOD0(GetEndTime, base::Time()); MOCK_METHOD1(SetDbHandle, void(int64)); diff --git a/content/browser/download/mock_download_manager.h b/content/browser/download/mock_download_manager.h index cf08d82..6e9be3f 100644 --- a/content/browser/download/mock_download_manager.h +++ b/content/browser/download/mock_download_manager.h @@ -6,10 +6,9 @@ #define CONTENT_BROWSER_DOWNLOAD_MOCK_DOWNLOAD_MANAGER_H_ #pragma once -#include "content/browser/download/download_id.h" -#include "content/browser/download/download_id_factory.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_types.h" +#include "content/public/browser/download_id.h" #include "content/public/browser/download_item.h" #include "content/public/browser/download_manager.h" #include "content/public/browser/download_query.h" diff --git a/content/browser/download/mock_download_manager_delegate.h b/content/browser/download/mock_download_manager_delegate.h index 1fb62cb..ff48765 100644 --- a/content/browser/download/mock_download_manager_delegate.h +++ b/content/browser/download/mock_download_manager_delegate.h @@ -9,6 +9,7 @@ #include "base/compiler_specific.h" #include "base/file_path.h" #include "content/browser/download/download_types.h" +#include "content/public/browser/download_id.h" #include "content/public/browser/download_manager_delegate.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -25,6 +26,7 @@ class MockDownloadManagerDelegate : public content::DownloadManagerDelegate { // DownloadManagerDelegate functions: MOCK_METHOD1(SetDownloadManager, void(content::DownloadManager* dm)); MOCK_METHOD0(Shutdown, void()); + MOCK_METHOD0(GetNextId, content::DownloadId()); MOCK_METHOD1(ShouldStartDownload, bool(int32 download_id)); MOCK_METHOD3(ChooseDownloadPath, void(content::WebContents* web_contents, const FilePath& suggested_path, |