Fixes navigation issues with interstitial pages.

Updates NavigationController so that the back and forward menus can
be used while an interstitial page is showing.  Also fixes tab cloning
while an interstitial page is showing, so that the cloned tab does not
have the interstitial navigation entry.

BUG=37215
BUG=37894
TEST=SSLUITest.TestHTTPSExpiredCertAndGo{Back,Forward}
TEST=NavigationControllerTest.CloneOmitsInterstitials

Review URL: http://codereview.chromium.org/861001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41304 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 150 insertions, 12 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 1c75cbc..46672ec 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -777,17 +777,7 @@ void Browser::UpdateCommandsForFullscreenMode(bool is_fullscreen) {
 void Browser::GoBack(WindowOpenDisposition disposition) {
   UserMetrics::RecordAction("Back", profile_);
 
-  // If we are showing an interstitial, just hide it.
   TabContents* current_tab = GetSelectedTabContents();
-  if (current_tab->interstitial_page()) {
-    // The GoBack() case is a special case when an interstitial is shown because
-    // the "previous" page is still available, just hidden by the interstitial.
-    // We treat the back as a "Don't proceed", this hides the interstitial and
-    // reveals the previous page.
-    current_tab->interstitial_page()->DontProceed();
-    return;
-  }
-
   if (current_tab->controller().CanGoBack()) {
     NavigationController* controller = NULL;
     if (disposition == NEW_FOREGROUND_TAB ||
@@ -796,6 +786,11 @@ void Browser::GoBack(WindowOpenDisposition disposition) {
       tabstrip_model_.AddTabContents(cloned, -1, false,
                                      PageTransition::LINK,
                                      disposition == NEW_FOREGROUND_TAB);
+      if (current_tab->interstitial_page()) {
+        // The interstitial won't be copied to the new tab, so we don't need to
+        // go back.
+        return;
+      }
       controller = &cloned->controller();
     } else {
       // Default disposition is CURRENT_TAB.
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index bdaeb61..c21b481 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -220,6 +220,77 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndDontProceed) {
   CheckUnauthenticatedState(tab);
 }
 
+// Visits a page with https error and then goes back using GoToOffset.
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBack) {
+  scoped_refptr<HTTPTestServer> http_server = PlainServer();
+  ASSERT_TRUE(http_server.get() != NULL);
+  scoped_refptr<HTTPSTestServer> bad_https_server = BadCertServer();
+  ASSERT_TRUE(bad_https_server.get() != NULL);
+
+  // First navigate to an HTTP page.
+  ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW(
+      L"files/ssl/google.html"));
+  TabContents* tab = browser()->GetSelectedTabContents();
+  NavigationEntry* entry = tab->controller().GetActiveEntry();
+  ASSERT_TRUE(entry);
+
+  // Now go to a bad HTTPS page that shows an interstitial.
+  ui_test_utils::NavigateToURL(browser(),
+      bad_https_server->TestServerPageW(L"files/ssl/google.html"));
+  CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID,
+                                 true);  // Interstitial showing
+
+  // Simulate user clicking and holding on back button (crbug.com/37215).
+  tab->controller().GoToOffset(-1);
+
+  // We should be back at the original good page.
+  EXPECT_FALSE(browser()->GetSelectedTabContents()->interstitial_page());
+  CheckUnauthenticatedState(tab);
+}
+
+// Visits a page with https error and then goes forward using GoToOffset.
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoForward) {
+  scoped_refptr<HTTPTestServer> http_server = PlainServer();
+  ASSERT_TRUE(http_server.get() != NULL);
+  scoped_refptr<HTTPSTestServer> bad_https_server = BadCertServer();
+  ASSERT_TRUE(bad_https_server.get() != NULL);
+
+  // First navigate to two HTTP pages.
+  ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW(
+      L"files/ssl/google.html"));
+  TabContents* tab = browser()->GetSelectedTabContents();
+  NavigationEntry* entry1 = tab->controller().GetActiveEntry();
+  ASSERT_TRUE(entry1);
+  ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW(
+      L"files/ssl/blank_page.html"));
+  NavigationEntry* entry2 = tab->controller().GetActiveEntry();
+  ASSERT_TRUE(entry2);
+
+  // Now go back so that a page is in the forward history.
+  tab->controller().GoBack();
+  ui_test_utils::WaitForNavigation(&(tab->controller()));
+  ASSERT_TRUE(tab->controller().CanGoForward());
+  NavigationEntry* entry3 = tab->controller().GetActiveEntry();
+  ASSERT_TRUE(entry1 == entry3);
+
+  // Now go to a bad HTTPS page that shows an interstitial.
+  ui_test_utils::NavigateToURL(browser(),
+      bad_https_server->TestServerPageW(L"files/ssl/google.html"));
+  CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID,
+                                 true);  // Interstitial showing
+
+  // Simulate user clicking and holding on forward button.
+  tab->controller().GoToOffset(1);
+  ui_test_utils::WaitForNavigation(&(tab->controller()));
+
+  // We should be showing the second good page.
+  EXPECT_FALSE(browser()->GetSelectedTabContents()->interstitial_page());
+  CheckUnauthenticatedState(tab);
+  EXPECT_FALSE(tab->controller().CanGoForward());
+  NavigationEntry* entry4 = tab->controller().GetActiveEntry();
+  EXPECT_TRUE(entry2 == entry4);
+}
+
 // Open a page with a HTTPS error in a tab with no prior navigation (through a
 // link with a blank target).  This is to test that the lack of navigation entry
 // does not cause any problems (it was causing a crasher, see
diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc
index b468d11..555e655 100644
--- a/chrome/browser/tab_contents/interstitial_page.cc
+++ b/chrome/browser/tab_contents/interstitial_page.cc
@@ -463,6 +463,20 @@ void InterstitialPage::DontProceed() {
   // WARNING: we are now deleted!
 }
 
+void InterstitialPage::CancelForNavigation() {
+  // The user is trying to navigate away.  We should unblock the renderer and
+  // disable the interstitial, but keep it visible until the navigation
+  // completes.
+  Disable();
+  // If this interstitial was shown for a new navigation, allow any navigations
+  // on the original page to resume (e.g., subresource requests, XHRs, etc).
+  // Otherwise, cancel the pending, possibly dangerous navigations.
+  if (new_navigation_)
+    TakeActionOnResourceDispatcher(RESUME);
+  else
+    TakeActionOnResourceDispatcher(CANCEL);
+}
+
 void InterstitialPage::SetSize(const gfx::Size& size) {
 #if defined(OS_WIN) || defined(OS_LINUX)
   // When a tab is closed, we might be resized after our view was NULLed
diff --git a/chrome/browser/tab_contents/interstitial_page.h b/chrome/browser/tab_contents/interstitial_page.h
index ba3f41d..4db50e0 100644
--- a/chrome/browser/tab_contents/interstitial_page.h
+++ b/chrome/browser/tab_contents/interstitial_page.h
@@ -81,6 +81,11 @@ class InterstitialPage : public NotificationObserver,
   // Warning: 'this' has been deleted when this method returns.
   virtual void Proceed();
 
+  // Allows the user to navigate away by disabling the interstitial, canceling
+  // the pending request, and unblocking the hidden renderer.  The interstitial
+  // will stay visible until the navigation completes.
+  void CancelForNavigation();
+
   // Sizes the RenderViewHost showing the actual interstitial page contents.
   void SetSize(const gfx::Size& size);
 
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index 27d502a..85eaa1d 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/site_instance.h"
 #include "chrome/browser/sessions/session_types.h"
+#include "chrome/browser/tab_contents/interstitial_page.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
@@ -297,6 +298,13 @@ void NavigationController::GoBack() {
     return;
   }
 
+  // If an interstitial page is showing, going back is equivalent to hiding the
+  // interstitial.
+  if (tab_contents_->interstitial_page()) {
+    tab_contents_->interstitial_page()->DontProceed();
+    return;
+  }
+
   // Base the navigation on where we are now...
   int current_index = GetCurrentEntryIndex();
 
@@ -312,6 +320,14 @@ void NavigationController::GoForward() {
     return;
   }
 
+  // If an interstitial page is showing, the previous renderer is blocked and
+  // cannot make new requests.  Unblock (and disable) it to allow this
+  // navigation to succeed.  The interstitial will stay visible until the
+  // resulting DidNavigate.
+  if (tab_contents_->interstitial_page()) {
+    tab_contents_->interstitial_page()->CancelForNavigation();
+  }
+
   bool transient = (transient_entry_index_ != -1);
 
   // Base the navigation on where we are now...
@@ -345,6 +361,21 @@ void NavigationController::GoToIndex(int index) {
     }
   }
 
+  // If an interstitial page is showing, the previous renderer is blocked and
+  // cannot make new requests.
+  if (tab_contents_->interstitial_page()) {
+    if (index == GetCurrentEntryIndex() - 1) {
+      // Going back one entry is equivalent to hiding the interstitial.
+      tab_contents_->interstitial_page()->DontProceed();
+      return;
+    } else {
+      // Unblock the renderer (and disable the interstitial) to allow this
+      // navigation to succeed.  The interstitial will stay visible until the
+      // resulting DidNavigate.
+      tab_contents_->interstitial_page()->CancelForNavigation();
+    }
+  }
+
   DiscardNonCommittedEntries();
 
   pending_entry_index_ = index;
@@ -886,8 +917,12 @@ void NavigationController::CopyStateFrom(const NavigationController& source) {
 
   needs_reload_ = true;
   for (int i = 0; i < source.entry_count(); i++) {
-    entries_.push_back(linked_ptr<NavigationEntry>(
-        new NavigationEntry(*source.entries_[i])));
+    // When cloning a tab, copy all entries except interstitial pages
+    if (source.entries_[i].get()->page_type() !=
+        NavigationEntry::INTERSTITIAL_PAGE) {
+      entries_.push_back(linked_ptr<NavigationEntry>(
+          new NavigationEntry(*source.entries_[i])));
+    }
   }
 
   session_storage_namespace_id_ =
diff --git a/chrome/browser/tab_contents/navigation_controller_unittest.cc b/chrome/browser/tab_contents/navigation_controller_unittest.cc
index c0d75c2..f880eed 100644
--- a/chrome/browser/tab_contents/navigation_controller_unittest.cc
+++ b/chrome/browser/tab_contents/navigation_controller_unittest.cc
@@ -1546,6 +1546,24 @@ TEST_F(NavigationControllerTest, CloneAndGoBack) {
   EXPECT_FALSE(clone->controller().needs_reload());
 }
 
+// Make sure that cloning a tabcontents doesn't copy interstitials.
+TEST_F(NavigationControllerTest, CloneOmitsInterstitials) {
+  const GURL url1("http://foo1");
+  const GURL url2("http://foo2");
+
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+
+  // Add an interstitial entry.  Should be deleted with controller.
+  NavigationEntry* interstitial_entry = new NavigationEntry();
+  interstitial_entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE);
+  controller().AddTransientEntry(interstitial_entry);
+
+  scoped_ptr<TabContents> clone(controller().tab_contents()->Clone());
+
+  ASSERT_EQ(2, clone->controller().entry_count());
+}
+
 /* TODO(brettw) These test pass on my local machine but fail on the XP buildbot
    (but not Vista) cleaning up the directory after they run.
    This should be fixed.