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

chrome/common/net/url_fetcher_unittest.cc

Index: chrome/common/net/url_fetcher_unittest.cc
===================================================================
--- chrome/common/net/url_fetcher_unittest.cc	(revision 66188)
+++ chrome/common/net/url_fetcher_unittest.cc	(working copy)
@@ -8,11 +8,11 @@
 #include "build/build_config.h"
 #include "chrome/common/chrome_plugin_lib.h"
 #include "chrome/common/net/url_fetcher.h"
-#include "chrome/common/net/url_fetcher_protect.h"
 #include "chrome/common/net/url_request_context_getter.h"
 #include "net/http/http_response_headers.h"
-#include "net/url_request/url_request_unittest.h"
 #include "net/test/test_server.h"
+#include "net/url_request/request_throttler_manager.h"
+#include "net/url_request/url_request_unittest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(USE_NSS)
@@ -312,6 +312,7 @@
   fetcher_->set_request_context(new TestURLRequestContextGetter(
       io_message_loop_proxy()));
   start_time_ = Time::Now();
+  fetcher_->set_max_retries(11);

[willchan no longer on Chromium, 2010/11/24 00:23:07]

11 is a magic number.  Does this have any specific meaning?  Perhaps you can use
a named constant instead.

[yzshen, 2010/11/24 09:42:08]

It is only meaningful for this specific test. I had a hard time to come up with
a name better than kMaxRetriesForURLFetcherProtectTest. :(

I think we have a lot of magic number in this file (say, line 338). And I
haven't added this magic number, I just moved it here from the other part of
this file.

I personally think it is fine. Please let me know if you have better idea. :)

   fetcher_->Start();
 }
 
@@ -338,7 +339,7 @@
       fetcher_->Start();
     } else {
       // We have already sent 20 requests continuously. And we expect that
-      // it takes more than 1 second due to the overload pretection settings.
+      // it takes more than 1 second due to the overload protection settings.
       EXPECT_TRUE(Time::Now() - start_time_ >= one_second);
       URLFetcherTest::OnURLFetchComplete(source, url, status, response_code,
                                          cookies, data);
@@ -352,6 +353,7 @@
       io_message_loop_proxy()));
   fetcher_->set_automatically_retry_on_5xx(false);
   start_time_ = Time::Now();
+  fetcher_->set_max_retries(11);
   fetcher_->Start();
 }
 
@@ -419,6 +421,7 @@
   CancelTestURLRequestContextGetter* context_getter =
       new CancelTestURLRequestContextGetter(io_message_loop_proxy());
   fetcher_->set_request_context(context_getter);
+  fetcher_->set_max_retries(2);
   fetcher_->Start();
   // We need to wait for the creation of the URLRequestContext, since we
   // rely on it being destroyed as a signal to end the test.
@@ -500,14 +503,15 @@
 
   // Registers an entry for test url. It only allows 3 requests to be sent
   // in 200 milliseconds.
-  URLFetcherProtectManager* manager = URLFetcherProtectManager::GetInstance();
-  URLFetcherProtectEntry* entry =
-      new URLFetcherProtectEntry(200, 3, 11, 1, 2.0, 0, 256);
-  manager->Register(url.host(), entry);
+  scoped_refptr<RequestThrottlerEntry> entry(new RequestThrottlerEntry(
+      200, 3, 1, 0, 2.0, 0.0, 256));
+  RequestThrottlerManager::GetInstance()->OverrideEntryForTests(url, entry);
 
   CreateFetcher(url);
 
   MessageLoop::current()->Run();
+
+  RequestThrottlerManager::GetInstance()->EraseEntryForTests(url);
 }
 
 TEST_F(URLFetcherProtectTest, ServerUnavailable) {
@@ -520,14 +524,15 @@
   //     new_backoff = 2.0 * old_backoff + 0
   // and maximum backoff time is 256 milliseconds.
   // Maximum retries allowed is set to 11.
-  URLFetcherProtectManager* manager = URLFetcherProtectManager::GetInstance();
-  URLFetcherProtectEntry* entry =
-      new URLFetcherProtectEntry(200, 3, 11, 1, 2.0, 0, 256);
-  manager->Register(url.host(), entry);
+  scoped_refptr<RequestThrottlerEntry> entry(new RequestThrottlerEntry(
+      200, 3, 1, 0, 2.0, 0.0, 256));
+  RequestThrottlerManager::GetInstance()->OverrideEntryForTests(url, entry);
 
   CreateFetcher(url);
 
   MessageLoop::current()->Run();
+
+  RequestThrottlerManager::GetInstance()->EraseEntryForTests(url);
 }
 
 TEST_F(URLFetcherProtectTestPassedThrough, ServerUnavailablePropagateResponse) {
@@ -538,21 +543,21 @@
 
   // Registers an entry for test url. The backoff time is calculated by:
   //     new_backoff = 2.0 * old_backoff + 0
-  // and maximum backoff time is 256 milliseconds.
+  // and maximum backoff time is 150000 milliseconds.
   // Maximum retries allowed is set to 11.
-  URLFetcherProtectManager* manager = URLFetcherProtectManager::GetInstance();
+  scoped_refptr<RequestThrottlerEntry> entry(new RequestThrottlerEntry(
+      200, 3, 100, 0, 2.0, 0.0, 150000));
   // Total time if *not* for not doing automatic backoff would be 150s.
   // In reality it should be "as soon as server responds".
-  URLFetcherProtectEntry* entry =
-      new URLFetcherProtectEntry(200, 3, 11, 100, 2.0, 0, 150000);
-  manager->Register(url.host(), entry);
+  RequestThrottlerManager::GetInstance()->OverrideEntryForTests(url, entry);
 
   CreateFetcher(url);
 
   MessageLoop::current()->Run();
+
+  RequestThrottlerManager::GetInstance()->EraseEntryForTests(url);
 }
 
-
 TEST_F(URLFetcherBadHTTPSTest, BadHTTPSTest) {
   net::TestServer::HTTPSOptions https_options(
       net::TestServer::HTTPSOptions::CERT_EXPIRED);
@@ -573,10 +578,9 @@
   //     new_backoff = 2.0 * old_backoff + 0
   // The initial backoff is 2 seconds and maximum backoff is 4 seconds.
   // Maximum retries allowed is set to 2.
-  URLFetcherProtectManager* manager = URLFetcherProtectManager::GetInstance();
-  URLFetcherProtectEntry* entry =
-      new URLFetcherProtectEntry(200, 3, 2, 2000, 2.0, 0, 4000);
-  manager->Register(url.host(), entry);
+  scoped_refptr<RequestThrottlerEntry> entry(new RequestThrottlerEntry(
+      200, 3, 2000, 0, 2.0, 0.0, 4000));
+  RequestThrottlerManager::GetInstance()->OverrideEntryForTests(url, entry);
 
   // Create a separate thread that will create the URLFetcher.  The current
   // (main) thread will do the IO, and when the fetch is complete it will
@@ -588,6 +592,8 @@
   t.message_loop()->PostTask(FROM_HERE, new FetcherWrapperTask(this, url));
 
   MessageLoop::current()->Run();
+
+  RequestThrottlerManager::GetInstance()->EraseEntryForTests(url);
 }
 
 TEST_F(URLFetcherCancelTest, CancelWhileDelayedStartTaskPending) {
@@ -597,22 +603,13 @@
   GURL url(test_server.GetURL("files/server-unavailable.html"));
 
   // Register an entry for test url.
-  //
-  // Ideally we would mock URLFetcherProtectEntry to return XXX seconds
-  // in response to entry->UpdateBackoff(SEND).
-  //
-  // Unfortunately this function is time sensitive, so we fudge some numbers
-  // to make it at least somewhat likely to have a non-zero deferred
-  // delay when running.
-  //
-  // Using a sliding window of 2 seconds, and max of 1 request, under a fast
+  // Using a sliding window of 4 seconds, and max of 1 request, under a fast
   // run we expect to have a 4 second delay when posting the Start task.
-  URLFetcherProtectManager* manager = URLFetcherProtectManager::GetInstance();
-  URLFetcherProtectEntry* entry =
-      new URLFetcherProtectEntry(2000, 1, 2, 2000, 2.0, 0, 4000);
-  EXPECT_EQ(0, entry->UpdateBackoff(URLFetcherProtectEntry::SEND));
-  entry->UpdateBackoff(URLFetcherProtectEntry::SEND);  // Returns about 2000.
-  manager->Register(url.host(), entry);
+  scoped_refptr<RequestThrottlerEntry> entry(new RequestThrottlerEntry(
+      4000, 1, 2000, 0, 2.0, 0.0, 4000));
+  RequestThrottlerManager::GetInstance()->OverrideEntryForTests(url, entry);
+  // Fake that a request has just started.
+  entry->ReserveSendingTimeForNextRequest(base::TimeTicks());
 
   // The next request we try to send will be delayed by ~4 seconds.
   // The slower the test runs, the less the delay will be (since it takes the
@@ -623,6 +620,8 @@
   t.message_loop()->PostTask(FROM_HERE, new FetcherWrapperTask(this, url));
 
   MessageLoop::current()->Run();
+
+  RequestThrottlerManager::GetInstance()->EraseEntryForTests(url);
 }
 
 }  // namespace.