diff options
26 files changed, 204 insertions, 167 deletions
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index 707c75f..33b7733 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -260,8 +260,6 @@ DictionaryValue* AutomationProvider::GetDictionaryFromDownloadItem( dl_item_value->SetBoolean("is_paused", download->is_paused()); dl_item_value->SetBoolean("open_when_complete", download->open_when_complete()); - dl_item_value->SetBoolean("is_extension_install", - download->is_extension_install()); dl_item_value->SetBoolean("is_temporary", download->is_temporary()); dl_item_value->SetBoolean("is_otr", download->is_otr()); // incognito dl_item_value->SetString("state", state_to_string[download->state()]); diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc index 0e624d9..99267e9 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/chrome/browser/download/chrome_download_manager_delegate.cc @@ -10,6 +10,7 @@ #include "base/rand_util.h" #include "base/stringprintf.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/download/download_crx_util.h" #include "chrome/browser/download/download_extensions.h" #include "chrome/browser/download/download_file_picker.h" #include "chrome/browser/download/download_history.h" @@ -17,6 +18,7 @@ #include "chrome/browser/download/download_safe_browsing_client.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/download/save_package_file_picker.h" +#include "chrome/browser/extensions/crx_installer.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/prefs/pref_member.h" #include "chrome/browser/prefs/pref_service.h" @@ -31,6 +33,7 @@ #include "content/browser/download/download_manager.h" #include "content/browser/download/download_status_updater.h" #include "content/browser/tab_contents/tab_contents.h" +#include "content/common/notification_source.h" #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -42,6 +45,15 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile) ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() { } +bool ChromeDownloadManagerDelegate::IsExtensionDownload( + const DownloadItem* item) { + if (item->prompt_user_for_save_location()) + return false; + + return (item->mime_type() == Extension::kMimeType) || + UserScript::IsURLUserScript(item->GetURL(), item->mime_type()); +} + void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) { download_manager_ = dm; download_history_.reset(new DownloadHistory(profile_)); @@ -111,6 +123,36 @@ bool ChromeDownloadManagerDelegate::ShouldOpenFileBasedOnExtension( return download_prefs_->IsAutoOpenEnabledForExtension(extension); } +bool ChromeDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) { + if (!IsExtensionDownload(item)) + return true; + + download_crx_util::OpenChromeExtension(profile_, *item); + return false; +} + +bool ChromeDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) { + if (!IsExtensionDownload(item)) + return true; + + scoped_refptr<CrxInstaller> crx_installer = + download_crx_util::OpenChromeExtension(profile_, *item); + + // CRX_INSTALLER_DONE will fire when the install completes. Observe() + // will call CompleteDelayedDownload() on this item. If this DownloadItem is + // not around when CRX_INSTALLER_DONE fires, Complete() will not be called. + registrar_.Add(this, + chrome::NOTIFICATION_CRX_INSTALLER_DONE, + Source<CrxInstaller>(crx_installer.get())); + + crx_installers_[crx_installer.get()] = item->id(); + // The status text and percent complete indicator will change now + // that we are installing a CRX. Update observers so that they pick + // up the change. + item->UpdateObservers(); + return false; +} + bool ChromeDownloadManagerDelegate::GenerateFileHash() { #if defined(ENABLE_SAFE_BROWSING) return profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled) && @@ -216,6 +258,26 @@ void ChromeDownloadManagerDelegate::CheckDownloadUrlDone( &ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone)); } +// NotificationObserver implementation. +void ChromeDownloadManagerDelegate::Observe( + int type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK(type == chrome::NOTIFICATION_CRX_INSTALLER_DONE); + + registrar_.Remove(this, + chrome::NOTIFICATION_CRX_INSTALLER_DONE, + source); + + CrxInstaller* installer = Source<CrxInstaller>(source).ptr(); + int download_id = crx_installers_[installer]; + crx_installers_.erase(installer); + + DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id); + if (item) + item->CompleteDelayedDownload(); +} + void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone( int32 download_id, bool visited_referrer_before) { @@ -229,13 +291,6 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone( // Check whether this download is for an extension install or not. // Allow extensions to be explicitly saved. DownloadStateInfo state = download->state_info(); - if (!state.prompt_user_for_save_location) { - if (UserScript::IsURLUserScript(download->GetURL(), - download->mime_type()) || - (download->mime_type() == Extension::kMimeType)) { - state.is_extension_install = true; - } - } if (state.force_file_name.empty()) { FilePath generated_name; @@ -252,7 +307,7 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone( // "save as...". // 2) Filetypes marked "always open." If the user just wants this file // opened, don't bother asking where to keep it. - if (!state.is_extension_install && + if (!IsExtensionDownload(download) && !ShouldOpenFileBasedOnExtension(generated_name)) state.prompt_user_for_save_location = true; } @@ -413,7 +468,7 @@ bool ChromeDownloadManagerDelegate::IsDangerousFile( (!state.has_user_gesture || !visited_referrer_before)) return true; - if (state.is_extension_install) { + if (IsExtensionDownload(&download)) { // Extensions that are not from the gallery are considered dangerous. ExtensionService* service = profile_->GetExtensionService(); if (!service || !service->IsDownloadFromGallery(download.GetURL(), diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h index 9b6faab..f9e571c 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.h +++ b/chrome/browser/download/chrome_download_manager_delegate.h @@ -7,11 +7,15 @@ #pragma once #include "base/compiler_specific.h" +#include "base/hash_tables.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/task.h" #include "content/browser/download/download_manager_delegate.h" +#include "content/common/notification_observer.h" +#include "content/common/notification_registrar.h" +class CrxInstaller; class DownloadHistory; class DownloadItem; class DownloadManager; @@ -19,13 +23,30 @@ class DownloadPrefs; class Profile; struct DownloadStateInfo; +#if defined(COMPILER_GCC) +namespace __gnu_cxx { + +template<> +struct hash<CrxInstaller*> { + std::size_t operator()(CrxInstaller* const& p) const { + return reinterpret_cast<std::size_t>(p); + } +}; + +} // namespace __gnu_cxx +#endif + // This is the Chrome side helper for the download system. class ChromeDownloadManagerDelegate : public base::RefCountedThreadSafe<ChromeDownloadManagerDelegate>, - public DownloadManagerDelegate { + public DownloadManagerDelegate, + public NotificationObserver { public: explicit ChromeDownloadManagerDelegate(Profile* profile); + // Returns true if the given item is for an extension download. + static bool IsExtensionDownload(const DownloadItem* item); + void SetDownloadManager(DownloadManager* dm); virtual void Shutdown() OVERRIDE; @@ -35,6 +56,8 @@ class ChromeDownloadManagerDelegate void* data) OVERRIDE; virtual TabContents* GetAlternativeTabContentsToNotifyForDownload() OVERRIDE; virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE; + virtual bool ShouldOpenDownload(DownloadItem* item) OVERRIDE; + virtual bool ShouldCompleteDownload(DownloadItem* item) OVERRIDE; virtual bool GenerateFileHash() OVERRIDE; virtual void AddItemToPersistentStore(DownloadItem* item) OVERRIDE; virtual void UpdateItemInPersistentStore(DownloadItem* item) OVERRIDE; @@ -60,6 +83,11 @@ class ChromeDownloadManagerDelegate friend class base::RefCountedThreadSafe<ChromeDownloadManagerDelegate>; virtual ~ChromeDownloadManagerDelegate(); + // NotificationObserver implementation. + virtual void Observe(int type, + const NotificationSource& source, + const NotificationDetails& details) OVERRIDE; + // Callback function after url is checked with safebrowsing service. void CheckDownloadUrlDone(int32 download_id, bool is_dangerous_url); @@ -96,6 +124,12 @@ class ChromeDownloadManagerDelegate scoped_ptr<DownloadPrefs> download_prefs_; scoped_ptr<DownloadHistory> download_history_; + // Maps from pending extension installations to DownloadItem IDs. + typedef base::hash_map<CrxInstaller*, int> CrxInstallerMap; + CrxInstallerMap crx_installers_; + + NotificationRegistrar registrar_; + DISALLOW_COPY_AND_ASSIGN(ChromeDownloadManagerDelegate); }; diff --git a/chrome/browser/download/download_crx_util.cc b/chrome/browser/download/download_crx_util.cc index 9b5b45a..574c9ab9 100644 --- a/chrome/browser/download/download_crx_util.cc +++ b/chrome/browser/download/download_crx_util.cc @@ -50,7 +50,6 @@ scoped_refptr<CrxInstaller> OpenChromeExtension( Profile* profile, const DownloadItem& download_item) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(download_item.is_extension_install()); ExtensionService* service = profile->GetExtensionService(); CHECK(service); diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc index f709e74..3da198b 100644 --- a/chrome/browser/download/download_history.cc +++ b/chrome/browser/download/download_history.cc @@ -5,6 +5,7 @@ #include "chrome/browser/download/download_history.h" #include "base/logging.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/browser/history/history_marshaling.h" #include "chrome/browser/profiles/profile.h" #include "content/browser/download/download_item.h" @@ -73,7 +74,8 @@ void DownloadHistory::AddEntry( // you'd have to do enough downloading that your ISP would likely stab you in // the neck first. YMMV. HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS); - if (download_item->is_otr() || download_item->is_extension_install() || + if (download_item->is_otr() || + ChromeDownloadManagerDelegate::IsExtensionDownload(download_item) || download_item->is_temporary() || !hs) { callback->RunWithParams( history::DownloadCreateRequest::TupleType(download_item->id(), diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc index 7aeba8c..891a5ed 100644 --- a/chrome/browser/download/download_item_model.cc +++ b/chrome/browser/download/download_item_model.cc @@ -8,6 +8,7 @@ #include "base/i18n/rtl.h" #include "base/string16.h" #include "base/utf_string_conversions.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/common/time_format.h" #include "content/browser/download/download_item.h" #include "content/browser/download/save_package.h" @@ -56,7 +57,9 @@ string16 DownloadItemModel::GetStatusText() { string16 status_text; switch (download_->state()) { case DownloadItem::IN_PROGRESS: - if (download_->IsCrxInstallRuning()) { + if (ChromeDownloadManagerDelegate::IsExtensionDownload(download_) && + download_->all_data_saved() && + download_->state() == DownloadItem::IN_PROGRESS) { // The download is a CRX (app, extension, theme, ...) and it is // being unpacked and validated. status_text = l10n_util::GetStringUTF16( diff --git a/chrome/browser/download/download_shelf_context_menu.cc b/chrome/browser/download/download_shelf_context_menu.cc index bfdbcf2..de0a4f5 100644 --- a/chrome/browser/download/download_shelf_context_menu.cc +++ b/chrome/browser/download/download_shelf_context_menu.cc @@ -6,6 +6,7 @@ #include "chrome/browser/download/download_item_model.h" #include "chrome/browser/download/download_prefs.h" +#include "chrome/common/extensions/extension.h" #include "content/browser/download/download_item.h" #include "content/browser/download/download_manager.h" #include "grit/generated_resources.h" @@ -30,7 +31,8 @@ bool DownloadShelfContextMenu::IsCommandIdEnabled(int command_id) const { case OPEN_WHEN_COMPLETE: return download_item_->CanShowInFolder(); case ALWAYS_OPEN_TYPE: - return download_item_->CanOpenDownload(); + return download_item_->CanOpenDownload() && + !Extension::IsExtension(download_item_->state_info().target_name); case CANCEL: return download_item_->IsPartialDownload(); case TOGGLE_PAUSE: diff --git a/chrome/browser/download/mock_download_manager_delegate.cc b/chrome/browser/download/mock_download_manager_delegate.cc index dba9613b..9ffa9e1 100644 --- a/chrome/browser/download/mock_download_manager_delegate.cc +++ b/chrome/browser/download/mock_download_manager_delegate.cc @@ -30,6 +30,14 @@ bool MockDownloadManagerDelegate::ShouldOpenFileBasedOnExtension( return false; } +bool MockDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) { + return true; +} + +bool MockDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) { + return true; +} + bool MockDownloadManagerDelegate::GenerateFileHash() { return false; } diff --git a/chrome/browser/download/mock_download_manager_delegate.h b/chrome/browser/download/mock_download_manager_delegate.h index 68389d4..1ce087c 100644 --- a/chrome/browser/download/mock_download_manager_delegate.h +++ b/chrome/browser/download/mock_download_manager_delegate.h @@ -19,6 +19,8 @@ class MockDownloadManagerDelegate : public DownloadManagerDelegate { void* data) OVERRIDE; virtual TabContents* GetAlternativeTabContentsToNotifyForDownload() OVERRIDE; virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE; + virtual bool ShouldOpenDownload(DownloadItem* item) OVERRIDE; + virtual bool ShouldCompleteDownload(DownloadItem* item) OVERRIDE; virtual bool GenerateFileHash() OVERRIDE; virtual void AddItemToPersistentStore(DownloadItem* item) OVERRIDE; virtual void UpdateItemInPersistentStore(DownloadItem* item) OVERRIDE; diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 9071b40..72b6f33 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -33,6 +33,7 @@ #include "chrome/browser/character_encoding.h" #include "chrome/browser/debugger/devtools_toggle_action.h" #include "chrome/browser/debugger/devtools_window.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/browser/download/download_item_model.h" #include "chrome/browser/download/download_started_animation.h" #include "chrome/browser/extensions/crx_installer.h" @@ -3427,7 +3428,7 @@ void Browser::OnStartDownload(TabContents* source, DownloadItem* download) { #if defined(OS_CHROMEOS) // Don't show content browser for extension/theme downloads from gallery. ExtensionService* service = profile_->GetExtensionService(); - if (!download->is_extension_install() || + if (!ChromeDownloadManagerDelegate::IsExtensionDownload(download) || (service == NULL) || !service->IsDownloadFromGallery(download->GetURL(), download->referrer_url())) { @@ -3447,7 +3448,7 @@ void Browser::OnStartDownload(TabContents* source, DownloadItem* download) { // window is minimized, we're in a unit test, etc.). TabContents* shelf_tab = shelf->browser()->GetSelectedTabContents(); if ((download->total_bytes() > 0) && - (!download->is_extension_install() || + (!ChromeDownloadManagerDelegate::IsExtensionDownload(download) || ExtensionService::IsDownloadFromMiniGallery(download->GetURL())) && platform_util::IsVisible(shelf_tab->GetNativeView()) && ui::Animation::ShouldRenderRichAnimation()) { diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.mm b/chrome/browser/ui/cocoa/download/download_item_controller.mm index bbd17bc..eb854ba 100644 --- a/chrome/browser/ui/cocoa/download/download_item_controller.mm +++ b/chrome/browser/ui/cocoa/download/download_item_controller.mm @@ -10,6 +10,7 @@ #include "base/string_util.h" #include "base/sys_string_conversions.h" #include "base/utf_string_conversions.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/browser/download/download_item_model.h" #include "chrome/browser/download/download_shelf_context_menu.h" #include "chrome/browser/download/download_util.h" @@ -184,7 +185,8 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { DCHECK_EQ(downloadModel->download()->GetDangerType(), DownloadItem::DANGEROUS_FILE); alertIcon = rb.GetNativeImageNamed(IDR_WARNING); - if (downloadModel->download()->is_extension_install()) { + if (ChromeDownloadManagerDelegate::IsExtensionDownload( + downloadModel->download())) { dangerousWarning = l10n_util::GetNSStringWithFixup( IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); confirmButtonTitle = l10n_util::GetNSStringWithFixup( diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.cc b/chrome/browser/ui/gtk/download/download_item_gtk.cc index a90fc9e..5766738 100644 --- a/chrome/browser/ui/gtk/download/download_item_gtk.cc +++ b/chrome/browser/ui/gtk/download/download_item_gtk.cc @@ -11,6 +11,7 @@ #include "base/time.h" #include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/browser/download/download_item_model.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/ui/browser.h" @@ -222,9 +223,10 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf, // Create the ok button. GtkWidget* dangerous_accept = gtk_button_new_with_label( l10n_util::GetStringUTF8( - download_model->download()->is_extension_install() ? - IDS_CONTINUE_EXTENSION_DOWNLOAD : - IDS_CONFIRM_DOWNLOAD).c_str()); + ChromeDownloadManagerDelegate::IsExtensionDownload( + download_model->download()) ? + IDS_CONTINUE_EXTENSION_DOWNLOAD : + IDS_CONFIRM_DOWNLOAD).c_str()); g_signal_connect(dangerous_accept, "clicked", G_CALLBACK(OnDangerousAcceptThunk), this); gtk_util::CenterWidgetInHBox(dangerous_hbox_.get(), dangerous_accept, false, @@ -556,7 +558,7 @@ void DownloadItemGtk::UpdateDangerWarning() { // It's a dangerous file type (e.g.: an executable). DCHECK(get_download()->GetDangerType() == DownloadItem::DANGEROUS_FILE); - if (get_download()->is_extension_install()) { + if (ChromeDownloadManagerDelegate::IsExtensionDownload(get_download())) { dangerous_warning = l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); } else { diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc index 41ae73d..836504a 100644 --- a/chrome/browser/ui/views/download/download_item_view.cc +++ b/chrome/browser/ui/views/download/download_item_view.cc @@ -15,6 +15,7 @@ #include "base/sys_string_conversions.h" #include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/browser/download/download_item_model.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/themes/theme_service.h" @@ -215,7 +216,7 @@ DownloadItemView::DownloadItemView(DownloadItem* download, drop_down_state_ = DANGEROUS; save_button_ = new views::NativeTextButton(this, UTF16ToWide(l10n_util::GetStringUTF16( - download->is_extension_install() ? + ChromeDownloadManagerDelegate::IsExtensionDownload(download) ? IDS_CONTINUE_EXTENSION_DOWNLOAD : IDS_CONFIRM_DOWNLOAD))); save_button_->set_ignore_minimum_size(true); discard_button_ = new views::NativeTextButton( @@ -262,7 +263,7 @@ DownloadItemView::DownloadItemView(DownloadItem* download, // The download file has dangerous file type (e.g.: an executable). DCHECK(download->GetDangerType() == DownloadItem::DANGEROUS_FILE); warning_icon_ = rb.GetBitmapNamed(IDR_WARNING); - if (download->is_extension_install()) { + if (ChromeDownloadManagerDelegate::IsExtensionDownload(download)) { dangerous_label = l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); } else { diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc index 4d6670d..7519fb9 100644 --- a/chrome/browser/ui/webui/downloads_dom_handler.cc +++ b/chrome/browser/ui/webui/downloads_dom_handler.cc @@ -15,6 +15,7 @@ #include "base/utf_string_conversions.h" #include "base/values.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/download/chrome_download_manager_delegate.h" #include "chrome/browser/download/download_history.h" #include "chrome/browser/download/download_prefs.h" #include "chrome/browser/download/download_util.h" @@ -187,6 +188,14 @@ void DownloadsDOMHandler::ModelChanged() { sort(download_items_.begin(), download_items_.end(), DownloadItemSorter()); + // Remove any extension downloads. + for (size_t i = 0; i < download_items_.size();) { + if (ChromeDownloadManagerDelegate::IsExtensionDownload(download_items_[i])) + download_items_.erase(download_items_.begin() + i); + else + i++; + } + // Add ourself to all download items as an observer. for (OrderedDownloads::iterator it = download_items_.begin(); it != download_items_.end(); ++it) { diff --git a/chrome/test/pyautolib/pyauto.py b/chrome/test/pyautolib/pyauto.py index fe2143a..043e7bb 100644 --- a/chrome/test/pyautolib/pyauto.py +++ b/chrome/test/pyautolib/pyauto.py @@ -1112,7 +1112,6 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase): u'file_name': u'file.txt', u'full_path': u'/path/to/file.txt', u'id': 0, - u'is_extension_install': False, u'is_otr': False, u'is_paused': False, u'is_temporary': False, diff --git a/content/browser/download/download_create_info.cc b/content/browser/download/download_create_info.cc index 4b556ab..af87041 100644 --- a/content/browser/download/download_create_info.cc +++ b/content/browser/download/download_create_info.cc @@ -27,8 +27,7 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path, download_id(download_id), has_user_gesture(has_user_gesture), db_handle(0), - prompt_user_for_save_location(false), - is_extension_install(false) { + prompt_user_for_save_location(false) { } DownloadCreateInfo::DownloadCreateInfo() @@ -39,8 +38,7 @@ DownloadCreateInfo::DownloadCreateInfo() download_id(-1), has_user_gesture(false), db_handle(0), - prompt_user_for_save_location(false), - is_extension_install(false) { + prompt_user_for_save_location(false) { } DownloadCreateInfo::~DownloadCreateInfo() { diff --git a/content/browser/download/download_create_info.h b/content/browser/download/download_create_info.h index d2e5cd6..770ac264 100644 --- a/content/browser/download/download_create_info.h +++ b/content/browser/download/download_create_info.h @@ -95,9 +95,6 @@ struct DownloadCreateInfo { // The original name for a dangerous download. FilePath original_name; - // Whether this download is for extension install or not. - bool is_extension_install; - // The charset of the referring page where the download request comes from. // It's used to construct a suggested filename. std::string referrer_charset; diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc index 9126d7a..4374c04 100644 --- a/content/browser/download/download_item.cc +++ b/content/browser/download/download_item.cc @@ -14,13 +14,7 @@ #include "base/timer.h" #include "base/utf_string_conversions.h" #include "net/base/net_util.h" -#include "chrome/browser/download/download_crx_util.h" -#include "chrome/browser/download/download_extensions.h" #include "chrome/browser/download/download_util.h" -#include "chrome/browser/extensions/crx_installer.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/common/chrome_notification_types.h" -#include "chrome/common/extensions/extension.h" #include "content/browser/browser_thread.h" #include "content/browser/content_browser_client.h" #include "content/browser/download/download_create_info.h" @@ -30,7 +24,6 @@ #include "content/browser/download/download_persistent_store_info.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_stats.h" -#include "content/common/notification_source.h" // A DownloadItem normally goes through the following states: // * Created (when download starts) @@ -145,7 +138,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, is_temporary_(false), all_data_saved_(false), opened_(false), - open_enabled_(true) { + open_enabled_(true), + delegate_delayed_complete_(false) { if (IsInProgress()) state_ = CANCELLED; if (IsComplete()) @@ -159,8 +153,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, bool is_otr) : state_info_(info.original_name, info.save_info.file_path, info.has_user_gesture, info.prompt_user_for_save_location, - info.path_uniquifier, false, false, - info.is_extension_install), + info.path_uniquifier, false, false), request_handle_(info.request_handle), download_id_(info.download_id), full_path_(info.path), @@ -257,8 +250,7 @@ bool DownloadItem::CanShowInFolder() { } bool DownloadItem::CanOpenDownload() { - return !Extension::IsExtension(state_info_.target_name) && - !file_externally_removed_; + return !file_externally_removed_; } bool DownloadItem::ShouldOpenFileBasedOnExtension() { @@ -291,14 +283,8 @@ void DownloadItem::OpenDownload() { if (!open_enabled_) return; - if (is_extension_install()) { - download_crx_util::OpenChromeExtension( - Profile::FromBrowserContext(download_manager_->browser_context()), - *this); - return; - } - - content::GetContentClient()->browser()->OpenItem(full_path()); + if (download_manager_->delegate()->ShouldOpenDownload(this)) + content::GetContentClient()->browser()->OpenItem(full_path()); } void DownloadItem::ShowDownloadInShell() { @@ -392,6 +378,11 @@ void DownloadItem::MarkAsComplete() { TransitionTo(COMPLETE); } +void DownloadItem::CompleteDelayedDownload() { + auto_opened_ = true; + Completed(); +} + void DownloadItem::OnAllDataSaved(int64 size) { // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved. CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); @@ -418,9 +409,8 @@ void DownloadItem::Completed() { download_manager_->DownloadCompleted(id()); download_stats::RecordDownloadCompleted(start_tick_); - if (is_extension_install()) { - // Extensions should already have been unpacked and opened. - DCHECK(auto_opened_); + if (auto_opened_) { + // If it was already handled by the delegate, do nothing. } else if (open_when_complete() || ShouldOpenFileBasedOnExtension() || is_temporary()) { @@ -435,31 +425,6 @@ void DownloadItem::Completed() { } } -void DownloadItem::StartCrxInstall() { - // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved. - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - DCHECK(is_extension_install()); - DCHECK(all_data_saved_); - - scoped_refptr<CrxInstaller> crx_installer = - download_crx_util::OpenChromeExtension( - Profile::FromBrowserContext(download_manager_->browser_context()), - *this); - - // CRX_INSTALLER_DONE will fire when the install completes. Observe() - // will call Completed() on this item. If this DownloadItem is not - // around when CRX_INSTALLER_DONE fires, Complete() will not be called. - registrar_.Add(this, - chrome::NOTIFICATION_CRX_INSTALLER_DONE, - Source<CrxInstaller>(crx_installer.get())); - - // The status text and percent complete indicator will change now - // that we are installing a CRX. Update observers so that they pick - // up the change. - UpdateObservers(); -} - void DownloadItem::TransitionTo(DownloadState new_state) { if (state_ == new_state) return; @@ -486,26 +451,6 @@ void DownloadItem::UpdateTarget() { state_info_.target_name = full_path_.BaseName(); } -// NotificationObserver implementation. -void DownloadItem::Observe(int type, - const NotificationSource& source, - const NotificationDetails& details) { - // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved. - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - DCHECK(type == chrome::NOTIFICATION_CRX_INSTALLER_DONE); - - // No need to listen for CRX_INSTALLER_DONE anymore. - registrar_.Remove(this, - chrome::NOTIFICATION_CRX_INSTALLER_DONE, - source); - - auto_opened_ = true; - DCHECK(all_data_saved_); - - Completed(); -} - void DownloadItem::Interrupted(int64 size, int os_error) { // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved. CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); @@ -580,11 +525,9 @@ int64 DownloadItem::CurrentSpeed() const { } int DownloadItem::PercentComplete() const { - // We don't have an accurate way to estimate the time to unpack a CRX. - // The slowest part is re-encoding images, and time to do this depends on - // the contents of the image. If a CRX is being unpacked, indicate that - // we do not know how close to completion we are. - if (IsCrxInstallRuning() || total_bytes_ <= 0) + // If the delegate is delaying completion of the download, then we have no + // idea how long it will take. + if (delegate_delayed_complete_ || total_bytes_ <= 0) return -1; return static_cast<int>(received_bytes_ * 100.0 / total_bytes_); @@ -637,7 +580,6 @@ void DownloadItem::OnDownloadCompleting(DownloadFileManager* file_manager) { return; } - DCHECK(!is_extension_install()); Completed(); BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, @@ -657,13 +599,11 @@ void DownloadItem::OnDownloadRenamedToFinalName(const FilePath& full_path) { Rename(full_path); - if (is_extension_install()) { - StartCrxInstall(); - // Completed() will be called when the installer finishes. - return; + if (download_manager_->delegate()->ShouldCompleteDownload(this)) { + Completed(); + } else { + delegate_delayed_complete_ = true; } - - Completed(); } bool DownloadItem::MatchesQuery(const string16& query) const { @@ -830,7 +770,6 @@ std::string DownloadItem::DebugString(bool verbose) const { " total_bytes = %" PRId64 " received_bytes = %" PRId64 " is_paused = %c" - " is_extension_install = %c" " is_otr = %c" " safety_state = %s" " url_chain = \n\t\"%s\"\n\t" @@ -840,7 +779,6 @@ std::string DownloadItem::DebugString(bool verbose) const { total_bytes(), received_bytes(), is_paused() ? 'T' : 'F', - is_extension_install() ? 'T' : 'F', is_otr() ? 'T' : 'F', DebugSafetyStateString(safety_state()), url_list.c_str(), diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h index 5b2ae0a..40fdfb2 100644 --- a/content/browser/download/download_item.h +++ b/content/browser/download/download_item.h @@ -27,11 +27,8 @@ #include "base/timer.h" #include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_state_info.h" -#include "content/common/notification_observer.h" -#include "content/common/notification_registrar.h" #include "googleurl/src/gurl.h" -class CrxInstaller; class DownloadFileManager; class DownloadManager; struct DownloadCreateInfo; @@ -42,7 +39,7 @@ struct DownloadPersistentStoreInfo; // Destination tab's download view, may refer to a given DownloadItem. // // This is intended to be used only on the UI thread. -class DownloadItem : public NotificationObserver { +class DownloadItem { public: enum DownloadState { // Download is actively progressing. @@ -135,11 +132,6 @@ class DownloadItem : public NotificationObserver { // Notifies our observers periodically. void UpdateObservers(); - // NotificationObserver implementation. - virtual void Observe(int type, - const NotificationSource& source, - const NotificationDetails& details); - // Returns true if it is OK to open a folder which this file is inside. bool CanShowInFolder(); @@ -178,6 +170,10 @@ class DownloadItem : public NotificationObserver { // to display progress when the DownloadItem should be considered complete. void MarkAsComplete(); + // Called by the delegate after it delayed completing the download in + // DownloadManagerDelegate::ShouldCompleteDownload. + void CompleteDelayedDownload(); + // Called when all data has been saved. Only has display effects. void OnAllDataSaved(int64 size); @@ -298,9 +294,6 @@ class DownloadItem : public NotificationObserver { return state_info_.prompt_user_for_save_location; } bool is_otr() const { return is_otr_; } - bool is_extension_install() const { - return state_info_.is_extension_install; - } const FilePath& suggested_path() const { return state_info_.suggested_path; } bool is_temporary() const { return is_temporary_; } void set_opened(bool opened) { opened_ = opened; } @@ -329,13 +322,6 @@ class DownloadItem : public NotificationObserver { return state_info_.target_name != full_path_.BaseName(); } - // Is a CRX installer running on this download? - bool IsCrxInstallRuning() const { - return (is_extension_install() && - all_data_saved() && - state_ == IN_PROGRESS); - } - std::string DebugString(bool verbose) const; #ifdef UNIT_TEST @@ -359,10 +345,6 @@ class DownloadItem : public NotificationObserver { void StartProgressTimer(); void StopProgressTimer(); - // Call to install this item as a CRX. Should only be called on - // items which are CRXes. Use is_extension_install() to check. - void StartCrxInstall(); - // Call to transition state; all state transitions should go through this. void TransitionTo(DownloadState new_state); @@ -485,8 +467,8 @@ class DownloadItem : public NotificationObserver { // only. bool open_enabled_; - // DownloadItem observes CRX installs it initiates. - NotificationRegistrar registrar_; + // Did the delegate delay calling Complete on this download? + bool delegate_delayed_complete_; DISALLOW_COPY_AND_ASSIGN(DownloadItem); }; diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc index e211c96..5367038 100644 --- a/content/browser/download/download_manager.cc +++ b/content/browser/download/download_manager.cc @@ -163,7 +163,7 @@ void DownloadManager::SearchDownloads(const string16& query, it != history_downloads_.end(); ++it) { DownloadItem* download_item = it->second; - if (download_item->is_temporary() || download_item->is_extension_install()) + if (download_item->is_temporary()) continue; // Display Incognito downloads only in Incognito window, and vice versa. diff --git a/content/browser/download/download_manager_delegate.h b/content/browser/download/download_manager_delegate.h index fd2e2d6..13de586 100644 --- a/content/browser/download/download_manager_delegate.h +++ b/content/browser/download/download_manager_delegate.h @@ -35,36 +35,45 @@ class DownloadManagerDelegate { // Called when the download system wants to alert a TabContents that a // download has started, but the TabConetnts has gone away. This lets an - // embedder return an alternative TabContents. The embedder can return NULL. + // delegate return an alternative TabContents. The delegate can return NULL. virtual TabContents* GetAlternativeTabContentsToNotifyForDownload() = 0; // Tests if a file type should be opened automatically. virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) = 0; + // Allows the delegate to override the opening of a download. If it returns + // true then it's reponsible for opening the item. + virtual bool ShouldOpenDownload(DownloadItem* item) = 0; + + // Allows the delegate to override completing the download. The delegate needs + // to call DownloadItem::CompleteDelayedDownload when it's done with the item, + // and is responsible for opening it. + virtual bool ShouldCompleteDownload(DownloadItem* item) = 0; + // Returns true if we need to generate a binary hash for downloads. virtual bool GenerateFileHash() = 0; - // Notifies the embedder that a new download item is created. The - // DownloadManager waits for the embedder to add information about this - // download to its persistent store. When the embedder is done, it calls + // Notifies the delegate that a new download item is created. The + // DownloadManager waits for the delegate to add information about this + // download to its persistent store. When the delegate is done, it calls // DownloadManager::OnDownloadItemAddedToPersistentStore. virtual void AddItemToPersistentStore(DownloadItem* item) = 0; - // Notifies the embedder that information about the given download has change, + // Notifies the delegate that information about the given download has change, // so that it can update its persistent store. virtual void UpdateItemInPersistentStore(DownloadItem* item) = 0; - // Notifies the embedder that path for the download item has changed, so that + // Notifies the delegate that path for the download item has changed, so that // it can update its persistent store. virtual void UpdatePathForItemInPersistentStore( DownloadItem* item, const FilePath& new_path) = 0; - // Notifies the embedder that it should remove the download item from its + // Notifies the delegate that it should remove the download item from its // persistent store. virtual void RemoveItemFromPersistentStore(DownloadItem* item) = 0; - // Notifies the embedder to remove downloads from the given time range. + // Notifies the delegate to remove downloads from the given time range. virtual void RemoveItemsFromPersistentStoreBetween( const base::Time remove_begin, const base::Time remove_end) = 0; diff --git a/content/browser/download/download_request_handle.cc b/content/browser/download/download_request_handle.cc index 83b0df60..e5d9b60 100644 --- a/content/browser/download/download_request_handle.cc +++ b/content/browser/download/download_request_handle.cc @@ -5,9 +5,9 @@ #include "content/browser/download/download_request_handle.h" #include "base/stringprintf.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/tab_contents/tab_util.h" +#include "content/browser/browser_context.h" #include "content/browser/browser_thread.h" +#include "content/browser/renderer_host/render_view_host.h" #include "content/browser/renderer_host/resource_dispatcher_host.h" #include "content/browser/tab_contents/tab_contents.h" @@ -52,8 +52,12 @@ DownloadRequestHandle::DownloadRequestHandle(ResourceDispatcherHost* rdh, } TabContents* DownloadRequestHandle::GetTabContents() const { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - return tab_util::GetTabContentsByID(child_id_, render_view_id_); + RenderViewHost* render_view_host = + RenderViewHost::FromID(child_id_, render_view_id_); + if (!render_view_host) + return NULL; + + return render_view_host->delegate()->GetAsTabContents(); } DownloadManager* DownloadRequestHandle::GetDownloadManager() const { diff --git a/content/browser/download/download_state_info.cc b/content/browser/download/download_state_info.cc index 0631129..9e8c3a2 100644 --- a/content/browser/download/download_state_info.cc +++ b/content/browser/download/download_state_info.cc @@ -11,8 +11,7 @@ DownloadStateInfo::DownloadStateInfo() has_user_gesture(false), prompt_user_for_save_location(false), is_dangerous_file(false), - is_dangerous_url(false), - is_extension_install(false) { + is_dangerous_url(false) { } DownloadStateInfo::DownloadStateInfo( @@ -22,8 +21,7 @@ DownloadStateInfo::DownloadStateInfo( has_user_gesture(has_user_gesture), prompt_user_for_save_location(prompt_user_for_save_location), is_dangerous_file(false), - is_dangerous_url(false), - is_extension_install(false) { + is_dangerous_url(false) { } DownloadStateInfo::DownloadStateInfo( @@ -33,15 +31,13 @@ DownloadStateInfo::DownloadStateInfo( bool prompt_user_for_save_location, int uniquifier, bool dangerous_file, - bool dangerous_url, - bool extension_install) + bool dangerous_url) : target_name(target), path_uniquifier(uniquifier), has_user_gesture(has_user_gesture), prompt_user_for_save_location(prompt_user_for_save_location), is_dangerous_file(dangerous_file), is_dangerous_url(dangerous_url), - is_extension_install(extension_install), force_file_name(forced_name) { } diff --git a/content/browser/download/download_state_info.h b/content/browser/download/download_state_info.h index 5ca4c40..dca01c3 100644 --- a/content/browser/download/download_state_info.h +++ b/content/browser/download/download_state_info.h @@ -20,8 +20,7 @@ struct DownloadStateInfo { bool prompt_user_for_save_location, int uniquifier, bool dangerous_file, - bool dangerous_url, - bool extension_install); + bool dangerous_url); // Indicates if the download is dangerous. bool IsDangerous() const; @@ -52,9 +51,6 @@ struct DownloadStateInfo { // If safebrowsing believes this URL leads to malware. bool is_dangerous_url; - // True if this download is for extension install. - bool is_extension_install; - // True if this download's file name was specified initially. FilePath force_file_name; }; diff --git a/content/browser/zygote_host_linux.cc b/content/browser/zygote_host_linux.cc index dc2fc4e..9d1d02d 100644 --- a/content/browser/zygote_host_linux.cc +++ b/content/browser/zygote_host_linux.cc @@ -172,7 +172,7 @@ void ZygoteHost::Init(const std::string& sandbox_cmd) { // In the SUID sandbox, the real zygote is forked from the sandbox. // We need to look for it. // But first, wait for the zygote to tell us it's running. - // The sending code is in chrome/browser/zygote_main_linux.cc. + // The sending code is in content/browser/zygote_main_linux.cc. std::vector<int> fds_vec; const int kExpectedLength = sizeof(kZygoteMagic); char buf[kExpectedLength]; diff --git a/content/browser/zygote_main_linux.cc b/content/browser/zygote_main_linux.cc index aa2d9f9..aa363b6 100644 --- a/content/browser/zygote_main_linux.cc +++ b/content/browser/zygote_main_linux.cc @@ -111,7 +111,7 @@ class Zygote { if (g_suid_sandbox_active) { // Let the ZygoteHost know we are ready to go. - // The receiving code is in chrome/browser/zygote_host_linux.cc. + // The receiving code is in content/browser/zygote_host_linux.cc. std::vector<int> empty; bool r = UnixDomainSocket::SendMsg(kBrowserDescriptor, kZygoteMagic, sizeof(kZygoteMagic), empty); |