Fix a race in proxy_service_unittest.cc that was causing flakiness on purify b...

net/proxy/proxy_service_unittest.cc

 // Copyright (c) 2006-2008 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 "base/compiler_specific.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/proxy_config_service.h"
 #include "net/proxy/proxy_resolver.h"
 #include "net/proxy/proxy_script_fetcher.h"
@@ skipping 40 matching lines @@
   net::ProxyInfo info;
 
   // info is only returned if query_url in GetProxyForURL matches this:
   std::string info_predicate_query_host;
 
   // If true, then GetProxyForURL will fail, which simulates failure to
   // download or execute the PAC file.
   bool fail_get_proxy_for_url;
 };
 
-class SyncProxyService {
- public:
-  SyncProxyService(net::ProxyConfigService* config_service,
-                   net::ProxyResolver* resolver)
-      : io_thread_("IO_Thread"),
-        service_(config_service, resolver) {
-    base::Thread::Options options;
-    options.message_loop_type = MessageLoop::TYPE_IO;
-    io_thread_.StartWithOptions(options);
-    sync_proxy_service_ = new net::SyncProxyServiceHelper(
-        io_thread_.message_loop(), &service_);
-  }
-
-  int ResolveProxy(const GURL& url, net::ProxyInfo* proxy_info) {
-    return sync_proxy_service_->ResolveProxy(url, proxy_info);
-  }
-
-  int ReconsiderProxyAfterError(const GURL& url, net::ProxyInfo* proxy_info) {
-    return sync_proxy_service_->ReconsiderProxyAfterError(url, proxy_info);
-  }
-
- private:
-  base::Thread io_thread_;
-  net::ProxyService service_;
-  scoped_refptr<net::SyncProxyServiceHelper> sync_proxy_service_;
-};
-
 // ResultFuture is a handle to get at the result from
 // ProxyService::ResolveProxyForURL() that ran on another thread.
 class ResultFuture : public base::RefCountedThreadSafe<ResultFuture> {
  public:
   // |service| is the ProxyService to issue requests on, and |io_message_loop|
   // is the message loop where ProxyService lives.
   ResultFuture(MessageLoop* io_message_loop,
                net::ProxyService* service)
        : io_message_loop_(io_message_loop),
          service_(service),
@@ skipping 38 matching lines @@
   // Wait until the ProxyService completes this request.
   void WaitUntilCompleted() {
     DCHECK(MessageLoop::current() != io_message_loop_);
     completion_.Wait();
     DCHECK(did_complete_);
   }
 
  private:
   friend class ProxyServiceWithFutures;
 
-  // Start the request. Return once ProxyService::GetProxyForURL() returns.
+  typedef int (net::ProxyService::*RequestMethod)(const GURL&, net::ProxyInfo*,
+      net::CompletionCallback*, net::ProxyService::PacRequest**);
+
   void StartResolve(const GURL& url) {
+    StartRequest(url, &net::ProxyService::ResolveProxy);
+  }
+
+  // |proxy_info| is the *previous* result (that we are reconsidering).
+  void StartReconsider(const GURL& url, const net::ProxyInfo& proxy_info) {
+    proxy_info_ = proxy_info;
+    StartRequest(url, &net::ProxyService::ReconsiderProxyAfterError);
+  }
+
+  // Start the request. Return once ProxyService::GetProxyForURL() or
+  // ProxyService::ReconsiderProxyAfterError() returns.
+  void StartRequest(const GURL& url, RequestMethod method) {
     DCHECK(MessageLoop::current() != io_message_loop_);
     io_message_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-        this, &ResultFuture::DoStartResolve, url));
+        this, &ResultFuture::DoStartRequest, url, method));
     started_.Wait();
   }
 
   // Called on |io_message_loop_|.
-  void DoStartResolve(const GURL& url) {
+  void DoStartRequest(const GURL& url, RequestMethod method) {
     DCHECK(MessageLoop::current() == io_message_loop_);
-    int rv = service_->ResolveProxy(url, &proxy_info_, &callback_, &request_);
+    int rv = (service_->*method)(url, &proxy_info_, &callback_, &request_);
     if (rv != net::ERR_IO_PENDING) {
       // Completed synchronously.
       OnCompletion(rv);
     }
     started_.Signal();
   }
 
   // Called on |io_message_loop_|.
   void DoCancel() {
     DCHECK(MessageLoop::current() == io_message_loop_);
@@ skipping 31 matching lines @@
   int result_code_;
   net::ProxyInfo proxy_info_;
 };
 
 // Wraps a ProxyService running on its own IO thread.
 class ProxyServiceWithFutures {
  public:
   ProxyServiceWithFutures(net::ProxyConfigService* config_service,
                           net::ProxyResolver* resolver)
       : io_thread_("IO_Thread"),
-        service_(config_service, resolver) {
+        io_thread_state_(new IOThreadState) {
     base::Thread::Options options;
     options.message_loop_type = MessageLoop::TYPE_IO;
     io_thread_.StartWithOptions(options);
+
+    // Initialize state that lives on |io_thread_|.
+    io_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+        io_thread_state_.get(), &IOThreadState::DoInit,
+        config_service, resolver));
+    io_thread_state_->event.Wait();
+  }
+
+  ~ProxyServiceWithFutures() {
+    // Destroy state that lives on |io_thread_|.
+    io_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+        io_thread_state_.get(), &IOThreadState::DoDestroy));
+    io_thread_state_->event.Wait();
   }
 
   // Start the request on |io_thread_|, and return a handle that can be
   // used to access the results. The caller is responsible for freeing
   // the ResultFuture.
   void ResolveProxy(scoped_refptr<ResultFuture>* result, const GURL& url) {
-    (*result) = new ResultFuture(io_thread_.message_loop(), &service_);
+    *result = new ResultFuture(io_thread_.message_loop(),
+                               io_thread_state_->service);
     (*result)->StartResolve(url);
   }
 
+  // Same as above, but for "ReconsiderProxyAfterError()".
+  void ReconsiderProxyAfterError(scoped_refptr<ResultFuture>* result,
+                                 const GURL& url,
+                                 const net::ProxyInfo& proxy_info) {
+    *result = new ResultFuture(io_thread_.message_loop(),
+                               io_thread_state_->service);
+    (*result)->StartReconsider(url, proxy_info);
+  }
+
   void SetProxyScriptFetcher(net::ProxyScriptFetcher* proxy_script_fetcher) {
-    service_.SetProxyScriptFetcher(proxy_script_fetcher);
+    io_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+        io_thread_state_.get(), &IOThreadState::DoSetProxyScriptFetcher,
+        proxy_script_fetcher));
+    io_thread_state_->event.Wait();
   }
 
  private:
+  // Class that encapsulates the state living on IO thread. It needs to be
+  // ref-counted for posting tasks.
+  class IOThreadState : public base::RefCounted<IOThreadState> {
+   public:
+    IOThreadState() : event(false, false), service(NULL) {}
+
+    void DoInit(net::ProxyConfigService* config_service,
+                net::ProxyResolver* resolver) {
+      service = new net::ProxyService(config_service, resolver);
+      event.Signal();
+    }
+
+    void DoDestroy() {
+      delete service;
+      service = NULL;
+      event.Signal();
+    }
+
+    void DoSetProxyScriptFetcher(
+        net::ProxyScriptFetcher* proxy_script_fetcher) {
+      service->SetProxyScriptFetcher(proxy_script_fetcher);
+      event.Signal();
+    }
+
+    base::WaitableEvent event;
+    net::ProxyService* service;
+  };
+
   base::Thread io_thread_;
-  net::ProxyService service_;
+  scoped_refptr<IOThreadState> io_thread_state_;  // Lives on |io_thread_|.
+};
+
+// Wrapper around ProxyServiceWithFutures to do one request at a time.
+class SyncProxyService {
+ public:
+  SyncProxyService(net::ProxyConfigService* config_service,
+                   net::ProxyResolver* resolver)
+      : service_(config_service, resolver) {
+  }
+
+  int ResolveProxy(const GURL& url, net::ProxyInfo* proxy_info) {
+    scoped_refptr<ResultFuture> result;
+    service_.ResolveProxy(&result, url);
+    *proxy_info = result->GetProxyInfo();
+    return result->GetResultCode();
+  }
+
+  int ReconsiderProxyAfterError(const GURL& url, net::ProxyInfo* proxy_info) {
+    scoped_refptr<ResultFuture> result;
+    service_.ReconsiderProxyAfterError(&result, url, *proxy_info);
+    *proxy_info = result->GetProxyInfo();
+    return result->GetResultCode();
+  }
+
+ private:
+  ProxyServiceWithFutures service_;
 };
 
 // A ProxyResolver which can be set to block upon reaching GetProxyForURL.
 class BlockableProxyResolver : public net::ProxyResolver {
  public:
   BlockableProxyResolver() : net::ProxyResolver(true),
                              should_block_(false),
                              unblocked_(true, true),
       blocked_(true, false) {
   }
@@ skipping 676 matching lines @@
   result3->WaitUntilCompleted();
 
   EXPECT_FALSE(result1->IsCompleted());  // Cancelled.
   EXPECT_FALSE(result2->IsCompleted());  // Cancelled.
 
   EXPECT_TRUE(result3->IsCompleted());
   EXPECT_EQ(net::OK, result3->GetResultCode());
   EXPECT_EQ("pac-v1.request3:80",
             result3->GetProxyInfo().proxy_server().ToURI());
 }