Fixes for Safe Browsing with unrelated pending navigations.

chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc

Index: chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 91547a27db4542cce9f6a3b448b5c2c3fc39583c..deccf81608f354262e9c83947db22f06ae84670b 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -18,6 +18,7 @@
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/test/thread_test_helper.h"
 #include "base/time/time.h"
 #include "chrome/browser/bookmarks/startup_task_runner_service_factory.h"
@@ -36,6 +37,7 @@
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -43,6 +45,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/bookmarks/browser/startup_task_runner_service.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/cookies/cookie_store.h"
 #include "net/cookies/cookie_util.h"
@@ -74,12 +77,60 @@ namespace safe_browsing {
 
 namespace {
 
+const char kEmptyPage[] = "/empty.html";
+const char kMalwareFile[] = "/downloads/dangerous/dangerous.exe";
+const char kMalwarePage[] = "/safe_browsing/malware.html";
+const char kMalwareDelayedLoadsPage[] =
+    "/safe_browsing/malware_delayed_loads.html";
+const char kMalwareIFrame[] = "/safe_browsing/malware_iframe.html";
+const char kMalwareImg[] = "/safe_browsing/malware_image.png";
+const char kNeverCompletesPath[] = "/never_completes";
+
+class NeverCompletingHttpResponse : public net::test_server::HttpResponse {
+ public:
+  ~NeverCompletingHttpResponse() override {}
+
+  void SendResponse(
+      const net::test_server::SendBytesCallback& send,
+      const net::test_server::SendCompleteCallback& done) override {
+    // Do nothing. |done| is never called.
+  }
+};
+
+scoped_ptr<net::test_server::HttpResponse> HandleNeverCompletingRequests(
+    const net::test_server::HttpRequest& request) {
+  if (!base::StartsWith(request.relative_url, kNeverCompletesPath,
+                          base::CompareCase::SENSITIVE))
+    return scoped_ptr<net::test_server::HttpResponse>();
+  return make_scoped_ptr(new NeverCompletingHttpResponse());
+}
+
 void InvokeFullHashCallback(
     SafeBrowsingProtocolManager::FullHashCallback callback,
     const std::vector<SBFullHashResult>& result) {
   callback.Run(result, base::TimeDelta::FromMinutes(45));
 }
 
+class FakeSafeBrowsingUIManager : public SafeBrowsingUIManager {
+ public:
+  explicit FakeSafeBrowsingUIManager(SafeBrowsingService* service)
+      : SafeBrowsingUIManager(service) {}
+
+  void MaybeReportSafeBrowsingHit(
+      const safe_browsing::HitReport& hit_report) override {
+    EXPECT_FALSE(got_hit_report_);
+    got_hit_report_ = true;
+    hit_report_ = hit_report;
+    SafeBrowsingUIManager::MaybeReportSafeBrowsingHit(hit_report);
+  }
+
+  bool got_hit_report_ = false;
+  safe_browsing::HitReport hit_report_;
+
+ protected:
+  ~FakeSafeBrowsingUIManager() override {}
+};
+
 class FakeSafeBrowsingService : public SafeBrowsingService {
  public:
   explicit FakeSafeBrowsingService(const std::string& url_prefix)
@@ -95,6 +146,11 @@ class FakeSafeBrowsingService : public SafeBrowsingService {
     return config;
   }
 
+ protected:
+  SafeBrowsingUIManager* CreateUIManager() override {
+    return new FakeSafeBrowsingUIManager(this);
+  }
+
  private:
   ~FakeSafeBrowsingService() override {}
 
@@ -458,6 +514,8 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
   void SetUpInProcessBrowserTestFixture() override {
     base::FilePath test_data_dir;
     PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
+    embedded_test_server()->RegisterRequestHandler(
+        base::Bind(&HandleNeverCompletingRequests));
     embedded_test_server()->ServeFilesFromDirectory(test_data_dir);
     ASSERT_TRUE(embedded_test_server()->Start());
   }
@@ -534,6 +592,15 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
     }
   }
 
+  FakeSafeBrowsingUIManager* ui_manager() {
+    return static_cast<FakeSafeBrowsingUIManager*>(
+        g_browser_process->safe_browsing_service()->ui_manager().get());
+  }
+  bool got_hit_report() { return ui_manager()->got_hit_report_; }
+  const safe_browsing::HitReport& hit_report() {
+    return ui_manager()->hit_report_;
+  }
+
  protected:
   StrictMock<MockObserver> observer_;
 
@@ -613,15 +680,6 @@ class SafeBrowsingServiceMetadataTest
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingServiceMetadataTest);
 };
 
-namespace {
-
-const char kEmptyPage[] = "/empty.html";
-const char kMalwareFile[] = "/downloads/dangerous/dangerous.exe";
-const char kMalwarePage[] = "/safe_browsing/malware.html";
-const char kMalwareIFrame[] = "/safe_browsing/malware_iframe.html";
-const char kMalwareImg[] = "/safe_browsing/malware_image.png";
-
-// This test goes through DownloadResourceHandler.
 IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareMainFrame) {
   GURL url = embedded_test_server()->GetURL(kEmptyPage);
 
@@ -634,6 +692,12 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareMainFrame) {
   ui_test_utils::NavigateToURL(browser(), url);
   // All types should show the interstitial.
   EXPECT_TRUE(ShowingInterstitialPage());
+
+  EXPECT_TRUE(got_hit_report());
+  EXPECT_EQ(url, hit_report().malicious_url);
+  EXPECT_EQ(url, hit_report().page_url);
+  EXPECT_EQ(GURL(), hit_report().referrer_url);
+  EXPECT_FALSE(hit_report().is_subresource);
 }
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareIFrame) {
@@ -649,6 +713,12 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareIFrame) {
   ui_test_utils::NavigateToURL(browser(), main_url);
   // All types should show the interstitial.
   EXPECT_TRUE(ShowingInterstitialPage());
+
+  EXPECT_TRUE(got_hit_report());
+  EXPECT_EQ(iframe_url, hit_report().malicious_url);
+  EXPECT_EQ(main_url, hit_report().page_url);
+  EXPECT_EQ(GURL(), hit_report().referrer_url);
+  EXPECT_TRUE(hit_report().is_subresource);
 }
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareImg) {
@@ -676,9 +746,15 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingServiceMetadataTest, MalwareImg) {
     case METADATA_NONE:
     case METADATA_DISTRIBUTION:
       EXPECT_TRUE(ShowingInterstitialPage());
+      EXPECT_TRUE(got_hit_report());
+      EXPECT_EQ(img_url, hit_report().malicious_url);
+      EXPECT_EQ(main_url, hit_report().page_url);
+      EXPECT_EQ(GURL(), hit_report().referrer_url);
+      EXPECT_TRUE(hit_report().is_subresource);
       break;
     case METADATA_LANDING:
       EXPECT_FALSE(ShowingInterstitialPage());
+      EXPECT_FALSE(got_hit_report());
       break;
   }
 }
@@ -702,6 +778,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, UnwantedImgIgnored) {
   ui_test_utils::NavigateToURL(browser(), main_url);
 
   EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
 }
 
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, DISABLED_MalwareWithWhitelist) {
@@ -764,6 +841,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Prefetch) {
   SetupResponseForUrl(malware_url, malware_full_hash);
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
   Mock::VerifyAndClear(&observer_);
 
   // However, when we navigate to the malware page, we should still get
@@ -772,10 +850,190 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Prefetch) {
       .Times(1);
   ui_test_utils::NavigateToURL(browser(), malware_url);
   EXPECT_TRUE(ShowingInterstitialPage());
+  EXPECT_TRUE(got_hit_report());
   Mock::VerifyAndClear(&observer_);
 }
 
-}  // namespace
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, MainFrameHitWithReferrer) {
+  GURL first_url = embedded_test_server()->GetURL(kEmptyPage);
+  GURL bad_url = embedded_test_server()->GetURL(kMalwarePage);
+
+  SBFullHashResult malware_full_hash;
+  GenUrlFullhashResult(bad_url, MALWARE, &malware_full_hash);
+  SetupResponseForUrl(bad_url, malware_full_hash);
+
+  // Navigate to first, safe page.
+  ui_test_utils::NavigateToURL(browser(), first_url);
+  EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
+  Mock::VerifyAndClear(&observer_);
+
+  // Navigate to malware page, should show interstitial and have first page in
+  // referrer.
+  EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(bad_url)))
+      .Times(1);
+
+  chrome::NavigateParams params(browser(), bad_url, ui::PAGE_TRANSITION_LINK);
+  params.referrer.url = first_url;
+  ui_test_utils::NavigateToURL(&params);
+
+  EXPECT_TRUE(ShowingInterstitialPage());
+  EXPECT_TRUE(got_hit_report());
+  EXPECT_EQ(bad_url, hit_report().malicious_url);
+  EXPECT_EQ(bad_url, hit_report().page_url);
+  EXPECT_EQ(first_url, hit_report().referrer_url);
+  EXPECT_FALSE(hit_report().is_subresource);
+}
+
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
+                       SubResourceHitWithMainFrameReferrer) {
+  GURL first_url = embedded_test_server()->GetURL(kEmptyPage);
+  GURL second_url = embedded_test_server()->GetURL(kMalwarePage);
+  GURL bad_url = embedded_test_server()->GetURL(kMalwareImg);
+
+  SBFullHashResult malware_full_hash;
+  GenUrlFullhashResult(bad_url, MALWARE, &malware_full_hash);
+  SetupResponseForUrl(bad_url, malware_full_hash);
+
+  // Navigate to first, safe page.
+  ui_test_utils::NavigateToURL(browser(), first_url);
+  EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
+  Mock::VerifyAndClear(&observer_);
+
+  // Navigate to page which has malware subresource, should show interstitial
+  // and have first page in referrer.
+  EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(bad_url)))
+      .Times(1);
+
+  chrome::NavigateParams params(browser(), second_url,
+                                ui::PAGE_TRANSITION_LINK);
+  params.referrer.url = first_url;
+  ui_test_utils::NavigateToURL(&params);
+
+  EXPECT_TRUE(ShowingInterstitialPage());
+  EXPECT_TRUE(got_hit_report());
+  EXPECT_EQ(bad_url, hit_report().malicious_url);
+  EXPECT_EQ(second_url, hit_report().page_url);
+  EXPECT_EQ(first_url, hit_report().referrer_url);
+  EXPECT_TRUE(hit_report().is_subresource);
+}
+
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
+                       SubResourceHitWithMainFrameRendererInitiatedSlowLoad) {
+  GURL first_url = embedded_test_server()->GetURL(kEmptyPage);
+  GURL second_url = embedded_test_server()->GetURL(kMalwareDelayedLoadsPage);
+  GURL third_url = embedded_test_server()->GetURL(kNeverCompletesPath);
+  GURL bad_url = embedded_test_server()->GetURL(kMalwareImg);
+
+  SBFullHashResult malware_full_hash;
+  GenUrlFullhashResult(bad_url, MALWARE, &malware_full_hash);
+  SetupResponseForUrl(bad_url, malware_full_hash);
+
+  // Navigate to first, safe page.
+  ui_test_utils::NavigateToURL(browser(), first_url);
+  EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
+  Mock::VerifyAndClear(&observer_);
+
+  // Navigate to malware page. The malware subresources haven't loaded yet, so
+  // no interstitial should show yet.
+  chrome::NavigateParams params(browser(), second_url,
+                                ui::PAGE_TRANSITION_LINK);
+  params.referrer.url = first_url;
+  ui_test_utils::NavigateToURL(&params);
+
+  EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
+  Mock::VerifyAndClear(&observer_);
+
+  EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(bad_url)))
+      .Times(1);
+
+  WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::WindowedNotificationObserver load_stop_observer(
+      content::NOTIFICATION_LOAD_STOP,
+      content::Source<content::NavigationController>(
+           &contents->GetController()));
+  // Run javascript function in the page which starts a timer to load the
+  // malware image, and also starts a renderer-initiated top-level navigation to
+  // a site that does not respond.  Should show interstitial and have first page
+  // in referrer.
+  contents->GetMainFrame()->ExecuteJavaScriptForTests(
+      base::ASCIIToUTF16("navigateAndLoadMalwareImage()"));
+  load_stop_observer.Wait();
+
+  EXPECT_TRUE(ShowingInterstitialPage());
+  EXPECT_TRUE(got_hit_report());
+  // Report URLs should be for the current page, not the pending load.
+  EXPECT_EQ(bad_url, hit_report().malicious_url);
+  EXPECT_EQ(second_url, hit_report().page_url);
+  EXPECT_EQ(first_url, hit_report().referrer_url);
+  EXPECT_TRUE(hit_report().is_subresource);
+}
+
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
+                       SubResourceHitWithMainFrameBrowserInitiatedSlowLoad) {
+  GURL first_url = embedded_test_server()->GetURL(kEmptyPage);
+  GURL second_url = embedded_test_server()->GetURL(kMalwareDelayedLoadsPage);
+  GURL third_url = embedded_test_server()->GetURL(kNeverCompletesPath);
+  GURL bad_url = embedded_test_server()->GetURL(kMalwareImg);
+
+  SBFullHashResult malware_full_hash;
+  GenUrlFullhashResult(bad_url, MALWARE, &malware_full_hash);
+  SetupResponseForUrl(bad_url, malware_full_hash);
+
+  // Navigate to first, safe page.
+  ui_test_utils::NavigateToURL(browser(), first_url);
+  EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
+  Mock::VerifyAndClear(&observer_);
+
+  // Navigate to malware page. The malware subresources haven't loaded yet, so
+  // no interstitial should show yet.
+  chrome::NavigateParams params(browser(), second_url,
+                                ui::PAGE_TRANSITION_LINK);
+  params.referrer.url = first_url;
+  ui_test_utils::NavigateToURL(&params);
+
+  EXPECT_FALSE(ShowingInterstitialPage());
+  EXPECT_FALSE(got_hit_report());
+  Mock::VerifyAndClear(&observer_);
+
+  EXPECT_CALL(observer_, OnSafeBrowsingHit(IsUnsafeResourceFor(bad_url)))
+      .Times(1);
+
+  WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::RenderFrameHost* rfh = contents->GetMainFrame();
+  content::WindowedNotificationObserver load_stop_observer(
+      content::NOTIFICATION_LOAD_STOP,
+      content::Source<content::NavigationController>(
+           &contents->GetController()));
+  // Start a browser initiated top-level navigation to a site that does not
+  // respond.
+  ui_test_utils::NavigateToURLWithDisposition(browser(), third_url, CURRENT_TAB,
+                                              ui_test_utils::BROWSER_TEST_NONE);
+
+  // While the top-level navigation is pending, run javascript
+  // function in the page which loads the malware image.
+  rfh->ExecuteJavaScriptForTests(base::ASCIIToUTF16("loadMalwareImage()"));
+
+  // Wait for interstitial to show.
+  load_stop_observer.Wait();
+
+  EXPECT_TRUE(ShowingInterstitialPage());
+  EXPECT_TRUE(got_hit_report());
+  // Report URLs should be for the current page, not the pending load.
+  EXPECT_EQ(bad_url, hit_report().malicious_url);
+  EXPECT_EQ(second_url, hit_report().page_url);
+  EXPECT_EQ(first_url, hit_report().referrer_url);
+  EXPECT_TRUE(hit_report().is_subresource);
+}
+
+
+namespace {
 
 class TestSBClient : public base::RefCountedThreadSafe<TestSBClient>,
                      public SafeBrowsingDatabaseManager::Client {
@@ -852,10 +1110,10 @@ class TestSBClient : public base::RefCountedThreadSafe<TestSBClient>,
   DISALLOW_COPY_AND_ASSIGN(TestSBClient);
 };
 
+}  // namespace
+
 // These tests use SafeBrowsingService::Client to directly interact with
 // SafeBrowsingService.
-namespace {
-
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrl) {
   GURL badbin_url = embedded_test_server()->GetURL(kMalwareFile);
   std::vector<GURL> badbin_urls(1, badbin_url);
@@ -1089,8 +1347,6 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, StartAndStop) {
   EXPECT_FALSE(csd_service->enabled());
 }
 
-}  // namespace
-
 class SafeBrowsingServiceShutdownTest : public SafeBrowsingServiceTest {
  public:
   void TearDown() override {