diff options
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', |