Fix the root cause of flakiness in SafeBrowsingProtocolManagerTest.TestGetHashBackOffTimes

and undisable the test.

TEST=none
http://crbug.com/1880

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

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

3 files changed, 15 insertions, 16 deletions

diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 9740161..a720bcc 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -222,7 +222,7 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
           HandleReKey();
       }
     } else if (response_code >= 300) {
-      HandleGetHashError();
+      HandleGetHashError(Time::Now());
       SB_DLOG(INFO) << "SafeBrowsing GetHash request for: " << source->url()
                     << ", failed with error: " << response_code;
     }
@@ -626,9 +626,9 @@ void SafeBrowsingProtocolManager::HandleReKey() {
   IssueKeyRequest();
 }
 
-void SafeBrowsingProtocolManager::HandleGetHashError() {
+void SafeBrowsingProtocolManager::HandleGetHashError(const Time& now) {
   int next = GetNextBackOffTime(&gethash_error_count_, &gethash_back_off_mult_);
-  next_gethash_time_ = Time::Now() + TimeDelta::FromSeconds(next);
+  next_gethash_time_ = now + TimeDelta::FromSeconds(next);
 }
 
 void SafeBrowsingProtocolManager::UpdateFinished(bool success) {
diff --git a/chrome/browser/safe_browsing/protocol_manager.h b/chrome/browser/safe_browsing/protocol_manager.h
index ad6ec2f..ff67795 100644
--- a/chrome/browser/safe_browsing/protocol_manager.h
+++ b/chrome/browser/safe_browsing/protocol_manager.h
@@ -46,8 +46,7 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
   // Testing friends:
   FRIEND_TEST(SafeBrowsingProtocolManagerTest, TestBackOffTimes);
   FRIEND_TEST(SafeBrowsingProtocolManagerTest, TestChunkStrings);
-  FRIEND_TEST(SafeBrowsingProtocolManagerTest,
-              DISABLED_TestGetHashBackOffTimes);
+  FRIEND_TEST(SafeBrowsingProtocolManagerTest, TestGetHashBackOffTimes);
 
  public:
   SafeBrowsingProtocolManager(SafeBrowsingService* sb_service,
@@ -146,8 +145,9 @@ class SafeBrowsingProtocolManager : public URLFetcher::Delegate {
   // issue the request.
   void HandleReKey();
 
-  // Update internal state for each GetHash response error.
-  void HandleGetHashError();
+  // Update internal state for each GetHash response error, assuming that the
+  // current time is |now|.
+  void HandleGetHashError(const base::Time& now);
 
   // Helper function for update completion.
   void UpdateFinished(bool success);
diff --git a/chrome/browser/safe_browsing/protocol_manager_unittest.cc b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
index 57514ae..8af7a1b 100644
--- a/chrome/browser/safe_browsing/protocol_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
@@ -86,8 +86,7 @@ TEST_F(SafeBrowsingProtocolManagerTest, TestChunkStrings) {
   EXPECT_EQ(pm.FormatList(phish, true), "goog-phish-shavar;mac\n");
 }
 
-// Flakey, see http://code.google.com/p/chromium/issues/detail?id=1880
-TEST_F(SafeBrowsingProtocolManagerTest, DISABLED_TestGetHashBackOffTimes) {
+TEST_F(SafeBrowsingProtocolManagerTest, TestGetHashBackOffTimes) {
   SafeBrowsingProtocolManager pm(NULL, NULL, "", "");
 
   // No errors or back off time yet.
@@ -97,7 +96,7 @@ TEST_F(SafeBrowsingProtocolManagerTest, DISABLED_TestGetHashBackOffTimes) {
   Time now = Time::Now();
 
   // 1 error.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 1);
   TimeDelta margin = TimeDelta::FromSeconds(5);  // Fudge factor.
   Time future = now + TimeDelta::FromMinutes(1);
@@ -105,36 +104,36 @@ TEST_F(SafeBrowsingProtocolManagerTest, DISABLED_TestGetHashBackOffTimes) {
               pm.next_gethash_time_ <= future + margin);
 
   // 2 errors.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 2);
   EXPECT_TRUE(pm.next_gethash_time_ >= now + TimeDelta::FromMinutes(30));
   EXPECT_TRUE(pm.next_gethash_time_ <= now + TimeDelta::FromMinutes(60));
 
   // 3 errors.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 3);
   EXPECT_TRUE(pm.next_gethash_time_ >= now + TimeDelta::FromMinutes(60));
   EXPECT_TRUE(pm.next_gethash_time_ <= now + TimeDelta::FromMinutes(120));
 
   // 4 errors.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 4);
   EXPECT_TRUE(pm.next_gethash_time_ >= now + TimeDelta::FromMinutes(120));
   EXPECT_TRUE(pm.next_gethash_time_ <= now + TimeDelta::FromMinutes(240));
 
   // 5 errors.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 5);
   EXPECT_TRUE(pm.next_gethash_time_ >= now + TimeDelta::FromMinutes(240));
   EXPECT_TRUE(pm.next_gethash_time_ <= now + TimeDelta::FromMinutes(480));
 
   // 6 errors, reached max backoff.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 6);
   EXPECT_TRUE(pm.next_gethash_time_ == now + TimeDelta::FromMinutes(480));
 
   // 7 errors.
-  pm.HandleGetHashError();
+  pm.HandleGetHashError(now);
   EXPECT_EQ(pm.gethash_error_count_, 7);
   EXPECT_TRUE(pm.next_gethash_time_== now + TimeDelta::FromMinutes(480));
 }