Add caching to phishing client side detection.

chrome/browser/safe_browsing/client_side_detection_service_unittest.cc

Index: chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
diff --git a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
index b418d14a6e4de6fc90c6b4f9204e0aba1c50eb19..ce64fb1e0a12cfb796dd6241a40e13ebaa2ffa4c 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
@@ -94,6 +94,41 @@ class ClientSideDetectionServiceTest : public testing::Test {
     return csd_service_->phishing_report_times_;
   }
 
+  void TestUpdateCache() {
+    ClientSideDetectionService::PhishingCache& cache = csd_service_->cache_;
+    base::Time now = base::Time::Now();
+    base::Time time = base::Time::Now() -
+        ClientSideDetectionService::kNegativeCacheInterval +
+        base::TimeDelta::FromMinutes(2);

[Brian Ryner, 2011/02/08 20:16:48]

Did you mean to use the "now" variable here instead of calling base::Time::Now()
again?

Also, it's unlikely, but if the test is running really slowly, 2 minutes could
possibly elapse between this call and when UpdateCache is called, which would
cause this to be evicted.  It might be a little safer to increase this to at
least 5 minutes.

[gcasto (DO NOT USE), 2011/02/09 00:10:52]

Done.

+    cache[GURL("http://first.url.com/")] =
+        make_linked_ptr(new ClientSideDetectionService::CacheState(false,
+                                                                   time));
+
+    time = now - ClientSideDetectionService::kNegativeCacheInterval -
+        base::TimeDelta::FromHours(1);
+    cache[GURL("http://second.url.com/")] =
+        make_linked_ptr(new ClientSideDetectionService::CacheState(false,
+                                                                   time));
+
+    time = now - ClientSideDetectionService::kPositiveCacheInterval -
+        base::TimeDelta::FromMinutes(5);
+    cache[GURL("http://third.url.com/")] =
+        make_linked_ptr(new ClientSideDetectionService::CacheState(true, time));
+
+    time = now - ClientSideDetectionService::kPositiveCacheInterval +
+        base::TimeDelta::FromMinutes(5);
+    cache[GURL("http://fourth.url.com/")] =
+        make_linked_ptr(new ClientSideDetectionService::CacheState(true, time));
+
+    LOG(INFO) << "Size:" << cache.size();

[Brian Ryner, 2011/02/08 20:16:48]

Did you mean to leave this in?

[gcasto (DO NOT USE), 2011/02/09 00:10:52]

Removed.

+    csd_service_->UpdateCache();
+
+    // Only 2 elements should be in the cache, the first and the fourth.
+    EXPECT_EQ(2U, cache.size());
+    EXPECT_NE(cache.find(GURL("http://first.url.com/")), cache.end());
+    EXPECT_NE(cache.find(GURL("http://fourth.url.com/")), cache.end());
+  }
+
  protected:
   scoped_ptr<ClientSideDetectionService> csd_service_;
   scoped_ptr<FakeURLFetcherFactory> factory_;
@@ -188,16 +223,46 @@ TEST_F(ClientSideDetectionServiceTest, SendClientReportPhishingRequest) {
   SetClientReportPhishingResponse(response.SerializeAsString(),
                                   true /* success */);
   EXPECT_TRUE(SendClientReportPhishingRequest(url, score));
+
+  // Caching causes this to still count as phishy.
   response.set_phishy(false);
   SetClientReportPhishingResponse(response.SerializeAsString(),
                                   true /* success */);
-  EXPECT_FALSE(SendClientReportPhishingRequest(url, score));
+  EXPECT_TRUE(SendClientReportPhishingRequest(url, score));
+
+  // This request will fail and should not be cached.
+  GURL second_url("http://b.com/");
+  response.set_phishy(false);
+  SetClientReportPhishingResponse(response.SerializeAsString(),
+                                  false /* success*/);
+  EXPECT_FALSE(SendClientReportPhishingRequest(second_url, score));
+
+  // Verify that the previous request was not cached.
+  response.set_phishy(true);
+  SetClientReportPhishingResponse(response.SerializeAsString(),
+                                  true /* success */);
+  EXPECT_TRUE(SendClientReportPhishingRequest(second_url, score));
+
+  // This request is blocked because it's not in the cache and we have more
+  // than 3 requests.
+  GURL third_url("http://c.com");
+  response.set_phishy(true);
+  SetClientReportPhishingResponse(response.SerializeAsString(),
+                                  true /* success */);
+  EXPECT_FALSE(SendClientReportPhishingRequest(third_url, score));
+
+  // Verify that caching still works even when new requests are blocked.
+  response.set_phishy(true);
+  SetClientReportPhishingResponse(response.SerializeAsString(),
+                                  true /* success */);
+  EXPECT_TRUE(SendClientReportPhishingRequest(url, score));
 
   base::Time after = base::Time::Now();
 
-  // Check that we have recorded 3 requests, all within the correct time range.
+  // Check that we have recorded 4 requests, all within the correct time range.
+  // The blocked request and the cached requests should not be present.
   std::queue<base::Time>& report_times = GetPhishingReportTimes();
-  EXPECT_EQ(3U, report_times.size());
+  EXPECT_EQ(4U, report_times.size());
   while (!report_times.empty()) {
     base::Time time = report_times.back();
     report_times.pop();
@@ -224,4 +289,14 @@ TEST_F(ClientSideDetectionServiceTest, GetNumReportTest) {
   EXPECT_EQ(2, GetNumReportsPerDay());
 }
 
+TEST_F(ClientSideDetectionServiceTest, UpdateCacheTest) {
+  SetModelFetchResponse("bogus model", true /* success */);
+  ScopedTempDir tmp_dir;
+  ASSERT_TRUE(tmp_dir.CreateUniqueTempDir());
+  csd_service_.reset(ClientSideDetectionService::Create(
+      tmp_dir.path().AppendASCII("model"), NULL));
+
+  TestUpdateCache();
+}
+
 }  // namespace safe_browsing