diff options
25 files changed, 1378 insertions, 204 deletions
diff --git a/chrome/SConscript.unit_tests b/chrome/SConscript.unit_tests index 053787e..03a369d 100644 --- a/chrome/SConscript.unit_tests +++ b/chrome/SConscript.unit_tests @@ -195,6 +195,7 @@ if env_test['PLATFORM'] == 'win32': 'browser/profile_manager_unittest.cc', 'browser/renderer_security_policy_unittest.cc', 'browser/resource_dispatcher_host_unittest.cc', + 'browser/download_request_manager_unittest.cc', 'browser/rlz/rlz_unittest.cc', 'browser/safe_browsing/protocol_manager_unittest.cc', 'browser/safe_browsing/safe_browsing_database_unittest.cc', @@ -235,6 +236,7 @@ if env_test['PLATFORM'] == 'win32': 'renderer/net/render_dns_queue_unittest.cc', 'renderer/spellcheck_unittest.cc', 'test/test_notification_tracker.cc', + 'test/test_tab_contents.cc', 'test/testing_profile.cc', 'views/focus_manager_unittest.cc', 'views/grid_layout_unittest.cc', diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index c0c1b9c..1b5c699 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -3134,6 +3134,16 @@ each locale. --> <message name="IDS_BLOCKED_POPUP" desc="Text on a blocked popup's window titlebar."> Blocked Pop-up </message> + <!-- Multiple download warning--> + <message name="IDS_MULTI_DOWNLOAD_WARNING" desc="Warning invoked if multiple downloads are attempted without user interaction."> + This site is attempting to download multiple files. Do you want to allow this? + </message> + <message name="IDS_MULTI_DOWNLOAD_WARNING_ALLOW" desc="Text on the allow button in the IDS_MULTI_DOWNLOAD_WARNING dialog"> + Allow + </message> + <message name="IDS_MULTI_DOWNLOAD_WARNING_DENY" desc="Text on the deny button in the IDS_MULTI_DOWNLOAD_WARNING dialog"> + Deny + </message> </messages> </release> </grit> diff --git a/chrome/browser/SConscript b/chrome/browser/SConscript index 780a8a1..a6bccad 100644 --- a/chrome/browser/SConscript +++ b/chrome/browser/SConscript @@ -158,6 +158,7 @@ if env['PLATFORM'] == 'win32': 'download/download_item_model.cc', 'download/download_manager.cc', 'download/download_util.cc', + 'download/download_request_manager.cc', 'download/save_file.cc', 'download/save_file_manager.cc', 'download/save_item.cc', diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj index 2d3f6e8..e2a5f7e 100644 --- a/chrome/browser/browser.vcproj +++ b/chrome/browser/browser.vcproj @@ -2038,6 +2038,14 @@ > </File> <File + RelativePath=".\download\download_request_manager.cc" + > + </File> + <File + RelativePath=".\download\download_request_manager.h" + > + </File> + <File RelativePath=".\download\download_util.cc" > </File> diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h index cad8e89..eb67f8b 100644 --- a/chrome/browser/browser_process.h +++ b/chrome/browser/browser_process.h @@ -14,9 +14,13 @@ #include "base/basictypes.h" #include "base/message_loop.h" +#if defined(OS_WIN) +#include "chrome/browser/resource_dispatcher_host.h" +#endif // defined(OS_WIN) class AutomationProviderList; class ClipboardService; +class DownloadRequestManager; class GoogleURLTracker; class IconManager; class MetricsService; @@ -24,8 +28,8 @@ class NotificationService; class PrefService; class ProfileManager; class RenderProcessHost; -class ResourceDispatcherHost; class DebuggerWrapper; +class ResourceDispatcherHost; class WebAppInstallerService; class SuspendController; @@ -124,6 +128,11 @@ class BrowserProcess { virtual bool IsUsingNewFrames() = 0; #if defined(OS_WIN) + DownloadRequestManager* download_request_manager() { + ResourceDispatcherHost* rdh = resource_dispatcher_host(); + return rdh ? rdh->download_request_manager() : NULL; + } + // Returns an event that is signaled when the browser shutdown. virtual HANDLE shutdown_event() = 0; #endif diff --git a/chrome/browser/download/download_request_manager.cc b/chrome/browser/download/download_request_manager.cc new file mode 100644 index 0000000..57d3b43 --- /dev/null +++ b/chrome/browser/download/download_request_manager.cc @@ -0,0 +1,479 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/download/download_request_manager.h" + +#include "base/message_loop.h" +#include "base/thread.h" +#include "chrome/browser/navigation_controller.h" +#include "chrome/browser/navigation_entry.h" +#include "chrome/browser/constrained_window.h" +#include "chrome/browser/tab_contents.h" +#include "chrome/browser/tab_contents_delegate.h" +#include "chrome/browser/tab_util.h" +#include "chrome/common/l10n_util.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_service.h" +#include "chrome/views/dialog_delegate.h" +#include "chrome/views/message_box_view.h" + +#include "generated_resources.h" + +namespace { + +// DialogDelegateImpl ---------------------------------------------------------- + +// DialogDelegateImpl is the DialogDelegate implementation used to prompt the +// the user as to whether they want to allow multiple downloads. +// DialogDelegateImpl delegates the allow/cancel methods to the +// TabDownloadState. +// +// TabDownloadState does not directly implement DialogDelegate, rather it is +// split into DialogDelegateImpl as TabDownloadState may be deleted before +// the dialog. + +class DialogDelegateImpl : public views::DialogDelegate { + public: + DialogDelegateImpl(TabContents* tab, + DownloadRequestManager::TabDownloadState* host); + + void set_host(DownloadRequestManager::TabDownloadState* host) { + host_ = host; + } + + // Closes the prompt. + void CloseWindow(); + + private: + // DialogDelegate methods; + virtual bool Cancel(); + virtual bool Accept(); + virtual views::View* GetContentsView() { return message_view_; } + virtual std::wstring GetDialogButtonLabel(DialogButton button) const; + virtual int GetDefaultDialogButton() const { + return DIALOGBUTTON_CANCEL; + } + virtual void WindowClosing(); + + // The TabDownloadState we're displaying the dialog for. May be null. + DownloadRequestManager::TabDownloadState* host_; + + MessageBoxView* message_view_; + + ConstrainedWindow* window_; + + DISALLOW_COPY_AND_ASSIGN(DialogDelegateImpl); +}; + +} // namespace + +// TabDownloadState ------------------------------------------------------------ + +// TabDownloadState maintains the download state for a particular tab. +// TabDownloadState installs observers to update the download status +// appropriately. Additionally TabDownloadState prompts the user as necessary. +// TabDownloadState deletes itself (by invoking DownloadRequestManager::Remove) +// as necessary. + +class DownloadRequestManager::TabDownloadState : public NotificationObserver { + public: + // Creates a new TabDownloadState. |controller| is the controller the + // TabDownloadState tracks the state of and is the host for any dialogs that + // are displayed. |originating_controller| is used to determine the host of + // the initial download. If |originating_controller| is null, |controller| is + // used. |originating_controller| is typically null, but differs from + // |controller| in the case of a constrained popup requesting the download. + TabDownloadState(DownloadRequestManager* host, + NavigationController* controller, + NavigationController* originating_controller); + ~TabDownloadState(); + + // Status of the download. + void set_download_status(DownloadRequestManager::DownloadStatus status) { + status_ = status; + } + DownloadRequestManager::DownloadStatus download_status() const { + return status_; + } + + // Invoked when a user gesture occurs (mouse click, enter or space). This + // may result in invoking Remove on DownloadRequestManager. + void OnUserGesture(); + + // Asks the user if they really want to allow the download. + // See description above CanDownloadOnIOThread for details on lifetime of + // callback. + void PromptUserForDownload(TabContents* tab, + DownloadRequestManager::Callback* callback); + + // Are we showing a prompt to the user? + bool is_showing_prompt() const { return (dialog_delegate_ != NULL); } + + // NavigationController we're tracking. + NavigationController* controller() const { return controller_; } + + // Invoked from DialogDelegateImpl. Notifies the delegates and changes the + // status appropriately. + void Cancel(); + void Accept(); + + private: + // NotificationObserver method. + void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + + // Notifies the callbacks as to whether the download is allowed or not. + // Updates status_ appropriately. + void NotifyCallbacks(bool allow); + + DownloadRequestManager* host_; + + NavigationController* controller_; + + // Host of the first page the download started on. This may be empty. + std::string initial_page_host_; + + DownloadRequestManager::DownloadStatus status_; + + // Callbacks we need to notify. This is only non-empty if we're showing a + // dialog. + // See description above CanDownloadOnIOThread for details on lifetime of + // callbacks. + std::vector<DownloadRequestManager::Callback*> callbacks_; + + // Used to remove observers installed on NavigationController. + NotificationRegistrar registrar_; + + // Handles showing the dialog to the user, may be null. + DialogDelegateImpl* dialog_delegate_; + + DISALLOW_COPY_AND_ASSIGN(TabDownloadState); +}; + +DownloadRequestManager::TabDownloadState::TabDownloadState( + DownloadRequestManager* host, + NavigationController* controller, + NavigationController* originating_controller) + : host_(host), + controller_(controller), + status_(DownloadRequestManager::ALLOW_ONE_DOWNLOAD), + dialog_delegate_(NULL) { + Source<NavigationController> notification_source(controller); + registrar_.Add(this, NOTIFY_NAV_ENTRY_COMMITTED, notification_source); + registrar_.Add(this, NOTIFY_TAB_CLOSED, notification_source); + + NavigationEntry* active_entry = originating_controller ? + originating_controller->GetActiveEntry() : controller->GetActiveEntry(); + if (active_entry) + initial_page_host_ = active_entry->url().host(); +} + +DownloadRequestManager::TabDownloadState::~TabDownloadState() { + // We should only be destroyed after the callbacks have been notified. + DCHECK(callbacks_.empty()); + + // And we should have closed the message box. + DCHECK(!dialog_delegate_); +} + +void DownloadRequestManager::TabDownloadState::OnUserGesture() { + if (is_showing_prompt()) { + // Don't change the state if the user clicks on the page some where. + return; + } + + if (status_ != DownloadRequestManager::ALLOW_ALL_DOWNLOADS && + status_ != DownloadRequestManager::DOWNLOADS_NOT_ALLOWED) { + // Revert to default status. + host_->Remove(this); + // WARNING: We've been deleted. + return; + } +} + +void DownloadRequestManager::TabDownloadState::PromptUserForDownload( + TabContents* tab, + DownloadRequestManager::Callback* callback) { + callbacks_.push_back(callback); + + if (is_showing_prompt()) + return; // Already showing prompt. + + if (DownloadRequestManager::delegate_) + NotifyCallbacks(DownloadRequestManager::delegate_->ShouldAllowDownload()); + else + dialog_delegate_ = new DialogDelegateImpl(tab, this); +} + +void DownloadRequestManager::TabDownloadState::Cancel() { + NotifyCallbacks(false); +} + +void DownloadRequestManager::TabDownloadState::Accept() { + NotifyCallbacks(true); +} + +void DownloadRequestManager::TabDownloadState::Observe( + NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + if ((type != NOTIFY_NAV_ENTRY_COMMITTED && type != NOTIFY_TAB_CLOSED) || + Source<NavigationController>(source).ptr() != controller_) { + NOTREACHED(); + return; + } + + switch(type) { + case NOTIFY_NAV_ENTRY_COMMITTED: { + if (is_showing_prompt()) { + // We're prompting the user and they navigated away. Close the popup and + // cancel the downloads. + dialog_delegate_->CloseWindow(); + // After switch we'll notify callbacks and get deleted. + } else if (status_ == DownloadRequestManager::ALLOW_ALL_DOWNLOADS || + status_ == DownloadRequestManager::DOWNLOADS_NOT_ALLOWED) { + // User has either allowed all downloads or canceled all downloads. Only + // reset the download state if the user is navigating to a different + // host (or host is empty). + NavigationController::LoadCommittedDetails* load_details = + Details<NavigationController::LoadCommittedDetails>(details).ptr(); + NavigationEntry* entry = load_details->entry; + if (load_details->is_auto || !entry || + (!initial_page_host_.empty() && + !entry->url().host().empty() && + entry->url().host() == initial_page_host_)) { + return; + } + } // else case: we're not prompting user and user hasn't allowed or + // disallowed downloads, break so that we get deleted after switch. + break; + } + + case NOTIFY_TAB_CLOSED: + // Tab closed, no need to handle closing the dialog as it's owned by the + // TabContents, break so that we get deleted after switch. + break; + + default: + NOTREACHED(); + } + + NotifyCallbacks(false); + host_->Remove(this); +} + +void DownloadRequestManager::TabDownloadState::NotifyCallbacks(bool allow) { + if (dialog_delegate_) { + // Reset the delegate so we don't get notified again. + dialog_delegate_->set_host(NULL); + dialog_delegate_ = NULL; + } + status_ = allow ? + DownloadRequestManager::ALLOW_ALL_DOWNLOADS : + DownloadRequestManager::DOWNLOADS_NOT_ALLOWED; + std::vector<DownloadRequestManager::Callback*> callbacks; + callbacks.swap(callbacks_); + for (size_t i = 0; i < callbacks.size(); ++i) + host_->ScheduleNotification(callbacks[i], allow); +} + +namespace { + +// DialogDelegateImpl ---------------------------------------------------------- + +DialogDelegateImpl::DialogDelegateImpl( + TabContents* tab, + DownloadRequestManager::TabDownloadState* host) + : host_(host) { + message_view_ = new MessageBoxView( + MessageBoxView::kIsConfirmMessageBox, + l10n_util::GetString(IDS_MULTI_DOWNLOAD_WARNING), + std::wstring()); + window_ = tab->CreateConstrainedDialog(this, message_view_); +} + +void DialogDelegateImpl::CloseWindow() { + window_->CloseConstrainedWindow(); +} + +bool DialogDelegateImpl::Cancel() { + if (host_) + host_->Cancel(); + return true; +} + +bool DialogDelegateImpl::Accept() { + if (host_) + host_->Accept(); + return true; +} + +std::wstring DialogDelegateImpl::GetDialogButtonLabel( + DialogButton button) const { + if (button == DIALOGBUTTON_OK) + return l10n_util::GetString(IDS_MULTI_DOWNLOAD_WARNING_ALLOW); + if (button == DIALOGBUTTON_CANCEL) + return l10n_util::GetString(IDS_MULTI_DOWNLOAD_WARNING_DENY); + return std::wstring(); +} + +void DialogDelegateImpl::WindowClosing() { + DCHECK(!host_); + delete this; +} + +} // namespace + +// DownloadRequestManager ------------------------------------------------------ + +DownloadRequestManager::DownloadRequestManager(MessageLoop* io_loop, + MessageLoop* ui_loop) + : io_loop_(io_loop), + ui_loop_(ui_loop) { +} + +DownloadRequestManager::~DownloadRequestManager() { + // All the tabs should have closed before us, which sends notification and + // removes from state_map_. As such, there should be no pending callbacks. + DCHECK(state_map_.empty()); +} + +DownloadRequestManager::DownloadStatus + DownloadRequestManager::GetDownloadStatus(TabContents* tab) { + TabDownloadState* state = GetDownloadState(tab->controller(), NULL, false); + return state ? state->download_status() : ALLOW_ONE_DOWNLOAD; +} + +void DownloadRequestManager::CanDownloadOnIOThread(int render_process_host_id, + int render_view_id, + Callback* callback) { + // This is invoked on the IO thread. Schedule the task to run on the UI + // thread so that we can query UI state. + DCHECK(!io_loop_ || io_loop_ == MessageLoop::current()); + ui_loop_->PostTask(FROM_HERE, + NewRunnableMethod(this, &DownloadRequestManager::CanDownload, + render_process_host_id, render_view_id, callback)); +} + +void DownloadRequestManager::OnUserGesture(TabContents* tab) { + NavigationController* controller = tab->controller(); + if (!controller) { + NOTREACHED(); + return; + } + + TabDownloadState* state = GetDownloadState(controller, NULL, false); + if (!state) + return; + + state->OnUserGesture(); +} + +// static +void DownloadRequestManager::SetTestingDelegate(TestingDelegate* delegate) { + delegate_ = delegate; +} + +DownloadRequestManager::TabDownloadState* DownloadRequestManager:: + GetDownloadState(NavigationController* controller, + NavigationController* originating_controller, + bool create) { + DCHECK(controller); + StateMap::iterator i = state_map_.find(controller); + if (i != state_map_.end()) + return i->second; + + if (!create) + return NULL; + + TabDownloadState* state = + new TabDownloadState(this, controller, originating_controller); + state_map_[controller] = state; + return state; +} + +void DownloadRequestManager::CanDownload(int render_process_host_id, + int render_view_id, + Callback* callback) { + DCHECK(!ui_loop_ || MessageLoop::current() == ui_loop_); + + TabContents* originating_tab = + tab_util::GetTabContentsByID(render_process_host_id, render_view_id); + if (!originating_tab) { + // The tab was closed, don't allow the download. + ScheduleNotification(callback, false); + return; + } + CanDownloadImpl(originating_tab, callback); +} + +void DownloadRequestManager::CanDownloadImpl( + TabContents* originating_tab, + Callback* callback) { + TabContents* effective_tab = originating_tab; + if (effective_tab->delegate() && + effective_tab->delegate()->GetConstrainingContents(effective_tab)) { + // The tab requesting the download is a constrained popup that is not + // shown, treat the request as if it came from the parent. + effective_tab = + effective_tab->delegate()->GetConstrainingContents(effective_tab); + } + + NavigationController* controller = effective_tab->controller(); + DCHECK(controller); + TabDownloadState* state = GetDownloadState( + controller, originating_tab->controller(), true); + switch (state->download_status()) { + case ALLOW_ALL_DOWNLOADS: + ScheduleNotification(callback, true); + break; + + case ALLOW_ONE_DOWNLOAD: + state->set_download_status(PROMPT_BEFORE_DOWNLOAD); + ScheduleNotification(callback, true); + break; + + case DOWNLOADS_NOT_ALLOWED: + ScheduleNotification(callback, false); + break; + + case PROMPT_BEFORE_DOWNLOAD: + state->PromptUserForDownload(effective_tab, callback); + break; + + default: + NOTREACHED(); + } +} + +void DownloadRequestManager::ScheduleNotification(Callback* callback, + bool allow) { + if (io_loop_) { + io_loop_->PostTask(FROM_HERE, + NewRunnableMethod(this, &DownloadRequestManager::NotifyCallback, + callback, allow)); + } else { + NotifyCallback(callback, allow); + } +} + +void DownloadRequestManager::NotifyCallback(Callback* callback, bool allow) { + // We better be on the IO thread now. + DCHECK(!io_loop_ || MessageLoop::current() == io_loop_); + if (allow) + callback->ContinueDownload(); + else + callback->CancelDownload(); +} + +void DownloadRequestManager::Remove(TabDownloadState* state) { + DCHECK(state_map_.find(state->controller()) != state_map_.end()); + state_map_.erase(state->controller()); + delete state; +} + +// static +DownloadRequestManager::TestingDelegate* DownloadRequestManager::delegate_ = + NULL; diff --git a/chrome/browser/download/download_request_manager.h b/chrome/browser/download/download_request_manager.h new file mode 100644 index 0000000..90180f8 --- /dev/null +++ b/chrome/browser/download/download_request_manager.h @@ -0,0 +1,144 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_MANAGER_H_ +#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_MANAGER_H_ + +#include <map> +#include <vector> + +#include "base/ref_counted.h" + +class MessageLoop; +class NavigationController; +class TabContents; + +// DownloadRequestManager is responsible for determining whether a download +// should be allowed or not. It is designed to keep pages from downloading +// multiple files without user interaction. DownloadRequestManager is invoked +// from ResourceDispatcherHost any time a download begins +// (CanDownloadOnIOThread). The request is processed on the UI thread, and the +// request is notified (back on the IO thread) as to whether the download should +// be allowed or denied. +// +// Invoking CanDownloadOnIOThread notifies the callback and may update the +// download status. The following details the various states: +// . Each NavigationController initially starts out allowing a download +// (ALLOW_ONE_DOWNLOAD). +// . The first time CanDownloadOnIOThread is invoked the download is allowed and +// the state changes to PROMPT_BEFORE_DOWNLOAD. +// . If the state is PROMPT_BEFORE_DOWNLOAD and the user clicks the mouse, +// presses enter, the space bar or navigates to another page the state is +// reset to ALLOW_ONE_DOWNLOAD. +// . If a download is attempted and the state is PROMPT_BEFORE_DOWNLOAD the user +// is prompted as to whether the download is allowed or disallowed. The users +// choice stays until the user navigates to a different host. For example, if +// the user allowed the download, multiple downloads are allowed without any +// user intervention until the user navigates to a different host. + +class DownloadRequestManager : + public base::RefCountedThreadSafe<DownloadRequestManager> { + public: + class TabDownloadState; + + // Download status for a particular page. See class description for details. + enum DownloadStatus { + ALLOW_ONE_DOWNLOAD, + PROMPT_BEFORE_DOWNLOAD, + ALLOW_ALL_DOWNLOADS, + DOWNLOADS_NOT_ALLOWED + }; + + DownloadRequestManager(MessageLoop* io_loop, MessageLoop* ui_loop); + ~DownloadRequestManager(); + + // The callback from CanDownloadOnIOThread. This is invoked on the io thread. + class Callback { + public: + virtual void ContinueDownload() = 0; + virtual void CancelDownload() = 0; + }; + + // Returns the download status for a page. This does not change the state in + // anyway. + DownloadStatus GetDownloadStatus(TabContents* tab); + + // Updates the state of the page as necessary and notifies the callback. + // WARNING: both this call and the callback are invoked on the io thread. + // + // DownloadRequestManager does not retain/release the Callback. It is up to + // the caller to ensure the callback is valid until the request is complete. + void CanDownloadOnIOThread(int render_process_host_id, + int render_view_id, + Callback* callback); + + // Invoked when the user presses the mouse, enter key or space bar. This may + // change the download status for the page. See the class description for + // details. + void OnUserGesture(TabContents* tab); + + private: + friend class DownloadRequestManagerTest; + friend class TabDownloadState; + + // For unit tests. If non-null this is used instead of creating a dialog. + class TestingDelegate { + public: + virtual bool ShouldAllowDownload() = 0; + }; + static void SetTestingDelegate(TestingDelegate* delegate); + + // Gets the download state for the specified controller. If the + // TabDownloadState does not exist and |create| is true, one is created. + // See TabDownloadState's constructor description for details on the two + // controllers. + // + // The returned TabDownloadState is owned by the DownloadRequestManager and + // deleted when no longer needed (the Remove method is invoked). + TabDownloadState* GetDownloadState( + NavigationController* controller, + NavigationController* originating_controller, + bool create); + + // CanDownloadOnIOThread invokes this on the UI thread. This determines the + // tab and invokes CanDownloadImpl. + void CanDownload(int render_process_host_id, + int render_view_id, + Callback* callback); + + // Does the work of updating the download status on the UI thread and + // potentially prompting the user. + void CanDownloadImpl(TabContents* originating_tab, + Callback* callback); + + // Invoked on the UI thread. Schedules a call to NotifyCallback on the io + // thread. + void ScheduleNotification(Callback* callback, bool allow); + + // Notifies the callback. This *must* be invoked on the IO thread. + void NotifyCallback(Callback* callback, bool allow); + + // Removes the specified TabDownloadState from the internal map and deletes + // it. This has the effect of resetting the status for the tab to + // ALLOW_ONE_DOWNLOAD. + void Remove(TabDownloadState* state); + + // Two threads we use. NULL during testing, in which case messages are + // dispatched immediately. + MessageLoop* io_loop_; + MessageLoop* ui_loop_; + + // Maps from tab to download state. The download state for a tab only exists + // if the state is other than ALLOW_ONE_DOWNLOAD. Similarly once the state + // transitions from anything but ALLOW_ONE_DOWNLOAD back to ALLOW_ONE_DOWNLOAD + // the TabDownloadState is removed and deleted (by way of Remove). + typedef std::map<NavigationController*, TabDownloadState*> StateMap; + StateMap state_map_; + + static TestingDelegate* delegate_; + + DISALLOW_COPY_AND_ASSIGN(DownloadRequestManager); +}; + +#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_MANAGER_H_ diff --git a/chrome/browser/download/download_request_manager_unittest.cc b/chrome/browser/download/download_request_manager_unittest.cc new file mode 100644 index 0000000..9293426 --- /dev/null +++ b/chrome/browser/download/download_request_manager_unittest.cc @@ -0,0 +1,189 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/download/download_request_manager.h" +#include "chrome/browser/navigation_controller.h" +#include "chrome/test/test_tab_contents.h" +#include "chrome/test/testing_profile.h" +#include "testing/gtest/include/gtest/gtest.h" + +class DownloadRequestManagerTest : public testing::Test, + public DownloadRequestManager::Callback, + public DownloadRequestManager::TestingDelegate { + public: + virtual void SetUp() { + allow_download_ = true; + ask_allow_count_ = cancel_count_ = continue_count_ = 0; + factory_.reset(TestTabContentsFactory::CreateAndRegisterFactory()); + TestTabContents* contents = factory_->CreateInstanceImpl(); + contents->set_commit_on_navigate(true); + controller_ = new NavigationController(contents, &profile_); + download_request_manager_ = new DownloadRequestManager(NULL, NULL); + DownloadRequestManager::SetTestingDelegate(this); + } + + virtual void TearDown() { + controller_->Destroy(); + DownloadRequestManager::SetTestingDelegate(NULL); + } + + virtual void ContinueDownload() { + continue_count_++; + } + virtual void CancelDownload() { + cancel_count_++; + } + + void CanDownload() { + download_request_manager_->CanDownloadImpl( + controller_->active_contents(), this); + } + + virtual bool ShouldAllowDownload() { + ask_allow_count_++; + return allow_download_; + } + + protected: + TestingProfile profile_; + scoped_ptr<TestTabContentsFactory> factory_; + NavigationController* controller_; + scoped_refptr<DownloadRequestManager> download_request_manager_; + + // Number of times ContinueDownload was invoked. + int continue_count_; + + // Number of times CancelDownload was invoked. + int cancel_count_; + + // Whether the download should be allowed. + bool allow_download_; + + // Number of times ShouldAllowDownload was invoked. + int ask_allow_count_; +}; + +TEST_F(DownloadRequestManagerTest, Allow) { + // All tabs should initially start at ALLOW_ONE_DOWNLOAD. + ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // Ask if the tab can do a download. This moves to PROMPT_BEFORE_DOWNLOAD. + CanDownload(); + ASSERT_EQ(DownloadRequestManager::PROMPT_BEFORE_DOWNLOAD, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + // We should have been told we can download. + ASSERT_EQ(1, continue_count_); + ASSERT_EQ(0, cancel_count_); + ASSERT_EQ(0, ask_allow_count_); + continue_count_ = 0; + + // Ask again. This triggers asking the delegate for allow/disallow. + allow_download_ = true; + CanDownload(); + // This should ask us if the download is allowed. + ASSERT_EQ(1, ask_allow_count_); + ask_allow_count_ = 0; + ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + // We should have been told we can download. + ASSERT_EQ(1, continue_count_); + ASSERT_EQ(0, cancel_count_); + continue_count_ = 0; + + // Ask again and make sure continue is invoked. + CanDownload(); + // The state is at allow_all, which means the delegate shouldn't be asked. + ASSERT_EQ(0, ask_allow_count_); + ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + // We should have been told we can download. + ASSERT_EQ(1, continue_count_); + ASSERT_EQ(0, cancel_count_); + continue_count_ = 0; +} + +TEST_F(DownloadRequestManagerTest, ResetOnNavigation) { + controller_->LoadURL(GURL(factory_->scheme() + "://foo.com/bar"), 0); + + // Do two downloads, allowing the second so that we end up with allow all. + CanDownload(); + allow_download_ = true; + CanDownload(); + ask_allow_count_ = continue_count_ = cancel_count_ = 0; + ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // Navigate to a new URL with the same host, which shouldn't reset the allow + // all state. + controller_->LoadURL(GURL(factory_->scheme() + "://foo.com/bar2"), 0); + CanDownload(); + ASSERT_EQ(1, continue_count_); + ASSERT_EQ(0, cancel_count_); + ASSERT_EQ(0, ask_allow_count_); + ask_allow_count_ = continue_count_ = cancel_count_ = 0; + ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // Do a user gesture, because we're at allow all, this shouldn't change the + // state. + download_request_manager_->OnUserGesture(controller_->active_contents()); + ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // Navigate to a completely different host, which should reset the state. + controller_->LoadURL(GURL(factory_->scheme() + "://fooey.com"), 0); + ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); +} + +TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) { + controller_->LoadURL(GURL(factory_->scheme() + "://foo.com/bar"), 0); + + // Do one download, which should change to prompt before download. + CanDownload(); + ask_allow_count_ = continue_count_ = cancel_count_ = 0; + ASSERT_EQ(DownloadRequestManager::PROMPT_BEFORE_DOWNLOAD, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // Do a user gesture, which should reset back to allow one. + download_request_manager_->OnUserGesture(controller_->active_contents()); + ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // Ask twice, which triggers calling the delegate. Don't allow the download + // so that we end up with not allowed. + allow_download_ = false; + CanDownload(); + CanDownload(); + ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + + // A user gesture now should NOT change the state. + download_request_manager_->OnUserGesture(controller_->active_contents()); + ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); + // And make sure we really can't download. + ask_allow_count_ = continue_count_ = cancel_count_ = 0; + CanDownload(); + ASSERT_EQ(0, ask_allow_count_); + ASSERT_EQ(0, continue_count_); + ASSERT_EQ(1, cancel_count_); + // And the state shouldn't have changed. + ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED, + download_request_manager_->GetDownloadStatus( + controller_->active_contents())); +} diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc index 2e7b3cc..7bd683b 100644 --- a/chrome/browser/navigation_controller.cc +++ b/chrome/browser/navigation_controller.cc @@ -413,9 +413,6 @@ void NavigationController::RemoveEntryAtIndex(int index, } else if (last_committed_entry_index_ > index) { last_committed_entry_index_--; } - - // TODO(brettw) bug 1324021: we probably need some notification here so the - // session service can stay in sync. } void NavigationController::Destroy() { diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc index ed27d84..d5d2654 100644 --- a/chrome/browser/navigation_controller_unittest.cc +++ b/chrome/browser/navigation_controller_unittest.cc @@ -19,96 +19,23 @@ #include "chrome/common/notification_types.h" #include "chrome/common/stl_util-inl.h" #include "chrome/test/test_notification_tracker.h" +#include "chrome/test/test_tab_contents.h" #include "chrome/test/testing_profile.h" #include "net/base/net_util.h" #include "testing/gtest/include/gtest/gtest.h" namespace { -// TODO(darin): come up with a better way to define these integers -// TODO(acw): we should have a real dynamic factory for content types. -// That way we could have several implementation of -// TabContents::CreateWithType(). Once this is done we'll be able to -// have a unit test for NavigationController::Clone() -const TabContentsType kTestContentsType1 = - static_cast<TabContentsType>(TAB_CONTENTS_NUM_TYPES + 1); -const TabContentsType kTestContentsType2 = - static_cast<TabContentsType>(TAB_CONTENTS_NUM_TYPES + 2); - -// Tests can set this to set the site instance for all the test contents. This -// refcounted pointer will NOT be derefed on cleanup (the tests do this -// themselves). -static SiteInstance* site_instance; - -// TestContents ---------------------------------------------------------------- - -class TestContents : public TabContents { - public: - BEGIN_MSG_MAP(TestContents) - END_MSG_MAP() - - TestContents(TabContentsType type) : TabContents(type) { - } - - // Overridden from TabContents so we can provide a non-NULL site instance in - // some cases. To use, the test will have to set the site_instance_ member - // variable to some site instance it creates. - virtual SiteInstance* GetSiteInstance() const { - return site_instance; - } - - // Just record the navigation so it can be checked by the test case. We don't - // want the normal behavior of TabContents just saying it committed since we - // want to behave more like the renderer and call RendererDidNavigate. - virtual bool NavigateToPendingEntry(bool reload) { - return true; - } - - // Sets up a call to RendererDidNavigate pretending to be a main frame - // navigation to the given URL. - void CompleteNavigationAsRenderer(int page_id, const GURL& url) { - ViewHostMsg_FrameNavigate_Params params; - params.page_id = page_id; - params.url = url; - params.transition = PageTransition::LINK; - params.should_update_history = false; - params.gesture = NavigationGestureUser; - params.is_post = false; - - NavigationController::LoadCommittedDetails details; - controller()->RendererDidNavigate(params, false, &details); - } -}; - -class TestContentsFactory : public TabContentsFactory { - public: - TestContentsFactory(TabContentsType type, const char* scheme) - : type_(type), - scheme_(scheme) { - } - - virtual TabContents* CreateInstance() { - return new TestContents(type_); - } - - virtual bool CanHandleURL(const GURL& url) { - return url.SchemeIs(scheme_); - } - - private: - TabContentsType type_; - const char* scheme_; -}; - -TestContentsFactory factory1(kTestContentsType1, "test1"); -TestContentsFactory factory2(kTestContentsType2, "test2"); - // NavigationControllerTest ---------------------------------------------------- class NavigationControllerTest : public testing::Test, public TabContentsDelegate { public: - NavigationControllerTest() : contents(NULL), profile(NULL) { + NavigationControllerTest() + : contents(NULL), + profile(NULL), + factory1_(TestTabContentsFactory::CreateAndRegisterFactory()), + factory2_(TestTabContentsFactory::CreateAndRegisterFactory()) { } ~NavigationControllerTest() { @@ -118,20 +45,17 @@ class NavigationControllerTest : public testing::Test, // testing::Test methods: virtual void SetUp() { - TabContents::RegisterFactory(kTestContentsType1, &factory1); - TabContents::RegisterFactory(kTestContentsType2, &factory2); - if (!profile) profile = new TestingProfile(); - contents = new TestContents(kTestContentsType1); + contents = new TestTabContents(type1()); contents->set_delegate(this); contents->CreateView(::GetDesktopWindow(), gfx::Rect()); contents->SetupController(profile); } virtual void TearDown() { - site_instance = NULL; + TestTabContents::set_site_instance(NULL); // Make sure contents is valid. NavigationControllerHistoryTest ends up // resetting this before TearDown is invoked. @@ -144,9 +68,6 @@ class NavigationControllerTest : public testing::Test, contents->set_delegate(NULL); contents->CloseContents(); contents = NULL; - - TabContents::RegisterFactory(kTestContentsType1, NULL); - TabContents::RegisterFactory(kTestContentsType2, NULL); } // TabContentsDelegate methods (only care about ReplaceContents): @@ -159,7 +80,7 @@ class NavigationControllerTest : public testing::Test, virtual void ReplaceContents(TabContents* source, TabContents* new_contents) { contents->set_delegate(NULL); - contents = static_cast<TestContents*>(new_contents); + contents = static_cast<TestTabContents*>(new_contents); contents->set_delegate(this); } virtual void AddNewContents(TabContents*, @@ -178,12 +99,20 @@ class NavigationControllerTest : public testing::Test, virtual void URLStarredChanged(TabContents* source, bool starred) {} virtual void UpdateTargetURL(TabContents* source, const GURL& url) {}; - TestContents* contents; + TabContentsType type1() const { return factory1_->type(); } + TabContentsType type2() const { return factory2_->type(); } + + const std::string& scheme1() const { return factory1_->scheme(); } + const std::string& scheme2() const { return factory2_->scheme(); } + + TestTabContents* contents; Profile* profile; - + private: MessageLoopForUI message_loop_; + scoped_ptr<TestTabContentsFactory> factory1_; + scoped_ptr<TestTabContentsFactory> factory2_; }; // NavigationControllerHistoryTest --------------------------------------------- @@ -192,9 +121,9 @@ class NavigationControllerHistoryTest : public NavigationControllerTest { public: NavigationControllerHistoryTest() : profile_manager_(NULL), - url0("test1:foo1"), - url1("test1:foo1"), - url2("test1:foo1") { + url0(scheme1() + ":foo1"), + url1(scheme1() + ":foo1"), + url2(scheme1() + ":foo1") { } virtual ~NavigationControllerHistoryTest() { @@ -323,8 +252,8 @@ TEST_F(NavigationControllerTest, LoadURL) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); // Creating a pending notification should not have issued any of the @@ -393,7 +322,7 @@ TEST_F(NavigationControllerTest, LoadURL_SamePage) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); + const GURL url1(scheme1() + ":foo1"); contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); @@ -420,8 +349,8 @@ TEST_F(NavigationControllerTest, LoadURL_Discarded) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); @@ -449,13 +378,13 @@ TEST_F(NavigationControllerTest, LoadURL_NoPending) { RegisterForAllNavNotifications(¬ifications, contents->controller()); // First make an existing committed entry. - const GURL kExistingURL1("test1:eh"); + const GURL kExistingURL1(scheme1() + ":eh"); contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, kExistingURL1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Do a new navigation without making a pending one. - const GURL kNewURL("test1:see"); + const GURL kNewURL(scheme1() + ":see"); contents->CompleteNavigationAsRenderer(99, kNewURL); // There should no longer be any pending entry, and the third navigation we @@ -475,18 +404,18 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { RegisterForAllNavNotifications(¬ifications, contents->controller()); // First make an existing committed entry. - const GURL kExistingURL1("test1:eh"); + const GURL kExistingURL1(scheme1() + ":eh"); contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, kExistingURL1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Make a pending entry to somewhere new. - const GURL kExistingURL2("test1:bee"); + const GURL kExistingURL2(scheme1() + ":bee"); contents->controller()->LoadURL(kExistingURL2, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); // Before that commits, do a new navigation. - const GURL kNewURL("test1:see"); + const GURL kNewURL(scheme1() + ":see"); contents->CompleteNavigationAsRenderer(3, kNewURL); // There should no longer be any pending entry, and the third navigation we @@ -505,12 +434,12 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { RegisterForAllNavNotifications(¬ifications, contents->controller()); // First make some history. - const GURL kExistingURL1("test1:eh"); + const GURL kExistingURL1(scheme1() + ":eh"); contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, kExistingURL1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - const GURL kExistingURL2("test1:bee"); + const GURL kExistingURL2(scheme1() + ":bee"); contents->controller()->LoadURL(kExistingURL2, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(1, kExistingURL2); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); @@ -523,7 +452,7 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex()); // Before that commits, do a new navigation. - const GURL kNewURL("test1:see"); + const GURL kNewURL(scheme1() + ":see"); NavigationController::LoadCommittedDetails details; contents->CompleteNavigationAsRenderer(3, kNewURL); @@ -539,7 +468,7 @@ TEST_F(NavigationControllerTest, Reload) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); + const GURL url1(scheme1() + ":foo1"); contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); @@ -576,8 +505,8 @@ TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, url1); @@ -604,11 +533,11 @@ TEST_F(NavigationControllerTest, Back) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); + const GURL url1(scheme1() + ":foo1"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - const GURL url2("test1:foo2"); + const GURL url2(scheme1() + ":foo2"); contents->CompleteNavigationAsRenderer(1, url2); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); @@ -642,9 +571,9 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); - const GURL url3("test1:foo3"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); + const GURL url3(scheme1() + ":foo3"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, url1); @@ -685,9 +614,9 @@ TEST_F(NavigationControllerTest, Back_NewPending) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL kUrl1("test1:foo1"); - const GURL kUrl2("test1:foo2"); - const GURL kUrl3("test1:foo3"); + const GURL kUrl1(scheme1() + ":foo1"); + const GURL kUrl2(scheme1() + ":foo2"); + const GURL kUrl3(scheme1() + ":foo3"); // First navigate two places so we have some back history. contents->CompleteNavigationAsRenderer(0, kUrl1); @@ -712,9 +641,9 @@ TEST_F(NavigationControllerTest, Back_NewPending) { // Receives a back message when there is a different renavigation already // pending. TEST_F(NavigationControllerTest, Back_OtherBackPending) { - const GURL kUrl1("test1:foo1"); - const GURL kUrl2("test1:foo2"); - const GURL kUrl3("test1:foo3"); + const GURL kUrl1(scheme1() + ":foo1"); + const GURL kUrl2(scheme1() + ":foo2"); + const GURL kUrl3(scheme1() + ":foo3"); // First navigate three places so we have some back history. contents->CompleteNavigationAsRenderer(0, kUrl1); @@ -727,7 +656,7 @@ TEST_F(NavigationControllerTest, Back_OtherBackPending) { // That second URL should be the last committed and it should have gotten the // new title. EXPECT_EQ(kUrl2, contents->controller()->GetEntryWithPageID( - kTestContentsType1, NULL, 1)->url()); + type1(), NULL, 1)->url()); EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex()); EXPECT_EQ(-1, contents->controller()->GetPendingEntryIndex()); @@ -757,8 +686,8 @@ TEST_F(NavigationControllerTest, Forward) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); @@ -799,9 +728,9 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); - const GURL url3("test1:foo3"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); + const GURL url3(scheme1() + ":foo3"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); @@ -843,11 +772,11 @@ TEST_F(NavigationControllerTest, NewSubframe) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); + const GURL url1(scheme1() + ":foo1"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - const GURL url2("test1:foo2"); + const GURL url2(scheme1() + ":foo2"); ViewHostMsg_FrameNavigate_Params params; params.page_id = 1; params.url = url2; @@ -882,7 +811,7 @@ TEST_F(NavigationControllerTest, SubframeOnEmptyPage) { RegisterForAllNavNotifications(¬ifications, contents->controller()); // Navigation controller currently has no entries. - const GURL url("test1:foo2"); + const GURL url(scheme1() + ":foo2"); ViewHostMsg_FrameNavigate_Params params; params.page_id = 1; params.url = url; @@ -903,11 +832,11 @@ TEST_F(NavigationControllerTest, AutoSubframe) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); + const GURL url1(scheme1() + ":foo1"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - const GURL url2("test1:foo2"); + const GURL url2(scheme1() + ":foo2"); ViewHostMsg_FrameNavigate_Params params; params.page_id = 0; params.url = url2; @@ -932,12 +861,12 @@ TEST_F(NavigationControllerTest, BackSubframe) { RegisterForAllNavNotifications(¬ifications, contents->controller()); // Main page. - const GURL url1("test1:foo1"); + const GURL url1(scheme1() + ":foo1"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // First manual subframe navigation. - const GURL url2("test1:foo2"); + const GURL url2(scheme1() + ":foo2"); ViewHostMsg_FrameNavigate_Params params; params.page_id = 1; params.url = url2; @@ -954,7 +883,7 @@ TEST_F(NavigationControllerTest, BackSubframe) { EXPECT_EQ(2, contents->controller()->GetEntryCount()); // Second manual subframe navigation should also make a new entry. - const GURL url3("test1:foo3"); + const GURL url3(scheme1() + ":foo3"); params.page_id = 2; params.url = url3; EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, @@ -988,8 +917,8 @@ TEST_F(NavigationControllerTest, LinkClick) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); @@ -1013,12 +942,12 @@ TEST_F(NavigationControllerTest, InPage) { // Main page. Note that we need "://" so this URL is treated as "standard" // which are the only ones that can have a ref. - const GURL url1("test1://foo"); + const GURL url1(scheme1() + "://foo"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // First navigation. - const GURL url2("test1://foo#a"); + const GURL url2(scheme1() + "://foo#a"); ViewHostMsg_FrameNavigate_Params params; params.page_id = 1; params.url = url2; @@ -1077,13 +1006,13 @@ TEST_F(NavigationControllerTest, SwitchTypes) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo"); - const GURL url2("test2:foo"); + const GURL url1(scheme1() + ":foo"); + const GURL url2(scheme2() + ":foo"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - TestContents* initial_contents = contents; + TestTabContents* initial_contents = contents; contents->controller()->LoadURL(url2, PageTransition::TYPED); // The tab contents should have been replaced @@ -1126,13 +1055,13 @@ TEST_F(NavigationControllerTest, SwitchTypes_Discard) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo"); - const GURL url2("test2:foo"); + const GURL url1(scheme1() + ":foo"); + const GURL url2(scheme2() + ":foo"); contents->CompleteNavigationAsRenderer(0, url1); EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - TestContents* initial_contents = contents; + TestTabContents* initial_contents = contents; contents->controller()->LoadURL(url2, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); @@ -1161,9 +1090,9 @@ TEST_F(NavigationControllerTest, SwitchTypes_Discard) { // Tests that TabContentsTypes that are not in use are deleted (via a // TabContentsCollector task). Prevents regression of bug 1296773. TEST_F(NavigationControllerTest, SwitchTypesCleanup) { - const GURL url1("test1:foo"); - const GURL url2("test2:foo"); - const GURL url3("test2:bar"); + const GURL url1(scheme1() + ":foo"); + const GURL url2(scheme2() + ":foo"); + const GURL url3(scheme2() + ":bar"); // Note that we need the LoadURL calls so that pending entries and the // different tab contents types are created. "Renderer" navigations won't @@ -1188,9 +1117,9 @@ TEST_F(NavigationControllerTest, SwitchTypesCleanup) { // Now that the tasks have been flushed, the first tab type should be gone. ASSERT_TRUE( - contents->controller()->GetTabContents(kTestContentsType1) == NULL); + contents->controller()->GetTabContents(type1()) == NULL); ASSERT_EQ(contents, - contents->controller()->GetTabContents(kTestContentsType2)); + contents->controller()->GetTabContents(type2())); } namespace { @@ -1239,7 +1168,7 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { char buffer[128]; // Load up to the max count, all entries should be there. for (url_index = 0; url_index < kMaxEntryCount; url_index++) { - SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index); + SNPrintF(buffer, 128, (scheme1() + "://www.a.com/%d").c_str(), url_index); GURL url(buffer); contents->controller()->LoadURL(url, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(url_index, url); @@ -1251,7 +1180,7 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { PrunedListener listener(contents->controller()); // Navigate some more. - SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index); + SNPrintF(buffer, 128, (scheme1() + "://www.a.com/%d").c_str(), url_index); GURL url(buffer); contents->controller()->LoadURL(url, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(url_index, url); @@ -1265,11 +1194,11 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { // We expect http://www.a.com/0 to be gone. EXPECT_EQ(contents->controller()->GetEntryCount(), kMaxEntryCount); EXPECT_EQ(contents->controller()->GetEntryAtIndex(0)->url(), - GURL("test1://www.a.com/1")); + GURL(scheme1() + "://www.a.com/1")); // More navigations. for (int i = 0; i < 3; i++) { - SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index); + SNPrintF(buffer, 128, (scheme1() + "://www.a.com/%d").c_str(), url_index); url = GURL(buffer); contents->controller()->LoadURL(url, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(url_index, url); @@ -1277,7 +1206,7 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { } EXPECT_EQ(contents->controller()->GetEntryCount(), kMaxEntryCount); EXPECT_EQ(contents->controller()->GetEntryAtIndex(0)->url(), - GURL("test1://www.a.com/4")); + GURL(scheme1() + "://www.a.com/4")); NavigationController::set_max_entry_count(original_count); } @@ -1286,11 +1215,12 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { // everything is updated properly. This can be tricky since there is no // SiteInstance for the entries created initially. TEST_F(NavigationControllerTest, RestoreNavigate) { - site_instance = SiteInstance::CreateSiteInstance(profile); + SiteInstance* site_instance = SiteInstance::CreateSiteInstance(profile); + TestTabContents::set_site_instance(site_instance); site_instance->AddRef(); // Create a NavigationController with a restored set of tabs. - GURL url("test1:foo"); + GURL url(scheme1() + ":foo"); std::vector<TabNavigation> navigations; navigations.push_back(TabNavigation(0, url, L"Title", "state", PageTransition::LINK)); @@ -1326,18 +1256,19 @@ TEST_F(NavigationControllerTest, RestoreNavigate) { // Clean up the navigation controller. ClearContents(); controller->Destroy(); + TestTabContents::set_site_instance(NULL); site_instance->Release(); } // Make sure that the page type and stuff is correct after an interstitial. TEST_F(NavigationControllerTest, Interstitial) { // First navigate somewhere normal. - const GURL url1("test1:foo"); + const GURL url1(scheme1() + ":foo"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, url1); // Now navigate somewhere with an interstitial. - const GURL url2("test1:bar"); + const GURL url2(scheme1() + ":bar"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->controller()->GetPendingEntry()->set_page_type( NavigationEntry::INTERSTITIAL_PAGE); @@ -1353,13 +1284,13 @@ TEST_F(NavigationControllerTest, Interstitial) { } TEST_F(NavigationControllerTest, RemoveEntry) { - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); - const GURL url3("test1:foo3"); - const GURL url4("test1:foo4"); - const GURL url5("test1:foo5"); - const GURL pending_url("test1:pending"); - const GURL default_url("test1:default"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); + const GURL url3(scheme1() + ":foo3"); + const GURL url4(scheme1() + ":foo4"); + const GURL url5(scheme1() + ":foo5"); + const GURL pending_url(scheme1() + ":pending"); + const GURL default_url(scheme1() + ":default"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, url1); @@ -1416,12 +1347,12 @@ TEST_F(NavigationControllerTest, TransientEntry) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url0("test1:foo0"); - const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); - const GURL url3("test1:foo3"); - const GURL url4("test1:foo4"); - const GURL transient_url("test1:transient"); + const GURL url0(scheme1() + ":foo0"); + const GURL url1(scheme1() + ":foo1"); + const GURL url2(scheme1() + ":foo2"); + const GURL url3(scheme1() + ":foo3"); + const GURL url4(scheme1() + ":foo4"); + const GURL transient_url(scheme1() + ":transient"); contents->controller()->LoadURL(url0, PageTransition::TYPED); contents->CompleteNavigationAsRenderer(0, url0); diff --git a/chrome/browser/render_view_host.cc b/chrome/browser/render_view_host.cc index 54376cf..03eec5a 100644 --- a/chrome/browser/render_view_host.cc +++ b/chrome/browser/render_view_host.cc @@ -1112,6 +1112,14 @@ void RenderViewHost::UnhandledInputEvent(const WebInputEvent& event) { static_cast<const WebKeyboardEvent&>(event)); } +void RenderViewHost::ForwardKeyboardEvent(const WebKeyboardEvent& key_event) { + if (key_event.type == WebKeyboardEvent::CHAR && + (key_event.key_data == '\n' || key_event.key_data == ' ')) { + delegate_->OnEnterOrSpace(); + } + RenderWidgetHost::ForwardKeyboardEvent(key_event); +} + void RenderViewHost::OnMissingPluginStatus(int status) { delegate_->OnMissingPluginStatus(status); } diff --git a/chrome/browser/render_view_host.h b/chrome/browser/render_view_host.h index a20bac7a5..34616a3 100644 --- a/chrome/browser/render_view_host.h +++ b/chrome/browser/render_view_host.h @@ -385,6 +385,7 @@ class RenderViewHost : public RenderWidgetHost { protected: // Overridden from RenderWidgetHost: virtual void UnhandledInputEvent(const WebInputEvent& event); + virtual void ForwardKeyboardEvent(const WebKeyboardEvent& key_event); // IPC message handlers: void OnMsgCreateWindow(int route_id, HANDLE modal_dialog_event); diff --git a/chrome/browser/render_view_host_delegate.h b/chrome/browser/render_view_host_delegate.h index 4593f10..746861f0 100644 --- a/chrome/browser/render_view_host_delegate.h +++ b/chrome/browser/render_view_host_delegate.h @@ -359,6 +359,11 @@ class RenderViewHostDelegate { virtual void OnDidGetApplicationInfo( int32 page_id, const webkit_glue::WebApplicationInfo& app_info) { } + + // Notification the user has pressed enter or space while focus was on the + // page. This is used to avoid uninitiated user downloads (aka carpet + // bombing), see DownloadRequestManager for details. + virtual void OnEnterOrSpace() { } }; #endif // CHROME_BROWSER_RENDER_VIEW_HOST_DELEGATE_H__ diff --git a/chrome/browser/render_widget_host.h b/chrome/browser/render_widget_host.h index f3c1454..a581604 100644 --- a/chrome/browser/render_widget_host.h +++ b/chrome/browser/render_widget_host.h @@ -232,7 +232,7 @@ class RenderWidgetHost : public IPC::Channel::Listener { friend class RenderWidgetHostViewWin; void ForwardMouseEvent(const WebMouseEvent& mouse_event); - void ForwardKeyboardEvent(const WebKeyboardEvent& key_event); + virtual void ForwardKeyboardEvent(const WebKeyboardEvent& key_event); void ForwardWheelEvent(const WebMouseWheelEvent& wheel_event); void ForwardInputEvent(const WebInputEvent& input_event, int event_size); diff --git a/chrome/browser/resource_dispatcher_host.cc b/chrome/browser/resource_dispatcher_host.cc index 4b0f37c..597abba 100644 --- a/chrome/browser/resource_dispatcher_host.cc +++ b/chrome/browser/resource_dispatcher_host.cc @@ -15,6 +15,7 @@ #include "chrome/browser/cross_site_request_manager.h" #include "chrome/browser/download/download_file.h" #include "chrome/browser/download/download_manager.h" +#include "chrome/browser/download/download_request_manager.h" #include "chrome/browser/download/save_file_manager.h" #include "chrome/browser/external_protocol_handler.h" #include "chrome/browser/login_prompt.h" @@ -426,6 +427,181 @@ class ResourceDispatcherHost::DownloadEventHandler DISALLOW_EVIL_CONSTRUCTORS(DownloadEventHandler); }; +// DownloadThrottlingEventHandler---------------------------------------------- + +// DownloadThrottlingEventHandler is used to determine if a download should be +// allowed. When a DownloadThrottlingEventHandler is created it pauses the +// download and asks the DownloadRequestManager if the download should be +// allowed. The DownloadRequestManager notifies us asynchronously as to whether +// the download is allowed or not. If the download is allowed the request is +// resumed, a DownloadEventHandler is created and all EventHandler methods are +// delegated to it. If the download is not allowed the request is canceled. + +class ResourceDispatcherHost::DownloadThrottlingEventHandler : + public ResourceDispatcherHost::EventHandler, + public DownloadRequestManager::Callback { + public: + DownloadThrottlingEventHandler(ResourceDispatcherHost* host, + URLRequest* request, + const std::string& url, + int render_process_host_id, + int render_view_id, + int request_id, + bool in_complete) + : host_(host), + request_(request), + url_(url), + render_process_host_id_(render_process_host_id), + render_view_id_(render_view_id), + request_id_(request_id), + tmp_buffer_length_(0), + ignore_on_read_complete_(in_complete) { + // Pause the request. + host_->PauseRequest(render_process_host_id_, request_id_, true); + host_->download_request_manager()->CanDownloadOnIOThread( + render_process_host_id_, render_view_id, this); + } + + virtual ~DownloadThrottlingEventHandler() {} + + virtual bool OnUploadProgress(int request_id, + uint64 position, + uint64 size) { + if (download_handler_.get()) + return download_handler_->OnUploadProgress(request_id, position, size); + return true; + } + + virtual bool OnRequestRedirected(int request_id, const GURL& url) { + if (download_handler_.get()) + return download_handler_->OnRequestRedirected(request_id, url); + url_ = url.spec(); + return true; + } + + virtual bool OnResponseStarted(int request_id, Response* response) { + if (download_handler_.get()) + return download_handler_->OnResponseStarted(request_id, response); + response_ = response; + return true; + } + + virtual bool OnWillRead(int request_id, + char** buf, + int* buf_size, + int min_size) { + if (download_handler_.get()) + return download_handler_->OnWillRead(request_id, buf, buf_size, min_size); + + // We should only have this invoked once, as such we only deal with one + // tmp buffer. + DCHECK(!tmp_buffer_.get()); + if (min_size < 0) + min_size = 1024; + tmp_buffer_.reset(new char[min_size]); + *buf = tmp_buffer_.get(); + *buf_size = min_size; + return true; + } + + virtual bool OnReadCompleted(int request_id, int* bytes_read) { + if (ignore_on_read_complete_) { + // See comments above definition for details on this. + ignore_on_read_complete_ = false; + return true; + } + if (!*bytes_read) + return true; + + if (tmp_buffer_.get()) { + DCHECK(!tmp_buffer_length_); + tmp_buffer_length_ = *bytes_read; + if (download_handler_.get()) + CopyTmpBufferToDownloadHandler(); + return true; + } + if (download_handler_.get()) + return download_handler_->OnReadCompleted(request_id, bytes_read); + return true; + } + + virtual bool OnResponseCompleted(int request_id, + const URLRequestStatus& status) { + if (download_handler_.get()) + return download_handler_->OnResponseCompleted(request_id, status); + NOTREACHED(); + return true; + } + + void CancelDownload() { + host_->CancelRequest(render_process_host_id_, request_id_, false); + } + + void ContinueDownload() { + DCHECK(!download_handler_.get()); + download_handler_ = + new DownloadEventHandler(host_, + render_process_host_id_, + render_view_id_, + request_id_, + url_, + host_->download_file_manager(), + request_, + false); + if (response_.get()) + download_handler_->OnResponseStarted(request_id_, response_.get()); + + if (tmp_buffer_length_) + CopyTmpBufferToDownloadHandler(); + + // And let the request continue. + host_->PauseRequest(render_process_host_id_, request_id_, false); + } + + private: + void CopyTmpBufferToDownloadHandler() { + // Copy over the tmp buffer. + char* buffer; + int buf_size; + if (download_handler_->OnWillRead(request_id_, &buffer, &buf_size, + tmp_buffer_length_)) { + CHECK(buf_size >= tmp_buffer_length_); + memcpy(buffer, tmp_buffer_.get(), tmp_buffer_length_); + download_handler_->OnReadCompleted(request_id_, &tmp_buffer_length_); + } + tmp_buffer_length_ = 0; + tmp_buffer_.reset(); + } + + ResourceDispatcherHost* host_; + URLRequest* request_; + std::string url_; + int render_process_host_id_; + int render_view_id_; + int request_id_; + + // Handles the actual download. This is only created if the download is + // allowed to continue. + scoped_refptr<DownloadEventHandler> download_handler_; + + // Response supplied to OnResponseStarted. Only non-null if OnResponseStarted + // is invoked. + scoped_refptr<Response> response_; + + // If we're created by way of BufferedEventHandler we'll get one request for + // a buffer. This is that buffer. + scoped_array<char> tmp_buffer_; + int tmp_buffer_length_; + + // If true the next call to OnReadCompleted is ignored. This is used if we're + // paused during a call to OnReadCompleted. Pausing during OnReadCompleted + // results in two calls to OnReadCompleted for the same data. This make sure + // we ignore one of them. + bool ignore_on_read_complete_; + + DISALLOW_COPY_AND_ASSIGN(DownloadThrottlingEventHandler); +}; + // ---------------------------------------------------------------------------- @@ -859,7 +1035,7 @@ class ResourceDispatcherHost::BufferedEventHandler bool OnResponseStarted(int request_id, Response* response) { response_ = response; if (!DelayResponse()) - return CompleteResponseStarted(request_id); + return CompleteResponseStarted(request_id, false); return true; } @@ -888,8 +1064,9 @@ class ResourceDispatcherHost::BufferedEventHandler // Returns true if we have to keep buffering data. bool KeepBuffering(int bytes_read); - // Sends a pending OnResponseStarted notification. - bool CompleteResponseStarted(int request_id); + // Sends a pending OnResponseStarted notification. |in_complete| is true if + // this is invoked from |OnResponseCompleted|. + bool CompleteResponseStarted(int request_id, bool in_complete); scoped_refptr<ResourceDispatcherHost::EventHandler> real_handler_; scoped_refptr<Response> response_; @@ -941,7 +1118,7 @@ bool ResourceDispatcherHost::BufferedEventHandler::OnReadCompleted( *bytes_read = bytes_read_; // Done buffering, send the pending ResponseStarted event. - if (!CompleteResponseStarted(request_id)) + if (!CompleteResponseStarted(request_id, true)) return false; } @@ -1026,7 +1203,8 @@ bool ResourceDispatcherHost::BufferedEventHandler::KeepBuffering( } bool ResourceDispatcherHost::BufferedEventHandler::CompleteResponseStarted( - int request_id) { + int request_id, + bool in_complete) { // Check to see if we should forward the data from this request to the // download thread. // TODO(paulg): Only download if the context from the renderer allows it. @@ -1053,17 +1231,17 @@ bool ResourceDispatcherHost::BufferedEventHandler::CompleteResponseStarted( info->is_download = true; - scoped_refptr<DownloadEventHandler> download_handler = - new DownloadEventHandler(host_, - info->render_process_host_id, - info->render_view_id, - request_id, - request_->url().spec(), - host_->download_file_manager(), - request_, false); + scoped_refptr<DownloadThrottlingEventHandler> download_handler = + new DownloadThrottlingEventHandler(host_, + request_, + request_->url().spec(), + info->render_process_host_id, + info->render_view_id, + request_id, + in_complete); if (bytes_read_) { // a Read has already occurred and we need to copy the data into the - // DownloadEventHandler. + // EventHandler. char *buf = NULL; int buf_len = 0; download_handler->OnWillRead(request_id, &buf, &buf_len, bytes_read_); @@ -1236,6 +1414,7 @@ ResourceDispatcherHost::ResourceDispatcherHost(MessageLoop* io_loop) : ui_loop_(MessageLoop::current()), io_loop_(io_loop), download_file_manager_(new DownloadFileManager(ui_loop_, this)), + download_request_manager_(new DownloadRequestManager(io_loop, ui_loop_)), save_file_manager_(new SaveFileManager(ui_loop_, io_loop, this)), safe_browsing_(new SafeBrowsingService), request_id_(-1), @@ -1279,6 +1458,10 @@ void ResourceDispatcherHost::OnShutdown() { DCHECK(MessageLoop::current() == io_loop_); is_shutdown_ = true; STLDeleteValues(&pending_requests_); + // Make sure we shutdown the timer now, otherwise by the time our destructor + // runs if the timer is still running the Task is deleted twice (once by + // the MessageLoop and the second time by RepeatingTimer). + update_load_states_timer_.Stop(); } bool ResourceDispatcherHost::HandleExternalProtocol(int request_id, @@ -1970,7 +2153,7 @@ void ResourceDispatcherHost::ResumeRequest(const GlobalRequestID& request_id) { OnResponseStarted(i->second); } -bool ResourceDispatcherHost::Read(URLRequest *request, int *bytes_read) { +bool ResourceDispatcherHost::Read(URLRequest* request, int* bytes_read) { ExtraRequestInfo* info = ExtraInfoForRequest(request); DCHECK(!info->is_paused); @@ -2308,4 +2491,3 @@ void ResourceDispatcherHost::MaybeUpdateUploadProgress(ExtraRequestInfo *info, info->last_upload_position = position; } } - diff --git a/chrome/browser/resource_dispatcher_host.h b/chrome/browser/resource_dispatcher_host.h index db94fcd..d77454c 100644 --- a/chrome/browser/resource_dispatcher_host.h +++ b/chrome/browser/resource_dispatcher_host.h @@ -27,13 +27,14 @@ #include "webkit/glue/resource_type.h" class DownloadFileManager; -class SaveFileManager; +class DownloadRequestManager; +class LoginHandler; class MessageLoop; class PluginService; class SafeBrowsingService; +class SaveFileManager; class TabContents; class URLRequestContext; -class LoginHandler; struct ViewHostMsg_Resource_Request; struct ViewMsg_Resource_ResponseHead; @@ -72,7 +73,8 @@ class ResourceDispatcherHost : public URLRequest::Delegate { int min_size) = 0; // Data (*bytes_read bytes) was written into the buffer provided by - // OnWillRead. + // OnWillRead. A return value of false cancels the request, true continues + // reading data. virtual bool OnReadCompleted(int request_id, int* bytes_read) = 0; // The response is complete. The final response status is given. @@ -101,7 +103,7 @@ class ResourceDispatcherHost : public URLRequest::Delegate { // Holds the data we would like to associate with each request class ExtraRequestInfo : public URLRequest::UserData { - friend ResourceDispatcherHost; + friend class ResourceDispatcherHost; public: ExtraRequestInfo(EventHandler* handler, int request_id, @@ -296,6 +298,10 @@ class ResourceDispatcherHost : public URLRequest::Delegate { return download_file_manager_; } + DownloadRequestManager* download_request_manager() const { + return download_request_manager_.get(); + } + SaveFileManager* save_file_manager() const { return save_file_manager_; } @@ -357,12 +363,13 @@ class ResourceDispatcherHost : public URLRequest::Delegate { private: class AsyncEventHandler; - class SyncEventHandler; + class BufferedEventHandler; class CrossSiteNotifyTabTask; class DownloadEventHandler; - class BufferedEventHandler; + class DownloadThrottlingEventHandler; class SaveFileEventHandler; class ShutdownTask; + class SyncEventHandler; friend class ShutdownTask; @@ -378,7 +385,7 @@ class ResourceDispatcherHost : public URLRequest::Delegate { // Reads data from the response using our internal buffer as async IO. // Returns true if data is available immediately, false otherwise. If the // return value is false, we will receive a OnReadComplete() callback later. - bool Read(URLRequest *, int *bytes_read); + bool Read(URLRequest* request, int* bytes_read); // Internal function to finish an async IO which has completed. Returns // true if there is more data to read (e.g. we haven't read EOF yet and @@ -449,6 +456,9 @@ class ResourceDispatcherHost : public URLRequest::Delegate { // We own the download file writing thread and manager scoped_refptr<DownloadFileManager> download_file_manager_; + // Determines whether a download is allowed. + scoped_refptr<DownloadRequestManager> download_request_manager_; + // We own the save file manager. scoped_refptr<SaveFileManager> save_file_manager_; @@ -478,4 +488,3 @@ class ResourceDispatcherHost : public URLRequest::Delegate { }; #endif // CHROME_BROWSER_RESOURCE_DISPATCHER_HOST_H__ - diff --git a/chrome/browser/tab_contents_factory.cc b/chrome/browser/tab_contents_factory.cc index ffde997..19b40569 100644 --- a/chrome/browser/tab_contents_factory.cc +++ b/chrome/browser/tab_contents_factory.cc @@ -24,6 +24,18 @@ typedef std::map<TabContentsType, TabContentsFactory*> TabContentsFactoryMap; static TabContentsFactoryMap* g_extra_types; // Only allocated if needed. +// static +TabContentsType TabContentsFactory::NextUnusedType() { + int type = static_cast<int>(TAB_CONTENTS_NUM_TYPES); + if (g_extra_types) { + for (TabContentsFactoryMap::iterator i = g_extra_types->begin(); + i != g_extra_types->end(); ++i) { + type = std::max(type, static_cast<int>(i->first)); + } + } + return static_cast<TabContentsType>(type + 1); +} + /*static*/ TabContents* TabContents::CreateWithType(TabContentsType type, HWND parent, diff --git a/chrome/browser/tab_contents_factory.h b/chrome/browser/tab_contents_factory.h index c802cde..86c15ea 100644 --- a/chrome/browser/tab_contents_factory.h +++ b/chrome/browser/tab_contents_factory.h @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#ifndef CHROME_BROWSER_TAB_CONTENTS_FACTORY_H_ +#define CHROME_BROWSER_TAB_CONTENTS_FACTORY_H_ + #include <string> #include "chrome/browser/tab_contents_type.h" @@ -11,6 +14,9 @@ class TabContents; // TabContents::RegisterFactory. class TabContentsFactory { public: + // Returns the next unused TabContentsType after TAB_CONTENTS_NUM_TYPES. + static TabContentsType NextUnusedType(); + // Returns a new TabContents instance of the associated type. virtual TabContents* CreateInstance() = 0; @@ -19,3 +25,4 @@ class TabContentsFactory { virtual bool CanHandleURL(const GURL& url) = 0; }; +#endif // CHROME_BROWSER_TAB_CONTENTS_FACTORY_H_
\ No newline at end of file diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc index cdee437..e842db8 100644 --- a/chrome/browser/web_contents.cc +++ b/chrome/browser/web_contents.cc @@ -14,6 +14,7 @@ #include "chrome/browser/character_encoding.h" #include "chrome/browser/dom_operation_notification_details.h" #include "chrome/browser/download/download_manager.h" +#include "chrome/browser/download/download_request_manager.h" #include "chrome/browser/find_in_page_controller.h" #include "chrome/browser/find_notification_details.h" #include "chrome/browser/google_util.h" @@ -1484,6 +1485,13 @@ void WebContents::OnDidGetApplicationInfo( &GearsCreateShortcutCallbackFunctor::Run)); } +void WebContents::OnEnterOrSpace() { + // See comment in RenderViewHostDelegate::OnEnterOrSpace as to why we do this. + DownloadRequestManager* drm = g_browser_process->download_request_manager(); + if (drm) + drm->OnUserGesture(this); +} + // Stupid pass-through for RenderViewHostDelegate. void WebContents::HandleKeyboardEvent(const WebKeyboardEvent& event) { view_->HandleKeyboardEvent(event); diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h index 5dcb63d..8a5f6aa 100644 --- a/chrome/browser/web_contents.h +++ b/chrome/browser/web_contents.h @@ -320,6 +320,7 @@ class WebContents : public TabContents, virtual void OnDidGetApplicationInfo( int32 page_id, const webkit_glue::WebApplicationInfo& info); + virtual void OnEnterOrSpace(); // Stupid render view host view pass-throughs. virtual void HandleKeyboardEvent(const WebKeyboardEvent& event); diff --git a/chrome/browser/web_contents_view_win.cc b/chrome/browser/web_contents_view_win.cc index 5a377de..60e11ce 100644 --- a/chrome/browser/web_contents_view_win.cc +++ b/chrome/browser/web_contents_view_win.cc @@ -6,6 +6,8 @@ #include <windows.h> +#include "chrome/browser/browser_process.h" +#include "chrome/browser/download/download_request_manager.h" #include "chrome/browser/find_in_page_controller.h" #include "chrome/browser/render_view_context_menu.h" #include "chrome/browser/render_view_context_menu_controller.h" @@ -327,11 +329,16 @@ LRESULT WebContentsViewWin::OnMouseRange(UINT msg, switch (msg) { case WM_LBUTTONDOWN: case WM_MBUTTONDOWN: - case WM_RBUTTONDOWN: + case WM_RBUTTONDOWN: { // Make sure this TabContents is activated when it is clicked on. if (web_contents_->delegate()) web_contents_->delegate()->ActivateContents(web_contents_); + DownloadRequestManager* drm = + g_browser_process->download_request_manager(); + if (drm) + drm->OnUserGesture(web_contents_); break; + } case WM_MOUSEMOVE: // Let our delegate know that the mouse moved (useful for resetting status // bubble state). @@ -514,4 +521,3 @@ void WebContentsViewWin::WheelZoom(int distance) { web_contents_->delegate()->ContentsZoomChange(zoom_in); } } - diff --git a/chrome/test/test_tab_contents.cc b/chrome/test/test_tab_contents.cc new file mode 100644 index 0000000..7b43b76 --- /dev/null +++ b/chrome/test/test_tab_contents.cc @@ -0,0 +1,38 @@ +// Copyright (c) 2006-2008 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/test/test_tab_contents.h" + +#include "chrome/browser/navigation_entry.h" + +// static +SiteInstance* TestTabContents::site_instance_ = NULL; + +TestTabContents::TestTabContents(TabContentsType type) + : TabContents(type), + commit_on_navigate_(false), + next_page_id_(1) { +} + +bool TestTabContents::NavigateToPendingEntry(bool reload) { + if (commit_on_navigate_) { + CompleteNavigationAsRenderer(next_page_id_++, + controller()->GetPendingEntry()->url()); + } + return true; +} + +void TestTabContents::CompleteNavigationAsRenderer(int page_id, + const GURL& url) { + ViewHostMsg_FrameNavigate_Params params; + params.page_id = page_id; + params.url = url; + params.transition = PageTransition::LINK; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + + NavigationController::LoadCommittedDetails details; + controller()->RendererDidNavigate(params, false, &details); +} diff --git a/chrome/test/test_tab_contents.h b/chrome/test/test_tab_contents.h new file mode 100644 index 0000000..13b82e4 --- /dev/null +++ b/chrome/test/test_tab_contents.h @@ -0,0 +1,110 @@ +// Copyright (c) 2006-2008 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_TEST_TEST_TAB_CONTENTS_H_ +#define CHROME_BROWSER_TEST_TEST_TAB_CONTENTS_H_ + +#include "base/string_util.h" +#include "chrome/browser/tab_contents.h" +#include "chrome/browser/tab_contents_factory.h" + +// TabContents typed created by TestTabContentsFactory. +class TestTabContents : public TabContents { + public: + BEGIN_MSG_MAP(TestTabContents) + END_MSG_MAP() + + explicit TestTabContents(TabContentsType type); + + // Sets the site instance used by *ALL* TestTabContents. + static void set_site_instance(SiteInstance* site_instance) { + site_instance_ = site_instance; + } + + // Sets whether we commit the load on NavigateToPendingEntry. The default is + // false. + void set_commit_on_navigate(bool commit_on_navigate) { + commit_on_navigate_ = commit_on_navigate; + } + + // Overridden from TabContents so we can provide a non-NULL site instance in + // some cases. To use, the test will have to set the site_instance_ member + // variable to some site instance it creates. + virtual SiteInstance* GetSiteInstance() const { + return site_instance_; + } + + // Does one of two things. If |commit_on_navigate| is true the navigation is + // committed immediately. Otherwise this returns true and you must invoke + // CompleteNavigationAsRenderer when you want to commit the load. + virtual bool NavigateToPendingEntry(bool reload); + + // Sets up a call to RendererDidNavigate pretending to be a main frame + // navigation to the given URL. + void CompleteNavigationAsRenderer(int page_id, const GURL& url); + + private: + static SiteInstance* site_instance_; + + bool commit_on_navigate_; + int next_page_id_; + + DISALLOW_COPY_AND_ASSIGN(TestTabContents); +}; + +// TestTabContentsFactory is a TabContentsFactory that can be used for tests. +// +// Use the CreateAndRegisterFactory method to create and register a new +// TestTabContentsFactory. You can use scheme() to determine the resulting +// scheme and type() for the resulting TabContentsType. +// +// TestTabContentsFactory unregisters itself from the TabContentsFactory in its +// destructor. +class TestTabContentsFactory : public TabContentsFactory { + public: + // Creates a new TestTabContentsFactory and registers it for the next + // free TabContentsType. The destructor unregisters the factory. + static TestTabContentsFactory* CreateAndRegisterFactory() { + TabContentsType new_type = TabContentsFactory::NextUnusedType(); + TestTabContentsFactory* new_factory = + new TestTabContentsFactory( + new_type, "test" + IntToString(static_cast<int>(new_type))); + TabContents::RegisterFactory(new_type, new_factory); + return new_factory; + } + + TestTabContentsFactory(TabContentsType type, const std::string& scheme) + : type_(type), + scheme_(scheme) { + } + + ~TestTabContentsFactory() { + TabContents::RegisterFactory(type_, NULL); + } + + virtual TabContents* CreateInstance() { + return CreateInstanceImpl(); + } + + TestTabContents* CreateInstanceImpl() { + return new TestTabContents(type_); + } + + virtual bool CanHandleURL(const GURL& url) { + return url.SchemeIs(scheme_.c_str()); + } + + const std::string& scheme() const { return scheme_; } + + TabContentsType type() const { return type_; } + + private: + TabContentsType type_; + + const std::string scheme_; + + DISALLOW_COPY_AND_ASSIGN(TestTabContentsFactory); +}; + +#endif // CHROME_BROWSER_TEST_TEST_TAB_CONTENTS_H_ diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj index df63415..b63c26e 100644 --- a/chrome/test/unit/unittests.vcproj +++ b/chrome/test/unit/unittests.vcproj @@ -198,6 +198,14 @@ > </File> <File + RelativePath="..\test_tab_contents.cc" + > + </File> + <File + RelativePath="..\test_tab_contents.h" + > + </File> + <File RelativePath="..\..\..\net\url_request\url_request_test_job.cc" > </File> @@ -527,6 +535,14 @@ </File> </Filter> <Filter + Name="TestDownloadRequestManager" + > + <File + RelativePath="..\..\browser\download\download_request_manager_unittest.cc" + > + </File> + </Filter> + <Filter Name="TestTemplateURLParser" > <File diff --git a/chrome/views/message_box_view.h b/chrome/views/message_box_view.h index ad9a8fd..a15fa89 100644 --- a/chrome/views/message_box_view.h +++ b/chrome/views/message_box_view.h @@ -14,8 +14,9 @@ #include "chrome/views/text_field.h" #include "chrome/views/view.h" -// This class displays a message box within a constrained window -// with options for a message, a prompt, and OK and Cancel buttons. +// This class displays the contents of a message box. It is intended for use +// within a constrained window, and has options for a message, prompt, OK +// and Cancel buttons. class MessageBoxView : public views::View { public: // flags |