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 397 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 312 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) {

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

That's a mouthful!  Would a shorter name be worthwhile, or does this follow a
convention?

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

It's following the same convention as other tests in this file, though I agree
it's unwieldy.

+  // 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();
+
+  // 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,
        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