diff options
author | ahendrickson@chromium.org <ahendrickson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-23 19:15:01 +0000 |
---|---|---|
committer | ahendrickson@chromium.org <ahendrickson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-23 19:15:01 +0000 |
commit | 4cd82f72da57dac8a4b810d8a99fbe5b379cf593 (patch) | |
tree | 5c503dfa4605672086765819a2af1ab9f669dd2e | |
parent | 088e6a3814c7de976650e8ad2b5cb075656d8665 (diff) | |
download | chromium_src-4cd82f72da57dac8a4b810d8a99fbe5b379cf593.zip chromium_src-4cd82f72da57dac8a4b810d8a99fbe5b379cf593.tar.gz chromium_src-4cd82f72da57dac8a4b810d8a99fbe5b379cf593.tar.bz2 |
Part 2 of downloads data flow refactor.
Reduced the lifetime of DownloadCreateInfo.
DownloadCreateInfo now only lives until DownloadFileManager::CreateDownloadFile(). It's contents are put into DownloadItem, and then only the download ID is passed around to get at the information.
In addition, most of DownloadItem's members are grouped into sub-structures by functionality:
- history_
Passed to the history data base, and includes the data relevant to that.
- request_
Contains the information about the request, which is not present in the other structures.
- state_
Contains data that is used to manage the download process. It is part of DownloadItem because it is specific to that item, but it could instead be passed around as an extra parameter.
Some notes on the refactoring:
- DownloadItem is only accessed in the UI thread. This means that the state_ structure needs to be passed by value to DownloadManager::CheckIfSuggestedPathExists(), modified there, and then passed by value again to DownloadManager::OnPathExistenceAvailable(), where it is copied bach into DownloadItem.
BUG=78084,77188
TEST=Passes all download unit tests.
Review URL: http://codereview.chromium.org/6969009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86319 0039d316-1c4b-4281-b951-d872f2087c98
37 files changed, 964 insertions, 434 deletions
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index d8a5f20..da4cc5d 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -254,7 +254,7 @@ DictionaryValue* AutomationProvider::GetDictionaryFromDownloadItem( DictionaryValue* dl_item_value = new DictionaryValue; dl_item_value->SetInteger("id", static_cast<int>(download->id())); - dl_item_value->SetString("url", download->url().spec()); + dl_item_value->SetString("url", download->GetURL().spec()); dl_item_value->SetString("referrer_url", download->referrer_url().spec()); dl_item_value->SetString("file_name", download->GetFileNameToReportUser().value()); diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc index 9ac0a4c..7362f8e 100644 --- a/chrome/browser/download/download_browsertest.cc +++ b/chrome/browser/download/download_browsertest.cc @@ -15,7 +15,7 @@ #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_prefs.h" #include "chrome/browser/download/download_shelf.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/history/history.h" #include "chrome/browser/net/url_request_mock_http_job.h" #include "chrome/browser/net/url_request_slow_download_job.h" @@ -698,7 +698,7 @@ class DownloadsHistoryDataCollector { ui_test_utils::RunMessageLoop(); } - bool GetDownloadsHistoryEntry(DownloadCreateInfo* result) { + bool GetDownloadsHistoryEntry(DownloadHistoryInfo* result) { DCHECK(result); *result = result_; return result_valid_; @@ -706,9 +706,9 @@ class DownloadsHistoryDataCollector { private: void OnQueryDownloadsComplete( - std::vector<DownloadCreateInfo>* entries) { + std::vector<DownloadHistoryInfo>* entries) { result_valid_ = false; - for (std::vector<DownloadCreateInfo>::const_iterator it = entries->begin(); + for (std::vector<DownloadHistoryInfo>::const_iterator it = entries->begin(); it != entries->end(); ++it) { if (it->db_handle == download_db_handle_) { result_ = *it; @@ -718,7 +718,7 @@ class DownloadsHistoryDataCollector { MessageLoopForUI::current()->Quit(); } - DownloadCreateInfo result_; + DownloadHistoryInfo result_; bool result_valid_; int64 download_db_handle_; CancelableRequestConsumer callback_consumer_; @@ -1284,10 +1284,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadHistoryCheck) { DownloadsHistoryDataCollector history_collector( db_handle, browser()->profile()->GetDownloadManager()); - DownloadCreateInfo info; + DownloadHistoryInfo info; EXPECT_TRUE(history_collector.GetDownloadsHistoryEntry(&info)) << db_handle; EXPECT_EQ(file, info.path.BaseName()); - EXPECT_EQ(url, info.url()); + EXPECT_EQ(url, info.url); // Ignore start_time. EXPECT_EQ(origin_size, info.received_bytes); EXPECT_EQ(origin_size, info.total_bytes); diff --git a/chrome/browser/download/download_create_info.cc b/chrome/browser/download/download_create_info.cc new file mode 100644 index 0000000..4a64b64 --- /dev/null +++ b/chrome/browser/download/download_create_info.cc @@ -0,0 +1,78 @@ +// 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 "chrome/browser/download/download_create_info.h" + +#include <string> + +#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, + int32 download_id, + bool has_user_gesture) + : path(path), + url_chain(1, url), + path_uniquifier(0), + start_time(start_time), + received_bytes(received_bytes), + total_bytes(total_bytes), + state(state), + download_id(download_id), + has_user_gesture(has_user_gesture), + db_handle(0), + prompt_user_for_save_location(false), + is_dangerous_file(false), + is_dangerous_url(false), + is_extension_install(false) { +} + +DownloadCreateInfo::DownloadCreateInfo() + : path_uniquifier(0), + received_bytes(0), + total_bytes(0), + state(-1), + download_id(-1), + has_user_gesture(false), + db_handle(0), + prompt_user_for_save_location(false), + is_dangerous_file(false), + is_dangerous_url(false), + is_extension_install(false) { +} + +DownloadCreateInfo::~DownloadCreateInfo() { +} + +std::string DownloadCreateInfo::DebugString() const { + return base::StringPrintf("{" + " download_id = %d" + " url = \"%s\"" + " path = \"%" PRFilePath "\"" + " received_bytes = %" PRId64 + " total_bytes = %" PRId64 + " child_id = %d" + " render_view_id = %d" + " request_id = %d" + " prompt_user_for_save_location = %c" + " }", + download_id, + url().spec().c_str(), + path.value().c_str(), + received_bytes, + total_bytes, + process_handle.child_id(), + process_handle.render_view_id(), + process_handle.request_id(), + prompt_user_for_save_location ? 'T' : 'F'); +} + +const GURL& DownloadCreateInfo::url() const { + return url_chain.empty() ? GURL::EmptyGURL() : url_chain.back(); +} diff --git a/chrome/browser/download/download_create_info.h b/chrome/browser/download/download_create_info.h new file mode 100644 index 0000000..6e097ef --- /dev/null +++ b/chrome/browser/download/download_create_info.h @@ -0,0 +1,118 @@ +// 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 CHROME_BROWSER_DOWNLOAD_DOWNLOAD_CREATE_INFO_H_ +#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_CREATE_INFO_H_ +#pragma once + +#include <string> +#include <vector> + +#include "base/basictypes.h" +#include "base/file_path.h" +#include "base/time.h" +#include "chrome/browser/download/download_file.h" +#include "chrome/browser/download/download_process_handle.h" +#include "googleurl/src/gurl.h" + +// Used for informing the download manager of a new download, since we don't +// want to pass |DownloadItem|s between threads. +struct DownloadCreateInfo { + DownloadCreateInfo(const FilePath& path, + const GURL& url, + const base::Time& start_time, + int64 received_bytes, + int64 total_bytes, + int32 state, + int32 download_id, + bool has_user_gesture); + DownloadCreateInfo(); + ~DownloadCreateInfo(); + + std::string DebugString() const; + + // The URL from which we are downloading. This is the final URL after any + // redirection by the server for |url_chain|. + const GURL& url() const; + + // DownloadItem fields + // The path where we want to save the download file. + FilePath path; + + // The chain of redirects that leading up to and including the final URL. + std::vector<GURL> url_chain; + + // The URL that referred us. + GURL referrer_url; + + // The default path for the download (may be overridden). + FilePath suggested_path; + + // A number that should be added to the suggested path to make it unique. + // 0 means no number should be appended. Not actually stored in the db. + int path_uniquifier; + + // The time when the download started. + base::Time start_time; + + // The number of bytes that have been received. + int64 received_bytes; + + // The total download size. + int64 total_bytes; + + // The current state of the download. + int32 state; + + // The (per-session) ID of the download. + int32 download_id; + + // True if the download was initiated by user action. + bool has_user_gesture; + + // The handle to the process information. Used for operations outside the + // download system. + DownloadProcessHandle process_handle; + + // The handle of the download in the history database. + int64 db_handle; + + // The content-disposition string from the response header. + std::string content_disposition; + + // The mime type string from the response header (may be overridden). + std::string mime_type; + + // The value of the content type header sent with the downloaded item. It + // may be different from |mime_type|, which may be set based on heuristics + // which may look at the file extension and first few bytes of the file. + std::string original_mime_type; + + // True if we should display the 'save as...' UI and prompt the user + // for the download location. + // False if the UI should be supressed and the download performed to the + // default location. + bool prompt_user_for_save_location; + + // Whether this download file is potentially dangerous (ex: exe, dll, ...). + bool is_dangerous_file; + + // If safebrowsing believes this URL leads to malware. + bool is_dangerous_url; + + // The original name for a dangerous download. + FilePath original_name; + + // Whether this download is for extension install or not. + bool is_extension_install; + + // The charset of the referring page where the download request comes from. + // It's used to construct a suggested filename. + std::string referrer_charset; + + // The download file save info. + DownloadSaveInfo save_info; +}; + +#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_CREATE_INFO_H_ diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc index 264bae3..d1a3470 100644 --- a/chrome/browser/download/download_file.cc +++ b/chrome/browser/download/download_file.cc @@ -8,9 +8,9 @@ #include "base/file_util.h" #include "base/stringprintf.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_util.h" -#include "chrome/browser/history/download_create_info.h" #include "content/browser/browser_thread.h" DownloadFile::DownloadFile(const DownloadCreateInfo* info, diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc index faa85ba..0c200e5 100644 --- a/chrome/browser/download/download_file_manager.cc +++ b/chrome/browser/download/download_file_manager.cc @@ -10,10 +10,10 @@ #include "base/task.h" #include "base/utf_string_conversions.h" #include "build/build_config.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_process_handle.h" #include "chrome/browser/download/download_util.h" -#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/platform_util.h" #include "chrome/browser/profiles/profile.h" @@ -58,22 +58,24 @@ void DownloadFileManager::OnShutdown() { void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info, DownloadManager* download_manager, bool get_hash) { + DCHECK(info); VLOG(20) << __FUNCTION__ << "()" << " info = " << info->DebugString(); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + // Life of |info| ends here. No more references to it after this method. + scoped_ptr<DownloadCreateInfo> infop(info); + scoped_ptr<DownloadFile> download_file(new DownloadFile(info, download_manager)); if (!download_file->Initialize(get_hash)) { download_util::CancelDownloadRequest(resource_dispatcher_host_, info->process_handle); - delete info; return; } - DCHECK(GetDownloadFile(info->download_id) == NULL); - downloads_[info->download_id] = download_file.release(); - // TODO(phajdan.jr): fix the duplication of path info below. - info->path = info->save_info.file_path; + int32 id = info->download_id; + DCHECK(GetDownloadFile(id) == NULL); + downloads_[id] = download_file.release(); // The file is now ready, we can un-pause the request and start saving data. BrowserThread::PostTask( @@ -86,7 +88,7 @@ void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info, BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, NewRunnableMethod(download_manager, - &DownloadManager::StartDownload, info)); + &DownloadManager::StartDownload, id)); } void DownloadFileManager::ResumeDownloadRequest( @@ -151,6 +153,9 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) { return; } + // TODO(phajdan.jr): fix the duplication of path info below. + info->path = info->save_info.file_path; + manager->CreateDownloadItem(info); bool hash_needed = resource_dispatcher_host_->safe_browsing_service()-> @@ -158,8 +163,7 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) { BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, NewRunnableMethod(this, &DownloadFileManager::CreateDownloadFile, - info, - make_scoped_refptr(manager), hash_needed)); + info, make_scoped_refptr(manager), hash_needed)); } // We don't forward an update to the UI thread here, since we want to throttle @@ -175,12 +179,12 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) { contents.swap(buffer->contents); } - DownloadFile* download = GetDownloadFile(id); + DownloadFile* download_file = GetDownloadFile(id); for (size_t i = 0; i < contents.size(); ++i) { net::IOBuffer* data = contents[i].first; const int data_len = contents[i].second; - if (download) - download->AppendDataToFile(data->data(), data_len); + if (download_file) + download_file->AppendDataToFile(data->data(), data_len); data->Release(); } } @@ -195,27 +199,27 @@ void DownloadFileManager::OnResponseCompleted( << " security_info = \"" << security_info << "\""; DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); delete buffer; - DownloadFile* download = GetDownloadFile(id); - if (!download) + DownloadFile* download_file = GetDownloadFile(id); + if (!download_file) return; - download->Finish(); + download_file->Finish(); - DownloadManager* download_manager = download->GetDownloadManager(); + DownloadManager* download_manager = download_file->GetDownloadManager(); if (!download_manager) { CancelDownload(id); return; } std::string hash; - if (!download->GetSha256Hash(&hash)) + if (!download_file->GetSha256Hash(&hash)) hash.clear(); BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, NewRunnableMethod( download_manager, &DownloadManager::OnResponseCompleted, - id, download->bytes_so_far(), os_error, hash)); + id, download_file->bytes_so_far(), os_error, hash)); // We need to keep the download around until the UI thread has finalized // the name. } @@ -230,10 +234,10 @@ void DownloadFileManager::CancelDownload(int id) { if (it == downloads_.end()) return; - DownloadFile* download = it->second; + DownloadFile* download_file = it->second; VLOG(20) << __FUNCTION__ << "()" - << " download = " << download->DebugString(); - download->Cancel(); + << " download_file = " << download_file->DebugString(); + download_file->Cancel(); EraseDownload(id); } @@ -291,13 +295,14 @@ void DownloadFileManager::RenameInProgressDownloadFile( << " full_path = \"" << full_path.value() << "\""; DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DownloadFile* download = GetDownloadFile(id); - if (!download) + DownloadFile* download_file = GetDownloadFile(id); + if (!download_file) return; - VLOG(20) << __FUNCTION__ << "()" << " download = " << download->DebugString(); + VLOG(20) << __FUNCTION__ << "()" + << " download_file = " << download_file->DebugString(); - if (!download->Rename(full_path)) { + if (!download_file->Rename(full_path)) { // Error. Between the time the UI thread generated 'full_path' to the time // this code runs, something happened that prevents us from renaming. CancelDownloadOnRename(id); @@ -318,14 +323,15 @@ void DownloadFileManager::RenameCompletingDownloadFile( << " full_path = \"" << full_path.value() << "\""; DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DownloadFile* download = GetDownloadFile(id); - if (!download) + DownloadFile* download_file = GetDownloadFile(id); + if (!download_file) return; - DCHECK(download->GetDownloadManager()); - DownloadManager* download_manager = download->GetDownloadManager(); + DCHECK(download_file->GetDownloadManager()); + DownloadManager* download_manager = download_file->GetDownloadManager(); - VLOG(20) << __FUNCTION__ << "()" << " download = " << download->DebugString(); + VLOG(20) << __FUNCTION__ << "()" + << " download_file = " << download_file->DebugString(); int uniquifier = 0; FilePath new_path = full_path; @@ -344,7 +350,7 @@ void DownloadFileManager::RenameCompletingDownloadFile( } // Rename the file, overwriting if necessary. - if (!download->Rename(new_path)) { + if (!download_file->Rename(new_path)) { // Error. Between the time the UI thread generated 'full_path' to the time // this code runs, something happened that prevents us from renaming. CancelDownloadOnRename(id); @@ -354,7 +360,7 @@ void DownloadFileManager::RenameCompletingDownloadFile( #if defined(OS_MACOSX) // Done here because we only want to do this once; see // http://crbug.com/13120 for details. - download->AnnotateWithSourceInformation(); + download_file->AnnotateWithSourceInformation(); #endif BrowserThread::PostTask( @@ -369,16 +375,16 @@ void DownloadFileManager::RenameCompletingDownloadFile( void DownloadFileManager::CancelDownloadOnRename(int id) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DownloadFile* download = GetDownloadFile(id); - if (!download) + DownloadFile* download_file = GetDownloadFile(id); + if (!download_file) return; - DownloadManager* download_manager = download->GetDownloadManager(); + DownloadManager* download_manager = download_file->GetDownloadManager(); if (!download_manager) { // Without a download manager, we can't cancel the request normally, so we // need to do it here. The normal path will also update the download // history before cancelling the request. - download->CancelDownloadRequest(resource_dispatcher_host_); + download_file->CancelDownloadRequest(resource_dispatcher_host_); return; } diff --git a/chrome/browser/download/download_file_unittest.cc b/chrome/browser/download/download_file_unittest.cc index d982489..19d0712 100644 --- a/chrome/browser/download/download_file_unittest.cc +++ b/chrome/browser/download/download_file_unittest.cc @@ -6,13 +6,13 @@ #include "base/message_loop.h" #include "base/scoped_temp_dir.h" #include "base/string_number_conversions.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_file.h" #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_process_handle.h" #include "chrome/browser/download/download_status_updater.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/download/mock_download_manager.h" -#include "chrome/browser/history/download_create_info.h" #include "content/browser/browser_thread.h" #include "net/base/file_stream.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc index 388ffd3..b19f240 100644 --- a/chrome/browser/download/download_history.cc +++ b/chrome/browser/download/download_history.cc @@ -1,11 +1,11 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// 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 "chrome/browser/download/download_history.h" #include "base/logging.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/history/history_marshaling.h" #include "chrome/browser/download/download_item.h" #include "chrome/browser/profiles/profile.h" @@ -40,9 +40,9 @@ void DownloadHistory::Load(HistoryService::DownloadQueryCallback* callback) { } void DownloadHistory::AddEntry( - const DownloadCreateInfo& info, DownloadItem* download_item, HistoryService::DownloadCreateCallback* callback) { + DCHECK(download_item); // Do not store the download in the history database for a few special cases: // - incognito mode (that is the point of this mode) // - extensions (users don't think of extension installation as 'downloading') @@ -57,12 +57,15 @@ void DownloadHistory::AddEntry( if (download_item->is_otr() || download_item->is_extension_install() || download_item->is_temporary() || !hs) { callback->RunWithParams( - history::DownloadCreateRequest::TupleType(info, GetNextFakeDbHandle())); + history::DownloadCreateRequest::TupleType(download_item->id(), + GetNextFakeDbHandle())); delete callback; return; } - hs->CreateDownload(info, &history_consumer_, callback); + int32 id = download_item->id(); + DownloadHistoryInfo history_info = download_item->GetHistoryInfo(); + hs->CreateDownload(id, history_info, &history_consumer_, callback); } void DownloadHistory::UpdateEntry(DownloadItem* download_item) { diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h index 7fb114e..cdde668 100644 --- a/chrome/browser/download/download_history.h +++ b/chrome/browser/download/download_history.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// 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. @@ -31,8 +31,7 @@ class DownloadHistory { void Load(HistoryService::DownloadQueryCallback* callback); // Adds a new entry for a download to the history database. - void AddEntry(const DownloadCreateInfo& info, - DownloadItem* download_item, + void AddEntry(DownloadItem* download_item, HistoryService::DownloadCreateCallback* callback); // Updates the history entry for |download_item|. diff --git a/chrome/browser/download/download_item.cc b/chrome/browser/download/download_item.cc index 59126a8..17d4fcc 100644 --- a/chrome/browser/download/download_item.cc +++ b/chrome/browser/download/download_item.cc @@ -14,13 +14,15 @@ #include "base/timer.h" #include "base/utf_string_conversions.h" #include "net/base/net_util.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_extensions.h" #include "chrome/browser/download/download_file_manager.h" #include "chrome/browser/download/download_history.h" #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_prefs.h" +#include "chrome/browser/download/download_state_info.h" #include "chrome/browser/download/download_util.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/platform_util.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" @@ -114,14 +116,11 @@ DownloadItem::DangerType GetDangerType(bool dangerous_file, // Constructor for reading from the history service. DownloadItem::DownloadItem(DownloadManager* download_manager, - const DownloadCreateInfo& info) - : id_(-1), + const DownloadHistoryInfo& info) + : download_id_(-1), full_path_(info.path), - path_uniquifier_(0), - url_chain_(info.url_chain), + url_chain_(1, info.url), referrer_url_(info.referrer_url), - mime_type_(info.mime_type), - original_mime_type_(info.original_mime_type), total_bytes_(info.total_bytes), received_bytes_(info.received_bytes), start_tick_(base::TimeTicks()), @@ -132,12 +131,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, is_paused_(false), open_when_complete_(false), safety_state_(SAFE), - danger_type_(NOT_DANGEROUS), auto_opened_(false), - target_name_(info.original_name), - save_as_(false), is_otr_(false), - is_extension_install_(info.is_extension_install), is_temporary_(false), all_data_saved_(false), opened_(false) { @@ -152,13 +147,19 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, DownloadItem::DownloadItem(DownloadManager* download_manager, const DownloadCreateInfo& info, bool is_otr) - : id_(info.download_id), + : state_info_(info.original_name, info.save_info.file_path, + info.has_user_gesture, info.prompt_user_for_save_location, + info.path_uniquifier, info.is_dangerous_file, + info.is_dangerous_url, info.is_extension_install), + process_handle_(info.process_handle), + download_id_(info.download_id), full_path_(info.path), - path_uniquifier_(info.path_uniquifier), url_chain_(info.url_chain), referrer_url_(info.referrer_url), + content_disposition_(info.content_disposition), mime_type_(info.mime_type), original_mime_type_(info.original_mime_type), + referrer_charset_(info.referrer_charset), total_bytes_(info.total_bytes), received_bytes_(0), last_os_error_(0), @@ -171,14 +172,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, open_when_complete_(false), safety_state_(GetSafetyState(info.is_dangerous_file, info.is_dangerous_url)), - danger_type_(GetDangerType(info.is_dangerous_file, - info.is_dangerous_url)), auto_opened_(false), - target_name_(info.original_name), - process_handle_(info.process_handle), - save_as_(info.prompt_user_for_save_location), is_otr_(is_otr), - is_extension_install_(info.is_extension_install), is_temporary_(!info.save_info.file_path.empty()), all_data_saved_(false), opened_(false) { @@ -190,13 +185,10 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, const FilePath& path, const GURL& url, bool is_otr) - : id_(1), + : download_id_(1), full_path_(path), - path_uniquifier_(0), url_chain_(1, url), referrer_url_(GURL()), - mime_type_(std::string()), - original_mime_type_(std::string()), total_bytes_(0), received_bytes_(0), last_os_error_(0), @@ -208,11 +200,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, is_paused_(false), open_when_complete_(false), safety_state_(SAFE), - danger_type_(NOT_DANGEROUS), auto_opened_(false), - save_as_(false), is_otr_(is_otr), - is_extension_install_(false), is_temporary_(false), all_data_saved_(false), opened_(false) { @@ -237,7 +226,7 @@ void DownloadItem::UpdateObservers() { } bool DownloadItem::CanOpenDownload() { - return !Extension::IsExtension(target_name_); + return !Extension::IsExtension(state_info_.target_name); } bool DownloadItem::ShouldOpenFileBasedOnExtension() { @@ -290,7 +279,7 @@ void DownloadItem::ShowDownloadInShell() { void DownloadItem::DangerousDownloadValidated() { UMA_HISTOGRAM_ENUMERATION("Download.DangerousDownloadValidated", - danger_type_, + GetDangerType(), DANGEROUS_TYPE_MAX); download_manager_->DangerousDownloadValidated(this); } @@ -340,7 +329,7 @@ void DownloadItem::Cancel(bool update_history) { UpdateObservers(); StopProgressTimer(); if (update_history) - download_manager_->DownloadCancelled(id_); + download_manager_->DownloadCancelled(download_id_); } void DownloadItem::MarkAsComplete() { @@ -398,11 +387,11 @@ void DownloadItem::Interrupted(int64 size, int os_error) { void DownloadItem::Delete(DeleteReason reason) { switch (reason) { case DELETE_DUE_TO_USER_DISCARD: - UMA_HISTOGRAM_ENUMERATION("Download.UserDiscard", danger_type_, + UMA_HISTOGRAM_ENUMERATION("Download.UserDiscard", GetDangerType(), DANGEROUS_TYPE_MAX); break; case DELETE_DUE_TO_BROWSER_SHUTDOWN: - UMA_HISTOGRAM_ENUMERATION("Download.Discard", danger_type_, + UMA_HISTOGRAM_ENUMERATION("Download.Discard", GetDangerType(), DANGEROUS_TYPE_MAX); break; default: @@ -430,8 +419,8 @@ bool DownloadItem::TimeRemaining(base::TimeDelta* remaining) const { if (speed == 0) return false; - *remaining = - base::TimeDelta::FromSeconds((total_bytes_ - received_bytes_) / speed); + *remaining = base::TimeDelta::FromSeconds( + (total_bytes_ - received_bytes_) / speed); return true; } @@ -445,7 +434,9 @@ int64 DownloadItem::CurrentSpeed() const { int DownloadItem::PercentComplete() const { return (total_bytes_ > 0) ? - static_cast<int>(received_bytes_ * 100.0 / total_bytes_) : -1; + static_cast<int>(received_bytes_ * 100.0 / + total_bytes_) : + -1; } void DownloadItem::Rename(const FilePath& full_path) { @@ -458,7 +449,7 @@ void DownloadItem::Rename(const FilePath& full_path) { void DownloadItem::TogglePause() { DCHECK(IsInProgress()); - download_manager_->PauseDownload(id_, !is_paused_); + download_manager_->PauseDownload(download_id_, !is_paused_); is_paused_ = !is_paused_; UpdateObservers(); } @@ -487,7 +478,7 @@ void DownloadItem::OnDownloadCompleting(DownloadFileManager* file_manager) { void DownloadItem::OnDownloadRenamedToFinalName(const FilePath& full_path) { VLOG(20) << __FUNCTION__ << "()" - << " full_path = " << full_path.value() + << " full_path = \"" << full_path.value() << "\"" << " needed rename = " << NeedsRename() << " " << DebugString(false); DCHECK(NeedsRename()); @@ -503,7 +494,7 @@ bool DownloadItem::MatchesQuery(const string16& query) const { DCHECK_EQ(query, base::i18n::ToLower(query)); - string16 url_raw(base::i18n::ToLower(UTF8ToUTF16(url().spec()))); + string16 url_raw(base::i18n::ToLower(UTF8ToUTF16(GetURL().spec()))); if (url_raw.find(query) != string16::npos) return true; @@ -514,7 +505,8 @@ bool DownloadItem::MatchesQuery(const string16& query) const { // "/%E4%BD%A0%E5%A5%BD%E4%BD%A0%E5%A5%BD" PrefService* prefs = download_manager_->profile()->GetPrefs(); std::string languages(prefs->GetString(prefs::kAcceptLanguages)); - string16 url_formatted(base::i18n::ToLower(net::FormatUrl(url(), languages))); + string16 url_formatted( + base::i18n::ToLower(net::FormatUrl(GetURL(), languages))); if (url_formatted.find(query) != string16::npos) return true; @@ -526,50 +518,55 @@ bool DownloadItem::MatchesQuery(const string16& query) const { return (path.find(query) != string16::npos); } -void DownloadItem::SetFileCheckResults(const FilePath& path, - bool is_dangerous_file, - bool is_dangerous_url, - int path_uniquifier, - bool prompt, - bool is_extension_install, - const FilePath& original_name) { - VLOG(20) << " " << __FUNCTION__ << "()" - << " path = \"" << path.value() << "\"" - << " is_dangerous_file = " << is_dangerous_file - << " is_dangerous_url = " << is_dangerous_url - << " path_uniquifier = " << path_uniquifier - << " prompt = " << prompt - << " is_extension_install = " << is_extension_install - << " path = \"" << path.value() << "\"" - << " original_name = \"" << original_name.value() << "\"" - << " " << DebugString(true); - // Make sure the initial file name is set only once. - DCHECK(full_path_.empty()); - DCHECK(!path.empty()); +void DownloadItem::SetFileCheckResults(const DownloadStateInfo& state) { + VLOG(20) << " " << __FUNCTION__ << "()" << " this = " << DebugString(true); + state_info_ = state; + VLOG(20) << " " << __FUNCTION__ << "()" << " this = " << DebugString(true); + + safety_state_ = GetSafetyState(state_info_.is_dangerous_file, + state_info_.is_dangerous_url); +} + +void DownloadItem::UpdateTarget() { + if (state_info_.target_name.value().empty()) + state_info_.target_name = full_path_.BaseName(); +} + +DownloadItem::DangerType DownloadItem::GetDangerType() const { + return ::GetDangerType(state_info_.is_dangerous_file, + state_info_.is_dangerous_url); +} + +bool DownloadItem::IsDangerous() const { + return GetDangerType() != DownloadItem::NOT_DANGEROUS; +} - full_path_ = path; - safety_state_ = GetSafetyState(is_dangerous_file, is_dangerous_url); - danger_type_ = GetDangerType(is_dangerous_file, is_dangerous_url); - path_uniquifier_ = path_uniquifier; - save_as_ = prompt; - is_extension_install_ = is_extension_install; - target_name_ = original_name; +void DownloadItem::MarkUrlDangerous() { + state_info_.is_dangerous_url = true; +} - if (target_name_.value().empty()) - target_name_ = full_path_.BaseName(); +DownloadHistoryInfo DownloadItem::GetHistoryInfo() const { + return DownloadHistoryInfo(full_path(), + GetURL(), + referrer_url(), + start_time(), + received_bytes(), + total_bytes(), + state(), + db_handle()); } FilePath DownloadItem::GetTargetFilePath() const { - return full_path_.DirName().Append(target_name_); + return full_path_.DirName().Append(state_info_.target_name); } FilePath DownloadItem::GetFileNameToReportUser() const { - if (path_uniquifier_ > 0) { - FilePath name(target_name_); - download_util::AppendNumberToPath(&name, path_uniquifier_); + if (state_info_.path_uniquifier > 0) { + FilePath name(state_info_.target_name); + download_util::AppendNumberToPath(&name, state_info_.path_uniquifier); return name; } - return target_name_; + return state_info_.target_name; } FilePath DownloadItem::GetUserVerifiedFilePath() const { @@ -578,8 +575,7 @@ FilePath DownloadItem::GetUserVerifiedFilePath() const { } void DownloadItem::Init(bool start_timer) { - if (target_name_.value().empty()) - target_name_ = full_path_.BaseName(); + UpdateTarget(); if (start_timer) StartProgressTimer(); VLOG(20) << __FUNCTION__ << "() " << DebugString(true); @@ -596,7 +592,8 @@ bool DownloadItem::IsInProgress() const { } bool DownloadItem::IsCancelled() const { - return (state_ == CANCELLED) || (state_ == INTERRUPTED); + return (state_ == CANCELLED) || + (state_ == INTERRUPTED); } bool DownloadItem::IsInterrupted() const { @@ -607,9 +604,17 @@ bool DownloadItem::IsComplete() const { return (state_ == COMPLETE); } +const GURL& DownloadItem::GetURL() const { + return url_chain_.empty() ? + GURL::EmptyGURL() : url_chain_.back(); +} + std::string DownloadItem::DebugString(bool verbose) const { - std::string description = base::StringPrintf( - "{ id_ = %d state = %s", id_, DebugDownloadStateString(state())); + std::string description = + base::StringPrintf("{ id = %d" + " state = %s", + download_id_, + DebugDownloadStateString(state())); // Construct a string of the URL chain. std::string url_list("<none>"); @@ -619,7 +624,7 @@ std::string DownloadItem::DebugString(bool verbose) const { url_list = (*iter).spec(); ++iter; for ( ; verbose && (iter != last); ++iter) { - url_list += " -> "; + url_list += " ->\n\t"; const GURL& next_url = *iter; url_list += next_url.spec(); } @@ -629,12 +634,12 @@ std::string DownloadItem::DebugString(bool verbose) const { description += base::StringPrintf( " db_handle = %" PRId64 " total_bytes = %" PRId64 - " is_paused = " "%c" - " is_extension_install = " "%c" - " is_otr = " "%c" - " safety_state = " "%s" - " url_chain = " "\"%s\"" - " target_name_ = \"%" PRFilePath "\"" + " is_paused = %c" + " is_extension_install = %c" + " is_otr = %c" + " safety_state = %s" + " url_chain = \n\t\"%s\"\n\t" + " target_name = \"%" PRFilePath "\"" " full_path = \"%" PRFilePath "\"", db_handle(), total_bytes(), @@ -643,10 +648,13 @@ std::string DownloadItem::DebugString(bool verbose) const { is_otr() ? 'T' : 'F', DebugSafetyStateString(safety_state()), url_list.c_str(), - target_name_.value().c_str(), + state_info_.target_name.value().c_str(), full_path().value().c_str()); } else { description += base::StringPrintf(" url = \"%s\"", url_list.c_str()); } + + description += " }"; + return description; } diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h index 3a6bbd7..59b23fc 100644 --- a/chrome/browser/download/download_item.h +++ b/chrome/browser/download/download_item.h @@ -26,11 +26,13 @@ #include "base/time.h" #include "base/timer.h" #include "chrome/browser/download/download_process_handle.h" +#include "chrome/browser/download/download_state_info.h" #include "googleurl/src/gurl.h" class DownloadFileManager; class DownloadManager; struct DownloadCreateInfo; +struct DownloadHistoryInfo; // One DownloadItem per download. This is the model class that stores all the // state for a download. Multiple views, such as a tab's download shelf and the @@ -41,7 +43,7 @@ class DownloadItem { public: enum DownloadState { // Download is actively progressing. - IN_PROGRESS, + IN_PROGRESS = 0, // Download is completely finished. COMPLETE, @@ -54,7 +56,10 @@ class DownloadItem { REMOVING, // This state indicates that the download has been interrupted. - INTERRUPTED + INTERRUPTED, + + // Maximum value. + MAX_DOWNLOAD_STATE }; enum SafetyState { @@ -101,7 +106,7 @@ class DownloadItem { // Constructing from persistent store: DownloadItem(DownloadManager* download_manager, - const DownloadCreateInfo& info); + const DownloadHistoryInfo& info); // Constructing for a regular download: DownloadItem(DownloadManager* download_manager, @@ -122,7 +127,7 @@ class DownloadItem { // Notifies our observers periodically. void UpdateObservers(); - // Whether it is OK to open this download. + // Returns true if it is OK to open this download. bool CanOpenDownload(); // Tests if a file type should be opened automatically. @@ -191,19 +196,19 @@ class DownloadItem { // total size). int PercentComplete() const; - // Whether or not this download has saved all of its data. + // Called when the final path has been determined. + void OnPathDetermined(const FilePath& path) { full_path_ = path; } + + // Returns true if this download has saved all of its data. bool all_data_saved() const { return all_data_saved_; } - // Update the fields that may have changed in DownloadCreateInfo as a + // Update the fields that may have changed in DownloadStateInfo as a // result of analyzing the file and figuring out its type, location, etc. // May only be called once. - void SetFileCheckResults(const FilePath& path, - bool is_dangerous_file, - bool is_dangerous_url, - int path_uniquifier, - bool prompt, - bool is_extension_install, - const FilePath& original_name); + void SetFileCheckResults(const DownloadStateInfo& state); + + // Updates the target file. + void UpdateTarget(); // Update the download's path, the actual file is renamed on the download // thread. @@ -241,17 +246,24 @@ class DownloadItem { // Accessors DownloadState state() const { return state_; } FilePath full_path() const { return full_path_; } - void set_path_uniquifier(int uniquifier) { path_uniquifier_ = uniquifier; } - const GURL& url() const { return url_chain_.back(); } + void set_path_uniquifier(int uniquifier) { + state_info_.path_uniquifier = uniquifier; + } + const GURL& GetURL() const; + const std::vector<GURL>& url_chain() const { return url_chain_; } const GURL& original_url() const { return url_chain_.front(); } const GURL& referrer_url() const { return referrer_url_; } + std::string content_disposition() const { return content_disposition_; } std::string mime_type() const { return mime_type_; } std::string original_mime_type() const { return original_mime_type_; } + std::string referrer_charset() const { return referrer_charset_; } int64 total_bytes() const { return total_bytes_; } - void set_total_bytes(int64 total_bytes) { total_bytes_ = total_bytes; } + void set_total_bytes(int64 total_bytes) { + total_bytes_ = total_bytes; + } int64 received_bytes() const { return received_bytes_; } - int32 id() const { return id_; } + int32 id() const { return download_id_; } base::Time start_time() const { return start_time_; } void set_db_handle(int64 handle) { db_handle_ = handle; } int64 db_handle() const { return db_handle_; } @@ -262,16 +274,25 @@ class DownloadItem { void set_safety_state(SafetyState safety_state) { safety_state_ = safety_state; } - DangerType danger_type() { return danger_type_;} + // Why |safety_state_| is not SAFE. + DangerType GetDangerType() const; + bool IsDangerous() const; + void MarkUrlDangerous(); + bool auto_opened() { return auto_opened_; } - FilePath target_name() const { return target_name_; } - bool save_as() const { return save_as_; } + FilePath target_name() const { return state_info_.target_name; } + bool save_as() const { return state_info_.prompt_user_for_save_location; } bool is_otr() const { return is_otr_; } - bool is_extension_install() const { return is_extension_install_; } + bool is_extension_install() const { + return state_info_.is_extension_install; + } + FilePath suggested_path() const { return state_info_.suggested_path; } bool is_temporary() const { return is_temporary_; } void set_opened(bool opened) { opened_ = opened; } bool opened() const { return opened_; } + DownloadHistoryInfo GetHistoryInfo() const; + DownloadStateInfo state_info() const { return state_info_; } const DownloadProcessHandle& process_handle() const { return process_handle_; } @@ -280,7 +301,7 @@ class DownloadItem { FilePath GetTargetFilePath() const; // Returns the file-name that should be reported to the user, which is - // target_name_ possibly with the uniquifier number. + // target_name possibly with the uniquifier number. FilePath GetFileNameToReportUser() const; // Returns the user-verified target file path for the download. @@ -290,7 +311,7 @@ class DownloadItem { // Returns true if the current file name is not the final target name yet. bool NeedsRename() const { - return target_name_ != full_path_.BaseName(); + return state_info_.target_name != full_path_.BaseName(); } std::string DebugString(bool verbose) const; @@ -309,8 +330,15 @@ class DownloadItem { void StartProgressTimer(); void StopProgressTimer(); - // Request ID assigned by the ResourceDispatcherHost. - int32 id_; + // State information used by the download manager. + DownloadStateInfo state_info_; + + // The handle to the process information. Used for operations outside the + // download system. + DownloadProcessHandle process_handle_; + + // Download ID assigned by DownloadResourceHandler. + int32 download_id_; // Full path to the downloaded or downloading file. FilePath full_path_; @@ -325,13 +353,22 @@ class DownloadItem { // The URL of the page that initiated the download. GURL referrer_url_; - // The mimetype of the download + // Information from the request. + // Content-disposition field from the header. + std::string content_disposition_; + + // Mime-type from the header. Subject to change. std::string mime_type_; - // The value of the content type header received when downloading - // this item. |mime_type_| may be different because of type sniffing. + // The value of the content type header sent with the downloaded item. It + // may be different from |mime_type_|, which may be set based on heuristics + // which may look at the file extension and first few bytes of the file. std::string original_mime_type_; + // The charset of the referring page where the download request comes from. + // It's used to construct a suggested filename. + std::string referrer_charset_; + // Total bytes expected int64 total_bytes_; @@ -368,36 +405,18 @@ class DownloadItem { // A flag for indicating if the download should be opened at completion. bool open_when_complete_; - // Whether the download is considered potentially safe or dangerous + // Indicates if the download is considered potentially safe or dangerous // (executable files are typically considered dangerous). SafetyState safety_state_; - // Why |safety_state_| is not SAFE. - DangerType danger_type_; - - // Whether the download was auto-opened. We set this rather than using + // True if the download was auto-opened. We set this rather than using // an observer as it's frequently possible for the download to be auto opened // before the observer is added. bool auto_opened_; - // Dangerous downloads or ongoing downloads are given temporary names until - // the user approves them or the downloads finish. - // This stores their final target name. - FilePath target_name_; - - // The handle to the process information. Used for operations outside the - // download system. - DownloadProcessHandle process_handle_; - - // True if the item was downloaded as a result of 'save as...' - bool save_as_; - // True if the download was initiated in an incognito window. bool is_otr_; - // True if the item was downloaded for an extension installation. - bool is_extension_install_; - // True if the item was downloaded temporarily. bool is_temporary_; diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc index 54d937b..899fe65 100644 --- a/chrome/browser/download/download_manager.cc +++ b/chrome/browser/download/download_manager.cc @@ -17,6 +17,7 @@ #include "base/utf_string_conversions.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_extensions.h" #include "chrome/browser/download/download_file_manager.h" #include "chrome/browser/download/download_history.h" @@ -27,7 +28,7 @@ #include "chrome/browser/download/download_status_updater.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/platform_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/tab_contents/tab_util.h" @@ -253,36 +254,51 @@ bool DownloadManager::Init(Profile* profile) { // observers at this point. OnCreateDownloadEntryComplete() handles that // finalization of the the download creation as a callback from the // history thread. -void DownloadManager::StartDownload(DownloadCreateInfo* info) { +void DownloadManager::StartDownload(int32 download_id) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DownloadItem* download = GetActiveDownloadItem(download_id); + if (!download) + return; + // Create a client to verify download URL with safebrowsing. // It deletes itself after the callback. scoped_refptr<DownloadSBClient> sb_client = new DownloadSBClient( - info->download_id, info->url_chain, info->referrer_url); + download_id, download->url_chain(), download->referrer_url()); sb_client->CheckDownloadUrl( - info, NewCallback(this, &DownloadManager::CheckDownloadUrlDone)); + NewCallback(this, &DownloadManager::CheckDownloadUrlDone)); } -void DownloadManager::CheckDownloadUrlDone(DownloadCreateInfo* info, +void DownloadManager::CheckDownloadUrlDone(int32 download_id, bool is_dangerous_url) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(info); - info->is_dangerous_url = is_dangerous_url; + DownloadItem* download = GetActiveDownloadItem(download_id); + if (!download) + return; + + if (is_dangerous_url) + download->MarkUrlDangerous(); + + DownloadStateInfo state = download->state_info(); // Check whether this download is for an extension install or not. // Allow extensions to be explicitly saved. - if (!info->prompt_user_for_save_location) { - if (UserScript::IsURLUserScript(info->url(), info->mime_type) || - info->mime_type == Extension::kMimeType) { - info->is_extension_install = true; + if (!state.prompt_user_for_save_location) { + if (UserScript::IsURLUserScript(download->GetURL(), + download->mime_type()) || + (download->mime_type() == Extension::kMimeType)) { + state.is_extension_install = true; } } - if (info->save_info.file_path.empty()) { + if (state.force_file_name.empty()) { FilePath generated_name; - download_util::GenerateFileNameFromInfo(info, &generated_name); + download_util::GenerateFileNameFromRequest(download->GetURL(), + download->content_disposition(), + download->referrer_charset(), + download->mime_type(), + &generated_name); // Freeze the user's preference for showing a Save As dialog. We're going // to bounce around a bunch of threads and we don't want to worry about race @@ -294,32 +310,29 @@ void DownloadManager::CheckDownloadUrlDone(DownloadCreateInfo* info, // "save as...". // 2) Filetypes marked "always open." If the user just wants this file // opened, don't bother asking where to keep it. - if (!info->is_extension_install && + if (!state.is_extension_install && !ShouldOpenFileBasedOnExtension(generated_name)) - info->prompt_user_for_save_location = true; + state.prompt_user_for_save_location = true; } if (download_prefs_->IsDownloadPathManaged()) { - info->prompt_user_for_save_location = false; + state.prompt_user_for_save_location = false; } // Determine the proper path for a download, by either one of the following: // 1) using the default download directory. // 2) prompting the user. - if (info->prompt_user_for_save_location && !last_download_path_.empty()) { - info->suggested_path = last_download_path_; + if (state.prompt_user_for_save_location && !last_download_path_.empty()) { + state.suggested_path = last_download_path_; } else { - info->suggested_path = download_prefs_->download_path(); + state.suggested_path = download_prefs_->download_path(); } - info->suggested_path = info->suggested_path.Append(generated_name); + state.suggested_path = state.suggested_path.Append(generated_name); } else { - info->suggested_path = info->save_info.file_path; + state.suggested_path = state.force_file_name; } - if (!info->prompt_user_for_save_location && - info->save_info.file_path.empty()) { - info->is_dangerous_file = download_util::IsDangerous( - info, profile(), ShouldOpenFileBasedOnExtension(info->suggested_path)); - } + if (!state.prompt_user_for_save_location && state.force_file_name.empty()) + state.is_dangerous_file = IsDangerous(*download, state); // We need to move over to the download thread because we don't want to stat // the suggested path on the UI thread. @@ -330,14 +343,15 @@ void DownloadManager::CheckDownloadUrlDone(DownloadCreateInfo* info, NewRunnableMethod( this, &DownloadManager::CheckIfSuggestedPathExists, - info, + download_id, + state, download_prefs()->download_path())); } -void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info, +void DownloadManager::CheckIfSuggestedPathExists(int32 download_id, + DownloadStateInfo state, const FilePath& default_path) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DCHECK(info); // Make sure the default download directory exists. // TODO(phajdan.jr): only create the directory when we're sure the user @@ -346,18 +360,18 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info, // Check writability of the suggested path. If we can't write to it, default // to the user's "My Documents" directory. We'll prompt them in this case. - FilePath dir = info->suggested_path.DirName(); - FilePath filename = info->suggested_path.BaseName(); + FilePath dir = state.suggested_path.DirName(); + FilePath filename = state.suggested_path.BaseName(); if (!file_util::PathIsWritable(dir)) { VLOG(1) << "Unable to write to directory \"" << dir.value() << "\""; - info->prompt_user_for_save_location = true; - PathService::Get(chrome::DIR_USER_DOCUMENTS, &info->suggested_path); - info->suggested_path = info->suggested_path.Append(filename); + state.prompt_user_for_save_location = true; + PathService::Get(chrome::DIR_USER_DOCUMENTS, &state.suggested_path); + state.suggested_path = state.suggested_path.Append(filename); } // If the download is deemed dangerous, we'll use a temporary name for it. - if (info->IsDangerous()) { - info->original_name = FilePath(info->suggested_path).BaseName(); + if (state.IsDangerous()) { + state.target_name = FilePath(state.suggested_path).BaseName(); // Create a temporary file to hold the file until the user approves its // download. FilePath::StringType file_name; @@ -380,61 +394,73 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info, if (file_util::PathExists(path)) path = FilePath(); } - info->suggested_path = path; + state.suggested_path = path; } else { // Do not add the path uniquifier if we are saving to a specific path as in // the drag-out case. - if (info->save_info.file_path.empty()) { - info->path_uniquifier = download_util::GetUniquePathNumberWithCrDownload( - info->suggested_path); + if (state.force_file_name.empty()) { + state.path_uniquifier = download_util::GetUniquePathNumberWithCrDownload( + state.suggested_path); } // We know the final path, build it if necessary. - if (info->path_uniquifier > 0) { - download_util::AppendNumberToPath(&(info->suggested_path), - info->path_uniquifier); + if (state.path_uniquifier > 0) { + download_util::AppendNumberToPath(&(state.suggested_path), + state.path_uniquifier); // Setting path_uniquifier to 0 to make sure we don't try to unique it // later on. - info->path_uniquifier = 0; - } else if (info->path_uniquifier == -1) { + state.path_uniquifier = 0; + } else if (state.path_uniquifier == -1) { // We failed to find a unique path. We have to prompt the user. VLOG(1) << "Unable to find a unique path for suggested path \"" - << info->suggested_path.value() << "\""; - info->prompt_user_for_save_location = true; + << state.suggested_path.value() << "\""; + state.prompt_user_for_save_location = true; } } // Create an empty file at the suggested path so that we don't allocate the // same "non-existant" path to multiple downloads. // See: http://code.google.com/p/chromium/issues/detail?id=3662 - if (!info->prompt_user_for_save_location && - info->save_info.file_path.empty()) { - if (info->IsDangerous()) - file_util::WriteFile(info->suggested_path, "", 0); + if (!state.prompt_user_for_save_location && + state.force_file_name.empty()) { + if (state.IsDangerous()) + file_util::WriteFile(state.suggested_path, "", 0); else file_util::WriteFile(download_util::GetCrDownloadPath( - info->suggested_path), "", 0); + state.suggested_path), "", 0); } BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, NewRunnableMethod(this, &DownloadManager::OnPathExistenceAvailable, - info)); + download_id, + state)); } -void DownloadManager::OnPathExistenceAvailable(DownloadCreateInfo* info) { - VLOG(20) << __FUNCTION__ << "()" << " info = " << info->DebugString(); +void DownloadManager::OnPathExistenceAvailable( + int32 download_id, DownloadStateInfo new_state) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(info); - if (info->prompt_user_for_save_location) { + DownloadItem* download = GetActiveDownloadItem(download_id); + if (!download) + return; + + VLOG(20) << __FUNCTION__ << "()" + << " download = " << download->DebugString(true); + + download->SetFileCheckResults(new_state); + + FilePath suggested_path = download->suggested_path(); + + if (download->save_as()) { // We must ask the user for the place to put the download. if (!select_file_dialog_.get()) select_file_dialog_ = SelectFileDialog::Create(this); - TabContents* contents = info->process_handle.GetTabContents(); + DownloadProcessHandle process_handle = download->process_handle(); + TabContents* contents = process_handle.GetTabContents(); SelectFileDialog::FileTypeInfo file_type_info; - FilePath::StringType extension = info->suggested_path.Extension(); + FilePath::StringType extension = suggested_path.Extension(); if (!extension.empty()) { extension.erase(extension.begin()); // drop the . file_type_info.extensions.resize(1); @@ -443,17 +469,21 @@ void DownloadManager::OnPathExistenceAvailable(DownloadCreateInfo* info) { file_type_info.include_all_files = true; gfx::NativeWindow owning_window = contents ? platform_util::GetTopLevel(contents->GetNativeView()) : NULL; + // |id_ptr| will be deleted in either FileSelected() or + // FileSelectionCancelled(). + int32* id_ptr = new int32; + *id_ptr = download_id; select_file_dialog_->SelectFile(SelectFileDialog::SELECT_SAVEAS_FILE, string16(), - info->suggested_path, + suggested_path, &file_type_info, 0, FILE_PATH_LITERAL(""), - contents, owning_window, info); + contents, owning_window, + reinterpret_cast<void*>(id_ptr)); FOR_EACH_OBSERVER(Observer, observers_, - SelectFileDialogDisplayed(info->download_id)); + SelectFileDialogDisplayed(download_id)); } else { // No prompting for download, just continue with the suggested name. - info->path = info->suggested_path; - AttachDownloadItem(info); + ContinueDownloadWithPath(download, suggested_path); } } @@ -462,52 +492,52 @@ void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info) { DownloadItem* download = new DownloadItem(this, *info, profile_->IsOffTheRecord()); - DCHECK(!ContainsKey(in_progress_, info->download_id)); - DCHECK(!ContainsKey(active_downloads_, info->download_id)); + int32 download_id = info->download_id; + DCHECK(!ContainsKey(in_progress_, download_id)); + DCHECK(!ContainsKey(active_downloads_, download_id)); downloads_.insert(download); - active_downloads_[info->download_id] = download; + active_downloads_[download_id] = download; } -void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info) { - VLOG(20) << __FUNCTION__ << "()" << " info = " << info->DebugString(); - +void DownloadManager::ContinueDownloadWithPath(DownloadItem* download, + const FilePath& chosen_file) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(download); - // Life of |info| ends here. No more references to it after this method. - scoped_ptr<DownloadCreateInfo> infop(info); + int32 download_id = download->id(); // NOTE(ahendrickson) Eventually |active_downloads_| will replace // |in_progress_|, but we don't want to change the semantics yet. - DCHECK(!ContainsKey(in_progress_, info->download_id)); - DCHECK(ContainsKey(active_downloads_, info->download_id)); - DownloadItem* download = active_downloads_[info->download_id]; - DCHECK(download != NULL); + DCHECK(!ContainsKey(in_progress_, download_id)); DCHECK(ContainsKey(downloads_, download)); + DCHECK(ContainsKey(active_downloads_, download_id)); - download->SetFileCheckResults(info->path, - info->is_dangerous_file, - info->is_dangerous_url, - info->path_uniquifier, - info->prompt_user_for_save_location, - info->is_extension_install, - info->original_name); - in_progress_[info->download_id] = download; + // Make sure the initial file name is set only once. + DCHECK(download->full_path().empty()); + download->OnPathDetermined(chosen_file); + download->UpdateTarget(); + + VLOG(20) << __FUNCTION__ << "()" + << " download = " << download->DebugString(true); + + in_progress_[download_id] = download; UpdateAppIcon(); // Reflect entry into in_progress_. // Rename to intermediate name. FilePath download_path; - if (info->IsDangerous()) { + if (download->IsDangerous()) { // The download is not safe. We can now rename the file to its // tentative name using RenameInProgressDownloadFile. // NOTE: The |Rename| below will be a no-op for dangerous files, as we're // renaming it to the same name. - download_path = info->path; + download_path = download->full_path(); } else { // The download is a safe download. We need to // rename it to its intermediate '.crdownload' path. The final // name after user confirmation will be set from // DownloadItem::OnDownloadCompleting. - download_path = download_util::GetCrDownloadPath(info->path); + download_path = + download_util::GetCrDownloadPath(download->full_path()); } BrowserThread::PostTask( @@ -518,7 +548,7 @@ void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info) { download->Rename(download_path); - download_history_->AddEntry(*info, download, + download_history_->AddEntry(download, NewCallback(this, &DownloadManager::OnCreateDownloadEntryComplete)); } @@ -591,7 +621,7 @@ void DownloadManager::CheckDownloadHashDone(int32 download_id, return; DVLOG(1) << "CheckDownloadHashDone, url: " - << active_downloads_[download_id]->url().spec(); + << active_downloads_[download_id]->GetURL().spec(); } bool DownloadManager::IsDownloadReadyForCompletion(DownloadItem* download) { @@ -732,9 +762,11 @@ void DownloadManager::OnDownloadError(int32 download_id, DownloadItem* download = it->second; - VLOG(20) << "Error " << os_error << " at offset " - << download->received_bytes() << " for download = " - << download->DebugString(true); + VLOG(20) << __FUNCTION__ << "()" << " Error " << os_error + << " at offset " << download->received_bytes() + << " for download = " << download->DebugString(true); + + download->Interrupted(size, os_error); // TODO(ahendrickson) - Remove this when we add resuming of interrupted // downloads, as we will keep the download item around in that case. @@ -748,8 +780,6 @@ void DownloadManager::OnDownloadError(int32 download_id, download_history_->UpdateEntry(download); } - download->Interrupted(size, os_error); - BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, NewRunnableMethod( @@ -966,19 +996,69 @@ int64 DownloadManager::GetTotalDownloadBytes() { void DownloadManager::FileSelected(const FilePath& path, int index, void* params) { - DownloadCreateInfo* info = reinterpret_cast<DownloadCreateInfo*>(params); - if (info->prompt_user_for_save_location) + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + int32* id_ptr = reinterpret_cast<int32*>(params); + DCHECK(id_ptr != NULL); + int32 download_id = *id_ptr; + delete id_ptr; + + DownloadItem* download = GetActiveDownloadItem(download_id); + if (!download) + return; + VLOG(20) << __FUNCTION__ << "()" << " path = \"" << path.value() << "\"" + << " download = " << download->DebugString(true); + + if (download->save_as()) last_download_path_ = path.DirName(); - info->path = path; - AttachDownloadItem(info); + // Make sure the initial file name is set only once. + ContinueDownloadWithPath(download, path); } void DownloadManager::FileSelectionCanceled(void* params) { // The user didn't pick a place to save the file, so need to cancel the // download that's already in progress to the temporary location. - DownloadCreateInfo* info = reinterpret_cast<DownloadCreateInfo*>(params); - DownloadCancelledInternal(info->download_id, info->process_handle); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + int32* id_ptr = reinterpret_cast<int32*>(params); + DCHECK(id_ptr != NULL); + int32 download_id = *id_ptr; + delete id_ptr; + + DownloadItem* download = GetActiveDownloadItem(download_id); + if (!download) + return; + + VLOG(20) << __FUNCTION__ << "()" + << " download = " << download->DebugString(true); + + DownloadCancelledInternal(download_id, download->process_handle()); +} + +// TODO(phajdan.jr): This is apparently not being exercised in tests. +bool DownloadManager::IsDangerous(const DownloadItem& download, + const DownloadStateInfo& state) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + bool auto_open = ShouldOpenFileBasedOnExtension(state.suggested_path); + download_util::DownloadDangerLevel danger_level = + download_util::GetFileDangerLevel(state.suggested_path.BaseName()); + + if (danger_level == download_util::Dangerous) + return !(auto_open && state.has_user_gesture); + + if (danger_level == download_util::AllowOnUserGesture && + !state.has_user_gesture) + return true; + + if (state.is_extension_install) { + // Extensions that are not from the gallery are considered dangerous. + ExtensionService* service = profile()->GetExtensionService(); + if (!service || !service->IsDownloadFromGallery(download.GetURL(), + download.referrer_url())) + return true; + } + return false; } void DownloadManager::DangerousDownloadValidated(DownloadItem* download) { @@ -993,9 +1073,9 @@ void DownloadManager::DangerousDownloadValidated(DownloadItem* download) { // Operations posted to us from the history service ---------------------------- // The history service has retrieved all download entries. 'entries' contains -// 'DownloadCreateInfo's in sorted order (by ascending start_time). +// 'DownloadHistoryInfo's in sorted order (by ascending start_time). void DownloadManager::OnQueryDownloadEntriesComplete( - std::vector<DownloadCreateInfo>* entries) { + std::vector<DownloadHistoryInfo>* entries) { for (size_t i = 0; i < entries->size(); ++i) { DownloadItem* download = new DownloadItem(this, entries->at(i)); DCHECK(!ContainsKey(history_downloads_, download->db_handle())); @@ -1010,16 +1090,15 @@ void DownloadManager::OnQueryDownloadEntriesComplete( // Once the new DownloadItem's creation info has been committed to the history // service, we associate the DownloadItem with the db handle, update our // 'history_downloads_' map and inform observers. -void DownloadManager::OnCreateDownloadEntryComplete( - DownloadCreateInfo info, - int64 db_handle) { +void DownloadManager::OnCreateDownloadEntryComplete(int32 download_id, + int64 db_handle) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DownloadMap::iterator it = in_progress_.find(info.download_id); - DCHECK(it != in_progress_.end()); + DownloadItem* download = GetActiveDownloadItem(download_id); + if (!download) + return; - DownloadItem* download = it->second; VLOG(20) << __FUNCTION__ << "()" << " db_handle = " << db_handle - << " download_id = " << info.download_id + << " download_id = " << download_id << " download = " << download->DebugString(true); // It's not immediately obvious, but HistoryBackend::CreateDownload() can @@ -1038,7 +1117,7 @@ void DownloadManager::OnCreateDownloadEntryComplete( // Show in the appropriate browser UI. // This includes buttons to save or cancel, for a dangerous download. - ShowDownloadInBrowser(&info.process_handle, download); + ShowDownloadInBrowser(download); // Inform interested objects about the new download. NotifyModelChanged(); @@ -1055,22 +1134,20 @@ void DownloadManager::OnCreateDownloadEntryComplete( } else { DCHECK(download->IsCancelled()) << " download = " << download->DebugString(true); - in_progress_.erase(it); - active_downloads_.erase(info.download_id); + in_progress_.erase(download_id); + active_downloads_.erase(download_id); download_history_->UpdateEntry(download); download->UpdateObservers(); } } -void DownloadManager::ShowDownloadInBrowser( - DownloadProcessHandle* process_handle, DownloadItem* download) { - if (!process_handle) - return; +void DownloadManager::ShowDownloadInBrowser(DownloadItem* download) { // The 'contents' may no longer exist if the user closed the tab before we // get this start completion event. If it does, tell the origin TabContents // to display its download shelf. - TabContents* contents = process_handle->GetTabContents(); + DownloadProcessHandle process_handle = download->process_handle(); + TabContents* contents = process_handle.GetTabContents(); TabContentsWrapper* wrapper = NULL; if (contents) wrapper = TabContentsWrapper::GetCurrentWrapperForContents(contents); @@ -1099,16 +1176,25 @@ void DownloadManager::NotifyModelChanged() { FOR_EACH_OBSERVER(Observer, observers_, ModelChanged()); } -DownloadItem* DownloadManager::GetDownloadItem(int id) { +DownloadItem* DownloadManager::GetDownloadItem(int download_id) { + // The |history_downloads_| map is indexed by the download's db_handle, + // not its id, so we have to iterate. for (DownloadMap::iterator it = history_downloads_.begin(); it != history_downloads_.end(); ++it) { DownloadItem* item = it->second; - if (item->id() == id) + if (item->id() == download_id) return item; } return NULL; } +DownloadItem* DownloadManager::GetActiveDownloadItem(int download_id) { + DCHECK(ContainsKey(active_downloads_, download_id)); + DownloadItem* download = active_downloads_[download_id]; + DCHECK(download != NULL); + return download; +} + // Confirm that everything in all maps is also in |downloads_|, and that // everything in |downloads_| is also in some other map. void DownloadManager::AssertContainersConsistent() const { diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h index 5cc4e0e..f01cb62 100644 --- a/chrome/browser/download/download_manager.h +++ b/chrome/browser/download/download_manager.h @@ -41,6 +41,7 @@ #include "base/observer_list.h" #include "base/scoped_ptr.h" #include "base/time.h" +#include "chrome/browser/download/download_item.h" #include "chrome/browser/download/download_status_updater_delegate.h" #include "chrome/browser/download/download_process_handle.h" #include "chrome/browser/ui/shell_dialogs.h" @@ -48,7 +49,6 @@ class DownloadFileManager; class DownloadHistory; -class DownloadItem; class DownloadPrefs; class DownloadStatusUpdater; class GURL; @@ -56,6 +56,7 @@ class Profile; class ResourceDispatcherHost; class TabContents; struct DownloadCreateInfo; +struct DownloadHistoryInfo; struct DownloadSaveInfo; // Browser's download manager: manages all downloads and destination view. @@ -114,7 +115,7 @@ class DownloadManager bool Init(Profile* profile); // Notifications sent from the download thread to the UI thread - void StartDownload(DownloadCreateInfo* info); + void StartDownload(int32 id); void UpdateDownload(int32 download_id, int64 size); // |hash| is sha256 hash for the downloaded file. It is empty when the hash // is not available. @@ -190,13 +191,11 @@ class DownloadManager // Methods called on completion of a query sent to the history system. void OnQueryDownloadEntriesComplete( - std::vector<DownloadCreateInfo>* entries); - void OnCreateDownloadEntryComplete( - DownloadCreateInfo info, int64 db_handle); + std::vector<DownloadHistoryInfo>* entries); + void OnCreateDownloadEntryComplete(int32 download_id, int64 db_handle); // Display a new download in the appropriate browser UI. - void ShowDownloadInBrowser(DownloadProcessHandle* process_handle, - DownloadItem* download); + void ShowDownloadInBrowser(DownloadItem* download); // The number of in progress (including paused) downloads. int in_progress_count() const { @@ -228,11 +227,18 @@ class DownloadManager virtual void FileSelected(const FilePath& path, int index, void* params); virtual void FileSelectionCanceled(void* params); + // Returns true if this download should show the "dangerous file" warning. + // Various factors are considered, such as the type of the file, whether a + // user action initiated the download, and whether the user has explicitly + // marked the file type as "auto open". + bool IsDangerous(const DownloadItem& download, + const DownloadStateInfo& state); + // Called when the user has validated the download of a dangerous file. void DangerousDownloadValidated(DownloadItem* download); // Callback function after url is checked with safebrowsing service. - void CheckDownloadUrlDone(DownloadCreateInfo* info, bool is_dangerous_url); + void CheckDownloadUrlDone(int32 download_id, bool is_dangerous_url); // Callback function after download file hash is checked with safebrowsing // service. @@ -273,17 +279,20 @@ class DownloadManager // Called on the download thread to check whether the suggested file path // exists. We don't check if the file exists on the UI thread to avoid UI // stalls from interacting with the file system. - void CheckIfSuggestedPathExists(DownloadCreateInfo* info, + void CheckIfSuggestedPathExists(int32 download_id, + DownloadStateInfo state, const FilePath& default_path); // Called on the UI thread once the DownloadManager has determined whether the // suggested file path exists. - void OnPathExistenceAvailable(DownloadCreateInfo* info); + void OnPathExistenceAvailable(int32 download_id, + DownloadStateInfo new_state); // Called back after a target path for the file to be downloaded to has been // determined, either automatically based on the suggested file name, or by // the user in a Save As dialog box. - void AttachDownloadItem(DownloadCreateInfo* info); + void ContinueDownloadWithPath(DownloadItem* download, + const FilePath& chosen_file); // Download cancel helper function. // |process_handle| is passed by value because it is ultimately passed to @@ -313,8 +322,14 @@ class DownloadManager // Inform observers that the model has changed. void NotifyModelChanged(); + // Get the download item from the history map. Useful after the item has + // been removed from the active map, or was retrieved from the history DB. DownloadItem* GetDownloadItem(int id); + // Get the download item from the active map. Useful when the item is not + // yet in the history map. + DownloadItem* GetActiveDownloadItem(int id); + // Debugging routine to confirm relationship between below // containers; no-op if NDEBUG. void AssertContainersConsistent() const; diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc index 271b1f9..fe1376c 100644 --- a/chrome/browser/download/download_manager_unittest.cc +++ b/chrome/browser/download/download_manager_unittest.cc @@ -10,6 +10,7 @@ #include "base/stl_util-inl.h" #include "base/string_util.h" #include "build/build_config.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_file.h" #include "chrome/browser/download/download_file_manager.h" #include "chrome/browser/download/download_item.h" @@ -18,7 +19,6 @@ #include "chrome/browser/download/download_status_updater.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/download/mock_download_manager.h" -#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/common/pref_names.h" #include "chrome/test/testing_profile.h" @@ -61,8 +61,8 @@ class DownloadManagerTest : public testing::Test { download_manager_->FileSelected(path, index, params); } - void AttachDownloadItem(DownloadCreateInfo* info) { - download_manager_->AttachDownloadItem(info); + void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) { + download_manager_->ContinueDownloadWithPath(download, path); } void OnDownloadError(int32 download_id, int64 size, int os_error) { @@ -312,11 +312,11 @@ TEST_F(DownloadManagerTest, StartDownload) { DownloadFile* download_file(new DownloadFile(info, download_manager_)); AddDownloadToFileManager(info->download_id, download_file); download_file->Initialize(false); - download_manager_->StartDownload(info); + download_manager_->StartDownload(info->download_id); message_loop_.RunAllPending(); - // NOTE: At this point, |AttachDownloadItem| will have been run if we don't - // need to prompt the user, so |info| could have been destructed. + // NOTE: At this point, |ContinueDownloadWithPath| will have been run if + // we don't need to prompt the user, so |info| could have been destructed. // This means that we can't check any of its values. // However, SelectFileObserver will have recorded any attempt to open the // select file dialog. @@ -324,7 +324,7 @@ TEST_F(DownloadManagerTest, StartDownload) { observer.ShowedFileDialogForId(i)); // If the Save As dialog pops up, it never reached - // DownloadManager::AttachDownloadItem(), and never deleted info or + // DownloadManager::ContinueDownloadWithPath(), and never deleted info or // completed. This cleans up info. // Note that DownloadManager::FileSelectionCanceled() is never called. if (observer.ShowedFileDialogForId(i)) { @@ -347,7 +347,7 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) { info->url_chain.push_back(GURL()); info->is_dangerous_file = kDownloadRenameCases[i].is_dangerous_file; info->is_dangerous_url = kDownloadRenameCases[i].is_dangerous_url; - FilePath new_path(kDownloadRenameCases[i].suggested_path); + const FilePath new_path(kDownloadRenameCases[i].suggested_path); MockDownloadFile* download_file( new MockDownloadFile(info, download_manager_)); @@ -372,12 +372,14 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) { } download_manager_->CreateDownloadItem(info); + int32* id_ptr = new int32; + *id_ptr = i; // Deleted in FileSelected(). if (kDownloadRenameCases[i].finish_before_rename) { OnAllDataSaved(i, 1024, std::string("fake_hash")); message_loop_.RunAllPending(); - FileSelected(new_path, i, info); + FileSelected(new_path, i, id_ptr); } else { - FileSelected(new_path, i, info); + FileSelected(new_path, i, id_ptr); message_loop_.RunAllPending(); OnAllDataSaved(i, 1024, std::string("fake_hash")); } @@ -425,8 +427,7 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) { download_file->AppendDataToFile(kTestData, kTestDataLen); - info->path = new_path; - AttachDownloadItem(info); + ContinueDownloadWithPath(download, new_path); message_loop_.RunAllPending(); EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); @@ -489,8 +490,7 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) { EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state()); scoped_ptr<ItemObserver> observer(new ItemObserver(download)); - info->path = new_path; - AttachDownloadItem(info); + ContinueDownloadWithPath(download, new_path); message_loop_.RunAllPending(); EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); @@ -568,8 +568,7 @@ TEST_F(DownloadManagerTest, DownloadOverwriteTest) { // |download_file| is owned by DownloadFileManager. AddDownloadToFileManager(info->download_id, download_file); - info->path = new_path; - AttachDownloadItem(info); + ContinueDownloadWithPath(download, new_path); message_loop_.RunAllPending(); EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); diff --git a/chrome/browser/download/download_safe_browsing_client.cc b/chrome/browser/download/download_safe_browsing_client.cc index 36cf5fb..1acc45c 100644 --- a/chrome/browser/download/download_safe_browsing_client.cc +++ b/chrome/browser/download/download_safe_browsing_client.cc @@ -10,8 +10,8 @@ #include "base/metrics/histogram.h" #include "base/metrics/stats_counters.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_manager.h" -#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/safe_browsing/safe_browsing_util.h" #include "chrome/common/chrome_switches.h" #include "content/browser/browser_thread.h" @@ -23,8 +23,7 @@ DownloadSBClient::DownloadSBClient(int32 download_id, const std::vector<GURL>& url_chain, const GURL& referrer_url) - : info_(NULL), - download_id_(download_id), + : download_id_(download_id), url_chain_(url_chain), referrer_url_(referrer_url) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); @@ -36,22 +35,19 @@ DownloadSBClient::DownloadSBClient(int32 download_id, DownloadSBClient::~DownloadSBClient() {} -void DownloadSBClient::CheckDownloadUrl(DownloadCreateInfo* info, - UrlDoneCallback* callback) { +void DownloadSBClient::CheckDownloadUrl(UrlDoneCallback* callback) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); // It is not allowed to call this method twice. CHECK(!url_done_callback_.get() && !hash_done_callback_.get()); CHECK(callback); - CHECK(info); - info_ = info; start_time_ = base::TimeTicks::Now(); url_done_callback_.reset(callback); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, NewRunnableMethod(this, &DownloadSBClient::CheckDownloadUrlOnIOThread, - info->url_chain)); + url_chain_)); } void DownloadSBClient::CheckDownloadHash(const std::string& hash, @@ -125,7 +121,7 @@ void DownloadSBClient::SafeBrowsingCheckUrlDone( DVLOG(1) << "SafeBrowsingCheckUrlDone with result: " << result; bool is_dangerous = result != SafeBrowsingService::SAFE; - url_done_callback_->Run(info_, is_dangerous); + url_done_callback_->Run(download_id_, is_dangerous); if (sb_service_.get() && sb_service_->download_protection_enabled()) { UMA_HISTOGRAM_TIMES("SB2.DownloadUrlCheckDuration", diff --git a/chrome/browser/download/download_safe_browsing_client.h b/chrome/browser/download/download_safe_browsing_client.h index 7623733..98fb220 100644 --- a/chrome/browser/download/download_safe_browsing_client.h +++ b/chrome/browser/download/download_safe_browsing_client.h @@ -11,8 +11,6 @@ #include "base/time.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" -struct DownloadCreateInfo; - // This is a helper class used by DownloadManager to check a download URL with // SafeBrowsingService. The client is refcounted and will be released once // there is no reference to it. @@ -32,7 +30,7 @@ class DownloadSBClient : public SafeBrowsingService::Client, public base::RefCountedThreadSafe<DownloadSBClient> { public: - typedef Callback2<DownloadCreateInfo*, bool>::Type UrlDoneCallback; + typedef Callback2<int32, bool>::Type UrlDoneCallback; typedef Callback2<int32, bool>::Type HashDoneCallback; DownloadSBClient(int32 download_id, @@ -43,7 +41,7 @@ class DownloadSBClient // For each DownloadSBClient instance, either CheckDownloadUrl or // CheckDownloadHash can be called, and be called only once. // DownloadSBClient instance. - void CheckDownloadUrl(DownloadCreateInfo* info, UrlDoneCallback* callback); + void CheckDownloadUrl(UrlDoneCallback* callback); void CheckDownloadHash(const std::string& hash, HashDoneCallback* callback); private: @@ -91,9 +89,6 @@ class DownloadSBClient scoped_ptr<UrlDoneCallback> url_done_callback_; scoped_ptr<HashDoneCallback> hash_done_callback_; - // Not owned by this class. - DownloadCreateInfo* info_; - int32 download_id_; scoped_refptr<SafeBrowsingService> sb_service_; diff --git a/chrome/browser/download/download_state_info.cc b/chrome/browser/download/download_state_info.cc new file mode 100644 index 0000000..7df6b4f --- /dev/null +++ b/chrome/browser/download/download_state_info.cc @@ -0,0 +1,51 @@ + +// 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 "chrome/browser/download/download_state_info.h" + +#include "chrome/browser/download/download_item.h" + +DownloadStateInfo::DownloadStateInfo() + : path_uniquifier(0), + has_user_gesture(false), + prompt_user_for_save_location(false), + is_dangerous_file(false), + is_dangerous_url(false), + is_extension_install(false) { +} + +DownloadStateInfo::DownloadStateInfo( + bool has_user_gesture, + bool prompt_user_for_save_location) + : path_uniquifier(0), + has_user_gesture(has_user_gesture), + prompt_user_for_save_location(prompt_user_for_save_location), + is_dangerous_file(false), + is_dangerous_url(false), + is_extension_install(false) { +} + +DownloadStateInfo::DownloadStateInfo( + const FilePath& target, + const FilePath& forced_name, + bool has_user_gesture, + bool prompt_user_for_save_location, + int uniquifier, + bool dangerous_file, + bool dangerous_url, + bool extension_install) + : target_name(target), + path_uniquifier(uniquifier), + has_user_gesture(has_user_gesture), + prompt_user_for_save_location(prompt_user_for_save_location), + is_dangerous_file(dangerous_file), + is_dangerous_url(dangerous_url), + is_extension_install(extension_install), + force_file_name(forced_name) { +} + +bool DownloadStateInfo::IsDangerous() const { + return is_dangerous_url || is_dangerous_file; +} diff --git a/chrome/browser/download/download_state_info.h b/chrome/browser/download/download_state_info.h new file mode 100644 index 0000000..09182db --- /dev/null +++ b/chrome/browser/download/download_state_info.h @@ -0,0 +1,62 @@ +// 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 CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATE_INFO_H_ +#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATE_INFO_H_ +#pragma once + +#include "base/file_path.h" + +// Contains information relating to the process of determining what to do with +// the download. +struct DownloadStateInfo { + DownloadStateInfo(); + DownloadStateInfo(bool has_user_gesture, + bool prompt_user_for_save_location); + DownloadStateInfo(const FilePath& target, + const FilePath& forced_name, + bool has_user_gesture, + bool prompt_user_for_save_location, + int uniquifier, + bool dangerous_file, + bool dangerous_url, + bool extension_install); + + // Indicates if the download is dangerous. + bool IsDangerous() const; + + // The original name for a dangerous download, specified by the request. + FilePath target_name; + + // The path where we save the download. Typically generated. + FilePath suggested_path; + + // A number that should be added to the suggested path to make it unique. + // 0 means no number should be appended. It is eventually incorporated + // into the final file name. + int path_uniquifier; + + // True if the download is the result of user action. + bool has_user_gesture; + + // True if we should display the 'save as...' UI and prompt the user + // for the download location. + // False if the UI should be suppressed and the download performed to the + // default location. + bool prompt_user_for_save_location; + + // True if this download file is potentially dangerous (ex: exe, dll, ...). + bool is_dangerous_file; + + // If safebrowsing believes this URL leads to malware. + bool is_dangerous_url; + + // True if this download is for extension install. + bool is_extension_install; + + // True if this download's file name was specified initially. + FilePath force_file_name; +}; + +#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATE_INFO_H_ diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc index ea5bec3..ee2fe3a 100644 --- a/chrome/browser/download/download_util.cc +++ b/chrome/browser/download/download_util.cc @@ -26,6 +26,7 @@ #include "base/value_conversions.h" #include "base/values.h" #include "base/win/windows_version.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_extensions.h" #include "chrome/browser/download/download_item.h" #include "chrome/browser/download/download_item_model.h" @@ -34,7 +35,6 @@ #include "chrome/browser/extensions/crx_installer.h" #include "chrome/browser/extensions/extension_install_ui.h" #include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/history/download_create_info.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/chrome_paths.h" @@ -281,11 +281,17 @@ void GenerateExtension(const FilePath& file_name, generated_extension->swap(extension); } -void GenerateFileNameFromInfo(DownloadCreateInfo* info, - FilePath* generated_name) { - GenerateFileNameInternal(GURL(info->url()), info->content_disposition, - info->referrer_charset, std::string(), - info->mime_type, generated_name); +void GenerateFileNameFromRequest(const GURL& url, + const std::string& content_disposition, + const std::string& referrer_charset, + const std::string& mime_type, + FilePath* generated_name) { + GenerateFileNameInternal(url, + content_disposition, + referrer_charset, + std::string(), + mime_type, + generated_name); } void GenerateFileNameFromSuggestedName(const GURL& url, @@ -336,7 +342,7 @@ void OpenChromeExtension(Profile* profile, ExtensionService* service = profile->GetExtensionService(); CHECK(service); NotificationService* nservice = NotificationService::current(); - GURL nonconst_download_url = download_item.url(); + GURL nonconst_download_url = download_item.GetURL(); nservice->Notify(NotificationType::EXTENSION_READY_FOR_INSTALL, Source<DownloadManager>(download_manager), Details<GURL>(&nonconst_download_url)); @@ -345,18 +351,18 @@ void OpenChromeExtension(Profile* profile, service->MakeCrxInstaller(new ExtensionInstallUI(profile))); installer->set_delete_source(true); - if (UserScript::IsURLUserScript(download_item.url(), + if (UserScript::IsURLUserScript(download_item.GetURL(), download_item.mime_type())) { installer->InstallUserScript(download_item.full_path(), - download_item.url()); + download_item.GetURL()); return; } bool is_gallery_download = service->IsDownloadFromGallery( - download_item.url(), download_item.referrer_url()); + download_item.GetURL(), download_item.referrer_url()); installer->set_original_mime_type(download_item.original_mime_type()); installer->set_apps_require_extension_mime_type(true); - installer->set_original_url(download_item.url()); + installer->set_original_url(download_item.GetURL()); installer->set_is_gallery_install(is_gallery_download); installer->set_allow_silent_install(is_gallery_download); installer->set_install_cause(extension_misc::INSTALL_CAUSE_USER_DOWNLOAD); @@ -642,16 +648,16 @@ DictionaryValue* CreateDownloadItemValue(DownloadItem* download, int id) { string16 file_name = download->GetFileNameToReportUser().LossyDisplayName(); file_name = base::i18n::GetDisplayStringInLTRDirectionality(file_name); file_value->SetString("file_name", file_name); - file_value->SetString("url", download->url().spec()); + file_value->SetString("url", download->GetURL().spec()); file_value->SetBoolean("otr", download->is_otr()); if (download->IsInProgress()) { if (download->safety_state() == DownloadItem::DANGEROUS) { file_value->SetString("state", "DANGEROUS"); - DCHECK(download->danger_type() == DownloadItem::DANGEROUS_FILE || - download->danger_type() == DownloadItem::DANGEROUS_URL); + DCHECK(download->GetDangerType() == DownloadItem::DANGEROUS_FILE || + download->GetDangerType() == DownloadItem::DANGEROUS_URL); const char* danger_type_value = - download->danger_type() == DownloadItem::DANGEROUS_FILE ? + download->GetDangerType() == DownloadItem::DANGEROUS_FILE ? "DANGEROUS_FILE" : "DANGEROUS_URL"; file_value->SetString("danger_type", danger_type_value); } else if (download->is_paused()) { @@ -907,22 +913,4 @@ FilePath GetCrDownloadPath(const FilePath& suggested_path) { return FilePath(file_name); } -// TODO(erikkay,phajdan.jr): This is apparently not being exercised in tests. -bool IsDangerous(DownloadCreateInfo* info, Profile* profile, bool auto_open) { - DownloadDangerLevel danger_level = GetFileDangerLevel( - info->suggested_path.BaseName()); - if (danger_level == Dangerous) - return !(auto_open && info->has_user_gesture); - if (danger_level == AllowOnUserGesture && !info->has_user_gesture) - return true; - if (info->is_extension_install) { - // Extensions that are not from the gallery are considered dangerous. - ExtensionService* service = profile->GetExtensionService(); - if (!service || - !service->IsDownloadFromGallery(info->url(), info->referrer_url)) - return true; - } - return false; -} - } // namespace download_util diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h index 707dc07..3fedb5a 100644 --- a/chrome/browser/download/download_util.h +++ b/chrome/browser/download/download_util.h @@ -61,8 +61,11 @@ void GenerateExtension(const FilePath& file_name, FilePath::StringType* generated_extension); // Create a file name based on the response from the server. -void GenerateFileNameFromInfo(DownloadCreateInfo* info, - FilePath* generated_name); +void GenerateFileNameFromRequest(const GURL& url, + const std::string& content_disposition, + const std::string& referrer_charset, + const std::string& mime_type, + FilePath* generated_name); void GenerateFileNameFromSuggestedName(const GURL& url, const std::string& suggested_name, @@ -269,12 +272,6 @@ void EraseUniqueDownloadFiles(const FilePath& path_prefix); // Returns a .crdownload intermediate path for the |suggested_path|. FilePath GetCrDownloadPath(const FilePath& suggested_path); -// Returns true if this download should show the "dangerous file" warning. -// Various factors are considered, such as the type of the file, whether a -// user action initiated the download, and whether the user has explictly -// marked the file type as "auto open". -bool IsDangerous(DownloadCreateInfo* info, Profile* profile, bool auto_open); - } // namespace download_util #endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_UTIL_H_ diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc index f0f122e..0f8dca6 100644 --- a/chrome/browser/history/download_database.cc +++ b/chrome/browser/history/download_database.cc @@ -12,7 +12,7 @@ #include "base/utf_string_conversions.h" #include "build/build_config.h" #include "chrome/browser/download/download_item.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" // Download schema: // @@ -81,7 +81,7 @@ bool DownloadDatabase::DropDownloadTable() { } void DownloadDatabase::QueryDownloads( - std::vector<DownloadCreateInfo>* results) { + std::vector<DownloadHistoryInfo>* results) { results->clear(); sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, @@ -93,11 +93,11 @@ void DownloadDatabase::QueryDownloads( return; while (statement.Step()) { - DownloadCreateInfo info; + DownloadHistoryInfo info; info.db_handle = statement.ColumnInt64(0); info.path = ColumnFilePath(statement, 1); - info.url_chain.push_back(GURL(statement.ColumnString(2))); + info.url = GURL(statement.ColumnString(2)); info.start_time = base::Time::FromTimeT(statement.ColumnInt64(3)); info.received_bytes = statement.ColumnInt64(4); info.total_bytes = statement.ColumnInt64(5); @@ -145,7 +145,7 @@ bool DownloadDatabase::CleanUpInProgressEntries() { return statement.Run(); } -int64 DownloadDatabase::CreateDownload(const DownloadCreateInfo& info) { +int64 DownloadDatabase::CreateDownload(const DownloadHistoryInfo& info) { sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, "INSERT INTO downloads " "(full_path, url, start_time, received_bytes, total_bytes, state) " @@ -154,7 +154,7 @@ int64 DownloadDatabase::CreateDownload(const DownloadCreateInfo& info) { return 0; BindFilePath(statement, info.path, 0); - statement.BindString(1, info.url().spec()); + statement.BindString(1, info.url.spec()); statement.BindInt64(2, info.start_time.ToTimeT()); statement.BindInt64(3, info.received_bytes); statement.BindInt64(4, info.total_bytes); diff --git a/chrome/browser/history/download_database.h b/chrome/browser/history/download_database.h index 56f650a..bef60a1 100644 --- a/chrome/browser/history/download_database.h +++ b/chrome/browser/history/download_database.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// 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. @@ -8,7 +8,7 @@ #include "chrome/browser/history/history_types.h" -struct DownloadCreateInfo; +struct DownloadHistoryInfo; class FilePath; namespace sql { @@ -25,7 +25,7 @@ class DownloadDatabase { virtual ~DownloadDatabase(); // Get all the downloads from the database. - void QueryDownloads(std::vector<DownloadCreateInfo>* results); + void QueryDownloads(std::vector<DownloadHistoryInfo>* results); // Update the state of one download. Returns true if successful. bool UpdateDownload(int64 received_bytes, int32 state, DownloadID db_handle); @@ -40,7 +40,7 @@ class DownloadDatabase { bool CleanUpInProgressEntries(); // Create a new database entry for one download and return its primary db id. - int64 CreateDownload(const DownloadCreateInfo& info); + int64 CreateDownload(const DownloadHistoryInfo& info); // Remove a download from the database. void RemoveDownload(DownloadID db_handle); diff --git a/chrome/browser/history/download_history_info.cc b/chrome/browser/history/download_history_info.cc new file mode 100644 index 0000000..439f714 --- /dev/null +++ b/chrome/browser/history/download_history_info.cc @@ -0,0 +1,35 @@ +// 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 "chrome/browser/history/download_history_info.h" + +#include "chrome/browser/download/download_item.h" + +DownloadHistoryInfo::DownloadHistoryInfo() + : received_bytes(0), + total_bytes(0), + state(0), + db_handle(0) { +} + +DownloadHistoryInfo::DownloadHistoryInfo(const FilePath& path, + const GURL& url, + const GURL& referrer, + const base::Time& start, + int64 received, + int64 total, + int32 download_state, + int64 handle) + : path(path), + url(url), + referrer_url(referrer), + start_time(start), + received_bytes(received), + total_bytes(total), + state(download_state), + db_handle(handle) { +} + +DownloadHistoryInfo::~DownloadHistoryInfo() { // For linux-clang. +} diff --git a/chrome/browser/history/download_history_info.h b/chrome/browser/history/download_history_info.h new file mode 100644 index 0000000..e7c35d4 --- /dev/null +++ b/chrome/browser/history/download_history_info.h @@ -0,0 +1,60 @@ +// 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. +// +// Download struct used for informing and querying the history service. + +#ifndef CHROME_BROWSER_HISTORY_DOWNLOAD_HISTORY_INFO_H_ +#define CHROME_BROWSER_HISTORY_DOWNLOAD_HISTORY_INFO_H_ +#pragma once + +#include <vector> + +#include "base/file_path.h" +#include "base/time.h" +#include "googleurl/src/gurl.h" + +class DownloadItem; + +// Contains the information that is stored in the download history database +// (or refers to it). Managed by the DownloadItem. +struct DownloadHistoryInfo { + // TODO(ahendrickson) -- Reduce the number of constructors. + DownloadHistoryInfo(); + DownloadHistoryInfo(const FilePath& path, + const GURL& url, + const GURL& referrer, + const base::Time& start, + int64 received, + int64 total, + int32 download_state, + int64 handle); + ~DownloadHistoryInfo(); // For linux-clang. + + // The final path where the download is saved. + FilePath path; + + // The URL from which we are downloading. This is the final URL after any + // redirection by the server for |url_chain|. + GURL url; + + // The URL that referred us. + GURL referrer_url; + + // The time when the download started. + base::Time start_time; + + // The number of bytes received (so far). + int64 received_bytes; + + // The total number of bytes in the download. + int64 total_bytes; + + // The current state of the download. + int32 state; + + // The handle of the download in the database. + int64 db_handle; +}; + +#endif // CHROME_BROWSER_HISTORY_DOWNLOAD_HISTORY_INFO_H_ diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc index 4ec7301..c099322 100644 --- a/chrome/browser/history/history.cc +++ b/chrome/browser/history/history.cc @@ -32,7 +32,7 @@ #include "base/task.h" #include "chrome/browser/autocomplete/history_url_provider.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/history/history_backend.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/history_types.h" @@ -514,11 +514,13 @@ HistoryService::Handle HistoryService::QueryURL( // Handle creation of a download by creating an entry in the history service's // 'downloads' table. HistoryService::Handle HistoryService::CreateDownload( - const DownloadCreateInfo& create_info, + int32 id, + const DownloadHistoryInfo& create_info, CancelableRequestConsumerBase* consumer, HistoryService::DownloadCreateCallback* callback) { return Schedule(PRIORITY_NORMAL, &HistoryBackend::CreateDownload, consumer, - new history::DownloadCreateRequest(callback), create_info); + new history::DownloadCreateRequest(callback), id, + create_info); } // Handle queries for a list of all downloads in the history database's diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h index 5fcf1ea..b976c6a 100644 --- a/chrome/browser/history/history.h +++ b/chrome/browser/history/history.h @@ -27,7 +27,7 @@ #include "content/common/page_transition_types.h" class BookmarkService; -struct DownloadCreateInfo; +struct DownloadHistoryInfo; class FilePath; class GURL; class HistoryURLProvider; @@ -416,24 +416,25 @@ class HistoryService : public CancelableRequestProvider, // Implemented by the caller of 'CreateDownload' below, and is called when the // history service has created a new entry for a download in the history db. - typedef Callback2<DownloadCreateInfo, int64>::Type + typedef Callback2<int32, int64>::Type DownloadCreateCallback; // Begins a history request to create a new persistent entry for a download. // 'info' contains all the download's creation state, and 'callback' runs // when the history service request is complete. - Handle CreateDownload(const DownloadCreateInfo& info, + Handle CreateDownload(int32 id, + const DownloadHistoryInfo& info, CancelableRequestConsumerBase* consumer, DownloadCreateCallback* callback); // Implemented by the caller of 'QueryDownloads' below, and is called when the // history service has retrieved a list of all download state. The call - typedef Callback1<std::vector<DownloadCreateInfo>*>::Type + typedef Callback1<std::vector<DownloadHistoryInfo>*>::Type DownloadQueryCallback; // Begins a history request to retrieve the state of all downloads in the // history db. 'callback' runs when the history service request is complete, - // at which point 'info' contains an array of DownloadCreateInfo, one per + // at which point 'info' contains an array of DownloadHistoryInfo, one per // download. Handle QueryDownloads(CancelableRequestConsumerBase* consumer, DownloadQueryCallback* callback); diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc index 6f5d89f..438f784 100644 --- a/chrome/browser/history/history_backend.cc +++ b/chrome/browser/history/history_backend.cc @@ -20,7 +20,7 @@ #include "base/time.h" #include "chrome/browser/autocomplete/history_url_provider.h" #include "chrome/browser/bookmarks/bookmark_service.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/history/history_publisher.h" #include "chrome/browser/history/in_memory_history_backend.h" @@ -1132,13 +1132,13 @@ void HistoryBackend::UpdateDownloadPath(const FilePath& path, // Create a new download entry and pass back the db_handle to it. void HistoryBackend::CreateDownload( scoped_refptr<DownloadCreateRequest> request, - const DownloadCreateInfo& create_info) { + int32 id, + const DownloadHistoryInfo& history_info) { int64 db_handle = 0; if (!request->canceled()) { if (db_.get()) - db_handle = db_->CreateDownload(create_info); - request->ForwardResult(DownloadCreateRequest::TupleType(create_info, - db_handle)); + db_handle = db_->CreateDownload(history_info); + request->ForwardResult(DownloadCreateRequest::TupleType(id, db_handle)); } } diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h index 9bf8340..08eb0ff 100644 --- a/chrome/browser/history/history_backend.h +++ b/chrome/browser/history/history_backend.h @@ -25,7 +25,7 @@ #include "chrome/browser/search_engines/template_url_id.h" class BookmarkService; -struct DownloadCreateInfo; +struct DownloadHistoryInfo; class TestingProfile; struct ThumbnailScore; @@ -240,7 +240,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>, void UpdateDownload(int64 received_bytes, int32 state, int64 db_handle); void UpdateDownloadPath(const FilePath& path, int64 db_handle); void CreateDownload(scoped_refptr<DownloadCreateRequest> request, - const DownloadCreateInfo& info); + int32 id, + const DownloadHistoryInfo& info); void RemoveDownload(int64 db_handle); void RemoveDownloadsBetween(const base::Time remove_begin, const base::Time remove_end); diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h index f063fe79..5f9a97e 100644 --- a/chrome/browser/history/history_marshaling.h +++ b/chrome/browser/history/history_marshaling.h @@ -56,7 +56,7 @@ typedef CancelableRequest<FaviconService::FaviconDataCallback> // Downloads ------------------------------------------------------------------ typedef CancelableRequest1<HistoryService::DownloadQueryCallback, - std::vector<DownloadCreateInfo> > + std::vector<DownloadHistoryInfo> > DownloadQueryRequest; typedef CancelableRequest<HistoryService::DownloadCreateCallback> diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc index 2d26f1f..361a737 100644 --- a/chrome/browser/history/history_unittest.cc +++ b/chrome/browser/history/history_unittest.cc @@ -37,7 +37,7 @@ #include "base/task.h" #include "base/utf_string_conversions.h" #include "chrome/browser/download/download_item.h" -#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/history/download_history_info.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/history_backend.h" #include "chrome/browser/history/history_database.h" @@ -184,8 +184,14 @@ class HistoryTest : public testing::Test { } int64 AddDownload(int32 state, const Time& time) { - DownloadCreateInfo download(FilePath(FILE_PATH_LITERAL("foo-path")), - GURL("foo-url"), time, 0, 512, state, 0, false); + DownloadHistoryInfo download(FilePath(FILE_PATH_LITERAL("foo-path")), + GURL("foo-url"), + GURL(""), + time, + 0, + 512, + state, + 0); return db_->CreateDownload(download); } @@ -304,7 +310,7 @@ TEST_F(HistoryTest, ClearBrowsingData_Downloads) { Time month_ago = now - TimeDelta::FromDays(30); // Initially there should be nothing in the downloads database. - std::vector<DownloadCreateInfo> downloads; + std::vector<DownloadHistoryInfo> downloads; db_->QueryDownloads(&downloads); EXPECT_EQ(0U, downloads.size()); diff --git a/chrome/browser/renderer_host/download_resource_handler.cc b/chrome/browser/renderer_host/download_resource_handler.cc index bbcab26..df96cff 100644 --- a/chrome/browser/renderer_host/download_resource_handler.cc +++ b/chrome/browser/renderer_host/download_resource_handler.cc @@ -10,11 +10,11 @@ #include "base/metrics/histogram.h" #include "base/metrics/stats_counters.h" #include "base/stringprintf.h" +#include "chrome/browser/download/download_create_info.h" #include "chrome/browser/download/download_item.h" #include "chrome/browser/download/download_file_manager.h" #include "chrome/browser/download/download_process_handle.h" #include "chrome/browser/download/download_util.h" -#include "chrome/browser/history/download_create_info.h" #include "content/browser/browser_thread.h" #include "content/browser/renderer_host/global_request_id.h" #include "content/browser/renderer_host/resource_dispatcher_host.h" diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index f5f57fb..8970ac7 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -3471,7 +3471,7 @@ void Browser::OnStartDownload(DownloadItem* download, TabContentsWrapper* tab) { // Don't show content browser for extension/theme downloads from gallery. if (download->is_extension_install()) { ExtensionService* service = profile_->GetExtensionService(); - if (service && service->IsDownloadFromGallery(download->url(), + if (service && service->IsDownloadFromGallery(download->GetURL(), download->referrer_url())) { return; } @@ -3488,7 +3488,7 @@ void Browser::OnStartDownload(DownloadItem* download, TabContentsWrapper* tab) { // For non-theme extensions, we don't show the download animation. if (download->is_extension_install() && - !ExtensionService::IsDownloadFromMiniGallery(download->url())) + !ExtensionService::IsDownloadFromMiniGallery(download->GetURL())) return; TabContents* current_tab = GetSelectedTabContents(); diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.mm b/chrome/browser/ui/cocoa/download/download_item_controller.mm index 9f2bef0..d22474a 100644 --- a/chrome/browser/ui/cocoa/download/download_item_controller.mm +++ b/chrome/browser/ui/cocoa/download/download_item_controller.mm @@ -171,7 +171,7 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { // The dangerous download label, button text and icon are different under // different cases. - if (downloadModel->download()->danger_type() == + if (downloadModel->download()->GetDangerType() == DownloadItem::DANGEROUS_URL) { // Safebrowsing shows the download URL leads to malicious file. alertIcon = rb.GetNativeImageNamed(IDR_SAFEBROWSING_WARNING); @@ -180,7 +180,7 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { confirmButtonTitle = l10n_util::GetNSStringWithFixup(IDS_SAVE_DOWNLOAD); } else { // It's a dangerous file type (e.g.: an executable). - DCHECK_EQ(downloadModel->download()->danger_type(), + DCHECK_EQ(downloadModel->download()->GetDangerType(), DownloadItem::DANGEROUS_FILE); alertIcon = rb.GetNativeImageNamed(IDR_WARNING); if (downloadModel->download()->is_extension_install()) { diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.cc b/chrome/browser/ui/gtk/download/download_item_gtk.cc index 71c5601..25c94e8 100644 --- a/chrome/browser/ui/gtk/download/download_item_gtk.cc +++ b/chrome/browser/ui/gtk/download/download_item_gtk.cc @@ -533,13 +533,14 @@ void DownloadItemGtk::UpdateDangerWarning() { string16 dangerous_warning; // The dangerous download label text is different for different cases. - if (get_download()->danger_type() == DownloadItem::DANGEROUS_URL) { + if (get_download()->GetDangerType() == DownloadItem::DANGEROUS_URL) { // Safebrowsing shows the download URL leads to malicious file. dangerous_warning = l10n_util::GetStringUTF16(IDS_PROMPT_UNSAFE_DOWNLOAD_URL); } else { // It's a dangerous file type (e.g.: an executable). - DCHECK(get_download()->danger_type() == DownloadItem::DANGEROUS_FILE); + DCHECK(get_download()->GetDangerType() == + DownloadItem::DANGEROUS_FILE); if (get_download()->is_extension_install()) { dangerous_warning = l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); @@ -589,7 +590,7 @@ void DownloadItemGtk::UpdateDangerWarning() { void DownloadItemGtk::UpdateDangerIcon() { if (theme_service_->UsingNativeTheme()) { const char* stock = - get_download()->danger_type() == DownloadItem::DANGEROUS_URL ? + get_download()->GetDangerType() == DownloadItem::DANGEROUS_URL ? GTK_STOCK_DIALOG_ERROR : GTK_STOCK_DIALOG_WARNING; gtk_image_set_from_stock( GTK_IMAGE(dangerous_image_), stock, GTK_ICON_SIZE_SMALL_TOOLBAR); @@ -597,7 +598,7 @@ void DownloadItemGtk::UpdateDangerIcon() { // Set the warning icon. ResourceBundle& rb = ResourceBundle::GetSharedInstance(); int pixbuf_id = - get_download()->danger_type() == DownloadItem::DANGEROUS_URL ? + get_download()->GetDangerType() == DownloadItem::DANGEROUS_URL ? IDR_SAFEBROWSING_WARNING : IDR_WARNING; GdkPixbuf* download_pixbuf = rb.GetNativeImageNamed(pixbuf_id); gtk_image_set_from_pixbuf(GTK_IMAGE(dangerous_image_), download_pixbuf); diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc index 0be0d11..a909766 100644 --- a/chrome/browser/ui/views/download/download_item_view.cc +++ b/chrome/browser/ui/views/download/download_item_view.cc @@ -255,14 +255,14 @@ DownloadItemView::DownloadItemView(DownloadItem* download, // The dangerous download label text and icon are different // under different cases. string16 dangerous_label; - if (download->danger_type() == DownloadItem::DANGEROUS_URL) { + if (download->GetDangerType() == DownloadItem::DANGEROUS_URL) { // Safebrowsing shows the download URL leads to malicious file. warning_icon_ = rb.GetBitmapNamed(IDR_SAFEBROWSING_WARNING); dangerous_label = l10n_util::GetStringUTF16(IDS_PROMPT_UNSAFE_DOWNLOAD_URL); } else { // The download file has dangerous file type (e.g.: an executable). - DCHECK(download->danger_type() == DownloadItem::DANGEROUS_FILE); + DCHECK(download->GetDangerType() == DownloadItem::DANGEROUS_FILE); warning_icon_ = rb.GetBitmapNamed(IDR_WARNING); if (download->is_extension_install()) { dangerous_label = diff --git a/chrome/browser/ui/webui/chromeos/imageburner_ui.cc b/chrome/browser/ui/webui/chromeos/imageburner_ui.cc index 9e768be..31e18b6 100644 --- a/chrome/browser/ui/webui/chromeos/imageburner_ui.cc +++ b/chrome/browser/ui/webui/chromeos/imageburner_ui.cc @@ -613,7 +613,7 @@ void ImageBurnResourceManager::ModelChanged() { for (std::vector<DownloadItem*>::const_iterator it = downloads.begin(); it != downloads.end(); ++it) { - if ((*it)->url() == config_file_url_) { + if ((*it)->GetURL() == config_file_url_) { download_item_observer_added_ = true; (*it)->AddObserver(this); active_download_item_ = *it; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index fb25d3d..cb8611f 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -792,6 +792,8 @@ 'browser/dom_operation_notification_details.h', 'browser/download/base_file.cc', 'browser/download/base_file.h', + 'browser/download/download_create_info.cc', + 'browser/download/download_create_info.h', 'browser/download/download_extensions.cc', 'browser/download/download_extensions.h', 'browser/download/download_file.cc', @@ -820,6 +822,8 @@ 'browser/download/download_shelf_context_menu.cc', 'browser/download/download_shelf_context_menu.h', 'browser/download/download_started_animation.h', + 'browser/download/download_state_info.cc', + 'browser/download/download_state_info.h', 'browser/download/download_status_updater.cc', 'browser/download/download_status_updater.h', 'browser/download/download_status_updater_delegate.h', @@ -1136,10 +1140,10 @@ 'browser/hang_monitor/hung_window_detector.h', 'browser/history/archived_database.cc', 'browser/history/archived_database.h', - 'browser/history/download_create_info.cc', - 'browser/history/download_create_info.h', 'browser/history/download_database.cc', 'browser/history/download_database.h', + 'browser/history/download_history_info.cc', + 'browser/history/download_history_info.h', 'browser/history/expire_history_backend.cc', 'browser/history/expire_history_backend.h', 'browser/history/history.cc', |