Fixing a bug with interstitial pages triggered with malware.

BUG=http://crbug.com/17627
TEST=See bug.

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

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

2 files changed, 58 insertions, 4 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index 3a963c3..b66df16 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -461,7 +461,18 @@ void SafeBrowsingBlockingPage::ShowBlockingPage(
   TabContents* tab_contents = tab_util::GetTabContentsByID(
       unsafe_resource.render_process_host_id, unsafe_resource.render_view_id);
 
-  if (!InterstitialPage::GetInterstitialPage(tab_contents)) {
+  InterstitialPage* interstitial =
+      InterstitialPage::GetInterstitialPage(tab_contents);
+  if (interstitial &&
+      unsafe_resource.resource_type == ResourceType::MAIN_FRAME) {
+    // There is already an interstitial showing and we are about to display a
+    // new one for the main frame. Just hide the current one, it is now
+    // irrelevent
+    interstitial->DontProceed();
+    interstitial = NULL;
+  }
+
+  if (!interstitial) {
     // There are no interstitial currently showing in that tab, go ahead and
     // show this interstitial.
     std::vector<SafeBrowsingService::UnsafeResource> resources;
@@ -476,9 +487,7 @@ void SafeBrowsingBlockingPage::ShowBlockingPage(
     return;
   }
 
-  // Let's queue the interstitial.
-  // Note we only expect resources from the page at this point.
-  DCHECK(unsafe_resource.resource_type != ResourceType::MAIN_FRAME);
+  // This is an interstitial for a page's resource, let's queue it.
   UnsafeResourceMap* unsafe_resource_map = GetUnsafeResourcesMap();
   (*unsafe_resource_map)[tab_contents].push_back(unsafe_resource);
 }
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
index e0a0f77..de0ba2d 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
@@ -102,6 +102,13 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness,
     contents()->TestDidNavigate(contents_->render_view_host(), params);
   }
 
+  void GoBack() {
+    NavigationEntry* entry = contents()->controller().GetEntryAtOffset(-1);
+    ASSERT_TRUE(entry);
+    contents()->controller().GoBack();
+    Navigate(entry->url().spec().c_str(), entry->page_id());
+  }
+
   void ShowInterstitial(ResourceType::Type resource_type,
                         const char* url) {
     SafeBrowsingService::UnsafeResource resource;
@@ -355,3 +362,41 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMultipleMalwareResourceProceed) {
   ASSERT_EQ(1, controller().entry_count());
   EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec());
 }
+
+// Tests showing a blocking page then navigating back and forth to make sure the
+// controller entries are OK.  http://crbug.com/17627
+TEST_F(SafeBrowsingBlockingPageTest, NavigatingBackAndForth) {
+  // Navigate somewhere.
+  Navigate(kGoodURL, 1);
+
+  // Now navigate to a bad page triggerring an interstitial.
+  controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
+  ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
+  SafeBrowsingBlockingPage* sb_interstitial = GetSafeBrowsingBlockingPage();
+  ASSERT_TRUE(sb_interstitial);
+
+  // Proceed, then navigate back.
+  ProceedThroughInterstitial(sb_interstitial);
+  Navigate(kBadURL, 2);  // Commit the navigation.
+  GoBack();
+
+  // We are back on the good page.
+  sb_interstitial = GetSafeBrowsingBlockingPage();
+  ASSERT_FALSE(sb_interstitial);
+  ASSERT_EQ(2, controller().entry_count());
+  EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec());
+
+  // Navigate forward to the malware URL.
+  contents()->controller().GoForward();
+  ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
+  sb_interstitial = GetSafeBrowsingBlockingPage();
+  ASSERT_TRUE(sb_interstitial);
+
+  // Let's proceed and make sure everything is OK (bug 17627).
+  ProceedThroughInterstitial(sb_interstitial);
+  Navigate(kBadURL, 2);  // Commit the navigation.
+  sb_interstitial = GetSafeBrowsingBlockingPage();
+  ASSERT_FALSE(sb_interstitial);
+  ASSERT_EQ(2, controller().entry_count());
+  EXPECT_EQ(kBadURL, controller().GetActiveEntry()->url().spec());
+}