Fixes for Safe Browsing with unrelated pending navigations.

chrome/browser/safe_browsing/client_side_detection_host_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
@@ skipping 333 matching lines @@
     EXPECT_EQ(resource.is_subresource,
               csd_host_->unsafe_resource_->is_subresource);
     EXPECT_EQ(resource.threat_type, csd_host_->unsafe_resource_->threat_type);
     EXPECT_TRUE(csd_host_->unsafe_resource_->callback.is_null());
     EXPECT_EQ(resource.render_process_host_id,
               csd_host_->unsafe_resource_->render_process_host_id);
     EXPECT_EQ(resource.render_view_id,
               csd_host_->unsafe_resource_->render_view_id);
   }
 
-  void SetUnsafeSubResourceForCurrent() {
+  void SetUnsafeSubResourceForCurrent(bool expect_unsafe_resource) {
     UnsafeResource resource;
     resource.url = GURL("http://www.malware.com/");
     resource.original_url = web_contents()->GetURL();
     resource.is_subresource = true;
     resource.threat_type = SB_THREAT_TYPE_URL_MALWARE;
     resource.callback = base::Bind(&EmptyUrlCheckCallback);
     resource.callback_thread =
         BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO);
     resource.render_process_host_id = web_contents()->GetRenderProcessHost()->
         GetID();
     resource.render_view_id =
         web_contents()->GetRenderViewHost()->GetRoutingID();
     csd_host_->OnSafeBrowsingHit(resource);
     resource.callback.Reset();
-    ASSERT_TRUE(csd_host_->DidShowSBInterstitial());
-    TestUnsafeResourceCopied(resource);
+    ASSERT_EQ(expect_unsafe_resource, csd_host_->DidShowSBInterstitial());
+    if (expect_unsafe_resource)
+      TestUnsafeResourceCopied(resource);
   }
 
   void NavigateWithSBHitAndCommit(const GURL& url) {
     // Create a pending navigation.
     controller().LoadURL(
         url, content::Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
 
     ASSERT_TRUE(pending_rvh());
     if (web_contents()->GetRenderViewHost()->GetProcess()->GetID() ==
         pending_rvh()->GetProcess()->GetID()) {
@@ skipping 27 matching lines @@
     controller().LoadURL(
         safe_url, content::Referrer(), ui::PAGE_TRANSITION_LINK,
         std::string());
 
     ASSERT_TRUE(pending_rvh());
     if (web_contents()->GetRenderViewHost()->GetProcess()->GetID() ==
         pending_rvh()->GetProcess()->GetID()) {
       EXPECT_NE(web_contents()->GetRenderViewHost()->GetRoutingID(),
                 pending_rvh()->GetRoutingID());
     }
-    ASSERT_FALSE(csd_host_->DidShowSBInterstitial());
 
     content::WebContentsTester::For(web_contents())->CommitPendingNavigation();
     ASSERT_FALSE(csd_host_->DidShowSBInterstitial());
   }
 
   void CheckIPUrlEqual(const std::vector<IPUrlInfo>& expect,
                        const std::vector<IPUrlInfo>& result) {
     ASSERT_EQ(expect.size(), result.size());
 
     for (unsigned int i = 0; i < expect.size(); ++i) {
@@ skipping 254 matching lines @@
   ClientPhishingRequest verdict;
   verdict.set_url(url.spec());
   verdict.set_client_score(0.1f);
   verdict.set_is_phishing(false);
 
   // First we have to navigate to the URL to set the unique page ID.
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
                                 &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
-  SetUnsafeSubResourceForCurrent();
+  SetUnsafeSubResourceForCurrent(true /* expect_unsafe_resource */);
 
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _, CallbackIsNull()))
       .WillOnce(DoAll(DeleteArg<0>(), QuitUIMessageLoop()));
   std::vector<GURL> redirect_chain;
   redirect_chain.push_back(url);
   SetRedirectChain(redirect_chain);
   OnPhishingDetectionDone(verdict.SerializeAsString());
   base::MessageLoop::current()->Run();
@@ skipping 37 matching lines @@
   OnPhishingDetectionDone(verdict.SerializeAsString());
   base::MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
 
   ExpectPreClassificationChecks(start_url, &kFalse, &kFalse, &kFalse, &kFalse,
                                 &kFalse, &kFalse, &kFalse, &kFalse);
   NavigateWithoutSBHitAndCommit(start_url);
   WaitAndCheckPreClassificationChecks();
 }
 
+TEST_F(
+    ClientSideDetectionHostTest,
+    OnPhishingDetectionDoneVerdictNotPhishingButSBMatchOnSubresourceWhileNavPending) {

[Bernhard Bauer, 2015/12/17 16:02:02]

Umm... I think you might win an award for longest test name ever. 😉

+  // When a malware hit happens on a committed page while a slow pending load is
+  // in progress, the csd report should be sent for the committed page.
+
+  // Do an initial navigation to a safe host.
+  GURL start_url("http://safe.example.com/");
+  ExpectPreClassificationChecks(
+      start_url, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse, &kFalse,
+      &kFalse);
+  NavigateAndCommit(start_url);
+  WaitAndCheckPreClassificationChecks();
+
+  // Now navigate to a different host which does not have a SB hit.
+  GURL url("http://not-malware-not-phishing-but-malware-subresource.com/");
+  ClientPhishingRequest verdict;
+  verdict.set_url(url.spec());
+  verdict.set_client_score(0.1f);
+  verdict.set_is_phishing(false);
+
+  ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
+                                &kFalse, &kFalse, &kFalse, &kFalse);
+  NavigateWithoutSBHitAndCommit(url);
+
+  // Simulate a subresource malware hit on committed page.
+  SetUnsafeSubResourceForCurrent(true /* expect_unsafe_resource */);
+
+  // Create a pending navigation, but don't commit it.
+  GURL pending_url("http://slow.example.com/");
+  content::WebContentsTester::For(web_contents())->StartNavigation(pending_url);
+
+  WaitAndCheckPreClassificationChecks();
+
+  // Even though we have a pending navigation, the DidShowSBInterstitial check
+  // should apply to the committed navigation, so we should get a report even
+  // though the verdict has is_phishing = false.
+  EXPECT_CALL(*csd_service_,
+              SendClientReportPhishingRequest(
+                  Pointee(PartiallyEqualVerdict(verdict)), _, CallbackIsNull()))
+      .WillOnce(DoAll(DeleteArg<0>(), QuitUIMessageLoop()));
+  std::vector<GURL> redirect_chain;
+  redirect_chain.push_back(url);
+  SetRedirectChain(redirect_chain);
+  OnPhishingDetectionDone(verdict.SerializeAsString());
+  base::MessageLoop::current()->Run();
+  EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
+}
+
+TEST_F(ClientSideDetectionHostTest, SafeBrowsingHitOnFreshTab) {
+  // A fresh WebContents should not have any NavigationEntries yet. (See
+  // https://crbug.com/524208.)
+  EXPECT_EQ(nullptr, controller().GetLastCommittedEntry());
+  EXPECT_EQ(nullptr, controller().GetPendingEntry());
+
+  // Simulate a subresource malware hit (this could happen if the WebContents
+  // was created with window.open, and had content injected into it).
+  SetUnsafeSubResourceForCurrent(false /* expect_unsafe_resource */);
+
+  // If the test didn't crash, we're good. (Nothing else to test, since there
+  // was no DidNavigateMainFrame, CSD won't do anything with this hit.)
+}
+
 TEST_F(ClientSideDetectionHostTest,
        DidStopLoadingShowMalwareInterstitial) {
   // Case 9: client thinks the page match malware IP and so does the server.
   // We show an sub-resource malware interstitial.
   MockBrowserFeatureExtractor* mock_extractor =
       new StrictMock<MockBrowserFeatureExtractor>(
           web_contents(),
           csd_host_.get());
   SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
 
@@ skipping 368 matching lines @@
   EXPECT_EQ(url, resource.url);
   EXPECT_EQ(url, resource.original_url);
 
   ExpectStartPhishingDetection(NULL);
 
   // Showing a phishing warning will invalidate all the weak pointers which
   // means we will not extract malware features.
   ExpectShouldClassifyForMalwareResult(false);
 }
 }  // namespace safe_browsing