diff options
33 files changed, 1876 insertions, 143 deletions
@@ -192,7 +192,7 @@ }, 'downloads_ext': { 'filepath': 'chrome/browser/extensions/api/downloads/|'\ - 'chrome/common/extensions/api/downloads.idl', + 'chrome/common/extensions/api/downloads.*.idl', }, 'downloads_ui': { 'filepath': 'chrome/browser/resources/downloads/|' \ diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc index 4c6d779..f22aa6f 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/chrome/browser/download/chrome_download_manager_delegate.cc @@ -24,6 +24,8 @@ #include "chrome/browser/download/download_history.h" #include "chrome/browser/download/download_path_reservation_tracker.h" #include "chrome/browser/download/download_prefs.h" +#include "chrome/browser/download/download_service.h" +#include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/download/download_status_updater.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/download/save_package_file_picker.h" @@ -208,17 +210,10 @@ ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() { void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) { download_manager_ = dm; -#if !defined(OS_ANDROID) - extension_event_router_.reset(new ExtensionDownloadsEventRouter( - profile_, download_manager_)); -#endif } void ChromeDownloadManagerDelegate::Shutdown() { download_prefs_.reset(); -#if !defined(OS_ANDROID) - extension_event_router_.reset(); -#endif } DownloadId ChromeDownloadManagerDelegate::GetNextId() { @@ -630,6 +625,22 @@ void ChromeDownloadManagerDelegate::Observe( callback.Run(installer->did_handle_successfully()); } +struct ChromeDownloadManagerDelegate::ContinueFilenameDeterminationInfo { + ContinueFilenameDeterminationInfo(); + ~ContinueFilenameDeterminationInfo(); + + int32 download_id; + content::DownloadTargetCallback callback; + content::DownloadDangerType danger_type; + bool visited_referrer_before; + bool should_prompt; +}; + +ChromeDownloadManagerDelegate::ContinueFilenameDeterminationInfo:: + ContinueFilenameDeterminationInfo() {} +ChromeDownloadManagerDelegate::ContinueFilenameDeterminationInfo:: + ~ContinueFilenameDeterminationInfo() {} + void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone( int32 download_id, const content::DownloadTargetCallback& callback, @@ -645,12 +656,12 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone( bool should_prompt = (download->GetTargetDisposition() == DownloadItem::TARGET_DISPOSITION_PROMPT); bool is_forced_path = !download->GetForcedFilePath().empty(); + base::FilePath generated_name; base::FilePath suggested_path; // Check whether this download is for an extension install or not. // Allow extensions to be explicitly saved. if (!is_forced_path) { - base::FilePath generated_name; GenerateFileNameFromRequest( *download, &generated_name, @@ -687,6 +698,78 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone( suggested_path = download->GetForcedFilePath(); } + ContinueFilenameDeterminationInfo continue_info; + continue_info.download_id = download_id; + continue_info.callback = callback; + continue_info.danger_type = danger_type; + continue_info.visited_referrer_before = visited_referrer_before; + continue_info.should_prompt = should_prompt; + + base::Closure filename_determined = base::Bind( + &ChromeDownloadManagerDelegate::ContinueDeterminingFilename, + this, continue_info, suggested_path, is_forced_path); +#if defined(OS_ANDROID) + filename_determined.Run(); +#else + if (is_forced_path || + !DownloadServiceFactory::GetForProfile(profile_) + ->GetExtensionEventRouter()) { + filename_determined.Run(); + } else { + DownloadService* service = DownloadServiceFactory::GetForProfile(profile_); + ExtensionDownloadsEventRouter* router = service->GetExtensionEventRouter(); + ExtensionDownloadsEventRouter::FilenameChangedCallback overriding = + base::Bind(&ChromeDownloadManagerDelegate::OnExtensionOverridingFilename, + this, continue_info); + router->OnDeterminingFilename( + download, generated_name, filename_determined, overriding); + } +#endif +} + +void ChromeDownloadManagerDelegate::OnExtensionOverridingFilename( + const ContinueFilenameDeterminationInfo& continue_info, + const base::FilePath& changed_filename, + bool overwrite) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DownloadItem* download = + download_manager_->GetDownload(continue_info.download_id); + if (!download || (download->GetState() != DownloadItem::IN_PROGRESS)) + return; + // If an extension overrides the filename, then the target directory will be + // forced to download_prefs_->DownloadPath() since extensions cannot place + // downloaded files anywhere except there. This prevents subdirectories from + // accumulating: if an extension is allowed to say that a file should go in + // last_download_path/music/foo.mp3, then last_download_path will accumulate + // the subdirectory /music/ so that the next download may end up in + // Downloads/music/music/music/bar.mp3. + base::FilePath temp_filename(download_prefs_->DownloadPath().Append( + changed_filename).NormalizePathSeparators()); + // Do not pass a mime type to GenerateSafeFileName so that it does not force + // the filename to have an extension if the (chrome) extension does not + // suggest it. + net::GenerateSafeFileName(std::string(), false, &temp_filename); + // If |is_forced_path| were true, then extensions would not have been + // consulted, so use |overwrite| instead of |is_forced_path|. This does NOT + // set DownloadItem::GetForcedFilePath()! + ContinueDeterminingFilename(continue_info, temp_filename, overwrite); +} + +void ChromeDownloadManagerDelegate::ContinueDeterminingFilename( + const ContinueFilenameDeterminationInfo& continue_info, + const base::FilePath& suggested_path, + bool is_forced_path) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + int32 download_id = continue_info.download_id; + const content::DownloadTargetCallback& callback = continue_info.callback; + content::DownloadDangerType danger_type = continue_info.danger_type; + bool visited_referrer_before = continue_info.visited_referrer_before; + bool should_prompt = continue_info.should_prompt; + DownloadItem* download = + download_manager_->GetDownload(download_id); + if (!download || (download->GetState() != DownloadItem::IN_PROGRESS)) + return; + // If the download hasn't already been marked dangerous (could be // DANGEROUS_URL), check if it is a dangerous file. if (danger_type == content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS) { diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h index eb4150d..a9a081a 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.h +++ b/chrome/browser/download/chrome_download_manager_delegate.h @@ -19,7 +19,6 @@ #include "content/public/browser/notification_registrar.h" class DownloadPrefs; -class ExtensionDownloadsEventRouter; class PrefRegistrySyncable; class Profile; @@ -145,6 +144,8 @@ class ChromeDownloadManagerDelegate private: friend class base::RefCountedThreadSafe<ChromeDownloadManagerDelegate>; + struct ContinueFilenameDeterminationInfo; + // content::NotificationObserver implementation. virtual void Observe(int type, const content::NotificationSource& source, @@ -198,6 +199,22 @@ class ChromeDownloadManagerDelegate const base::FilePath& reserved_path, bool reserved_path_verified); + // When an extension opts to change a download's target filename, this + // sanitizes it before continuing with the filename determination process. + void OnExtensionOverridingFilename( + const ContinueFilenameDeterminationInfo& continue_info, + const base::FilePath& changed_filename, + bool overwrite); + + // When extensions either opt not to change a download's target filename, or + // the changed filename has been sanitized, this method continues with the + // filename determination process, optionally prompting the user to manually + // set the filename. + void ContinueDeterminingFilename( + const ContinueFilenameDeterminationInfo& continue_info, + const base::FilePath& suggested_path, + bool is_forced_path); + // Called on the UI thread once the final target path is available. void OnTargetPathDetermined( int32 download_id, @@ -227,20 +244,6 @@ class ChromeDownloadManagerDelegate content::NotificationRegistrar registrar_; - // On Android, GET downloads are not handled by the DownloadManager. - // Once we have extensions on android, we probably need the EventRouter - // in ContentViewDownloadDelegate which knows about both GET and POST - // downloads. -#if !defined(OS_ANDROID) - // The ExtensionDownloadsEventRouter dispatches download creation, change, and - // erase events to extensions. Like ChromeDownloadManagerDelegate, it's a - // chrome-level concept and its lifetime should match DownloadManager. There - // should be a separate EDER for on-record and off-record managers. - // There does not appear to be a separate ExtensionSystem for on-record and - // off-record profiles, so ExtensionSystem cannot own the EDER. - scoped_ptr<ExtensionDownloadsEventRouter> extension_event_router_; -#endif - // The directory most recently chosen by the user in response to a Save As // dialog for a regular download. base::FilePath last_download_path_; diff --git a/chrome/browser/download/download_service.cc b/chrome/browser/download/download_service.cc index 5572dbf..13fceb8 100644 --- a/chrome/browser/download/download_service.cc +++ b/chrome/browser/download/download_service.cc @@ -10,6 +10,7 @@ #include "chrome/browser/download/download_history.h" #include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/download/download_status_updater.h" +#include "chrome/browser/extensions/api/downloads/downloads_api.h" #include "chrome/browser/history/history_service.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/net/chrome_net_log.h" @@ -45,6 +46,11 @@ ChromeDownloadManagerDelegate* DownloadService::GetDownloadManagerDelegate() { manager_delegate_->SetDownloadManager(manager); +#if !defined(OS_ANDROID) + extension_event_router_.reset(new ExtensionDownloadsEventRouter( + profile_, manager)); +#endif + if (!profile_->IsOffTheRecord()) { HistoryService* hs = HistoryServiceFactory::GetForProfile( profile_, Profile::EXPLICIT_ACCESS); @@ -120,6 +126,9 @@ void DownloadService::Shutdown() { // manually earlier. See http://crbug.com/131692 BrowserContext::GetDownloadManager(profile_)->Shutdown(); } +#if !defined(OS_ANDROID) + extension_event_router_.reset(); +#endif manager_delegate_ = NULL; download_history_.reset(); } diff --git a/chrome/browser/download/download_service.h b/chrome/browser/download/download_service.h index 1c72b29..601b3ed 100644 --- a/chrome/browser/download/download_service.h +++ b/chrome/browser/download/download_service.h @@ -15,6 +15,7 @@ class ChromeDownloadManagerDelegate; class DownloadHistory; +class ExtensionDownloadsEventRouter; class Profile; namespace content { @@ -35,6 +36,12 @@ class DownloadService : public ProfileKeyedService { // no HistoryService for profile. DownloadHistory* GetDownloadHistory(); +#if !defined(OS_ANDROID) + ExtensionDownloadsEventRouter* GetExtensionEventRouter() { + return extension_event_router_.get(); + } +#endif + // Has a download manager been created? bool HasCreatedDownloadManager(); @@ -65,6 +72,20 @@ class DownloadService : public ProfileKeyedService { scoped_ptr<DownloadHistory> download_history_; + // On Android, GET downloads are not handled by the DownloadManager. + // Once we have extensions on android, we probably need the EventRouter + // in ContentViewDownloadDelegate which knows about both GET and POST + // downloads. +#if !defined(OS_ANDROID) + // The ExtensionDownloadsEventRouter dispatches download creation, change, and + // erase events to extensions. Like ChromeDownloadManagerDelegate, it's a + // chrome-level concept and its lifetime should match DownloadManager. There + // should be a separate EDER for on-record and off-record managers. + // There does not appear to be a separate ExtensionSystem for on-record and + // off-record profiles, so ExtensionSystem cannot own the EDER. + scoped_ptr<ExtensionDownloadsEventRouter> extension_event_router_; +#endif + DISALLOW_COPY_AND_ASSIGN(DownloadService); }; diff --git a/chrome/browser/extensions/api/downloads/downloads_api.cc b/chrome/browser/extensions/api/downloads/downloads_api.cc index 117c93a..60831b9 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api.cc +++ b/chrome/browser/extensions/api/downloads/downloads_api.cc @@ -14,9 +14,11 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/callback.h" +#include "base/files/file_path.h" #include "base/json/json_writer.h" #include "base/lazy_instance.h" #include "base/logging.h" +#include "base/memory/weak_ptr.h" #include "base/metrics/histogram.h" #include "base/stl_util.h" #include "base/string16.h" @@ -34,6 +36,9 @@ #include "chrome/browser/extensions/event_names.h" #include "chrome/browser/extensions/event_router.h" #include "chrome/browser/extensions/extension_function_dispatcher.h" +#include "chrome/browser/extensions/extension_info_map.h" +#include "chrome/browser/extensions/extension_prefs.h" +#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/icon_loader.h" #include "chrome/browser/icon_manager.h" @@ -68,16 +73,20 @@ using content::DownloadManager; namespace download_extension_errors { // Error messages -const char kGenericError[] = "I'm afraid I can't do that."; -const char kIconNotFoundError[] = "Icon not found."; +const char kGenericError[] = "I'm afraid I can't do that"; +const char kIconNotFoundError[] = "Icon not found"; const char kInvalidDangerTypeError[] = "Invalid danger type"; +const char kInvalidFilenameError[] = "Invalid filename"; const char kInvalidFilterError[] = "Invalid query filter"; -const char kInvalidOperationError[] = "Invalid operation."; +const char kInvalidOperationError[] = "Invalid operation"; const char kInvalidOrderByError[] = "Invalid orderBy field"; const char kInvalidQueryLimit[] = "Invalid query limit"; const char kInvalidStateError[] = "Invalid state"; -const char kInvalidURLError[] = "Invalid URL."; +const char kInvalidURLError[] = "Invalid URL"; const char kNotImplementedError[] = "NotImplemented."; +const char kTooManyListenersError[] = "Each extension may have at most one " + "onDeterminingFilename listener between all of its renderer execution " + "contexts."; } // namespace download_extension_errors @@ -185,14 +194,12 @@ DownloadItem::DownloadState StateEnumFromString(const std::string& state) { return DownloadItem::MAX_DOWNLOAD_STATE; } -bool ValidateFilename(const string16& filename) { - // TODO(benjhayden): More robust validation of filename. - if ((filename.find('/') != string16::npos) || - (filename.find('\\') != string16::npos)) - return false; - if (filename.size() >= 2u && filename[0] == L'.' && filename[1] == L'.') - return false; - return true; +bool ValidateFilename(const base::FilePath& filename) { + return !filename.empty() && + (filename == filename.StripTrailingSeparators()) && + (filename.BaseName().value() != base::FilePath::kCurrentDirectory) && + !filename.ReferencesParent() && + !filename.IsAbsolute(); } std::string TimeToISO8601(const base::Time& t) { @@ -264,8 +271,7 @@ bool DownloadFileIconExtractorImpl::ExtractIconURLForPath( IconManager* im = g_browser_process->icon_manager(); // The contents of the file at |path| may have changed since a previous // request, in which case the associated icon may also have changed. - // Therefore, for the moment we always call LoadIcon instead of attempting - // a LookupIcon. + // Therefore, always call LoadIcon instead of attempting a LookupIcon. im->LoadIcon(path, icon_size, base::Bind(&DownloadFileIconExtractorImpl::OnIconLoadComplete, @@ -411,7 +417,7 @@ void CompileDownloadQueryOrderBy( std::vector<std::string> order_by_strs; base::SplitString(order_by_str, ' ', &order_by_strs); for (std::vector<std::string>::const_iterator iter = order_by_strs.begin(); - iter != order_by_strs.end(); ++iter) { + iter != order_by_strs.end(); ++iter) { std::string term_str = *iter; if (term_str.empty()) continue; @@ -508,31 +514,6 @@ void RunDownloadQuery( query_out.Search(all_items.begin(), all_items.end(), results); } -void DispatchEventInternal( - Profile* target_profile, - const char* event_name, - const std::string& json_args, - scoped_ptr<base::ListValue> event_args) { - if (!extensions::ExtensionSystem::Get(target_profile)->event_router()) - return; - scoped_ptr<extensions::Event> event(new extensions::Event( - event_name, event_args.Pass())); - event->restrict_to_profile = target_profile; - extensions::ExtensionSystem::Get(target_profile)->event_router()-> - BroadcastEvent(event.Pass()); - ExtensionDownloadsEventRouter::DownloadsNotificationSource - notification_source; - notification_source.event_name = event_name; - notification_source.profile = target_profile; - content::Source<ExtensionDownloadsEventRouter::DownloadsNotificationSource> - content_source(¬ification_source); - std::string args_copy(json_args); - content::NotificationService::current()->Notify( - chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT, - content_source, - content::Details<std::string>(&args_copy)); -} - class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data { public: static ExtensionDownloadsEventRouterData* Get(DownloadItem* download_item) { @@ -546,7 +527,8 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data { scoped_ptr<base::DictionaryValue> json_item) : updated_(0), changed_fired_(0), - json_(json_item.Pass()) { + json_(json_item.Pass()), + determined_overwrite_(false) { download_item->SetUserData(kKey, this); } @@ -565,16 +547,166 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data { void OnItemUpdated() { ++updated_; } void OnChangedFired() { ++changed_fired_; } + void set_filename_change_callbacks( + const base::Closure& no_change, + const ExtensionDownloadsEventRouter::FilenameChangedCallback& change) { + filename_no_change_ = no_change; + filename_change_ = change; + } + + void ClearPendingDeterminers() { + determined_filename_.clear(); + determined_overwrite_ = false; + determiner_ = DeterminerInfo(); + filename_no_change_ = base::Closure(); + filename_change_ = ExtensionDownloadsEventRouter::FilenameChangedCallback(); + weak_ptr_factory_.reset(); + determiners_.clear(); + } + + void DeterminerRemoved(const std::string& extension_id) { + for (DeterminerInfoVector::iterator iter = determiners_.begin(); + iter != determiners_.end();) { + if (iter->extension_id == extension_id) { + iter = determiners_.erase(iter); + } else { + ++iter; + } + } + // If we just removed the last unreported determiner, then we need to call a + // callback. + CheckAllDeterminersCalled(); + } + + void AddPendingDeterminer(const std::string& extension_id, + const base::Time& installed) { + for (size_t index = 0; index < determiners_.size(); ++index) { + if (determiners_[index].extension_id == extension_id) { + DCHECK(false) << extension_id; + return; + } + } + determiners_.push_back(DeterminerInfo(extension_id, installed)); + } + + bool DeterminerAlreadyReported(const std::string& extension_id) { + for (size_t index = 0; index < determiners_.size(); ++index) { + if (determiners_[index].extension_id == extension_id) { + return determiners_[index].reported; + } + } + return false; + } + + // Returns false if this extension_id was not expected or if this extension_id + // has already reported, regardless of whether the filename is valid. + bool DeterminerCallback( + const std::string& extension_id, + const base::FilePath& filename, + bool overwrite) { + bool found_info = false; + for (size_t index = 0; index < determiners_.size(); ++index) { + if (determiners_[index].extension_id == extension_id) { + found_info = true; + if (determiners_[index].reported) + return false; + determiners_[index].reported = true; + // Do not use filename if another determiner has already overridden the + // filename and they take precedence. Extensions that were installed + // later take precedence over previous extensions. + if (ValidateFilename(filename) && + (determiner_.extension_id.empty() || + (determiners_[index].install_time > determiner_.install_time))) { + determined_filename_ = filename; + determined_overwrite_ = overwrite; + determiner_ = determiners_[index]; + } + break; + } + } + if (!found_info) + return false; + CheckAllDeterminersCalled(); + return true; + } + private: + struct DeterminerInfo { + DeterminerInfo(); + DeterminerInfo(const std::string& e_id, + const base::Time& installed); + ~DeterminerInfo(); + + std::string extension_id; + base::Time install_time; + bool reported; + }; + typedef std::vector<DeterminerInfo> DeterminerInfoVector; + static const char kKey[]; + // This is safe to call even while not waiting for determiners to call back; + // in that case, the callbacks will be null so they won't be Run. + void CheckAllDeterminersCalled() { + for (DeterminerInfoVector::iterator iter = determiners_.begin(); + iter != determiners_.end(); ++iter) { + if (!iter->reported) + return; + } + if (determined_filename_.empty()) { + if (!filename_no_change_.is_null()) + filename_no_change_.Run(); + } else { + if (!filename_change_.is_null()) + filename_change_.Run(determined_filename_, determined_overwrite_); + } + // Don't clear determiners_ immediately in case there's a second listener + // for one of the extensions, so that DetermineFilename can return + // kTooManyListenersError. After a few seconds, DetermineFilename will + // return kInvalidOperationError instead of kTooManyListenersError so that + // determiners_ doesn't keep hogging memory. + weak_ptr_factory_.reset( + new base::WeakPtrFactory<ExtensionDownloadsEventRouterData>(this)); + MessageLoopForUI::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&ExtensionDownloadsEventRouterData::ClearPendingDeterminers, + weak_ptr_factory_->GetWeakPtr()), + base::TimeDelta::FromSeconds(30)); + } + int updated_; int changed_fired_; scoped_ptr<base::DictionaryValue> json_; + base::Closure filename_no_change_; + ExtensionDownloadsEventRouter::FilenameChangedCallback filename_change_; + + DeterminerInfoVector determiners_; + + base::FilePath determined_filename_; + bool determined_overwrite_; + DeterminerInfo determiner_; + + scoped_ptr<base::WeakPtrFactory<ExtensionDownloadsEventRouterData> > + weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(ExtensionDownloadsEventRouterData); }; +ExtensionDownloadsEventRouterData::DeterminerInfo::DeterminerInfo( + const std::string& e_id, + const base::Time& installed) + : extension_id(e_id), + install_time(installed), + reported(false) { +} + +ExtensionDownloadsEventRouterData::DeterminerInfo::DeterminerInfo() + : reported(false) { +} + +ExtensionDownloadsEventRouterData::DeterminerInfo::~DeterminerInfo() {} + const char ExtensionDownloadsEventRouterData::kKey[] = "DownloadItem ExtensionDownloadsEventRouterData"; @@ -589,8 +721,8 @@ class ManagerDestructionObserver : public DownloadManager::Observer { if (!(*manager_file_existence_last_checked_)[manager]) (*manager_file_existence_last_checked_)[manager] = new ManagerDestructionObserver(manager); - (*manager_file_existence_last_checked_)[manager] - ->CheckForHistoryFilesRemovalInternal(); + (*manager_file_existence_last_checked_)[manager]-> + CheckForHistoryFilesRemovalInternal(); } private: @@ -634,9 +766,23 @@ class ManagerDestructionObserver : public DownloadManager::Observer { std::map<DownloadManager*, ManagerDestructionObserver*>* ManagerDestructionObserver::manager_file_existence_last_checked_ = NULL; +void OnDeterminingFilenameWillDispatchCallback( + bool* any_determiners, + ExtensionDownloadsEventRouterData* data, + Profile* profile, + const extensions::Extension* extension, + ListValue* event_args) { + *any_determiners = true; + base::Time installed = extensions::ExtensionSystem::Get( + profile)->extension_service()->extension_prefs()-> + GetInstallTime(extension->id()); + data->AddPendingDeterminer(extension->id(), installed); +} + } // namespace DownloadsDownloadFunction::DownloadsDownloadFunction() {} + DownloadsDownloadFunction::~DownloadsDownloadFunction() {} bool DownloadsDownloadFunction::RunImpl() { @@ -673,8 +819,14 @@ bool DownloadsDownloadFunction::RunImpl() { string16 filename16; EXTENSION_FUNCTION_VALIDATE(options_value->GetString( kFilenameKey, &filename16)); - if (!ValidateFilename(filename16)) { - error_ = download_extension_errors::kGenericError; +#if defined(OS_WIN) + base::FilePath file_path(filename16); +#elif defined(OS_POSIX) + base::FilePath file_path(*options.filename.get()); +#endif + if (!ValidateFilename(file_path) || + (file_path.DirName().value() != base::FilePath::kCurrentDirectory)) { + error_ = download_extension_errors::kInvalidFilenameError; return false; } // TODO(benjhayden) Ensure that this filename is interpreted as a path @@ -733,6 +885,7 @@ void DownloadsDownloadFunction::OnStarted( } DownloadsSearchFunction::DownloadsSearchFunction() {} + DownloadsSearchFunction::~DownloadsSearchFunction() {} bool DownloadsSearchFunction::RunImpl() { @@ -770,6 +923,7 @@ bool DownloadsSearchFunction::RunImpl() { } DownloadsPauseFunction::DownloadsPauseFunction() {} + DownloadsPauseFunction::~DownloadsPauseFunction() {} bool DownloadsPauseFunction::RunImpl() { @@ -793,6 +947,7 @@ bool DownloadsPauseFunction::RunImpl() { } DownloadsResumeFunction::DownloadsResumeFunction() {} + DownloadsResumeFunction::~DownloadsResumeFunction() {} bool DownloadsResumeFunction::RunImpl() { @@ -807,7 +962,7 @@ bool DownloadsResumeFunction::RunImpl() { error_ = download_extension_errors::kInvalidOperationError; } else { // Note that if the item isn't paused, this will be a no-op, and - // we will silently treat the extension call as a success. + // the extension call will seem successful. download_item->Resume(); } if (error_.empty()) @@ -816,6 +971,7 @@ bool DownloadsResumeFunction::RunImpl() { } DownloadsCancelFunction::DownloadsCancelFunction() {} + DownloadsCancelFunction::~DownloadsCancelFunction() {} bool DownloadsCancelFunction::RunImpl() { @@ -827,13 +983,13 @@ bool DownloadsCancelFunction::RunImpl() { if (download_item != NULL) download_item->Cancel(true); // |download_item| can be NULL if the download ID was invalid or if the - // download is not currently active. Either way, we don't consider it a - // failure. + // download is not currently active. Either way, it's not a failure. RecordApiFunctions(DOWNLOADS_FUNCTION_CANCEL); return true; } DownloadsEraseFunction::DownloadsEraseFunction() {} + DownloadsEraseFunction::~DownloadsEraseFunction() {} bool DownloadsEraseFunction::RunImpl() { @@ -862,20 +1018,8 @@ bool DownloadsEraseFunction::RunImpl() { return true; } -DownloadsSetDestinationFunction::DownloadsSetDestinationFunction() {} -DownloadsSetDestinationFunction::~DownloadsSetDestinationFunction() {} - -bool DownloadsSetDestinationFunction::RunImpl() { - scoped_ptr<extensions::api::downloads::SetDestination::Params> params( - extensions::api::downloads::SetDestination::Params::Create(*args_)); - EXTENSION_FUNCTION_VALIDATE(params.get()); - error_ = download_extension_errors::kNotImplementedError; - if (error_.empty()) - RecordApiFunctions(DOWNLOADS_FUNCTION_SET_DESTINATION); - return error_.empty(); -} - DownloadsAcceptDangerFunction::DownloadsAcceptDangerFunction() {} + DownloadsAcceptDangerFunction::~DownloadsAcceptDangerFunction() {} bool DownloadsAcceptDangerFunction::RunImpl() { @@ -919,6 +1063,7 @@ void DownloadsAcceptDangerFunction::DangerPromptCallback( } DownloadsShowFunction::DownloadsShowFunction() {} + DownloadsShowFunction::~DownloadsShowFunction() {} bool DownloadsShowFunction::RunImpl() { @@ -937,6 +1082,7 @@ bool DownloadsShowFunction::RunImpl() { } DownloadsOpenFunction::DownloadsOpenFunction() {} + DownloadsOpenFunction::~DownloadsOpenFunction() {} bool DownloadsOpenFunction::RunImpl() { @@ -955,6 +1101,7 @@ bool DownloadsOpenFunction::RunImpl() { } DownloadsDragFunction::DownloadsDragFunction() {} + DownloadsDragFunction::~DownloadsDragFunction() {} bool DownloadsDragFunction::RunImpl() { @@ -1009,8 +1156,8 @@ bool DownloadsGetFileIconFunction::RunImpl() { return false; } // In-progress downloads return the intermediate filename for GetFullPath() - // which doesn't have the final extension. Therefore we won't be able to - // derive a good file icon for it. So we use GetTargetFilePath() instead. + // which doesn't have the final extension. Therefore a good file icon can't be + // found, so use GetTargetFilePath() instead. DCHECK(icon_extractor_.get()); DCHECK(icon_size == 16 || icon_size == 32); EXTENSION_FUNCTION_VALIDATE(icon_extractor_->ExtractIconURLForPath( @@ -1038,12 +1185,147 @@ ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter( ALLOW_THIS_IN_INITIALIZER_LIST(notifier_(manager, this)) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(profile_); + extensions::EventRouter* router = extensions::ExtensionSystem::Get(profile_)-> + event_router(); + if (router) + router->RegisterObserver( + this, extensions::event_names::kOnDownloadDeterminingFilename); } ExtensionDownloadsEventRouter::~ExtensionDownloadsEventRouter() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + extensions::EventRouter* router = extensions::ExtensionSystem::Get(profile_)-> + event_router(); + if (router) + router->UnregisterObserver(this); +} + +// The method by which extensions hook into the filename determination process +// is based on the method by which the omnibox API allows extensions to hook +// into the omnibox autocompletion process. Extensions that wish to play a part +// in the filename determination process call +// chrome.downloads.onDeterminingFilename.addListener, which adds an +// EventListener object to ExtensionEventRouter::listeners(). +// +// When a download's filename is being determined, +// ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone (CVRBD) passes +// 2 callbacks to ExtensionDownloadsEventRouter::OnDeterminingFilename (ODF), +// which stores the callbacks in the item's ExtensionDownloadsEventRouterData +// (EDERD) along with all of the extension IDs that are listening for +// onDeterminingFilename events. ODF dispatches +// chrome.downloads.onDeterminingFilename. +// +// When the extension's event handler calls |suggestCallback|, +// downloads_custom_bindings.js calls +// DownloadsInternalDetermineFilenameFunction::RunImpl, which calls +// EDER::DetermineFilename, which notifies the item's EDERD. +// +// When the last extension's event handler returns, EDERD calls one of the two +// callbacks that CVRBD passed to ODF, allowing CDMD to complete the filename +// determination process. If multiple extensions wish to override the filename, +// then the extension that was last installed wins. + +void ExtensionDownloadsEventRouter::OnDeterminingFilename( + DownloadItem* item, + const base::FilePath& suggested_path, + const base::Closure& no_change, + const FilenameChangedCallback& change) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + ExtensionDownloadsEventRouterData* data = + ExtensionDownloadsEventRouterData::Get(item); + data->ClearPendingDeterminers(); + data->set_filename_change_callbacks(no_change, change); + bool any_determiners = false; + base::DictionaryValue* json = DownloadItemToJSON( + item, profile_->IsOffTheRecord()).release(); + json->SetString(kFilenameKey, suggested_path.LossyDisplayName()); + DispatchEvent(extensions::event_names::kOnDownloadDeterminingFilename, + false, + base::Bind(&OnDeterminingFilenameWillDispatchCallback, + &any_determiners, + data), + json); + if (!any_determiners) { + data->ClearPendingDeterminers(); + no_change.Run(); + } +} + +bool ExtensionDownloadsEventRouter::DetermineFilename( + Profile* profile, + bool include_incognito, + const std::string& ext_id, + int download_id, + const base::FilePath& filename, + bool overwrite, + std::string* error) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DownloadItem* item = GetDownload(profile, include_incognito, download_id); + if (!item) { + *error = download_extension_errors::kInvalidOperationError; + return false; + } + ExtensionDownloadsEventRouterData* data = + ExtensionDownloadsEventRouterData::Get(item); + if (!data) { + *error = download_extension_errors::kInvalidOperationError; + return false; + } + // maxListeners=1 in downloads.idl and suggestCallback in + // downloads_custom_bindings.js should prevent duplicate DeterminerCallback + // calls from the same renderer, but an extension may have more than one + // renderer, so don't DCHECK(!reported). + if (data->DeterminerAlreadyReported(ext_id)) { + *error = download_extension_errors::kTooManyListenersError; + return false; + } + if (!item->IsInProgress()) { + *error = download_extension_errors::kInvalidOperationError; + return false; + } + if (!data->DeterminerCallback(ext_id, filename, overwrite)) { + // Nobody expects this ext_id! + *error = download_extension_errors::kInvalidOperationError; + return false; + } + if (!filename.empty() && !ValidateFilename(filename)) { + // If this is moved to before DeterminerCallback(), then it will block + // forever waiting for this ext_id to report. + *error = download_extension_errors::kInvalidFilenameError; + return false; + } + return true; +} + +void ExtensionDownloadsEventRouter::OnListenerRemoved( + const extensions::EventListenerInfo& details) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (details.event_name != + extensions::event_names::kOnDownloadDeterminingFilename) + return; + DownloadManager* manager = notifier_.GetManager(); + if (!manager) + return; + DownloadManager::DownloadVector items; + manager->GetAllDownloads(&items); + // Notify any items that may be waiting for callbacks from this + // extension/determiner. + for (DownloadManager::DownloadVector::const_iterator iter = + items.begin(); + iter != items.end(); ++iter) { + ExtensionDownloadsEventRouterData* data = + ExtensionDownloadsEventRouterData::Get(*iter); + // This will almost always be a no-op, however, it is possible for an + // extension renderer to be unloaded while a download item is waiting + // for a determiner. In that case, the download item should proceed. + if (data) + data->DeterminerRemoved(details.extension_id); + } } +// That's all the methods that have to do with filename determination. The rest +// have to do with the other, less special events. + void ExtensionDownloadsEventRouter::OnDownloadCreated( DownloadManager* manager, DownloadItem* download_item) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); @@ -1053,6 +1335,8 @@ void ExtensionDownloadsEventRouter::OnDownloadCreated( scoped_ptr<base::DictionaryValue> json_item( DownloadItemToJSON(download_item, profile_->IsOffTheRecord())); DispatchEvent(extensions::event_names::kOnDownloadCreated, + true, + extensions::Event::WillDispatchCallback(), json_item->DeepCopy()); if (!ExtensionDownloadsEventRouterData::Get(download_item)) new ExtensionDownloadsEventRouterData(download_item, json_item.Pass()); @@ -1072,7 +1356,7 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated( return; } scoped_ptr<base::DictionaryValue> new_json(DownloadItemToJSON( - download_item, profile_->IsOffTheRecord())); + download_item, profile_->IsOffTheRecord())); scoped_ptr<base::DictionaryValue> delta(new base::DictionaryValue()); delta->SetInteger(kIdKey, download_item->GetId()); std::set<std::string> new_fields; @@ -1112,7 +1396,10 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated( // changed. Replace the stored json with the new json. data->OnItemUpdated(); if (changed) { - DispatchEvent(extensions::event_names::kOnDownloadChanged, delta.release()); + DispatchEvent(extensions::event_names::kOnDownloadChanged, + true, + extensions::Event::WillDispatchCallback(), + delta.release()); data->OnChangedFired(); } data->set_json(new_json.Pass()); @@ -1124,33 +1411,42 @@ void ExtensionDownloadsEventRouter::OnDownloadRemoved( if (download_item->IsTemporary()) return; DispatchEvent(extensions::event_names::kOnDownloadErased, + true, + extensions::Event::WillDispatchCallback(), base::Value::CreateIntegerValue(download_item->GetId())); } void ExtensionDownloadsEventRouter::DispatchEvent( - const char* event_name, base::Value* arg) { + const char* event_name, + bool include_incognito, + const extensions::Event::WillDispatchCallback& will_dispatch_callback, + base::Value* arg) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (!extensions::ExtensionSystem::Get(profile_)->event_router()) + return; scoped_ptr<base::ListValue> args(new base::ListValue()); args->Append(arg); std::string json_args; base::JSONWriter::Write(args.get(), &json_args); - // There is a one EDER for each on-record Profile, and a separate EDER for - // each off-record Profile, so there is exactly one EDER for each - // DownloadManager. EDER only watches its own DM, so all the items that an - // EDER sees are either all on-record or all off-record. However, we want - // extensions in off-record contexts to see on-record items. So, if this EDER - // is watching an on-record DM, and there is a corresponding off-record - // Profile, then dispatch this event to both the on-record Profile and the - // off-record Profile. There may or may not be an off-record Profile, so send - // a *copy* of |args| to the off-record Profile, and Pass() |args| - // to the Profile that we know is there. - if (profile_->HasOffTheRecordProfile() && - !profile_->IsOffTheRecord()) { - DispatchEventInternal( - profile_->GetOffTheRecordProfile(), - event_name, - json_args, - scoped_ptr<base::ListValue>(args->DeepCopy())); - } - DispatchEventInternal(profile_, event_name, json_args, args.Pass()); + scoped_ptr<extensions::Event> event(new extensions::Event( + event_name, args.Pass())); + // The downloads system wants to share on-record events with off-record + // extension renderers even in incognito_split_mode because that's how + // chrome://downloads works. The "restrict_to_profile" mechanism does not + // anticipate this, so it does not automatically prevent sharing off-record + // events with on-record extension renderers. + event->restrict_to_profile = + (include_incognito && !profile_->IsOffTheRecord()) ? NULL : profile_; + event->will_dispatch_callback = will_dispatch_callback; + extensions::ExtensionSystem::Get(profile_)->event_router()-> + BroadcastEvent(event.Pass()); + DownloadsNotificationSource notification_source; + notification_source.event_name = event_name; + notification_source.profile = profile_; + content::Source<DownloadsNotificationSource> content_source( + ¬ification_source); + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT, + content_source, + content::Details<std::string>(&json_args)); } diff --git a/chrome/browser/extensions/api/downloads/downloads_api.h b/chrome/browser/extensions/api/downloads/downloads_api.h index 08714eb..fa63302 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api.h +++ b/chrome/browser/extensions/api/downloads/downloads_api.h @@ -12,6 +12,7 @@ #include "base/string16.h" #include "base/values.h" #include "chrome/browser/download/all_download_item_notifier.h" +#include "chrome/browser/extensions/event_router.h" #include "chrome/browser/extensions/extension_function.h" #include "content/public/browser/download_id.h" #include "content/public/browser/download_item.h" @@ -35,6 +36,7 @@ namespace download_extension_errors { extern const char kGenericError[]; extern const char kIconNotFoundError[]; extern const char kInvalidDangerTypeError[]; +extern const char kInvalidFilenameError[]; extern const char kInvalidFilterError[]; extern const char kInvalidOperationError[]; extern const char kInvalidOrderByError[]; @@ -42,6 +44,7 @@ extern const char kInvalidQueryLimit[]; extern const char kInvalidStateError[]; extern const char kInvalidURLError[]; extern const char kNotImplementedError[]; +extern const char kTooManyListenersError[]; } // namespace download_extension_errors @@ -126,20 +129,6 @@ class DownloadsEraseFunction : public SyncExtensionFunction { DISALLOW_COPY_AND_ASSIGN(DownloadsEraseFunction); }; -class DownloadsSetDestinationFunction : public AsyncExtensionFunction { - public: - DECLARE_EXTENSION_FUNCTION("downloads.setDestination", - DOWNLOADS_SETDESTINATION) - DownloadsSetDestinationFunction(); - virtual bool RunImpl() OVERRIDE; - - protected: - virtual ~DownloadsSetDestinationFunction(); - - private: - DISALLOW_COPY_AND_ASSIGN(DownloadsSetDestinationFunction); -}; - class DownloadsAcceptDangerFunction : public AsyncExtensionFunction { public: DECLARE_EXTENSION_FUNCTION("downloads.acceptDanger", DOWNLOADS_ACCEPTDANGER) @@ -212,14 +201,45 @@ class DownloadsGetFileIconFunction : public AsyncExtensionFunction { // Observes a single DownloadManager and many DownloadItems and dispatches // onCreated and onErased events. -class ExtensionDownloadsEventRouter - : public AllDownloadItemNotifier::Observer { +class ExtensionDownloadsEventRouter : public extensions::EventRouter::Observer, + public AllDownloadItemNotifier::Observer { public: + typedef base::Callback<void(const base::FilePath& changed_filename, + bool overwrite)> FilenameChangedCallback; + + // A downloads.onDeterminingFilename listener has returned. If the extension + // wishes to override the download's filename, then |filename| will be + // non-empty. |filename| will be interpreted as a relative path, appended to + // the default downloads directory. If the extension wishes to overwrite any + // existing files, then |overwrite| will be true. Returns true on success, + // false otherwise. + static bool DetermineFilename( + Profile* profile, + bool include_incognito, + const std::string& ext_id, + int download_id, + const base::FilePath& filename, + bool overwrite, + std::string* error); + explicit ExtensionDownloadsEventRouter( Profile* profile, content::DownloadManager* manager); virtual ~ExtensionDownloadsEventRouter(); - // AllDownloadItemNotifier::Observer interface + // Called by ChromeDownloadManagerDelegate during the filename determination + // process, allows extensions to change the item's target filename. If no + // extension wants to change the target filename, then |no_change| will be + // called and the filename determination process will continue as normal. If + // an extension wants to change the target filename, then |change| will be + // called with the new filename and a flag indicating whether the new file + // should overwrite any old files of the same name. + void OnDeterminingFilename( + content::DownloadItem* item, + const base::FilePath& suggested_path, + const base::Closure& no_change, + const FilenameChangedCallback& change); + + // AllDownloadItemNotifier::Observer virtual void OnDownloadCreated( content::DownloadManager* manager, content::DownloadItem* download_item) OVERRIDE; @@ -230,6 +250,10 @@ class ExtensionDownloadsEventRouter content::DownloadManager* manager, content::DownloadItem* download_item) OVERRIDE; + // extensions::EventRouter::Observer + virtual void OnListenerRemoved( + const extensions::EventListenerInfo& details) OVERRIDE; + // Used for testing. struct DownloadsNotificationSource { std::string event_name; @@ -237,7 +261,11 @@ class ExtensionDownloadsEventRouter }; private: - void DispatchEvent(const char* event_name, base::Value* json_arg); + void DispatchEvent( + const char* event_name, + bool include_incognito, + const extensions::Event::WillDispatchCallback& will_dispatch_callback, + base::Value* json_arg); Profile* profile_; AllDownloadItemNotifier notifier_; diff --git a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc index d2d6cc9..7421bed 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc +++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc @@ -19,6 +19,7 @@ #include "chrome/browser/extensions/event_names.h" #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_function_test_utils.h" +#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/history/download_row.h" #include "chrome/browser/net/url_request_mock_util.h" #include "chrome/browser/profiles/profile.h" @@ -37,6 +38,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/common/page_transition_types.h" #include "content/public/test/download_test_observer.h" +#include "content/public/test/test_file_error_injector.h" #include "content/test/net/url_request_slow_download_job.h" #include "net/base/data_url.h" #include "net/base/net_util.h" @@ -103,8 +105,9 @@ class DownloadsEventsListener : public content::NotificationObserver { if ((profile_ != other.profile_) || (event_name_ != other.event_name_)) return false; - if ((event_name_ == events::kOnDownloadCreated || - event_name_ == events::kOnDownloadChanged) && + if (((event_name_ == events::kOnDownloadDeterminingFilename) || + (event_name_ == events::kOnDownloadCreated) || + (event_name_ == events::kOnDownloadChanged)) && args_.get() && other.args_.get()) { base::ListValue* left_list = NULL; @@ -258,6 +261,27 @@ class DownloadExtensionTest : public ExtensionApiTest { CHECK(extension_); } + content::RenderProcessHost* AddFilenameDeterminer() { + content::WebContents* tab = chrome::AddSelectedTabWithURL( + current_browser(), + extension_->GetResourceURL("empty.html"), + content::PAGE_TRANSITION_LINK); + extensions::ExtensionSystem::Get(current_browser()->profile())-> + event_router()->AddEventListener( + extensions::event_names::kOnDownloadDeterminingFilename, + tab->GetRenderProcessHost(), + GetExtensionId()); + return tab->GetRenderProcessHost(); + } + + void RemoveFilenameDeterminer(content::RenderProcessHost* host) { + extensions::ExtensionSystem::Get(current_browser()->profile())-> + event_router()->RemoveEventListener( + extensions::event_names::kOnDownloadDeterminingFilename, + host, + GetExtensionId()); + } + Browser* current_browser() { return current_browser_; } // InProcessBrowserTest @@ -313,6 +337,9 @@ class DownloadExtensionTest : public ExtensionApiTest { std::string GetExtensionURL() { return extension_->url().spec(); } + std::string GetExtensionId() { + return extension_->id(); + } std::string GetFilename(const char* path) { std::string result = @@ -675,7 +702,7 @@ class TestURLRequestContext : public net::URLRequestContext { DISALLOW_COPY_AND_ASSIGN(TestURLRequestContext); }; -// TODO(benjhayden): Comment. +// Writes an HTML5 file so that it can be downloaded. class HTML5FileWriter { public: HTML5FileWriter( @@ -1462,6 +1489,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, std::string download_url = test_server()->GetURL("slow?0").spec(); GoOnTheRecord(); + // Start downloading a file. scoped_ptr<base::Value> result(RunFunctionAndReturnResult( new DownloadsDownloadFunction(), base::StringPrintf( "[{\"url\": \"%s\"}]", download_url.c_str()))); @@ -1502,6 +1530,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, GoOffTheRecord(); std::string download_url = test_server()->GetURL("slow?0").spec(); + // Start downloading a file. scoped_ptr<base::Value> result(RunFunctionAndReturnResult( new DownloadsDownloadFunction(), base::StringPrintf( "[{\"url\": \"%s\"}]", download_url.c_str()))); @@ -1593,7 +1622,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, std::string download_url = test_server()->GetURL("slow?0").spec(); GoOnTheRecord(); - EXPECT_STREQ(download_extension_errors::kGenericError, + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, RunFunctionAndReturnError(new DownloadsDownloadFunction(), base::StringPrintf( "[{\"url\": \"%s\"," @@ -1609,7 +1638,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, std::string download_url = test_server()->GetURL("slow?0").spec(); GoOnTheRecord(); - EXPECT_STREQ(download_extension_errors::kGenericError, + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, RunFunctionAndReturnError(new DownloadsDownloadFunction(), base::StringPrintf( "[{\"url\": \"%s\"," @@ -1690,7 +1719,6 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, DownloadExtensionTest_Download_DataURL) { LoadExtension("downloads_split"); - CHECK(StartTestServer()); std::string download_url = "data:text/plain,hello"; GoOnTheRecord(); @@ -1738,7 +1766,6 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, MAYBE_DownloadExtensionTest_Download_File) { GoOnTheRecord(); - CHECK(StartTestServer()); LoadExtension("downloads_split"); std::string download_url = "file:///"; #if defined(OS_WIN) @@ -2145,3 +2172,1032 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, EXPECT_TRUE(file_util::ReadFileToString(item->GetFullPath(), &disk_data)); EXPECT_STREQ(kPayloadData, disk_data.c_str()); } + +IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_NoChange) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + // Wait for the onCreated and onDeterminingFilename events. + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + std::string error; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(), + false, + &error)); + EXPECT_EQ("", error); + + // The download should complete successfully. + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_DangerousOverride) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + std::string error; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("overridden.swf")), + false, + &error)); + EXPECT_EQ("", error); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"danger\": {" + " \"previous\":\"safe\"," + " \"current\":\"file\"}," + " \"dangerAccepted\": {" + " \"current\":false}}]", + result_id))); + + item->DangerousDownloadValidated(); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"dangerAccepted\": {" + " \"previous\":false," + " \"current\":true}}]", + result_id))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("overridden.swf.crdownload").c_str(), + GetFilename("overridden.swf").c_str()))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_ReferencesParentInvalid) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + std::string error; + ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("sneaky/../../sneaky.txt")), + false, + &error)); + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, error.c_str()); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_CurDirInvalid) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + std::string error; + ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL(".")), + false, + &error)); + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, error.c_str()); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_ParentDirInvalid) { + CHECK(StartTestServer()); + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + std::string error; + ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("..")), + false, + &error)); + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, error.c_str()); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_AbsPathInvalid) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. Absolute paths should be rejected. + std::string error; + ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + downloads_directory().Append(FILE_PATH_LITERAL("sneaky.txt")), + false, + &error)); + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, error.c_str()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_EmptyBasenameInvalid) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. Empty basenames should be rejected. + std::string error; + ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("foo/")), + false, + &error)); + EXPECT_STREQ(download_extension_errors::kInvalidFilenameError, error.c_str()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_Override) { + GoOnTheRecord(); + LoadExtension("downloads_split"); + AddFilenameDeterminer(); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + std::string error; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(), + false, + &error)); + EXPECT_EQ("", error); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("slow.txt.crdownload").c_str(), + GetFilename("slow.txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); + + // Start downloading a file. + result.reset(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller2(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + // Also test that DetermineFilename allows (chrome) extensions to set + // filenames without (filename) extensions. (Don't ask about v8 extensions or + // python extensions or kernel extensions or firefox extensions...) + error = ""; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("foo")), + true, + &error)); + EXPECT_EQ("", error); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("foo.crdownload").c_str(), + GetFilename("foo").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); +} + +// TODO test precedence rules: install_time + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_RemoveFilenameDeterminer) { + CHECK(StartTestServer()); + GoOnTheRecord(); + LoadExtension("downloads_split"); + content::RenderProcessHost* host = AddFilenameDeterminer(); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + // Start downloading a file. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Remove a determiner while waiting for it. + RemoveFilenameDeterminer(host); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_IncognitoSplit) { + LoadExtension("downloads_split"); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + GoOnTheRecord(); + AddFilenameDeterminer(); + + GoOffTheRecord(); + AddFilenameDeterminer(); + + // Start an on-record download. + GoOnTheRecord(); + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + // Wait for the onCreated and onDeterminingFilename events. + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"incognito\": false," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename events. + std::string error; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + current_browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("42.txt")), + false, + &error)); + EXPECT_EQ("", error); + + // The download should complete successfully. + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("42.txt.crdownload").c_str(), + GetFilename("42.txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); + + // Start an incognito download for comparison. + GoOffTheRecord(); + result.reset(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller2(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": true," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + // On-Record renderers should not see events for off-record items. + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"incognito\": true," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + error = ""; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + current_browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("5.txt")), + false, + &error)); + EXPECT_EQ("", error); + + // The download should complete successfully. + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("5.txt.crdownload").c_str(), + GetFilename("5.txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); +} + +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + DownloadExtensionTest_OnDeterminingFilename_IncognitoSpanning) { + LoadExtension("downloads_spanning"); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + + GoOnTheRecord(); + AddFilenameDeterminer(); + + // There is a single extension renderer that sees both on-record and + // off-record events. The extension functions see the on-record profile with + // include_incognito=true. + + // Start an on-record download. + GoOnTheRecord(); + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + // Wait for the onCreated and onDeterminingFilename events. + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"incognito\": false," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename events. + std::string error; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + current_browser()->profile(), + true, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("42.txt")), + false, + &error)); + EXPECT_EQ("", error); + + // The download should complete successfully. + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("42.txt.crdownload").c_str(), + GetFilename("42.txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); + + // Start an incognito download for comparison. + GoOffTheRecord(); + result.reset(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller2(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": true," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"incognito\": true," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + // Respond to the onDeterminingFilename. + error = ""; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + current_browser()->profile(), + true, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("42.txt")), + false, + &error)); + EXPECT_EQ("", error); + + // The download should complete successfully. + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("42 (1).txt.crdownload").c_str(), + GetFilename("42 (1).txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); +} + +// Test download interruption while extensions determining filename, re-run +// through fan-out and fan-in. +// TODO(rdsmith): FILE_OPERATION_INITIALIZE is not right for this test. +IN_PROC_BROWSER_TEST_F( + DownloadExtensionTest, + FAILS_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume) { + LoadExtension("downloads_split"); + CHECK(StartTestServer()); + std::string download_url = test_server()->GetURL("slow?0").spec(); + GoOnTheRecord(); + AddFilenameDeterminer(); + + // TODO Interrupt the download instead of responding to onDeterminingFilename. + scoped_refptr<content::TestFileErrorInjector> injector( + content::TestFileErrorInjector::Create( + GetCurrentManager())); + content::TestFileErrorInjector::FileErrorInfo error_info = { + download_url, + content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE, + 0, + content::DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE}; + injector->AddError(error_info); + injector->InjectErrors(); + + // Start a download. + scoped_ptr<base::Value> result(RunFunctionAndReturnResult( + new DownloadsDownloadFunction(), base::StringPrintf( + "[{\"url\": \"%s\"}]", download_url.c_str()))); + ASSERT_TRUE(result.get()); + int result_id = -1; + ASSERT_TRUE(result->GetAsInteger(&result_id)); + DownloadItem* item = GetCurrentManager()->GetDownload(result_id); + ASSERT_TRUE(item); + ScopedCancellingItem canceller(item); + ASSERT_EQ(download_url, item->GetOriginalUrl().spec()); + + // Wait for the onCreated and onDeterminingFilename event. + ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, + base::StringPrintf("[{\"danger\": \"safe\"," + " \"incognito\": false," + " \"id\": %d," + " \"mime\": \"text/plain\"," + " \"paused\": false," + " \"url\": \"%s\"}]", + result_id, + download_url.c_str()))); + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"incognito\": false," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"interrupted\"}}]", + result_id))); + ASSERT_TRUE(item->IsInterrupted()); + item->ResumeInterruptedDownload(); + + // Wait for and respond to the onDeterminingFilename event. + ASSERT_TRUE(WaitFor( + events::kOnDownloadDeterminingFilename, + base::StringPrintf("[{\"id\": %d," + " \"incognito\": false," + " \"filename\":\"slow.txt\"}]", + result_id))); + ASSERT_TRUE(item->GetTargetFilePath().empty()); + ASSERT_TRUE(item->IsInProgress()); + std::string error; + ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( + current_browser()->profile(), + false, + GetExtensionId(), + result_id, + base::FilePath(FILE_PATH_LITERAL("42.txt")), + false, + &error)); + EXPECT_EQ("", error); + + // The download should complete successfully. + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"filename\": {" + " \"previous\": \"%s\"," + " \"current\": \"%s\"}," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id, + GetFilename("42.txt.crdownload").c_str(), + GetFilename("42.txt").c_str()))); + ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, + base::StringPrintf("[{\"id\": %d," + " \"state\": {" + " \"previous\": \"in_progress\"," + " \"current\": \"complete\"}}]", + result_id))); +} + +// TODO(benjhayden) Figure out why DisableExtension() does not fire +// OnListenerRemoved. diff --git a/chrome/browser/extensions/api/downloads_internal/downloads_internal_api.cc b/chrome/browser/extensions/api/downloads_internal/downloads_internal_api.cc new file mode 100644 index 0000000..7d6fb79 --- /dev/null +++ b/chrome/browser/extensions/api/downloads_internal/downloads_internal_api.cc @@ -0,0 +1,35 @@ +// Copyright (c) 2012 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/extensions/api/downloads_internal/downloads_internal_api.h" + +#include "chrome/browser/extensions/api/downloads/downloads_api.h" +#include "chrome/common/extensions/api/downloads_internal.h" + +DownloadsInternalDetermineFilenameFunction:: + DownloadsInternalDetermineFilenameFunction() {} + +DownloadsInternalDetermineFilenameFunction:: + ~DownloadsInternalDetermineFilenameFunction() {} + +typedef extensions::api::downloads_internal::DetermineFilename::Params + DetermineFilenameParams; + +bool DownloadsInternalDetermineFilenameFunction::RunImpl() { + scoped_ptr<DetermineFilenameParams> params( + DetermineFilenameParams::Create(*args_)); + EXTENSION_FUNCTION_VALIDATE(params.get()); + base::FilePath::StringType filename; + if (params->filename.get()) { + EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &filename)); + } + return ExtensionDownloadsEventRouter::DetermineFilename( + profile(), + include_incognito(), + GetExtension()->id(), + params->download_id, + base::FilePath(filename), + params->overwrite && *params->overwrite, + &error_); +} diff --git a/chrome/browser/extensions/api/downloads_internal/downloads_internal_api.h b/chrome/browser/extensions/api/downloads_internal/downloads_internal_api.h new file mode 100644 index 0000000..69780e0 --- /dev/null +++ b/chrome/browser/extensions/api/downloads_internal/downloads_internal_api.h @@ -0,0 +1,25 @@ +// Copyright (c) 2012 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_EXTENSIONS_API_DOWNLOADS_INTERNAL_DOWNLOADS_INTERNAL_API_H_ +#define CHROME_BROWSER_EXTENSIONS_API_DOWNLOADS_INTERNAL_DOWNLOADS_INTERNAL_API_H_ + +#include "chrome/browser/extensions/extension_function.h" + +class DownloadsInternalDetermineFilenameFunction + : public AsyncExtensionFunction { + public: + DECLARE_EXTENSION_FUNCTION("downloadsInternal.determineFilename", + DOWNLOADSINTERNAL_DETERMINEFILENAME); + DownloadsInternalDetermineFilenameFunction(); + virtual bool RunImpl() OVERRIDE; + + protected: + virtual ~DownloadsInternalDetermineFilenameFunction(); + + private: + DISALLOW_COPY_AND_ASSIGN(DownloadsInternalDetermineFilenameFunction); +}; + +#endif // CHROME_BROWSER_EXTENSIONS_API_DOWNLOADS_INTERNAL_DOWNLOADS_INTERNAL_API_H_ diff --git a/chrome/browser/extensions/api/web_request/web_request_api.cc b/chrome/browser/extensions/api/web_request/web_request_api.cc index 7a09ec4..24c9f52 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api.cc +++ b/chrome/browser/extensions/api/web_request/web_request_api.cc @@ -369,6 +369,9 @@ void SendOnMessageEventOnUI( // Represents a single unique listener to an event, along with whatever filter // parameters and extra_info_spec were specified at the time the listener was // added. +// NOTE(benjhayden) New APIs should not use this sub_event_name trick! It does +// not play well with event pages. See downloads.onDeterminingFilename and +// ExtensionDownloadsEventRouter for an alternative approach. struct ExtensionWebRequestEventRouter::EventListener { std::string extension_id; std::string extension_name; diff --git a/chrome/browser/extensions/event_names.cc b/chrome/browser/extensions/event_names.cc index 5e53c30..3db4f58 100644 --- a/chrome/browser/extensions/event_names.cc +++ b/chrome/browser/extensions/event_names.cc @@ -53,6 +53,7 @@ const char kOnDialError[] = "dial.onError"; const char kOnDownloadCreated[] = "downloads.onCreated"; const char kOnDownloadChanged[] = "downloads.onChanged"; const char kOnDownloadErased[] = "downloads.onErased"; +const char kOnDownloadDeterminingFilename[] = "downloads.onDeterminingFilename"; const char kOnSettingsChanged[] = "storage.onChanged"; diff --git a/chrome/browser/extensions/event_names.h b/chrome/browser/extensions/event_names.h index c4b10c3..a292497 100644 --- a/chrome/browser/extensions/event_names.h +++ b/chrome/browser/extensions/event_names.h @@ -59,6 +59,7 @@ extern const char kOnDialError[]; extern const char kOnDownloadCreated[]; extern const char kOnDownloadChanged[]; extern const char kOnDownloadErased[]; +extern const char kOnDownloadDeterminingFilename[]; // Settings. extern const char kOnSettingsChanged[]; diff --git a/chrome/browser/extensions/extension_function_histogram_value.h b/chrome/browser/extensions/extension_function_histogram_value.h index d1babe8..0f4f0f3 100644 --- a/chrome/browser/extensions/extension_function_histogram_value.h +++ b/chrome/browser/extensions/extension_function_histogram_value.h @@ -160,7 +160,6 @@ enum HistogramValue { TABS_RELOAD, WINDOWS_CREATE, DEVELOPERPRIVATE_LOADUNPACKED, - DOWNLOADS_SETDESTINATION, EXPERIMENTAL_PROCESSES_GETPROCESSIDFORTAB, BOOKMARKS_GETCHILDREN, BROWSERACTION_GETTITLE, @@ -172,6 +171,7 @@ enum HistogramValue { TABS_GETCURRENT, FONTSETTINGS_CLEARDEFAULTFIXEDFONTSIZE, MEDIAPLAYERPRIVATE_CLOSEWINDOW, + DOWNLOADSINTERNAL_DETERMINEFILENAME, WEBREQUESTINTERNAL_ADDEVENTLISTENER, CLOUDPRINTPRIVATE_GETPRINTERS, STORAGE_SET, diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index 5affe99..48201a1 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -203,6 +203,8 @@ 'browser/extensions/api/downloads/downloads_api.h', 'browser/extensions/api/extension_action/extension_action_api.cc', 'browser/extensions/api/extension_action/extension_action_api.h', + 'browser/extensions/api/downloads_internal/downloads_internal_api.cc', + 'browser/extensions/api/downloads_internal/downloads_internal_api.h', 'browser/extensions/api/extension_action/extension_page_actions_api_constants.cc', 'browser/extensions/api/extension_action/extension_page_actions_api_constants.h', 'browser/extensions/api/file_handlers/app_file_handler_util.cc', diff --git a/chrome/common/extensions/api/api.gyp b/chrome/common/extensions/api/api.gyp index ed53462..62e9a2c 100644 --- a/chrome/common/extensions/api/api.gyp +++ b/chrome/common/extensions/api/api.gyp @@ -37,6 +37,7 @@ 'dial.idl', 'downloads.idl', 'echo_private.json', + 'downloads_internal.idl', 'events.json', 'experimental_accessibility.json', 'experimental_app.json', diff --git a/chrome/common/extensions/api/downloads.idl b/chrome/common/extensions/api/downloads.idl index 1807715b..0ad5a4a 100644 --- a/chrome/common/extensions/api/downloads.idl +++ b/chrome/common/extensions/api/downloads.idl @@ -12,6 +12,20 @@ namespace downloads { DOMString value; }; + // $ref:onDeterminingFilename listeners may pass a $ref:FilenameSuggestion + // object to <code>suggest()</code> in order to override a download's target + // filename. + dictionary FilenameSuggestion { + // The $ref:DownloadItem's new target $ref:DownloadItem.filename, as a path + // relative to the user's default Downloads directory, possibly containing + // subdirectories. Absolute paths, empty paths, and paths containing + // back-references ".." will be ignored. + DOMString filename; + + // Whether to overwrite any existing files. + boolean? overwrite; + }; + [inline_doc] enum HttpMethod {GET, POST}; [inline_doc] dictionary DownloadOptions { @@ -19,7 +33,9 @@ namespace downloads { DOMString url; // A file path relative to the Downloads directory to contain the downloaded - // file. + // file. See $ref:onDeterminingFilename for how to dynamically suggest a + // filename after the file's MIME type and a tentative filename have been + // determined. DOMString? filename; // Use a file-chooser to allow the user to select a filename. @@ -313,6 +329,8 @@ namespace downloads { callback EraseCallback = void(long[] erasedIds); callback NullCallback = void(); callback GetFileIconCallback = void(optional DOMString iconURL); + callback SuggestFilenameCallback = void( + optional FilenameSuggestion suggestion); interface Functions { // Download a URL. If the URL uses the HTTP[S] protocol, then the request @@ -323,10 +341,9 @@ namespace downloads { // <code>callback</code> will be called with the new $ref:DownloadItem's // <code>downloadId</code>. If there was an error starting the download, // then <code>callback</code> will be called with - // <code>downloadId=undefined</code> and - // $ref:runtime.lastError - // will contain a descriptive string. The error strings are not guaranteed - // to remain backwards compatible between releases. You must not parse it. + // <code>downloadId=undefined</code> and $ref:runtime.lastError will contain + // a descriptive string. The error strings are not guaranteed to remain + // backwards compatible between releases. Extensions must not parse it. // |options|: What to download and how. // |callback|: Called with the id of the new $ref:DownloadItem. static void download(DownloadOptions options, @@ -396,9 +413,6 @@ namespace downloads { // then <code>callback</code> will be called. static void erase(DownloadQuery query, optional EraseCallback callback); - // TODO(benjhayden) Comment. - [nodoc] static void setDestination(long downloadId, DOMString relativePath); - // Prompt the user to accept a dangerous download. Does not automatically // accept dangerous downloads. If the download is accepted, then an // $ref:onChanged event will fire, otherwise nothing will happen. When all @@ -427,5 +441,29 @@ namespace downloads { // except <code>bytesReceived</code> changes, this event fires with the // <code>downloadId</code> and an object containing the properties that changed. static void onChanged(DownloadDelta downloadDelta); + + // During the filename determination process, extensions will be given the + // opportunity to override the target $ref:DownloadItem.filename. Each + // extension may not register more than one listener for this event. Each + // listener must call <code>suggest</code> exactly once, either + // synchronously or asynchronously. If the listener calls + // <code>suggest</code> asynchronously, then it must return + // <code>true</code>. If the listener neither calls <code>suggest</code> + // synchronously nor returns <code>true</code>, then <code>suggest</code> + // will be called automatically. The $ref:DownloadItem will not complete + // until all listeners have called <code>suggest</code>. Listeners may call + // <code>suggest</code> without any arguments in order to allow the download + // to use <code>downloadItem.filename</code> for its filename, or pass a + // $ref:FilenameSuggestion object to <code>suggest</code> in order to + // override the target filename. If more than one extension overrides the + // filename, then the last extension installed whose listener passes a + // $ref:FilenameSuggestion object to <code>suggest</code> wins. In order to + // avoid confusion regarding which extension will win, users should not + // install extensions that may conflict. If the download is initiated by + // $ref:download and the target filename is known before the MIME type and + // tentative filename have been determined, use + // $ref:DownloadOptions.filename instead. + [maxListeners=1] static void onDeterminingFilename( + DownloadItem downloadItem, SuggestFilenameCallback suggest); }; }; diff --git a/chrome/common/extensions/api/downloads_internal.idl b/chrome/common/extensions/api/downloads_internal.idl new file mode 100644 index 0000000..449ae87 --- /dev/null +++ b/chrome/common/extensions/api/downloads_internal.idl @@ -0,0 +1,16 @@ +// Copyright (c) 2012 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. + +[permissions=downloads_internal, internal=true, nodoc=true] +namespace downloadsInternal { + interface Functions { + // Secretly called when onDeterminingFilename handlers return. + static void determineFilename(long downloadId, + optional DOMString filename, + optional boolean overwrite); + }; + + interface Events { + }; +}; diff --git a/chrome/common/extensions/docs/examples/api/downloads/downloads_overwrite/bg.js b/chrome/common/extensions/docs/examples/api/downloads/downloads_overwrite/bg.js new file mode 100644 index 0000000..d7d9047 --- /dev/null +++ b/chrome/common/extensions/docs/examples/api/downloads/downloads_overwrite/bg.js @@ -0,0 +1,11 @@ +// Copyright (c) 2012 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. + +// Force all downloads to overwrite any existing files instead of inserting +// ' (1)', ' (2)', etc. + +chrome.downloads.onDeterminingFilename.addListener(function(item, suggest) { + suggest({filename: item.filename, + overwrite: true}); +}); diff --git a/chrome/common/extensions/docs/examples/api/downloads/downloads_overwrite/manifest.json b/chrome/common/extensions/docs/examples/api/downloads/downloads_overwrite/manifest.json new file mode 100644 index 0000000..c1f8999 --- /dev/null +++ b/chrome/common/extensions/docs/examples/api/downloads/downloads_overwrite/manifest.json @@ -0,0 +1,14 @@ +{ + "name": "Downloads Overwrite Existing Files", + "description": "All downloads overwrite existing files instead of adding ' (1)', ' (2)', etc.", + "version": "1", + "minimum_chrome_version": "26.0.1428", + "background": { + "scripts": ["bg.js"], + "persistent": false + }, + "permissions": [ + "downloads" + ], + "manifest_version": 2 +} diff --git a/chrome/common/extensions/permissions/api_permission.cc b/chrome/common/extensions/permissions/api_permission.cc index 818e801..c4d0a12 100644 --- a/chrome/common/extensions/permissions/api_permission.cc +++ b/chrome/common/extensions/permissions/api_permission.cc @@ -252,6 +252,7 @@ void APIPermissionInfo::RegisterAllPermissions( { APIPermission::kDeveloperPrivate, "developerPrivate", kFlagCannotBeOptional }, { APIPermission::kDial, "dial", kFlagCannotBeOptional }, + { APIPermission::kDownloadsInternal, "downloadsInternal" }, { APIPermission::kFileBrowserHandlerInternal, "fileBrowserHandlerInternal", kFlagCannotBeOptional }, { APIPermission::kFileBrowserPrivate, "fileBrowserPrivate", diff --git a/chrome/common/extensions/permissions/api_permission.h b/chrome/common/extensions/permissions/api_permission.h index 10659bd..5eefdf1 100644 --- a/chrome/common/extensions/permissions/api_permission.h +++ b/chrome/common/extensions/permissions/api_permission.h @@ -67,6 +67,7 @@ class APIPermission { kDeveloperPrivate, kDevtools, kDownloads, + kDownloadsInternal, kEchoPrivate, kExperimental, kFileBrowserHandler, diff --git a/chrome/common/extensions/permissions/permission_set.cc b/chrome/common/extensions/permissions/permission_set.cc index 4c4327c..b235a4d 100644 --- a/chrome/common/extensions/permissions/permission_set.cc +++ b/chrome/common/extensions/permissions/permission_set.cc @@ -511,6 +511,10 @@ std::set<std::string> PermissionSet::GetDistinctHosts( } void PermissionSet::InitImplicitPermissions() { + // The downloads permission implies the internal version as well. + if (apis_.find(APIPermission::kDownloads) != apis_.end()) + apis_.insert(APIPermission::kDownloadsInternal); + // The webRequest permission implies the internal version as well. if (apis_.find(APIPermission::kWebRequest) != apis_.end()) apis_.insert(APIPermission::kWebRequestInternal); diff --git a/chrome/common/extensions/permissions/permission_set_unittest.cc b/chrome/common/extensions/permissions/permission_set_unittest.cc index 695f89a9..cd6a7fb 100644 --- a/chrome/common/extensions/permissions/permission_set_unittest.cc +++ b/chrome/common/extensions/permissions/permission_set_unittest.cc @@ -714,6 +714,7 @@ TEST_F(PermissionsTest, PermissionMessages) { skip.insert(APIPermission::kCloudPrintPrivate); skip.insert(APIPermission::kDeveloperPrivate); skip.insert(APIPermission::kDial); + skip.insert(APIPermission::kDownloadsInternal); skip.insert(APIPermission::kEchoPrivate); skip.insert(APIPermission::kFileBrowserHandlerInternal); skip.insert(APIPermission::kFileBrowserPrivate); diff --git a/chrome/renderer/extensions/dispatcher.cc b/chrome/renderer/extensions/dispatcher.cc index 6f519d7..c085cbd 100644 --- a/chrome/renderer/extensions/dispatcher.cc +++ b/chrome/renderer/extensions/dispatcher.cc @@ -786,6 +786,8 @@ void Dispatcher::PopulateSourceMap() { IDR_DECLARATIVE_CONTENT_CUSTOM_BINDINGS_JS); source_map_.RegisterSource("declarativeWebRequest", IDR_DECLARATIVE_WEBREQUEST_CUSTOM_BINDINGS_JS); + source_map_.RegisterSource("downloads", + IDR_DOWNLOADS_CUSTOM_BINDINGS_JS); source_map_.RegisterSource( "experimental.mediaGalleries", IDR_EXPERIMENTAL_MEDIA_GALLERIES_CUSTOM_BINDINGS_JS); diff --git a/chrome/renderer/resources/extensions/downloads_custom_bindings.js b/chrome/renderer/resources/extensions/downloads_custom_bindings.js new file mode 100644 index 0000000..fd17af2 --- /dev/null +++ b/chrome/renderer/resources/extensions/downloads_custom_bindings.js @@ -0,0 +1,48 @@ +// Copyright (c) 2013 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. + +// Custom bindings for the downloads API. + +var chromeHidden = requireNative('chrome_hidden').GetChromeHidden(); + +chromeHidden.Event.registerArgumentMassager( + 'downloads.onDeterminingFilename', + function massage_determining_filename(args, dispatch) { + var downloadItem = args[0]; + // Copy the id so that extensions can't change it. + var downloadId = downloadItem.id; + var suggestable = true; + function suggestCallback(result) { + if (!suggestable) { + console.error('suggestCallback may not be called more than once.'); + return; + } + suggestable = false; + if ((typeof(result) == 'object') && + result.filename && + (typeof(result.filename) == 'string') && + ((result.overwrite === undefined) || + (typeof(result.overwrite) == 'boolean'))) { + chromeHidden.internalAPIs.downloadsInternal.determineFilename( + downloadId, + result.filename, + result.overwrite || false); + } else { + chromeHidden.internalAPIs.downloadsInternal.determineFilename( + downloadId); + } + } + try { + var results = dispatch([downloadItem, suggestCallback]); + var async = (results && + results.results && + (results.results.length != 0) && + (results.results[0] === true)); + if (suggestable && !async) + suggestCallback(); + } catch (e) { + suggestCallback(); + throw e; + } +}); diff --git a/chrome/renderer/resources/extensions/event.js b/chrome/renderer/resources/extensions/event.js index 0ccd586..59e04d5 100644 --- a/chrome/renderer/resources/extensions/event.js +++ b/chrome/renderer/resources/extensions/event.js @@ -213,6 +213,7 @@ var result = event.dispatch_(args, listenerIDs); if (result) DCHECK(!result.validationErrors, result.validationErrors); + return result; }; if (eventArgumentMassagers[name]) diff --git a/chrome/renderer/resources/extensions/web_request_custom_bindings.js b/chrome/renderer/resources/extensions/web_request_custom_bindings.js index b1b29b5..135da99 100644 --- a/chrome/renderer/resources/extensions/web_request_custom_bindings.js +++ b/chrome/renderer/resources/extensions/web_request_custom_bindings.js @@ -62,6 +62,9 @@ WebRequestEvent.prototype.addListener = function(cb, opt_filter, opt_extraInfo) { if (!this.eventOptions_.supportsListeners) throw new Error('This event does not support listeners.'); + // NOTE(benjhayden) New APIs should not use this subEventName trick! It does + // not play well with event pages. See downloads.onDeterminingFilename and + // ExtensionDownloadsEventRouter for an alternative approach. var subEventName = GetUniqueSubEventName(this.eventName_); // Note: this could fail to validate, in which case we would not add the // subEvent listener. diff --git a/chrome/renderer/resources/renderer_resources.grd b/chrome/renderer/resources/renderer_resources.grd index 65dcd9f..8a95160 100644 --- a/chrome/renderer/resources/renderer_resources.grd +++ b/chrome/renderer/resources/renderer_resources.grd @@ -53,6 +53,7 @@ without changes to the corresponding grd file. fb9 --> <include name="IDR_CONTEXT_MENUS_CUSTOM_BINDINGS_JS" file="extensions\context_menus_custom_bindings.js" type="BINDATA" /> <include name="IDR_DECLARATIVE_CONTENT_CUSTOM_BINDINGS_JS" file="extensions\declarative_content_custom_bindings.js" type="BINDATA" /> <include name="IDR_DECLARATIVE_WEBREQUEST_CUSTOM_BINDINGS_JS" file="extensions\declarative_webrequest_custom_bindings.js" type="BINDATA" /> + <include name="IDR_DOWNLOADS_CUSTOM_BINDINGS_JS" file="extensions\downloads_custom_bindings.js" type="BINDATA" /> <include name="IDR_BLUETOOTH_CUSTOM_BINDINGS_JS" file="extensions\bluetooth_custom_bindings.js" type="BINDATA" /> <include name="IDR_PERMISSIONS_CUSTOM_BINDINGS_JS" file="extensions\permissions_custom_bindings.js" type="BINDATA" /> <include name="IDR_EXPERIMENTAL_MEDIA_GALLERIES_CUSTOM_BINDINGS_JS" file="extensions\experimental.media_galleries_custom_bindings.js" type="BINDATA" /> diff --git a/chrome/test/data/extensions/api_test/downloads_spanning/empty.html b/chrome/test/data/extensions/api_test/downloads_spanning/empty.html new file mode 100644 index 0000000..ec31027 --- /dev/null +++ b/chrome/test/data/extensions/api_test/downloads_spanning/empty.html @@ -0,0 +1,4 @@ +<!-- +DownloadExtensionTest needs an Extension object and a RenderViewHost to pass to +some ExtensionFunctions. This extension doesn't need to do anything. +--> diff --git a/chrome/test/data/extensions/api_test/downloads_spanning/manifest.json b/chrome/test/data/extensions/api_test/downloads_spanning/manifest.json new file mode 100644 index 0000000..e5b463f --- /dev/null +++ b/chrome/test/data/extensions/api_test/downloads_spanning/manifest.json @@ -0,0 +1,13 @@ +{ + "name": "downloads incognito spanning apitest", + "version": "0.1", + "manifest_version": 2, + "description": "downloads incognito spanning apitest", + "incognito": "spanning", + "web_accessible_resources": ["empty.html"], + "permissions": [ + "downloads", + "http://*/*", + "file://*" + ] +} diff --git a/chrome/test/data/extensions/api_test/downloads_split/manifest.json b/chrome/test/data/extensions/api_test/downloads_split/manifest.json index 89e42db..7b2baea 100644 --- a/chrome/test/data/extensions/api_test/downloads_split/manifest.json +++ b/chrome/test/data/extensions/api_test/downloads_split/manifest.json @@ -4,6 +4,7 @@ "manifest_version": 2, "description": "downloads incognito split apitest", "incognito": "split", + "web_accessible_resources": ["empty.html"], "permissions": [ "http://*/*", "file://*" diff --git a/tools/json_schema_compiler/idl_schema.py b/tools/json_schema_compiler/idl_schema.py index 62e0e56..8c1bc76 100644 --- a/tools/json_schema_compiler/idl_schema.py +++ b/tools/json_schema_compiler/idl_schema.py @@ -164,6 +164,16 @@ class Member(object): for property_name in ('OPTIONAL', 'nodoc', 'nocompile', 'nodart'): if self.node.GetProperty(property_name): properties[property_name.lower()] = True + for option_name, sanitizer in [ + ('maxListeners', int), + ('supportsFilters', lambda s: s == 'true'), + ('supportsListeners', lambda s: s == 'true'), + ('supportsRules', lambda s: s == 'true')]: + if self.node.GetProperty(option_name): + if 'options' not in properties: + properties['options'] = {} + properties['options'][option_name] = sanitizer(self.node.GetProperty( + option_name)) is_function = False parameter_comments = OrderedDict() for node in self.node.children: |