Move cache and report checking to before classification is run.

This change also makes it so that a positive cache hit will immediately show
a warning page without running classification.

BUG=
TEST=Ran ClientSideDetectionServiceTest and ClientSideDetectionHostTest

Review URL: http://codereview.chromium.org/6652013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@78106 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 16 insertions, 47 deletions

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 1ce8b03..485abc9 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
@@ -136,13 +136,13 @@ class ClientSideDetectionServiceTest : public testing::Test {
     // While 3 elements remain, only the first and the fourth are actually
     // valid.
     bool is_phishing;
-    EXPECT_TRUE(csd_service_->GetCachedResult(GURL("http://first.url.com"),
-                                              &is_phishing));
+    EXPECT_TRUE(csd_service_->GetValidCachedResult(
+        GURL("http://first.url.com"), &is_phishing));
     EXPECT_FALSE(is_phishing);
-    EXPECT_FALSE(csd_service_->GetCachedResult(GURL("http://third.url.com"),
-                                               &is_phishing));
-    EXPECT_TRUE(csd_service_->GetCachedResult(GURL("http://fourth.url.com"),
-                                              &is_phishing));
+    EXPECT_FALSE(csd_service_->GetValidCachedResult(
+        GURL("http://third.url.com"), &is_phishing));
+    EXPECT_TRUE(csd_service_->GetValidCachedResult(
+        GURL("http://fourth.url.com"), &is_phishing));
     EXPECT_TRUE(is_phishing);
   }
 
@@ -241,62 +241,31 @@ TEST_F(ClientSideDetectionServiceTest, SendClientReportPhishingRequest) {
                                   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_TRUE(SendClientReportPhishingRequest(url, score));
-
-  // This request will fail and should not be cached.
+  // This request will fail
   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));
-
-  // Verify that we allow cache refreshing even when requests are blocked.
-  base::Time cache_time = base::Time::Now() - base::TimeDelta::FromHours(1);
-  SetCache(second_url, true, cache_time);
-
-  // Even though this element is in the cache, it's not currently valid so
-  // we make request and return that value instead.
-  response.set_phishy(false);
-  SetClientReportPhishingResponse(response.SerializeAsString(),
-                                  true /* success */);
-  EXPECT_FALSE(SendClientReportPhishingRequest(second_url, score));
-
   base::Time after = base::Time::Now();
 
-  // Check that we have recorded 5 requests, all within the correct time range.
-  // The blocked request and the cached requests should not be present.
+  // Check that we have recorded all 3 requests within the correct time range.
   std::queue<base::Time>& report_times = GetPhishingReportTimes();
-  EXPECT_EQ(5U, report_times.size());
+  EXPECT_EQ(3U, report_times.size());
   while (!report_times.empty()) {
     base::Time time = report_times.back();
     report_times.pop();
     EXPECT_LE(before, time);
     EXPECT_GE(after, time);
   }
+
+  // Only the first url should be in the cache.
+  bool is_phishing;
+  EXPECT_TRUE(csd_service_->IsInCache(url));
+  EXPECT_TRUE(csd_service_->GetValidCachedResult(url, &is_phishing));
+  EXPECT_TRUE(is_phishing);
+  EXPECT_FALSE(csd_service_->IsInCache(second_url));
 }
 
 TEST_F(ClientSideDetectionServiceTest, GetNumReportTest) {