[NoStatePrefetch] Implement FINAL_STATUS_SAFEBROWSING

chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc

Index: chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
diff --git a/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc b/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
index a14ad92b52bd494d7856eab917ba6c61e43d3f8b..1a065f031dc80586b980bf881d1a7679a5de5764 100644
--- a/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
+++ b/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
@@ -86,6 +86,7 @@ class NoStatePrefetchBrowserTest
   }
 
  protected:
+  // Pass FINAL_STATUS_MAX to skip the final status check.
   std::unique_ptr<TestPrerender> PrefetchFromURL(
       const GURL& target_url,
       FinalStatus expected_final_status) {
@@ -95,6 +96,8 @@ class NoStatePrefetchBrowserTest
                                                          expected_final_status);
     std::vector<std::unique_ptr<TestPrerender>> prerenders =
         NavigateWithPrerenders(loader_url, expected_final_status_queue);
+    if (expected_final_status == FINAL_STATUS_MAX)
+      prerenders[0]->contents()->set_skip_final_checks(true);
     prerenders[0]->WaitForStop();
     return std::move(prerenders[0]);
   }
@@ -467,23 +470,48 @@ IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, Jpeg) {
   counter.WaitForCount(1);
 }
 
-// Checks that nothing is prefetched from malware sites.
-// TODO(mattcary): disabled as prefetch process teardown is racey with prerender
-// contents destruction, can fix when prefetch prerenderers are destroyed
-// deterministically.
+// If the main resource is unsafe, the whole prefetch is cancelled.
 IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest,
-                       DISABLED_PrerenderSafeBrowsingTopLevel) {
+                       PrerenderSafeBrowsingTopLevel) {
   GURL url = src_server()->GetURL(MakeAbsolute(kPrefetchPage));
   GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
       url, safe_browsing::SB_THREAT_TYPE_URL_MALWARE);
-  // Prefetch resources are blocked, but the prerender is not killed in any
-  // special way.
-  // TODO(mattcary): since the prerender will count itself as loaded even if the
-  // fetch of the main resource fails, the test doesn't actually confirm what we
-  // want it to confirm. This may be fixed by planned changes to the prerender
-  // lifecycle.
+
+  RequestCounter main_counter;
+  CountRequestFor(kPrefetchPage, &main_counter);
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+
   std::unique_ptr<TestPrerender> prerender =
       PrefetchFromFile(kPrefetchPage, FINAL_STATUS_SAFE_BROWSING);
+
+  main_counter.WaitForCount(0);
+  script_counter.WaitForCount(0);
+
+  // Verify that the page load did not happen.
+  prerender->WaitForLoads(0);
+}
+
+// If a subresource is unsafe, the corresponding request is cancelled.
+IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest,
+                       PrerenderSafeBrowsingSubresource) {
+  GURL url = src_server()->GetURL(MakeAbsolute(kPrefetchScript));
+  GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl(
+      url, safe_browsing::SB_THREAT_TYPE_URL_MALWARE);
+
+  RequestCounter main_counter;
+  CountRequestFor(kPrefetchPage, &main_counter);
+  RequestCounter script_counter;
+  CountRequestFor(kPrefetchScript, &script_counter);
+
+  // The final status can be FINAL_STATUS_NOSTATE_PREFETCH_FINISHED or
+  // FINAL_STATUS_SAFE_BROWSING, depending wether the PrerenderContents was
+  // already destroyed when the subresource is loaded.

[mattcary, 2017/01/06 16:37:20]

It seems a shame to not to have a reliable final status, especially since if we
did we could use this as a reliable counter for how much we prefetch unsafe
resources.

Maybe we could add a histogram back in
safe_browsing_resource_throttle::DestroyPrerenderContents, so we could at least
count correctly... but then we have to do subtraction between histograms to get
accurate counts, which is hard.

Hmmm. I'm not sure. I guess this is fine for now until we think of something
better.

[droger, 2017/01/06 16:43:42]

The easy way to make it reliable would be to call
Destroy(FINAL_STATUS_SAFE_BROWSING) only for the main resource.

In that case, subresource would still be cancelled, but the status would be
FINAL_STATUS_NOSTATE_PREFETCH_FINISHED.

And then we can revisit once we know what we want to do for subresources (which
is not very clear yet).

[droger, 2017/01/06 17:14:16]

Done.

+  std::unique_ptr<TestPrerender> prerender =
+      PrefetchFromFile(kPrefetchPage, FINAL_STATUS_MAX);
+
+  main_counter.WaitForCount(1);
+  script_counter.WaitForCount(0);
 }
 
 // Checks that prefetching a page does not add it to browsing history.