From 7ae1b97cae65107ca09124e98365e4de9ddec32d Mon Sep 17 00:00:00 2001 From: "jcampan@chromium.org" Date: Thu, 8 Jan 2009 00:13:45 +0000 Subject: Attempt at fixing bug 5800. When pressing back on an interstitial with a cross site navigation pending, we should cancel the pending without running the unload handler on the previous page. BUG=5800 TEST=See bug. Review URL: http://codereview.chromium.org/17061 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@7703 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/render_view_host_manager.cc | 7 +++ chrome/browser/render_view_host_manager.h | 4 ++ .../renderer_host/cross_site_resource_handler.cc | 46 +++++++++++++--- chrome/browser/ssl_uitest.cc | 62 +++++++++++++++++++++- chrome/browser/web_contents.h | 4 ++ 5 files changed, 115 insertions(+), 8 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/render_view_host_manager.cc b/chrome/browser/render_view_host_manager.cc index 5ad576b..c5e4d00 100644 --- a/chrome/browser/render_view_host_manager.cc +++ b/chrome/browser/render_view_host_manager.cc @@ -490,3 +490,10 @@ void RenderViewHostManager::CancelPendingRenderView() { pending_render_view_host_ = NULL; } +void RenderViewHostManager::CrossSiteNavigationCanceled() { + DCHECK(cross_navigation_pending_); + cross_navigation_pending_ = false; + if (pending_render_view_host_) + CancelPendingRenderView(); +} + diff --git a/chrome/browser/render_view_host_manager.h b/chrome/browser/render_view_host_manager.h index 40eabce..3c2f61d 100644 --- a/chrome/browser/render_view_host_manager.h +++ b/chrome/browser/render_view_host_manager.h @@ -120,6 +120,10 @@ class RenderViewHostManager { void OnCrossSiteResponse(int new_render_process_host_id, int new_request_id); + // Notifies that the navigation that initiated a cross-site transition has + // been canceled. + void CrossSiteNavigationCanceled(); + // Called when a provisional load on the given renderer is aborted. void RendererAbortedProvisionalLoad(RenderViewHost* render_view_host); diff --git a/chrome/browser/renderer_host/cross_site_resource_handler.cc b/chrome/browser/renderer_host/cross_site_resource_handler.cc index aaf579b..d597732 100644 --- a/chrome/browser/renderer_host/cross_site_resource_handler.cc +++ b/chrome/browser/renderer_host/cross_site_resource_handler.cc @@ -5,6 +5,8 @@ #include "chrome/browser/renderer_host/cross_site_resource_handler.h" #include "chrome/browser/render_view_host.h" +#include "chrome/browser/tab_util.h" +#include "chrome/browser/web_contents.h" namespace { // Task to notify the WebContents that a cross-site response has begun, so that @@ -35,6 +37,25 @@ class CrossSiteNotifyTabTask : public Task { int render_view_id_; int request_id_; }; + +class CancelPendingRenderViewTask : public Task { + public: + CancelPendingRenderViewTask(int render_process_host_id, + int render_view_id) + : render_process_host_id_(render_process_host_id), + render_view_id_(render_view_id) {} + + void Run() { + WebContents* web_contents = + tab_util::GetWebContentsByID(render_process_host_id_, render_view_id_); + if (web_contents) + web_contents->CrossSiteNavigationCanceled(); + } + + private: + int render_process_host_id_; + int render_view_id_; +}; } CrossSiteResourceHandler::CrossSiteResourceHandler( @@ -116,12 +137,25 @@ bool CrossSiteResourceHandler::OnResponseCompleted( return next_handler_->OnResponseCompleted(request_id, status); } else { // Some types of failures will call OnResponseCompleted without calling - // CrossSiteResourceHandler::OnResponseStarted. We should wait now for - // the cross-site transition. Also continue with the logic below to - // remember that we completed during the cross-site transition. - ResourceDispatcherHost::GlobalRequestID global_id(render_process_host_id_, - request_id); - StartCrossSiteTransition(request_id, NULL, global_id); + // CrossSiteResourceHandler::OnResponseStarted. + if (status.status() == URLRequestStatus::CANCELED) { + // Here the request was canceled, which happens when selecting "take me + // back" from an interstitial. Nothing to do but cancel the pending + // render view host. + CancelPendingRenderViewTask* task = + new CancelPendingRenderViewTask(render_process_host_id_, + render_view_id_); + rdh_->ui_loop()->PostTask(FROM_HERE, task); + return next_handler_->OnResponseCompleted(request_id, status); + } else { + // An error occured, we should wait now for the cross-site transition, + // so that the error message (e.g., 404) can be displayed to the user. + // Also continue with the logic below to remember that we completed + // during the cross-site transition. + ResourceDispatcherHost::GlobalRequestID global_id( + render_process_host_id_, request_id); + StartCrossSiteTransition(request_id, NULL, global_id); + } } } diff --git a/chrome/browser/ssl_uitest.cc b/chrome/browser/ssl_uitest.cc index ccef280..94fcf70 100644 --- a/chrome/browser/ssl_uitest.cc +++ b/chrome/browser/ssl_uitest.cc @@ -125,8 +125,8 @@ TEST_F(SSLUITest, TestOKHTTPS) { EXPECT_EQ(NavigationEntry::SSLStatus::NORMAL_CONTENT, mixed_content_state); } -// Visits a page with https error: -TEST_F(SSLUITest, TestHTTPSExpiredCert) { +// Visits a page with https error and proceed: +TEST_F(SSLUITest, TestHTTPSExpiredCertAndProceed) { scoped_ptr bad_https_server(BadCertServer()); scoped_ptr tab(GetActiveTabProxy()); NavigateTab(tab.get(), @@ -157,6 +157,64 @@ TEST_F(SSLUITest, TestHTTPSExpiredCert) { EXPECT_EQ(NavigationEntry::SSLStatus::NORMAL_CONTENT, mixed_content_state); } +// Visits a page with https error and don't proceed (and ensure we can still +// navigate at that point): +TEST_F(SSLUITest, TestHTTPSExpiredCertAndDontProceed) { + scoped_ptr http_server(PlainServer()); + scoped_ptr good_https_server(GoodCertServer()); + scoped_ptr bad_https_server(BadCertServer()); + scoped_ptr tab(GetActiveTabProxy()); + + // First navigate to an OK page. + NavigateTab(tab.get(), + good_https_server->TestServerPageW(L"files/ssl/google.html")); + + GURL cross_site_url = + bad_https_server->TestServerPageW(L"files/ssl/google.html"); + // Change the host name from 127.0.0.1 to localhost so it triggers a + // cross-site navigation so we can test http://crbug.com/5800 is gone. + ASSERT_EQ("127.0.0.1", cross_site_url.host()); + GURL::Replacements replacements; + std::string new_host("localhost"); + replacements.SetHostStr(new_host); + cross_site_url = cross_site_url.ReplaceComponents(replacements); + + // Now go to a bad HTTPS page. + NavigateTab(tab.get(), cross_site_url); + + // An interstitial should be showing. + NavigationEntry::PageType page_type; + EXPECT_TRUE(tab->GetPageType(&page_type)); + EXPECT_EQ(NavigationEntry::INTERSTITIAL_PAGE, page_type); + + SecurityStyle security_style; + int cert_status; + int mixed_content_state; + EXPECT_TRUE(tab->GetSecurityState(&security_style, &cert_status, + &mixed_content_state)); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, security_style); + EXPECT_EQ(NavigationEntry::SSLStatus::NORMAL_CONTENT, mixed_content_state); + + // Simulate user clicking "Take me back". + EXPECT_TRUE(tab->TakeActionOnSSLBlockingPage(false)); + + // We should be back to the original good page. + EXPECT_TRUE(tab->GetPageType(&page_type)); + EXPECT_EQ(NavigationEntry::NORMAL_PAGE, page_type); + EXPECT_TRUE(tab->GetSecurityState(&security_style, &cert_status, + &mixed_content_state)); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, security_style); + EXPECT_EQ(0, cert_status & net::CERT_STATUS_ALL_ERRORS); + EXPECT_EQ(NavigationEntry::SSLStatus::NORMAL_CONTENT, mixed_content_state); + + // Try to navigate to a new page. (to make sure bug 5800 is fixed). + NavigateTab(tab.get(), + http_server->TestServerPageW(L"files/ssl/google.html")); + EXPECT_TRUE(tab->GetSecurityState(&security_style, &cert_status, + &mixed_content_state)); + EXPECT_EQ(SECURITY_STYLE_UNAUTHENTICATED, security_style); +} + // // Mixed contents // diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h index b14d07a..0001b3f 100644 --- a/chrome/browser/web_contents.h +++ b/chrome/browser/web_contents.h @@ -199,6 +199,10 @@ class WebContents : public TabContents, render_view_host()->SetPageEncoding(encoding); } + void CrossSiteNavigationCanceled() { + render_manager_.CrossSiteNavigationCanceled(); + } + protected: // Should be deleted via CloseContents. virtual ~WebContents(); -- cgit v1.1