Fixes for Safe Browsing with unrelated pending navigations.

chrome/browser/safe_browsing/ui_manager_unittest.cc

Index: chrome/browser/safe_browsing/ui_manager_unittest.cc
diff --git a/chrome/browser/safe_browsing/ui_manager_unittest.cc b/chrome/browser/safe_browsing/ui_manager_unittest.cc
index 8b10dc76b9678d3462fa9041b6e057cffd15929c..145133efda2ce82385d2d0c6b3e07a9602107b14 100644
--- a/chrome/browser/safe_browsing/ui_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/ui_manager_unittest.cc
@@ -22,6 +22,8 @@ using content::BrowserThread;
 static const char* kGoodURL = "https://www.good.com";
 static const char* kBadURL = "https://www.malware.com";
 static const char* kBadURLWithPath = "https://www.malware.com/index.html";
+static const char* kAnotherBadURL = "https://www.badware.com";
+static const char* kLandingURL = "https://www.landing.com";
 
 namespace safe_browsing {
 
@@ -79,9 +81,12 @@ class SafeBrowsingUIManagerTest : public ChromeRenderViewHostTestHarness {
     ui_manager_->AddToWhitelist(resource);
   }
 
-  SafeBrowsingUIManager::UnsafeResource MakeUnsafeResource(const char* url) {
+  SafeBrowsingUIManager::UnsafeResource MakeUnsafeResource(
+      const char* url,
+      bool is_subresource) {
     SafeBrowsingUIManager::UnsafeResource resource;
     resource.url = GURL(url);
+    resource.is_subresource = is_subresource;
     resource.render_process_host_id =
         web_contents()->GetRenderProcessHost()->GetID();
     resource.render_view_id =
@@ -90,14 +95,14 @@ class SafeBrowsingUIManagerTest : public ChromeRenderViewHostTestHarness {
     return resource;
   }
 
-  SafeBrowsingUIManager::UnsafeResource MakeUnsafeResourceAndNavigate(
+  SafeBrowsingUIManager::UnsafeResource MakeUnsafeResourceAndStartNavigation(
       const char* url) {
-    SafeBrowsingUIManager::UnsafeResource resource = MakeUnsafeResource(url);
+    SafeBrowsingUIManager::UnsafeResource resource =
+        MakeUnsafeResource(url, false /* is_subresource */);
 
-    // The WC doesn't have a URL without a navigation. Normally the
-    // interstitial would provide this instead of a fully committed navigation.
-    EXPECT_FALSE(IsWhitelisted(resource));
-    NavigateAndCommit(GURL(url));
+    // The WC doesn't have a URL without a navigation. A main-frame malware
+    // unsafe resource must be a pending navigation.
+    content::WebContentsTester::For(web_contents())->StartNavigation(GURL(url));
     return resource;
   }
 
@@ -113,43 +118,89 @@ class SafeBrowsingUIManagerTest : public ChromeRenderViewHostTestHarness {
 
 TEST_F(SafeBrowsingUIManagerTest, Whitelist) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   AddToWhitelist(resource);
   EXPECT_TRUE(IsWhitelisted(resource));
 }
 
 TEST_F(SafeBrowsingUIManagerTest, WhitelistIgnoresSitesNotAdded) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kGoodURL);
+      MakeUnsafeResourceAndStartNavigation(kGoodURL);
   EXPECT_FALSE(IsWhitelisted(resource));
 }
 
 TEST_F(SafeBrowsingUIManagerTest, WhitelistIgnoresPath) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   AddToWhitelist(resource);
   EXPECT_TRUE(IsWhitelisted(resource));
 
+  content::WebContentsTester::For(web_contents())->CommitPendingNavigation();
+
   SafeBrowsingUIManager::UnsafeResource resource_path =
-      MakeUnsafeResource(kBadURLWithPath);
+      MakeUnsafeResourceAndStartNavigation(kBadURLWithPath);
   EXPECT_TRUE(IsWhitelisted(resource_path));
 }
 
 TEST_F(SafeBrowsingUIManagerTest, WhitelistIgnoresThreatType) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   AddToWhitelist(resource);
   EXPECT_TRUE(IsWhitelisted(resource));
 
   SafeBrowsingUIManager::UnsafeResource resource_phishing =
-      MakeUnsafeResource(kBadURL);
+      MakeUnsafeResource(kBadURL, false /* is_subresource */);
   resource_phishing.threat_type = SB_THREAT_TYPE_URL_PHISHING;
   EXPECT_TRUE(IsWhitelisted(resource_phishing));
 }
 
+TEST_F(SafeBrowsingUIManagerTest, WhitelistWithUnrelatedPendingLoad) {
+  // Commit load of landing page.
+  NavigateAndCommit(GURL(kLandingURL));
+  {
+    // Simulate subresource malware hit on the landing page.
+    SafeBrowsingUIManager::UnsafeResource resource =
+        MakeUnsafeResource(kBadURL, true /* is_subresource */);
+
+    // Start pending load to unrelated site.
+    content::WebContentsTester::For(web_contents())
+        ->StartNavigation(GURL(kGoodURL));
+
+    // Whitelist the resource on the landing page.
+    AddToWhitelist(resource);
+    EXPECT_TRUE(IsWhitelisted(resource));
+  }
+
+  // Commit the pending load of unrelated site.
+  content::WebContentsTester::For(web_contents())->CommitPendingNavigation();
+  {
+    // The unrelated site is not on the whitelist, even if the same subresource
+    // was on it.
+    SafeBrowsingUIManager::UnsafeResource resource =
+        MakeUnsafeResource(kBadURL, true /* is_subresource */);
+    EXPECT_FALSE(IsWhitelisted(resource));
+  }
+
+  // Navigate back to the original landing url.
+  NavigateAndCommit(GURL(kLandingURL));
+  {
+    SafeBrowsingUIManager::UnsafeResource resource =
+        MakeUnsafeResource(kBadURL, true /* is_subresource */);
+    // Original resource url is whitelisted.
+    EXPECT_TRUE(IsWhitelisted(resource));
+  }
+  {
+    // A different malware subresource on the same page is also whitelisted.
+    // (The whitelist is by the page url, not the resource url.)
+    SafeBrowsingUIManager::UnsafeResource resource2 =
+        MakeUnsafeResource(kAnotherBadURL, true /* is_subresource */);
+    EXPECT_TRUE(IsWhitelisted(resource2));
+  }
+}
+
 TEST_F(SafeBrowsingUIManagerTest, UICallbackProceed) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   SafeBrowsingCallbackWaiter waiter;
   resource.callback =
       base::Bind(&SafeBrowsingCallbackWaiter::OnBlockingPageDone,
@@ -167,7 +218,7 @@ TEST_F(SafeBrowsingUIManagerTest, UICallbackProceed) {
 
 TEST_F(SafeBrowsingUIManagerTest, UICallbackDontProceed) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   SafeBrowsingCallbackWaiter waiter;
   resource.callback =
       base::Bind(&SafeBrowsingCallbackWaiter::OnBlockingPageDone,
@@ -185,7 +236,7 @@ TEST_F(SafeBrowsingUIManagerTest, UICallbackDontProceed) {
 
 TEST_F(SafeBrowsingUIManagerTest, IOCallbackProceed) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   SafeBrowsingCallbackWaiter waiter;
   resource.callback =
       base::Bind(&SafeBrowsingCallbackWaiter::OnBlockingPageDoneOnIO,
@@ -203,7 +254,7 @@ TEST_F(SafeBrowsingUIManagerTest, IOCallbackProceed) {
 
 TEST_F(SafeBrowsingUIManagerTest, IOCallbackDontProceed) {
   SafeBrowsingUIManager::UnsafeResource resource =
-      MakeUnsafeResourceAndNavigate(kBadURL);
+      MakeUnsafeResourceAndStartNavigation(kBadURL);
   SafeBrowsingCallbackWaiter waiter;
   resource.callback =
       base::Bind(&SafeBrowsingCallbackWaiter::OnBlockingPageDoneOnIO,