Implement exponential back-off mechanism and enforce it at the URLRequestHttpJob level.

chrome/common/net/url_fetcher.cc

Index: chrome/common/net/url_fetcher.cc
===================================================================
--- chrome/common/net/url_fetcher.cc	(revision 66188)
+++ chrome/common/net/url_fetcher.cc	(working copy)
@@ -14,13 +14,14 @@
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/thread.h"
-#include "chrome/common/net/url_fetcher_protect.h"
 #include "chrome/common/net/url_request_context_getter.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/load_flags.h"
 #include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
+#include "net/url_request/request_throttler_manager.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 
@@ -53,6 +54,9 @@
   // safe to call this multiple times.
   void Stop();
 
+  // Reports that the received content was malformed.
+  void ReceivedContentWasMalformed();
+
   // URLRequest::Delegate implementation.
   virtual void OnResponseStarted(URLRequest* request);
   virtual void OnReadCompleted(URLRequest* request, int bytes_read);
@@ -84,14 +88,20 @@
 
   // Wrapper functions that allow us to ensure actions happen on the right
   // thread.
+  void StartURLRequestNow();
   void StartURLRequest();

[Jói, 2010/11/21 19:31:40]

It might be more readable to name this one StartURLRequestWhenAllowed (as
opposed to StartURLRequestNow).

[yzshen, 2010/11/22 17:35:01]

Done.
Since the method tries to decide an appropriate sending time, taking into
account the exponential back-off time as well as the sliding window (which is
not enforced by the network level), so I decided to call it
StartURLRequestWhenAppropriate.

   void CancelURLRequest();
   void OnCompletedURLRequest(const URLRequestStatus& status);
+  void NotifyMalformedContent();
 
   // Deletes the request, removes it from the registry, and removes the
   // destruction observer.
   void ReleaseRequest();
 
+  // Returns the max value of exponential back-off release time for
+  // |original_url_| and |url_|.
+  base::TimeTicks GetBackoffReleaseTime();
+
   URLFetcher* fetcher_;              // Corresponding fetcher object
   GURL original_url_;                // The URL we were asked to fetch
   GURL url_;                         // The URL we eventually wound up at
@@ -116,20 +126,24 @@
   std::string upload_content_;       // HTTP POST payload
   std::string upload_content_type_;  // MIME type of POST payload
 
-  // The overload protection entry for this URL.  This is used to
-  // incrementally back off how rapidly we'll send requests to a particular
-  // URL, to avoid placing too much demand on the remote resource.  We update
-  // this with the status of all requests as they return, and in turn use it
-  // to determine how long to wait before making another request.
-  URLFetcherProtectEntry* protect_entry_;
+  // Used to determine how long to wait before making a request or doing a
+  // retry.
+  // Both of them can only be accessed on the IO thread.
+  scoped_refptr<RequestThrottlerEntryInterface> original_url_throttler_entry_;

[Jói, 2010/11/21 19:31:40]

It's non-obvious why you need two (entry vs. original entry); perhaps a line or
two here would help make it clearer.

[yzshen, 2010/11/22 17:35:01]

Done.

+  scoped_refptr<RequestThrottlerEntryInterface> url_throttler_entry_;
+
   // |num_retries_| indicates how many times we've failed to successfully
   // fetch this URL.  Once this value exceeds the maximum number of retries
-  // specified by the protection manager, we'll give up.
+  // specified by the owner URLFetcher instance, we'll give up.
   int num_retries_;
 
   // True if the URLFetcher has been cancelled.
   bool was_cancelled_;
 
+  // Since GetBackoffReleaseTime() can only be called on the IO thread, we cache
+  // its value to be used on the creating thread.
+  base::TimeTicks backoff_release_time_;

[Jói, 2010/11/21 19:31:40]

I only see this set in Core::OnReadCompleted but it's used in
Core::OnCompletedURLRequest without even checking if it's 0. Forgive my lack of
deep understanding of URLFetcher, but is it guaranteed that OnReadCompleted gets
called before OnCompletedURLRequest ?

[yzshen, 2010/11/22 17:35:01]

Since OnCompletedURLRequest is only called by OnReadCompleted, I thought it's
okay.

[Jói, 2010/11/22 21:16:27]

OK.

+
   static base::LazyInstance<Registry> g_registry;
 
   friend class URLFetcher;
@@ -170,7 +184,8 @@
                        Delegate* d)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
       core_(new Core(this, url, request_type, d))),
-      automatically_retry_on_5xx_(true) {
+      automatically_retry_on_5xx_(true),
+      max_retries_(0) {
 }
 
 URLFetcher::~URLFetcher() {
@@ -197,8 +212,9 @@
       load_flags_(net::LOAD_NORMAL),
       response_code_(-1),
       buffer_(new net::IOBuffer(kBufferSize)),
-      protect_entry_(URLFetcherProtectManager::GetInstance()->Register(
-          original_url_.host())),
+      original_url_throttler_entry_(
+          RequestThrottlerManager::GetInstance()->RegisterRequestUrl(
+              original_url)),
       num_retries_(0),
       was_cancelled_(false) {
 }
@@ -214,10 +230,9 @@
   CHECK(request_context_getter_) << "We need an URLRequestContext!";
   io_message_loop_proxy_ = request_context_getter_->GetIOMessageLoopProxy();
   CHECK(io_message_loop_proxy_.get()) << "We need an IO message loop proxy";
-  io_message_loop_proxy_->PostDelayedTask(
-      FROM_HERE,
-      NewRunnableMethod(this, &Core::StartURLRequest),
-          protect_entry_->UpdateBackoff(URLFetcherProtectEntry::SEND));
+
+  io_message_loop_proxy_->PostTask(
+      FROM_HERE, NewRunnableMethod(this, &Core::StartURLRequest));
 }
 
 void URLFetcher::Core::Stop() {
@@ -230,6 +245,14 @@
   }
 }
 
+void URLFetcher::Core::ReceivedContentWasMalformed() {
+  DCHECK_EQ(MessageLoop::current(), delegate_loop_);
+  if (io_message_loop_proxy_.get()) {
+    io_message_loop_proxy_->PostTask(
+        FROM_HERE, NewRunnableMethod(this, &Core::NotifyMalformedContent));
+  }
+}
+
 void URLFetcher::Core::CancelAll() {
   g_registry.Get().CancelAll();
 }
@@ -257,6 +280,8 @@
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
   url_ = request->url();
+  url_throttler_entry_ =
+      RequestThrottlerManager::GetInstance()->RegisterRequestUrl(url_);
 
   do {
     if (!request_->status().is_success() || bytes_read <= 0)
@@ -269,17 +294,19 @@
 
   // See comments re: HEAD requests in OnResponseStarted().
   if (!request_->status().is_io_pending() || (request_type_ == HEAD)) {
+    backoff_release_time_ = GetBackoffReleaseTime();
+
     delegate_loop_->PostTask(FROM_HERE, NewRunnableMethod(
         this, &Core::OnCompletedURLRequest, request_->status()));
     ReleaseRequest();
   }
 }
 
-void URLFetcher::Core::StartURLRequest() {
+void URLFetcher::Core::StartURLRequestNow() {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
   if (was_cancelled_) {
-    // Since StartURLRequest() is posted as a *delayed* task, it may
+    // Since StartURLRequestNow() is posted as a *delayed* task, it may
     // run after the URLFetcher was already stopped.
     return;
   }
@@ -325,6 +352,24 @@
   request_->Start();
 }
 
+void URLFetcher::Core::StartURLRequest() {
+  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+
+  if (was_cancelled_)
+    return;
+
+  int64 delay = original_url_throttler_entry_->ReserveSendingTimeForNextRequest(
+      GetBackoffReleaseTime());
+  if (delay == 0) {
+    StartURLRequestNow();
+  } else {
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        NewRunnableMethod(this, &Core::StartURLRequestNow),
+        delay);
+  }
+}
+
 void URLFetcher::Core::CancelURLRequest() {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
@@ -344,30 +389,28 @@
   DCHECK(MessageLoop::current() == delegate_loop_);
 
   // Checks the response from server.
-  if (response_code_ >= 500) {
+  if (response_code_ >= 500 ||
+      status.os_error() == net::ERR_TEMPORARILY_THROTTLED_BY_DDOS) {
     // When encountering a server error, we will send the request again
     // after backoff time.
-    int64 back_off_time =
-        protect_entry_->UpdateBackoff(URLFetcherProtectEntry::FAILURE);
-    if (delegate_) {
-      fetcher_->backoff_delay_ =
-          base::TimeDelta::FromMilliseconds(back_off_time);
-    }
     ++num_retries_;
     // Restarts the request if we still need to notify the delegate.
     if (delegate_) {
+      fetcher_->backoff_delay_ = backoff_release_time_ - base::TimeTicks::Now();
+      if (fetcher_->backoff_delay_ < base::TimeDelta())
+        fetcher_->backoff_delay_ = base::TimeDelta();
+
       if (fetcher_->automatically_retry_on_5xx_ &&
-          num_retries_ <= protect_entry_->max_retries()) {
-        io_message_loop_proxy_->PostDelayedTask(
+          num_retries_ <= fetcher_->max_retries()) {
+        io_message_loop_proxy_->PostTask(
             FROM_HERE,
-            NewRunnableMethod(this, &Core::StartURLRequest), back_off_time);
+            NewRunnableMethod(this, &Core::StartURLRequest));
       } else {
         delegate_->OnURLFetchComplete(fetcher_, url_, status, response_code_,
                                       cookies_, data_);
       }
     }
   } else {
-    protect_entry_->UpdateBackoff(URLFetcherProtectEntry::SUCCESS);
     if (delegate_) {
       fetcher_->backoff_delay_ = base::TimeDelta();
       delegate_->OnURLFetchComplete(fetcher_, url_, status, response_code_,
@@ -376,13 +419,35 @@
   }
 }
 
+void URLFetcher::Core::NotifyMalformedContent() {
+  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  if (url_throttler_entry_ != NULL)
+    url_throttler_entry_->ReceivedContentWasMalformed();
+}
+
 void URLFetcher::Core::ReleaseRequest() {
   request_.reset();
   g_registry.Get().RemoveURLFetcherCore(this);
 }
 
+base::TimeTicks URLFetcher::Core::GetBackoffReleaseTime() {
+  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+
+  base::TimeTicks original_url_backoff =
+      original_url_throttler_entry_->GetExponentialBackoffReleaseTime();
+  base::TimeTicks destination_url_backoff;
+  if (url_throttler_entry_ != NULL &&
+      original_url_throttler_entry_ != url_throttler_entry_) {
+    destination_url_backoff =
+        url_throttler_entry_->GetExponentialBackoffReleaseTime();
+  }
+
+  return original_url_backoff > destination_url_backoff ?
+      original_url_backoff : destination_url_backoff;
+}
+
 void URLFetcher::set_upload_data(const std::string& upload_content_type,
-                     const std::string& upload_content) {
+                                 const std::string& upload_content) {
   core_->upload_content_type_ = upload_content_type;
   core_->upload_content_ = upload_content;
 }
@@ -426,6 +491,10 @@
   return core_->url_;
 }
 
+void URLFetcher::ReceivedContentWasMalformed() {
+  core_->ReceivedContentWasMalformed();
+}
+
 // static
 void URLFetcher::CancelAll() {
   Core::CancelAll();