Separate pre-classification checks for client-side malware and phishing

chrome/browser/safe_browsing/client_side_detection_service_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 <map>
 #include <queue>
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 454 matching lines @@
   EXPECT_TRUE(SendClientReportMalwareRequest(url));
   CheckConfirmedMalwareUrl(GURL("http://response-bad.com/"));
 
   // This request will fail
   response.set_blacklist(false);
   SetClientReportMalwareResponse(response.SerializeAsString(),
                                  net::HTTP_INTERNAL_SERVER_ERROR,
                                  net::URLRequestStatus::FAILED);
   EXPECT_FALSE(SendClientReportMalwareRequest(url));
 
-  // server blacklist decision is false, and response is succesful
+  // Server blacklist decision is false, and response is successful
   response.set_blacklist(false);
   SetClientReportMalwareResponse(response.SerializeAsString(), net::HTTP_OK,
                                  net::URLRequestStatus::SUCCESS);
   EXPECT_FALSE(SendClientReportMalwareRequest(url));
 
-  // Check that we have recorded all 4 requests within the correct time range.
+  // Check that we have recorded all 5 requests within the correct time range.
   base::Time after = base::Time::Now();
   std::queue<base::Time>& report_times = GetMalwareReportTimes();
-  EXPECT_EQ(4U, report_times.size());
+  EXPECT_EQ(5U, report_times.size());

[mattm, 2014/03/18 02:19:06]

changed from 4 to 5 because the limit is not enforced in CSDS now? Should test
the limit in client_side_detection_host_unittest.cc.

[noé, 2014/03/20 17:01:45]

The actual limit is checked below.  In the host unit-test we do check that
OverMalwareReportLimit is called on the mock object.

[mattm, 2014/03/20 22:51:40]

I only see OverPhishingReportLimit tested in the host unittest.

[noé, 2014/03/21 00:08:38]

You're right again.  fixed.

 
-  // Another normal behavior will fail because of the limit is hit
-  response.set_blacklist(true);
-  SetClientReportMalwareResponse(response.SerializeAsString(), net::HTTP_OK,
-                                 net::URLRequestStatus::SUCCESS);
-  EXPECT_FALSE(SendClientReportMalwareRequest(url));
+  // Check that the malware report limit was reached.
+  EXPECT_TRUE(csd_service_->OverMalwareReportLimit());
 
   report_times = GetMalwareReportTimes();
-  EXPECT_EQ(4U, report_times.size());
+  EXPECT_EQ(5U, report_times.size());
   while (!report_times.empty()) {
     base::Time time = report_times.back();
     report_times.pop();
     EXPECT_LE(before, time);
     EXPECT_GE(after, time);
   }
 }
 
 TEST_F(ClientSideDetectionServiceTest, GetNumReportTest) {
   SetModelFetchResponse("bogus model", net::HTTP_OK,
@@ skipping 194 matching lines @@
   EXPECT_CALL(*service, ScheduleFetchModel(_))
       .WillOnce(Invoke(service, &MockClientSideDetectionService::Schedule));
   EXPECT_CALL(*service, EndFetchModel(
       ClientSideDetectionService::MODEL_NOT_CHANGED))
       .WillOnce(Invoke(service, &MockClientSideDetectionService::Disable));
   csd_service_->SetEnabledAndRefreshState(true);
   EXPECT_FALSE(SendClientReportPhishingRequest(GURL("http://a.com/"), 0.4f));
   Mock::VerifyAndClearExpectations(service);
 }
 }  // namespace safe_browsing