Fix SB main frame/subresource interstitial interaction bug

This fixes a bug that caused commands to be non-functional on a
subresource triggered SB committed interstitial if a different type of
interstitial had been shown on the same webcontents previously. The bug
was triggered depending on the order different WebContentsObservers
were installed (which determines the order DidFinishNavigation is
called in them). This CL changes the observer that needs to fire first
so it uses ReadyToCommitNavigationInstead.

Bug: 1021334
Change-Id: I3e6683279523dc6b74a2266fdd2bf05e0ab4d132
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896346
Auto-Submit: Carlos IL <carlosil@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712796}

chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

@@ -1890,6 +1890,31 @@ IN_PROC_BROWSER_TEST_P(
       browser()->tab_strip_model()->GetActiveWebContents()));
 }
 
+// Tests that commands work in a subframe triggered interstitial if a different
+// interstitial has been shown previously on the same webcontents. Regression
+// test for crbug.com/1021334
+IN_PROC_BROWSER_TEST_P(
+    SafeBrowsingBlockingPageBrowserTestWithCommittedSBInterstitials,
+    IframeProceedAfterMainFrameInterstitial) {
+  // Navigate to a site that triggers an interstitial due to a bad main frame
+  // URL.
+  ui_test_utils::NavigateToURL(browser(),
+                               GURL(kChromeUISafeBrowsingMatchMalwareUrl));
+  EXPECT_TRUE(WaitForReady(browser()));
+  EXPECT_TRUE(ClickAndWaitForDetach("primary-button"));
+  AssertNoInterstitial(false);
+
+  // Navigate to a site that triggers an interstitial due to a bad iframe.
+  GURL url = SetupThreatIframeWarningAndNavigate();
+
+  // Commands should work.
+  EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
+  AssertNoInterstitial(true);  // Assert the interstitial is gone
+
+  EXPECT_EQ(url,
+            browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
+}
+
 INSTANTIATE_TEST_SUITE_P(
     SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting,
     SafeBrowsingBlockingPageBrowserTestWithCommittedSBInterstitials,

chrome/browser/safe_browsing/safe_browsing_subresource_tab_helper.cc

@@ -18,7 +18,7 @@ namespace safe_browsing {
 
 SafeBrowsingSubresourceTabHelper::~SafeBrowsingSubresourceTabHelper() {}
 
-void SafeBrowsingSubresourceTabHelper::DidFinishNavigation(
+void SafeBrowsingSubresourceTabHelper::ReadyToCommitNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->GetNetErrorCode() == net::ERR_BLOCKED_BY_CLIENT) {
     safe_browsing::SafeBrowsingService* service =

chrome/browser/safe_browsing/safe_browsing_subresource_tab_helper.h

@@ -22,7 +22,7 @@ class SafeBrowsingSubresourceTabHelper
   ~SafeBrowsingSubresourceTabHelper() override;
 
   // WebContentsObserver::
-  void DidFinishNavigation(
+  void ReadyToCommitNavigation(
       content::NavigationHandle* navigation_handle) override;
 
  private: