Use base::TimeDelta in SafeBrowsingProtocolManager.

chrome/browser/safe_browsing/protocol_manager.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 "chrome/browser/safe_browsing/protocol_manager.h"
 
 #ifndef NDEBUG
 #include "base/base64.h"
 #endif
 #include "base/environment.h"
@@ skipping 72 matching lines @@
     const std::string& client_name,
     net::URLRequestContextGetter* request_context_getter,
     const std::string& url_prefix,
     bool disable_auto_update)
     : sb_service_(sb_service),
       request_type_(NO_REQUEST),
       update_error_count_(0),
       gethash_error_count_(0),
       update_back_off_mult_(1),
       gethash_back_off_mult_(1),
-      next_update_sec_(-1),
+      next_update_interval_(base::TimeDelta::FromSeconds(
+          base::RandInt(60, kSbTimerStartIntervalSec))),
       update_state_(FIRST_REQUEST),
       chunk_pending_to_write_(false),
       update_size_(0),
       client_name_(client_name),
       request_context_getter_(request_context_getter),
       url_prefix_(url_prefix),
       disable_auto_update_(disable_auto_update) {
   DCHECK(!url_prefix_.empty());
 
   // Set the backoff multiplier fuzz to a random value between 0 and 1.
   back_off_fuzz_ = static_cast<float>(base::RandDouble());
-  // The first update must happen between 1-5 minutes of start up.
-  next_update_sec_ = base::RandInt(60, kSbTimerStartIntervalSec);
 
   chrome::VersionInfo version_info;
   if (!version_info.is_valid() || version_info.Version().empty())
     version_ = "0.1";
   else
     version_ = version_info.Version();
 }
 
 // static
 void SafeBrowsingProtocolManager::RecordGetHashResult(
@@ skipping 213 matching lines @@
       int next_update_sec = -1;
       bool reset = false;
       scoped_ptr<std::vector<SBChunkDelete> > chunk_deletes(
           new std::vector<SBChunkDelete>);
       std::vector<ChunkUrl> chunk_urls;
       if (!parser.ParseUpdate(data, length, &next_update_sec,
                               &reset, chunk_deletes.get(), &chunk_urls)) {
         return false;
       }
 
+      base::TimeDelta next_update_interval =
+          base::TimeDelta::FromSeconds(next_update_sec);
       last_update_ = Time::Now();
 
       if (update_state_ == FIRST_REQUEST)
         update_state_ = SECOND_REQUEST;
       else if (update_state_ == SECOND_REQUEST)
         update_state_ = NORMAL_REQUEST;
 
       // New time for the next update.
-      if (next_update_sec > 0) {
-        next_update_sec_ = next_update_sec;
+      if (next_update_interval > base::TimeDelta::FromSeconds(0)) {

[mattm, 2012/09/12 20:00:03]

These base::TimeDelta::FromSeconds(0)s could also just be written as
base::TimeDelta(), unless you think that makes it harder to read.

[cbentzel, 2012/09/12 21:32:47]

Done.

+        next_update_interval_ = next_update_interval;
       } else if (update_state_ == SECOND_REQUEST) {
-        next_update_sec_ = base::RandInt(15 * 60, 45 * 60);
+        next_update_interval_ = base::TimeDelta::FromSeconds(
+            base::RandInt(15, 45));
       }
 
       // New chunks to download.
       if (!chunk_urls.empty()) {
         UMA_HISTOGRAM_COUNTS("SB2.UpdateUrls", chunk_urls.size());
         for (size_t i = 0; i < chunk_urls.size(); ++i)
           chunk_request_urls_.push_back(chunk_urls[i]);
       }
 
       // Handle the case were the SafeBrowsing service tells us to dump our
@@ skipping 51 matching lines @@
 void SafeBrowsingProtocolManager::Initialize() {
   // Don't want to hit the safe browsing servers on build/chrome bots.
   scoped_ptr<base::Environment> env(base::Environment::Create());
   if (env->HasVar(env_vars::kHeadless))
     return;
 
   ScheduleNextUpdate(false /* no back off */);
 }
 
 void SafeBrowsingProtocolManager::ScheduleNextUpdate(bool back_off) {
-  DCHECK_GT(next_update_sec_, 0);
-
   if (disable_auto_update_) {
     // Unschedule any current timer.
     update_timer_.Stop();
     return;
   }
   // Reschedule with the new update.
-  const int next_update = GetNextUpdateTime(back_off);
-  ForceScheduleNextUpdate(next_update);
+  base::TimeDelta next_update_interval = GetNextUpdateInterval(back_off);
+  ForceScheduleNextUpdate(next_update_interval);
 }
 
 void SafeBrowsingProtocolManager::ForceScheduleNextUpdate(
-    const int next_update_msec) {
-  DCHECK_GE(next_update_msec, 0);
+    base::TimeDelta interval) {
+  DCHECK(interval > base::TimeDelta::FromSeconds(0));

[mattm, 2012/09/12 20:00:03]

Do the DCHECK_GE & friends not work with TimeDeltas?

(also this should be >=)

[cbentzel, 2012/09/12 21:32:47]

Thanks for the catch.

They don't work because operator<< isn't defined.

   // Unschedule any current timer.
   update_timer_.Stop();
-  update_timer_.Start(FROM_HERE, TimeDelta::FromMilliseconds(next_update_msec),
-                      this, &SafeBrowsingProtocolManager::GetNextUpdate);
+  update_timer_.Start(FROM_HERE, interval, this,
+                      &SafeBrowsingProtocolManager::GetNextUpdate);
 }
 
 // According to section 5 of the SafeBrowsing protocol specification, we must
-// back off after a certain number of errors. We only change 'next_update_sec_'
+// back off after a certain number of errors. We only change |next_update_sec_|
 // when we receive a response from the SafeBrowsing service.
-int SafeBrowsingProtocolManager::GetNextUpdateTime(bool back_off) {
-  int next = next_update_sec_;
+base::TimeDelta SafeBrowsingProtocolManager::GetNextUpdateInterval(
+    bool back_off) {
+  DCHECK(next_update_interval_ > base::TimeDelta::FromSeconds(0));
+  base::TimeDelta next = next_update_interval_;
   if (back_off) {
-    next = GetNextBackOffTime(&update_error_count_, &update_back_off_mult_);
+    next = GetNextBackOffInterval(&update_error_count_, &update_back_off_mult_);
   } else {
     // Successful response means error reset.
     update_error_count_ = 0;
     update_back_off_mult_ = 1;
   }
-  return next * 1000;  // milliseconds
+  return next;
 }
 
-int SafeBrowsingProtocolManager::GetNextBackOffTime(int* error_count,
-                                                    int* multiplier) {
+base::TimeDelta SafeBrowsingProtocolManager::GetNextBackOffInterval(
+    int* error_count, int* multiplier) const {
   DCHECK(multiplier && error_count);
   (*error_count)++;
   if (*error_count > 1 && *error_count < 6) {
-    int next = static_cast<int>(*multiplier * (1 + back_off_fuzz_) * 30 * 60);
+    base::TimeDelta next = base::TimeDelta::FromMinutes(
+        *multiplier * (1 + back_off_fuzz_) * 30);
     *multiplier *= 2;
     if (*multiplier > kSbMaxBackOff)
       *multiplier = kSbMaxBackOff;
     return next;
   }
-
   if (*error_count >= 6)
-    return 60 * 60 * 8;  // 8 hours
-
-  return 60;  // 1 minute
+    return base::TimeDelta::FromHours(8);
+  return base::TimeDelta::FromMinutes(1);
 }
 
 // This request requires getting a list of all the chunks for each list from the
 // database asynchronously. The request will be issued when we're called back in
 // OnGetChunksComplete.
 // TODO(paulg): We should get this at start up and maintain a ChunkRange cache
 //              to avoid hitting the database with each update request. On the
 //              otherhand, this request will only occur ~20-30 minutes so there
 //              isn't that much overhead. Measure!
 void SafeBrowsingProtocolManager::IssueUpdateRequest() {
@@ skipping 138 matching lines @@
   }
   if (!list.subs.empty()) {
     formatted_results.append("s:" + list.subs);
   }
   formatted_results.append("\n");
 
   return formatted_results;
 }
 
 void SafeBrowsingProtocolManager::HandleGetHashError(const Time& now) {
-  int next = GetNextBackOffTime(&gethash_error_count_, &gethash_back_off_mult_);
-  next_gethash_time_ = now + TimeDelta::FromSeconds(next);
+  base::TimeDelta next = GetNextBackOffInterval(
+      &gethash_error_count_, &gethash_back_off_mult_);
+  next_gethash_time_ = now + next;
 }
 
 void SafeBrowsingProtocolManager::UpdateFinished(bool success) {
   UMA_HISTOGRAM_COUNTS("SB2.UpdateSize", update_size_);
   update_size_ = 0;
   sb_service_->UpdateFinished(success);
 }
 
 std::string SafeBrowsingProtocolManager::ComposeUrl(
     const std::string& prefix, const std::string& method,
@@ skipping 86 matching lines @@
   if (!additional_query_.empty()) {
     if (next_url.find("?") != std::string::npos) {
       next_url.append("&");
     } else {
       next_url.append("?");
     }
     next_url.append(additional_query_);
   }
   return GURL(next_url);
 }