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

5 files changed, 115 insertions, 8 deletions

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<HTTPSTestServer> bad_https_server(BadCertServer());
   scoped_ptr<TabProxy> 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<TestServer> http_server(PlainServer());
+  scoped_ptr<HTTPSTestServer> good_https_server(GoodCertServer());
+  scoped_ptr<HTTPSTestServer> bad_https_server(BadCertServer());
+  scoped_ptr<TabProxy> 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();