The URL in the bug triggers an SSL interstitial for a redirect that redirects to a bad SSL page.

This happens because:
- when proceeding on an interstitial, we wait for the navigation to commit before hiding the current interstitial
- when showing an interstitial, if an interstitial is already showing we call DontProceed on it

In the case of this bug, the interstitial has already been approved when the showing of the 2nd interstitial calls DontProceed on it, causing bad things to happen.

The fix is easy: we shouldn't call DontProceed on an interstitial already "Proceeded".


BUG=9286
TEST=See bug.
Review URL: http://codereview.chromium.org/56019

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

3 files changed, 64 insertions, 2 deletions

diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc
index 0b5d818..32fe0b6 100644
--- a/chrome/browser/tab_contents/interstitial_page.cc
+++ b/chrome/browser/tab_contents/interstitial_page.cc
@@ -133,8 +133,13 @@ InterstitialPage::~InterstitialPage() {
 
 void InterstitialPage::Show() {
   // If an interstitial is already showing, close it before showing the new one.
-  if (tab_->interstitial_page())
-    tab_->interstitial_page()->DontProceed();
+  // Be careful not to take an action on the old interstitial more than once.
+  if (tab_->interstitial_page()) {
+    if (tab_->interstitial_page()->action_taken())
+      tab_->interstitial_page()->Hide();
+    else
+      tab_->interstitial_page()->DontProceed();
+  }
 
   // Block the resource requests for the render view host while it is hidden.
   TakeActionOnResourceDispatcher(BLOCK);
diff --git a/chrome/browser/tab_contents/interstitial_page.h b/chrome/browser/tab_contents/interstitial_page.h
index 0c9049c..3f5ceca 100644
--- a/chrome/browser/tab_contents/interstitial_page.h
+++ b/chrome/browser/tab_contents/interstitial_page.h
@@ -74,6 +74,8 @@ class InterstitialPage : public NotificationObserver,
   // Sizes the RenderViewHost showing the actual interstitial page contents.
   void SetSize(const gfx::Size& size);
 
+  bool action_taken() const { return action_taken_; }
+
  protected:
   // NotificationObserver method:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc
index 85458cb..8dab8c6 100644
--- a/chrome/browser/tab_contents/web_contents_unittest.cc
+++ b/chrome/browser/tab_contents/web_contents_unittest.cc
@@ -991,6 +991,61 @@ TEST_F(WebContentsTest, ShowInterstitialOnInterstitial) {
   EXPECT_EQ(2, controller()->GetEntryCount());
 }
 
+// Test showing an interstitial, proceeding and then navigating to another
+// interstitial.
+TEST_F(WebContentsTest, ShowInterstitialProceedShowInterstitial) {
+  // Navigate to a page so we have a navigation entry in the controller.
+  GURL start_url("http://www.google.com");
+  rvh()->SendNavigate(1, start_url);
+  EXPECT_EQ(1, controller()->GetEntryCount());
+
+  // Show an interstitial.
+  TestInterstitialPage::InterstitialState state1 =
+      TestInterstitialPage::UNDECIDED;
+  bool deleted1 = false;
+  GURL url1("http://interstitial1");
+  TestInterstitialPage* interstitial1 =
+      new TestInterstitialPage(contents(), true, url1, &state1, &deleted1);
+  TestInterstitialPageStateGuard state_guard1(interstitial1);
+  interstitial1->Show();
+  interstitial1->TestDidNavigate(1, url1);
+
+  // Take action.  The interstitial won't be hidden until the navigation is
+  // committed.
+  interstitial1->Proceed();
+  EXPECT_EQ(TestInterstitialPage::OKED, state1);
+
+  // Now show another interstitial (simulating the navigation causing another
+  // interstitial).
+  TestInterstitialPage::InterstitialState state2 =
+      TestInterstitialPage::UNDECIDED;
+  bool deleted2 = false;
+  GURL url2("http://interstitial2");
+  TestInterstitialPage* interstitial2 =
+      new TestInterstitialPage(contents(), true, url2, &state2, &deleted2);
+  TestInterstitialPageStateGuard state_guard2(interstitial2);
+  interstitial2->Show();
+  interstitial2->TestDidNavigate(1, url2);
+
+  // Showing interstitial2 should have caused interstitial1 to go away.
+  EXPECT_TRUE(deleted1);
+
+  // Let's make sure interstitial2 is working as intended.
+  ASSERT_FALSE(deleted2);
+  EXPECT_EQ(TestInterstitialPage::UNDECIDED, state2);
+  interstitial2->Proceed();
+  GURL landing_url("http://www.thepage.com");
+  rvh()->SendNavigate(2, landing_url);
+
+  EXPECT_TRUE(deleted2);
+  EXPECT_FALSE(contents()->showing_interstitial_page());
+  EXPECT_TRUE(contents()->interstitial_page() == NULL);
+  NavigationEntry* entry = controller()->GetActiveEntry();
+  ASSERT_TRUE(entry != NULL);
+  EXPECT_TRUE(entry->url() == landing_url);
+  EXPECT_EQ(2, controller()->GetEntryCount());
+}
+
 // Test that navigating away from an interstitial while it's loading cause it
 // not to show.
 TEST_F(WebContentsTest, NavigateBeforeInterstitialShows) {