Adds backoff and Min Wait Duration compliance to Protocolmanager pver4 handlers.

chrome/browser/safe_browsing/protocol_manager.cc

Index: chrome/browser/safe_browsing/protocol_manager.cc
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 02d1c536d148e0c21faa083e2267def710510f45..532b186ce73f3ad691ef02b95fa37bd3c6639a0a 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -133,6 +133,30 @@ SafeBrowsingProtocolManager* SafeBrowsingProtocolManager::Create(
                                          config);
 }
 
+// static
+// Backoff interval is MIN(((2^(n-1))*15 minutes) * (RAND + 1), 24 hours) where

[Nathan Parker, 2016/01/12 04:14:38]

Is there a public doc we can ref with this spec? I think it's not public yet...
but awoz might have one in the works.

[kcarattini, 2016/01/12 05:18:06]

I don't know of one either.

+// n is the number of consecutive errors.
+base::TimeDelta SafeBrowsingProtocolManager::GetNextV4BackOffInterval(
+    size_t* error_count,
+    size_t* multiplier) {
+  DCHECK(multiplier && error_count);
+  (*error_count)++;
+  if (*error_count > 1 && *error_count < 9) {
+    // With error count 9 and above we will hit the 24 hour max interval.
+    // Cap the multiplier here to prevent integer overflow errors.
+    *multiplier *= 2;
+  }
+  base::TimeDelta next = base::TimeDelta::FromMinutes(
+      *multiplier * (1 + base::RandDouble()) * 15);
+
+  base::TimeDelta day = base::TimeDelta::FromHours(24);
+
+  if (next < day)
+    return next;
+  else
+    return day;
+}
+
 SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
     SafeBrowsingProtocolManagerDelegate* delegate,
     net::URLRequestContextGetter* request_context_getter,
@@ -141,12 +165,15 @@ SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
       request_type_(NO_REQUEST),
       update_error_count_(0),
       gethash_error_count_(0),
+      gethash_v4_error_count_(0),
       update_back_off_mult_(1),
       gethash_back_off_mult_(1),
+      gethash_v4_back_off_mult_(1),
       next_update_interval_(base::TimeDelta::FromSeconds(
           base::RandInt(kSbTimerStartIntervalSecMin,
                         kSbTimerStartIntervalSecMax))),
       chunk_pending_to_write_(false),
+      next_gethash_v4_time_(Time::Now()),

[Nathan Parker, 2016/01/12 04:14:38]

How about just zero?  Then if the clock drifts backwards slightly, we don't have
an issue.

[kcarattini, 2016/01/12 05:18:06]

Done.

       version_(config.version),
       update_size_(0),
       client_name_(config.client_name),
@@ -280,6 +307,12 @@ bool SafeBrowsingProtocolManager::ParseV4HashResponse(
         response.negative_cache_duration().seconds());
   }
 
+  if (response.has_minimum_wait_duration()) {
+    // Seconds resolution is good enough so we ignore the nanos field.
+    next_gethash_v4_time_ = Time::Now() + base::TimeDelta::FromSeconds(
+        response.minimum_wait_duration().seconds());
+  }
+
   // Loop over the threat matches and fill in full_hashes.
   for (const ThreatMatch& match : response.matches()) {
     // Make sure the platform and threat entry type match.
@@ -324,7 +357,16 @@ void SafeBrowsingProtocolManager::GetV4FullHashes(
     ThreatType threat_type,
     FullHashCallback callback) {
   DCHECK(CalledOnValidThread());
-  // TODO(kcarattini): Implement backoff behavior.
+  // We need to wait the minimum waiting duration, and if we are in backoff,
+  // we need to check if we're past the next allowed time. If we are, we can
+  // proceed with the request. If not, we are required to return empty results
+  // (i.e. treat the page as safe).
+  if (Time::Now() <= next_gethash_v4_time_) {
+    // TODO(kcarattini): Add UMA recording.
+    std::vector<SBFullHashResult> full_hashes;
+    callback.Run(full_hashes, base::TimeDelta());
+    return;
+  }
 
   std::string req_base64 = GetV4HashRequest(prefixes, platforms, threat_type);
   GURL gethash_url = GetV4HashUrl(req_base64);
@@ -431,8 +473,8 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
     base::TimeDelta negative_cache_duration;
     if (status.is_success() && response_code == net::HTTP_OK) {
       // TODO(kcarattini): Add UMA reporting.
-      // TODO(kcarattini): Implement backoff and minimum waiting duration
-      // compliance.
+      gethash_v4_error_count_ = 0;

[Nathan Parker, 2016/01/12 04:14:38]

Might be tidier to put this in a ResetGetHashV4Errors() function so these values
are only set in a few small functions.

[kcarattini, 2016/01/12 05:18:06]

Done.

+      gethash_v4_back_off_mult_ = 1;
       std::string data;
       source->GetResponseAsString(&data);
       if (!ParseV4HashResponse(data, &full_hashes, &negative_cache_duration)) {
@@ -440,7 +482,7 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
         // TODO(kcarattini): Add UMA reporting.
       }
     } else {
-      // TODO(kcarattini): Handle error by setting backoff interval.
+      HandleGetHashV4Error(Time::Now());
       // TODO(kcarattini): Add UMA reporting.
       if (status.status() == net::URLRequestStatus::FAILED) {
         DVLOG(1) << "SafeBrowsing GetEncodedFullHashes request for: " <<
@@ -864,6 +906,13 @@ void SafeBrowsingProtocolManager::HandleGetHashError(const Time& now) {
   next_gethash_time_ = now + next;
 }
 
+void SafeBrowsingProtocolManager::HandleGetHashV4Error(const Time& now) {
+  DCHECK(CalledOnValidThread());
+  base::TimeDelta next = GetNextV4BackOffInterval(
+      &gethash_v4_error_count_, &gethash_v4_back_off_mult_);
+  next_gethash_v4_time_ = now + next;
+}
+
 void SafeBrowsingProtocolManager::UpdateFinished(bool success) {
   UpdateFinished(success, !success);
 }