Fix flaky DHCP WPAD tests. In part, switch to deterministic ways of

deciding test state. Where this is not possible, use more conservative
timeouts via TestTimeouts.

BUG=82988,82991
TEST=net_unittests

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

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

2 files changed, 39 insertions, 26 deletions

diff --git a/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc b/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
index afb9377..36d137c 100644
--- a/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
+++ b/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/perftimer.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/test/test_timeouts.h"
 #include "base/timer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
@@ -35,7 +36,7 @@ class MockDhcpProxyScriptAdapterFetcher
   explicit MockDhcpProxyScriptAdapterFetcher(URLRequestContext* context)
       : DhcpProxyScriptAdapterFetcher(context),
         dhcp_delay_ms_(1),
-        timeout_ms_(100),
+        timeout_ms_(TestTimeouts::action_timeout_ms()),
         configured_url_(kPacUrl),
         fetcher_delay_ms_(1),
         fetcher_result_(OK),
@@ -156,9 +157,7 @@ class FetcherClient {
   string16 pac_text_;
 };
 
-// Seems to be flaky under TSAN.
-// http://crbug.com/82991
-TEST(DhcpProxyScriptAdapterFetcher, FLAKY_NormalCaseURLNotInDhcp) {
+TEST(DhcpProxyScriptAdapterFetcher, NormalCaseURLNotInDhcp) {
   FetcherClient client;
   client.fetcher_->configured_url_ = "";
   client.RunTest();
@@ -178,7 +177,7 @@ TEST(DhcpProxyScriptAdapterFetcher, NormalCaseURLInDhcp) {
   EXPECT_EQ(GURL(kPacUrl), client.fetcher_->GetPacURL());
 }
 
-TEST(DhcpProxyScriptAdapterFetcher, FLAKY_TimeoutDuringDhcp) {
+TEST(DhcpProxyScriptAdapterFetcher, TimeoutDuringDhcp) {
   // Does a Fetch() with a long enough delay on accessing DHCP that the
   // fetcher should time out.  This is to test a case manual testing found,
   // where under certain circumstances (e.g. adapter enabled for DHCP and
@@ -186,16 +185,15 @@ TEST(DhcpProxyScriptAdapterFetcher, FLAKY_TimeoutDuringDhcp) {
   // present on the network) accessing DHCP can take on the order of tens
   // of seconds.
   FetcherClient client;
-  client.fetcher_->dhcp_delay_ms_ = 20 * 1000;
+  client.fetcher_->dhcp_delay_ms_ = TestTimeouts::action_max_timeout_ms();
   client.fetcher_->timeout_ms_ = 25;
 
   PerfTimer timer;
   client.RunTest();
+  // An error different from this would be received if the timeout didn't
+  // kick in.
   client.WaitForResult(ERR_TIMED_OUT);
 
-  // The timeout should occur within about 25 ms, way before the 20s set as
-  // the API delay above.
-  ASSERT_GT(base::TimeDelta::FromMilliseconds(35), timer.Elapsed());
   ASSERT_TRUE(client.fetcher_->DidFinish());
   EXPECT_EQ(ERR_TIMED_OUT, client.fetcher_->GetResult());
   EXPECT_EQ(string16(L""), client.fetcher_->GetPacScript());
@@ -205,7 +203,6 @@ TEST(DhcpProxyScriptAdapterFetcher, FLAKY_TimeoutDuringDhcp) {
 
 TEST(DhcpProxyScriptAdapterFetcher, CancelWhileDhcp) {
   FetcherClient client;
-  client.fetcher_->dhcp_delay_ms_ = 10;
   client.RunTest();
   client.fetcher_->Cancel();
   MessageLoop::current()->RunAllPending();
diff --git a/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc b/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
index 9a8bab5..ec2134f 100644
--- a/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
+++ b/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/message_loop.h"
 #include "base/perftimer.h"
 #include "base/rand_util.h"
+#include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "net/base/completion_callback.h"
 #include "net/proxy/dhcp_proxy_script_adapter_fetcher_win.h"
@@ -263,8 +264,8 @@ class DummyDhcpProxyScriptAdapterFetcher
 class MockDhcpProxyScriptFetcherWin : public DhcpProxyScriptFetcherWin {
  public:
   MockDhcpProxyScriptFetcherWin()
-      : DhcpProxyScriptFetcherWin(new TestURLRequestContext()),
-        next_adapter_fetcher_index_(0) {
+      : DhcpProxyScriptFetcherWin(new TestURLRequestContext()) {
+    ResetTestState();
   }
 
   // Adds a fetcher object to the queue of fetchers used by
@@ -298,7 +299,7 @@ class MockDhcpProxyScriptFetcherWin : public DhcpProxyScriptFetcherWin {
   }
 
   int ImplGetMaxWaitMs() OVERRIDE {
-    return 25;
+    return max_wait_ms_;
   }
 
   void ResetTestState() {
@@ -306,6 +307,11 @@ class MockDhcpProxyScriptFetcherWin : public DhcpProxyScriptFetcherWin {
     adapter_fetchers_.clear();
     // String pointers contained herein will have been freed during test.
     adapter_names_.clear();
+    max_wait_ms_ = TestTimeouts::tiny_timeout_ms();
+  }
+
+  bool HasPendingFetchers() {
+    return num_pending_fetchers_ > 0;
   }
 
   int next_adapter_fetcher_index_;
@@ -315,6 +321,8 @@ class MockDhcpProxyScriptFetcherWin : public DhcpProxyScriptFetcherWin {
   std::vector<DhcpProxyScriptAdapterFetcher*> adapter_fetchers_;
 
   std::vector<std::string> adapter_names_;
+
+  int max_wait_ms_;
 };
 
 class FetcherClient {
@@ -404,7 +412,8 @@ void TestNormalCaseURLConfiguredMultipleAdaptersWithTimeout(
       "most_preferred", true, ERR_PAC_NOT_IN_DHCP, L"", 1);
   // This will time out.
   client->fetcher_.ConfigureAndPushBackAdapter(
-      "second", false, ERR_IO_PENDING, L"bingo", 1000);
+      "second", false, ERR_IO_PENDING, L"bingo",
+      TestTimeouts::action_timeout_ms());
   client->fetcher_.ConfigureAndPushBackAdapter(
       "third", true, OK, L"rocko", 1);
   client->RunTest();
@@ -425,7 +434,8 @@ void TestFailureCaseURLConfiguredMultipleAdaptersWithTimeout(
       "most_preferred", true, ERR_PAC_NOT_IN_DHCP, L"", 1);
   // This will time out.
   client->fetcher_.ConfigureAndPushBackAdapter(
-      "second", false, ERR_IO_PENDING, L"bingo", 1000);
+      "second", false, ERR_IO_PENDING, L"bingo",
+      TestTimeouts::action_timeout_ms());
   // This is the first non-ERR_PAC_NOT_IN_DHCP error and as such
   // should be chosen.
   client->fetcher_.ConfigureAndPushBackAdapter(
@@ -449,7 +459,8 @@ void TestFailureCaseNoURLConfigured(FetcherClient* client) {
       "most_preferred", true, ERR_PAC_NOT_IN_DHCP, L"", 1);
   // This will time out.
   client->fetcher_.ConfigureAndPushBackAdapter(
-      "second", false, ERR_IO_PENDING, L"bingo", 1000);
+      "second", false, ERR_IO_PENDING, L"bingo",
+      TestTimeouts::action_timeout_ms());
   // This is the first non-ERR_PAC_NOT_IN_DHCP error and as such
   // should be chosen.
   client->fetcher_.ConfigureAndPushBackAdapter(
@@ -487,26 +498,31 @@ void TestShortCircuitLessPreferredAdapters(FetcherClient* client) {
   client->fetcher_.ConfigureAndPushBackAdapter(
       "2", true, OK, L"bingo", 1);
   client->fetcher_.ConfigureAndPushBackAdapter(
-      "3", true, OK, L"wrongo", 1000);
+      "3", true, OK, L"wrongo", TestTimeouts::action_max_timeout_ms());
+
+  // Increase the timeout to ensure the short circuit mechanism has
+  // time to kick in before the timeout waiting for more adapters kicks in.
+  client->fetcher_.max_wait_ms_ = TestTimeouts::action_timeout_ms();
 
   PerfTimer timer;
   client->RunTest();
   client->RunMessageLoopUntilComplete();
-  // Assert that the time passed is just less than the wait timer
-  // timeout (which we have mocked out above to be 25 ms), to avoid
-  // flakiness but still get a strong signal that it was the shortcut
-  // mechanism (in OnFetcherDone) that kicked in.
-  ASSERT_GT(TimeDelta::FromMilliseconds(23), timer.Elapsed());
+  ASSERT_TRUE(client->fetcher_.HasPendingFetchers());
+  // Assert that the time passed is definitely less than the wait timer
+  // timeout, to get a second signal that it was the shortcut mechanism
+  // (in OnFetcherDone) that kicked in, and not the timeout waiting for
+  // more adapters.
+  ASSERT_GT(base::TimeDelta::FromMilliseconds(
+      client->fetcher_.max_wait_ms_ - (client->fetcher_.max_wait_ms_ / 10)),
+      timer.Elapsed());
 }
 
-// Seems to be flaky under TSAN. http://crbug.com/82991
-TEST(DhcpProxyScriptFetcherWin, FLAKY_ShortCircuitLessPreferredAdapters) {
+TEST(DhcpProxyScriptFetcherWin, ShortCircuitLessPreferredAdapters) {
   FetcherClient client;
   TestShortCircuitLessPreferredAdapters(&client);
 }
 
-// Seems to be flaky under TSAN. http://crbug.com/82991
-TEST(DhcpProxyScriptFetcherWin, FLAKY_ReuseFetcher) {
+TEST(DhcpProxyScriptFetcherWin, ReuseFetcher) {
   FetcherClient client;
 
   // The ProxyScriptFetcher interface stipulates that only a single