Fixes for Safe Browsing with unrelated pending navigations.

chrome/browser/safe_browsing/threat_details_unittest.cc

Index: chrome/browser/safe_browsing/threat_details_unittest.cc
diff --git a/chrome/browser/safe_browsing/threat_details_unittest.cc b/chrome/browser/safe_browsing/threat_details_unittest.cc
index fa77f4a122ba7de366fa33f7bc6c47c1bbd7a208..e40fec5b3ed26a0a0dcce2aa6cc2d30dfadc56ff 100644
--- a/chrome/browser/safe_browsing/threat_details_unittest.cc
+++ b/chrome/browser/safe_browsing/threat_details_unittest.cc
@@ -22,6 +22,7 @@
 #include "components/history/core/browser/history_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/web_contents_tester.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
@@ -331,14 +332,18 @@ class ThreatDetailsTest : public ChromeRenderViewHostTestHarness {
 
 // Tests creating a simple threat report of a malware URL.
 TEST_F(ThreatDetailsTest, ThreatSubResource) {
-  // Start a load.
-  controller().LoadURL(
-      GURL(kLandingURL),
-      content::Referrer(GURL(kReferrerURL), blink::WebReferrerPolicyDefault),
-      ui::PAGE_TRANSITION_TYPED, std::string());
+  // Commit a load.
+  content::WebContentsTester::For(web_contents())
+      ->TestDidNavigateWithReferrer(
+          web_contents()->GetMainFrame(), 1 /* page_id */, 0 /* nav_entry_id */,
+          true /* did_create_new_entry */, GURL(kLandingURL),
+          content::Referrer(GURL(kReferrerURL),
+                            blink::WebReferrerPolicyDefault),
+          ui::PAGE_TRANSITION_TYPED);
 
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true, GURL(kThreatURL));
+  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true /* is_subresource */,
+               GURL(kThreatURL));
 
   scoped_refptr<ThreatDetailsWrap> report =
       new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource, NULL);
@@ -376,11 +381,12 @@ TEST_F(ThreatDetailsTest, ThreatSubResource) {
 // Tests creating a simple threat report of a phishing page where the
 // subresource has a different original_url.
 TEST_F(ThreatDetailsTest, ThreatSubResourceWithOriginalUrl) {
-  controller().LoadURL(GURL(kLandingURL), content::Referrer(),
-                       ui::PAGE_TRANSITION_TYPED, std::string());
+  content::WebContentsTester::For(web_contents())
+      ->NavigateAndCommit(GURL(kLandingURL));
 
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_URL_PHISHING, true, GURL(kThreatURL));
+  InitResource(&resource, SB_THREAT_TYPE_URL_PHISHING,
+               true /* is_subresource */, GURL(kThreatURL));
   resource.original_url = GURL(kOriginalLandingURL);
 
   scoped_refptr<ThreatDetailsWrap> report =
@@ -421,11 +427,12 @@ TEST_F(ThreatDetailsTest, ThreatSubResourceWithOriginalUrl) {
 
 // Tests creating a threat report of a UwS page with data from the renderer.
 TEST_F(ThreatDetailsTest, ThreatDOMDetails) {
-  controller().LoadURL(GURL(kLandingURL), content::Referrer(),
-                       ui::PAGE_TRANSITION_TYPED, std::string());
+  content::WebContentsTester::For(web_contents())
+      ->NavigateAndCommit(GURL(kLandingURL));
 
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_URL_UNWANTED, true, GURL(kThreatURL));
+  InitResource(&resource, SB_THREAT_TYPE_URL_UNWANTED,
+               true /* is_subresource */, GURL(kThreatURL));
 
   scoped_refptr<ThreatDetailsWrap> report =
       new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource, NULL);
@@ -482,11 +489,12 @@ TEST_F(ThreatDetailsTest, ThreatDOMDetails) {
 // Tests creating a threat report of a malware page where there are redirect
 // urls to an unsafe resource url.
 TEST_F(ThreatDetailsTest, ThreatWithRedirectUrl) {
-  controller().LoadURL(GURL(kLandingURL), content::Referrer(),
-                       ui::PAGE_TRANSITION_TYPED, std::string());
+  content::WebContentsTester::For(web_contents())
+      ->NavigateAndCommit(GURL(kLandingURL));
 
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true, GURL(kThreatURL));
+  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true /* is_subresource */,
+               GURL(kThreatURL));
   resource.original_url = GURL(kOriginalLandingURL);
 
   // add some redirect urls
@@ -537,15 +545,135 @@ TEST_F(ThreatDetailsTest, ThreatWithRedirectUrl) {
   VerifyResults(actual, expected);
 }
 
-// Tests the interaction with the HTTP cache.
-TEST_F(ThreatDetailsTest, HTTPCache) {
-  controller().LoadURL(GURL(kLandingURL), content::Referrer(),
+// Test collecting threat details for a blocked main frame load.
+TEST_F(ThreatDetailsTest, ThreatOnMainPageLoadBlocked) {
+  const char* kUnrelatedReferrerURL = "http://www.unrelatedreferrer.com/some/path";

[Charlie Reis, 2015/12/11 05:39:24]

nit: 80 chars

[mattm, 2015/12/15 01:42:25]

Done.

+  const char* kUnrelatedURL = "http://www.unrelated.com/some/path";
+
+  // Load and commit an unrelated URL. The ThreatDetails should not use this
+  // navigation entry.
+  content::WebContentsTester::For(web_contents())
+      ->TestDidNavigateWithReferrer(
+          web_contents()->GetMainFrame(), 1 /* page_id */, 0 /* nav_entry_id */,
+          true /* did_create_new_entry */, GURL(kUnrelatedURL),
+          content::Referrer(GURL(kUnrelatedReferrerURL),
+                            blink::WebReferrerPolicyDefault),
+          ui::PAGE_TRANSITION_TYPED);
+
+  // Start a pending load with a referrer.
+  controller().LoadURL(GURL(kLandingURL),
+                       content::Referrer(GURL(kReferrerURL),
+                                         blink::WebReferrerPolicyDefault),
                        ui::PAGE_TRANSITION_TYPED, std::string());
 
+  // Create UnsafeResource for the pending main page load.
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL, true,
+  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE,
+               false /* is_subresource */, GURL(kLandingURL));
+
+  // Start ThreatDetails collection.
+  scoped_refptr<ThreatDetailsWrap> report =
+      new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource, NULL);
+
+  // Simulate clicking don't proceed.
+  controller().DiscardNonCommittedEntries();
+
+  // Finish ThreatDetails collection.
+  std::string serialized = WaitForSerializedReport(
+      report.get(), false /* did_proceed*/, 1 /* num_visit */);
+
+  ClientSafeBrowsingReportRequest actual;
+  actual.ParseFromString(serialized);
+
+  ClientSafeBrowsingReportRequest expected;
+  expected.set_type(ClientSafeBrowsingReportRequest::URL_MALWARE);
+  expected.set_url(kLandingURL);
+  expected.set_page_url(kLandingURL);
+  // Note that the referrer policy is not actually enacted here, since that's
+  // done in Blink.
+  expected.set_referrer_url(kReferrerURL);
+  expected.set_did_proceed(false);
+  expected.set_repeat_visit(true);
+
+  ClientSafeBrowsingReportRequest::Resource* pb_resource =
+      expected.add_resources();
+  pb_resource->set_id(0);
+  pb_resource->set_url(kLandingURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(1);
+  pb_resource->set_url(kReferrerURL);
+
+  VerifyResults(actual, expected);
+}
+
+// Tests that a pending load does not interfere with collecting threat details
+// for the committed page.
+TEST_F(ThreatDetailsTest, ThreatWithPendingLoad) {
+  const char* kPendingReferrerURL = "http://www.pendingreferrer.com/some/path";
+  const char* kPendingURL = "http://www.pending.com/some/path";
+
+  // Load and commit the landing URL with a referrer.
+  content::WebContentsTester::For(web_contents())
+      ->TestDidNavigateWithReferrer(
+          web_contents()->GetMainFrame(), 1 /* page_id */, 0 /* nav_entry_id */,
+          true /* did_create_new_entry */, GURL(kLandingURL),
+          content::Referrer(GURL(kReferrerURL),
+                            blink::WebReferrerPolicyDefault),
+          ui::PAGE_TRANSITION_TYPED);
+
+  // Create UnsafeResource for fake sub-resource of landing page.
+  UnsafeResource resource;
+  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true /* is_subresource */,
                GURL(kThreatURL));
 
+  // Start a pending load before creating ThreatDetails.
+  controller().LoadURL(GURL(kPendingURL),
+                       content::Referrer(GURL(kPendingReferrerURL),
+                                         blink::WebReferrerPolicyDefault),
+                       ui::PAGE_TRANSITION_TYPED, std::string());
+
+  // Do ThreatDetails collection.
+  scoped_refptr<ThreatDetailsWrap> report =
+      new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource, NULL);
+  std::string serialized = WaitForSerializedReport(
+      report.get(), true /* did_proceed*/, 1 /* num_visit */);
+
+  ClientSafeBrowsingReportRequest actual;
+  actual.ParseFromString(serialized);
+
+  ClientSafeBrowsingReportRequest expected;
+  expected.set_type(ClientSafeBrowsingReportRequest::URL_MALWARE);
+  expected.set_url(kThreatURL);
+  expected.set_page_url(kLandingURL);
+  // Note that the referrer policy is not actually enacted here, since that's
+  // done in Blink.
+  expected.set_referrer_url(kReferrerURL);
+  expected.set_did_proceed(true);
+  expected.set_repeat_visit(true);
+
+  ClientSafeBrowsingReportRequest::Resource* pb_resource =
+      expected.add_resources();
+  pb_resource->set_id(0);
+  pb_resource->set_url(kLandingURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(1);
+  pb_resource->set_url(kThreatURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(2);
+  pb_resource->set_url(kReferrerURL);
+
+  VerifyResults(actual, expected);
+}
+
+// Tests the interaction with the HTTP cache.
+TEST_F(ThreatDetailsTest, HTTPCache) {
+  content::WebContentsTester::For(web_contents())
+      ->NavigateAndCommit(GURL(kLandingURL));
+
+  UnsafeResource resource;
+  InitResource(&resource, SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL,
+               true /* is_subresource */, GURL(kThreatURL));
+
   scoped_refptr<ThreatDetailsWrap> report =
       new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource,
                             profile()->GetRequestContext());
@@ -618,12 +746,12 @@ TEST_F(ThreatDetailsTest, HTTPCache) {
 
 // Tests the interaction with the HTTP cache (where the cache is empty).
 TEST_F(ThreatDetailsTest, HTTPCacheNoEntries) {
-  controller().LoadURL(GURL(kLandingURL), content::Referrer(),
-                       ui::PAGE_TRANSITION_TYPED, std::string());
+  content::WebContentsTester::For(web_contents())
+      ->NavigateAndCommit(GURL(kLandingURL));
 
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL, true,
-               GURL(kThreatURL));
+  InitResource(&resource, SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL,
+               true /* is_subresource */, GURL(kThreatURL));
 
   scoped_refptr<ThreatDetailsWrap> report =
       new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource,
@@ -676,11 +804,12 @@ TEST_F(ThreatDetailsTest, HistoryServiceUrls) {
   // Wait for history service operation finished.
   profile()->BlockUntilHistoryProcessesPendingRequests();
 
-  controller().LoadURL(GURL(kLandingURL), content::Referrer(),
-                       ui::PAGE_TRANSITION_TYPED, std::string());
+  content::WebContentsTester::For(web_contents())
+      ->NavigateAndCommit(GURL(kLandingURL));
 
   UnsafeResource resource;
-  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true, GURL(kThreatURL));
+  InitResource(&resource, SB_THREAT_TYPE_URL_MALWARE, true /* is_subresource */,
+               GURL(kThreatURL));
   scoped_refptr<ThreatDetailsWrap> report =
       new ThreatDetailsWrap(ui_manager_.get(), web_contents(), resource, NULL);