If we show a SafeBrowsing warning we always send the client-side detection

chrome/browser/safe_browsing/client_side_detection_host_unittest.cc

 // Copyright (c) 2011 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/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/task.h"
 #include "chrome/browser/safe_browsing/browser_feature_extractor.h"
 #include "chrome/browser/safe_browsing/client_side_detection_host.h"
@@ skipping 11 matching lines @@
 #include "content/browser/tab_contents/test_tab_contents.h"
 #include "googleurl/src/gurl.h"
 #include "ipc/ipc_test_sink.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::DeleteArg;
 using ::testing::DoAll;
 using ::testing::Eq;
+using ::testing::IsNull;
 using ::testing::Mock;
 using ::testing::NiceMock;
 using ::testing::NotNull;
 using ::testing::Pointee;
 using ::testing::Return;
 using ::testing::SaveArg;
 using ::testing::SetArgumentPointee;
 using ::testing::StrictMock;
 
 namespace {
@@ skipping 112 matching lines @@
     csd_host_->set_client_side_detection_service(csd_service_.get());
     csd_host_->set_safe_browsing_service(sb_service_.get());
   }
 
   virtual void TearDown() {
     TabContentsWrapperTestHarness::TearDown();
     io_thread_.reset();
     ui_thread_.reset();
   }
 
-  void OnDetectedPhishingSite(const std::string& verdict_str) {
+  void OnPhishingDetectionDone(const std::string& verdict_str) {
     // Make sure we have a valid BrowseInfo object set before we call this
     // method.
     csd_host_->browse_info_.reset(new BrowseInfo);
-    csd_host_->OnDetectedPhishingSite(verdict_str);
+    csd_host_->OnPhishingDetectionDone(verdict_str);
   }
 
   void FlushIOMessageLoop() {
     // If there was a message posted on the IO thread to display the
     // interstitial page we know that it would have been posted before
     // we put the quit message there.
     BrowserThread::PostTask(BrowserThread::IO,
                             FROM_HERE,
                             NewRunnableFunction(&QuitUIMessageLoopFromIO));
     MessageLoop::current()->Run();
@@ skipping 39 matching lines @@
     MessageLoop::current()->RunAllPending();
     EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
     EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
     EXPECT_TRUE(Mock::VerifyAndClear(mock_profile_));
   }
 
   void SetFeatureExtractor(BrowserFeatureExtractor* extractor) {
     csd_host_->feature_extractor_.reset(extractor);
   }
 
+  void SetUnsafeUniquePageIdToCurrent() {
+    csd_host_->unsafe_unique_page_id_ =
+        contents()->controller().GetActiveEntry()->unique_id();
+    ASSERT_TRUE(csd_host_->DidShowSBInterstitial());
+  }
+
  protected:
   scoped_ptr<ClientSideDetectionHost> csd_host_;
   scoped_ptr<StrictMock<MockClientSideDetectionService> > csd_service_;
   scoped_refptr<StrictMock<MockSafeBrowsingService> > sb_service_;
   MockTestingProfile* mock_profile_;  // We don't own this object
 
  private:
   scoped_ptr<BrowserThread> ui_thread_;
   scoped_ptr<BrowserThread> io_thread_;
 };
 
-TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteInvalidVerdict) {
+TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneInvalidVerdict) {
   // Case 0: renderer sends an invalid verdict string that we're unable to
   // parse.
   MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
       contents(),
       csd_service_.get());
   SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
-  OnDetectedPhishingSite("Invalid Protocol Buffer");
+  OnPhishingDetectionDone("Invalid Protocol Buffer");
   EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
 }
 
-TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteNotPhishing) {
+TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneNotPhishing) {
   // Case 1: client thinks the page is phishing.  The server does not agree.
   // No interstitial is shown.
   ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
   ClientPhishingRequest verdict;
   verdict.set_url("http://phishingurl.com/");
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb), QuitUIMessageLoop()));
-  OnDetectedPhishingSite(verdict.SerializeAsString());
+  OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
   ASSERT_TRUE(cb);
 
   // Make sure DoDisplayBlockingPage is not going to be called.
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_)).Times(0);
   cb->Run(GURL(verdict.url()), false);
   delete cb;
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
 }
 
-TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteDisabled) {
+TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneDisabled) {
   // Case 2: client thinks the page is phishing and so does the server but
   // showing the interstitial is disabled => no interstitial is shown.
   ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
   ClientPhishingRequest verdict;
   verdict.set_url("http://phishingurl.com/");
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb), QuitUIMessageLoop()));
-  OnDetectedPhishingSite(verdict.SerializeAsString());
+  OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
   ASSERT_TRUE(cb);
 
   // Make sure DoDisplayBlockingPage is not going to be called.
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_)).Times(0);
   cb->Run(GURL(verdict.url()), false);
   delete cb;
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
 }
 
-TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteShowInterstitial) {
+TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneShowInterstitial) {
   // Case 3: client thinks the page is phishing and so does the server.
   // We show an interstitial.
   ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
   GURL phishing_url("http://phishingurl.com/");
   ClientPhishingRequest verdict;
   verdict.set_url(phishing_url.spec());
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb), QuitUIMessageLoop()));
-  OnDetectedPhishingSite(verdict.SerializeAsString());
+  OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
   ASSERT_TRUE(cb);
 
   SafeBrowsingService::UnsafeResource resource;
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
   cb->Run(phishing_url, true);
   delete cb;
 
@@ skipping 16 matching lines @@
       NewRunnableMethod(
           sb_service_.get(),
           &MockSafeBrowsingService::InvokeOnBlockingPageComplete,
           resource.client));
   // Since the CsdClient object will be deleted on the UI thread I need
   // to run the UI message loop.  Post a task to stop the UI message loop
   // after the client object destructor is called.
   FlushIOMessageLoop();
 }
 
-TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteMultiplePings) {
+TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
   // Case 4 & 5: client thinks a page is phishing then navigates to
   // another page which is also considered phishing by the client
   // before the server responds with a verdict.  After a while the
   // server responds for both requests with a phishing verdict.  Only
   // a single interstitial is shown for the second URL.
   ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
   GURL phishing_url("http://phishingurl.com/");
   ClientPhishingRequest verdict;
   verdict.set_url(phishing_url.spec());
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb), QuitUIMessageLoop()));
-  OnDetectedPhishingSite(verdict.SerializeAsString());
+  OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
   ASSERT_TRUE(cb);
   GURL other_phishing_url("http://other_phishing_url.com/bla");
   ExpectPreClassificationChecks(other_phishing_url, &kFalse, &kFalse, &kFalse,
                                 &kFalse, &kFalse, &kFalse);
   // We navigate away.  The callback cb should be revoked.
   NavigateAndCommit(other_phishing_url);
   // Wait for the pre-classification checks to finish for other_phishing_url.
   WaitAndCheckPreClassificationChecks();
 
   ClientSideDetectionService::ClientReportPhishingRequestCallback* cb_other;
   verdict.set_url(other_phishing_url.spec());
   verdict.set_client_score(0.8f);
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(DoAll(DeleteArg<0>(),
                       SaveArg<1>(&cb_other),
                       QuitUIMessageLoop()));
-  OnDetectedPhishingSite(verdict.SerializeAsString());
+  OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
   ASSERT_TRUE(cb_other);
 
   // We expect that the interstitial is shown for the second phishing URL and
   // not for the first phishing URL.
   SafeBrowsingService::UnsafeResource resource;
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
 
@@ skipping 21 matching lines @@
       NewRunnableMethod(
           sb_service_.get(),
           &MockSafeBrowsingService::InvokeOnBlockingPageComplete,
           resource.client));
   // Since the CsdClient object will be deleted on the UI thread I need
   // to run the UI message loop.  Post a task to stop the UI message loop
   // after the client object destructor is called.
   FlushIOMessageLoop();
 }
 
+TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneVerdictNotPhishing) {
+  // Case 6: renderer sends a verdict string that isn't phishing.
+  MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
+      contents(),
+      csd_service_.get());
+  SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
+
+  ClientPhishingRequest verdict;
+  verdict.set_url("http://not-phishing.com/");
+  verdict.set_client_score(0.1f);
+  verdict.set_is_phishing(false);
+
+  EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
+  OnPhishingDetectionDone(verdict.SerializeAsString());
+  EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
+}
+
+TEST_F(ClientSideDetectionHostTest,
+       OnPhishingDetectionDoneVerdictNotPhishingButSBMatch) {
+  // Case 7: renderer sends a verdict string that isn't phishing but the URL
+  // was on the regular phishing or malware lists.
+  GURL url("http://not-phishing.com/");
+  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);
+  NavigateAndCommit(url);
+  WaitAndCheckPreClassificationChecks();
+  SetUnsafeUniquePageIdToCurrent();
+
+  EXPECT_CALL(*csd_service_,
+              SendClientReportPhishingRequest(
+                  Pointee(PartiallyEqualVerdict(verdict)), IsNull()))
+      .WillOnce(DoAll(DeleteArg<0>(), QuitUIMessageLoop()));

[Brian Ryner, 2011/07/19 21:27:10]

Is there an easy way to check that this situation doesn't cause the interstitial
to reappear?

[noelutz, 2011/07/19 22:28:08]

Well, I do check that NULL is passed for the callback.  Since no callback is
provided to SendClientReportPhishingRequest I don't see how an interstitial
could be shown.

[Brian Ryner, 2011/07/19 22:35:24]

Ah, I missed that.  Seems reasonable.

+  OnPhishingDetectionDone(verdict.SerializeAsString());
+  MessageLoop::current()->Run();
+  EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
+}
+
 TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) {
   // Test that canceling pending should classify requests works as expected.
 
   GURL first_url("http://first.phishy.url.com");
   // The proxy checks is done synchronously so check that it has been done
   // for the first URL.
   ExpectPreClassificationChecks(first_url, &kFalse, &kFalse, &kFalse, NULL,
                                 NULL, NULL);
   NavigateAndCommit(first_url);
 
@@ skipping 168 matching lines @@
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
   EXPECT_EQ(url, resource.url);
   EXPECT_EQ(url, resource.original_url);
   delete resource.client;
   msg = process()->sink().GetFirstMessageMatching(
       SafeBrowsingMsg_StartPhishingDetection::ID);
   ASSERT_FALSE(msg);
 }
 
 }  // namespace safe_browsing