If replacing a safebrowsing interstitial, don't call DontProceed on the old one.

chrome/browser/safe_browsing/safe_browsing_blocking_page.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Implementation of the SafeBrowsingBlockingPage class.
 
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 
 #include <string>
 
@@ skipping 436 matching lines @@
                                           main_frame_url, resources);
 }
 
 // static
 void SafeBrowsingBlockingPage::ShowBlockingPage(
     SafeBrowsingUIManager* ui_manager,
     const UnsafeResource& unsafe_resource) {
   DVLOG(1) << __func__ << " " << unsafe_resource.url.spec();
   WebContents* web_contents = unsafe_resource.web_contents_getter.Run();
 
-  InterstitialPage* interstitial =
-      InterstitialPage::GetInterstitialPage(web_contents);
-  if (interstitial && !unsafe_resource.is_subresource) {
-    // 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.
+  if (!InterstitialPage::GetInterstitialPage(web_contents) ||
+      !unsafe_resource.is_subresource) {
+    // There is no interstitial currently showing in that tab, or we are about
+    // to display a new one for the main frame. If there is already an
+    // interstitial, showing the new one will automatically hide the old one.

[Nathan Parker, 2016/11/17 20:08:24]

What causes the old one to get hidden?  And do we want a new one to show at all?
 It seems weird to have one show, then have another replace it.

[mattm, 2016/11/17 21:22:09]

https://cs.chromium.org/chromium/src/content/browser/frame_host/interstitial_...
I guess for the reload case, but this is the same path that gets hit if you see
a warning, type a different url into the omnibox, and then get a warning for
that page. Seems easier to just always display the new one than to add extra
code to try to figure out if it's the same as the old one.

     content::NavigationEntry* entry =
         unsafe_resource.GetNavigationEntryForResource();
     SafeBrowsingBlockingPage* blocking_page =
         CreateBlockingPage(ui_manager, web_contents,
                            entry ? entry->GetURL() : GURL(), unsafe_resource);
     blocking_page->Show();
     return;
   }
 
   // This is an interstitial for a page's resource, let's queue it.

[Nathan Parker, 2016/11/17 20:08:24]

note: felt mentioned that we no longer queue interstitials and show a sequence
of them, that this is effectively dead code. I haven't looked through this
recently to see what parts of it are dead.

[mattm, 2016/11/17 21:22:09]

That's doesn't seem to be entirely true. I think what was actually killed was
the multi-threat interstitial. This code is still active. So it will actually
queue up the resources and show a second interstitial, it'll just be another
regular interstitial instead of a multi-threat interstitial. (I just verified
this.) I don't know if this was the intended result or if this should be killed
too...

   UnsafeResourceMap* unsafe_resource_map = GetUnsafeResourcesMap();
   (*unsafe_resource_map)[web_contents].push_back(unsafe_resource);
 }
 
 // static
 bool SafeBrowsingBlockingPage::IsMainPageLoadBlocked(
     const UnsafeResourceList& unsafe_resources) {
   // If there is more than one unsafe resource, the main page load must not be
   // blocked. Otherwise, check if the one resource is.
   return unsafe_resources.size() == 1 &&
@@ skipping 273 matching lines @@
       l10n_util::GetStringFUTF16(IDS_PHISHING_V4_EXPLANATION_PARAGRAPH,
                                  GetFormattedHostName()));
   load_time_data->SetString(
       "finalParagraph",
       l10n_util::GetStringUTF16(IDS_PHISHING_V4_PROCEED_AND_REPORT_PARAGRAPH));
 
   PopulateExtendedReportingOption(load_time_data);
 }
 
 }  // namespace safe_browsing