Make HostResolver NonThreadSafe and not thread safe refcounted.

Required making SyncHostResolverBridge not use RefCountedThreadSafe.  I refactored the innards to have a thread safe refcounted Core implementation.

BUG=45298

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

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

13 files changed, 334 insertions, 206 deletions

diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h
index e4bf847..d798688 100644
--- a/net/base/host_resolver.h
+++ b/net/base/host_resolver.h
@@ -32,7 +32,7 @@ class NetworkChangeNotifier;
 // request at a time is to create a SingleRequestHostResolver wrapper around
 // HostResolver (which will automatically cancel the single request when it
 // goes out of scope).
-class HostResolver : public base::RefCountedThreadSafe<HostResolver> {
+class HostResolver : public base::RefCounted<HostResolver> {
  public:
   // The parameters for doing a Resolve(). |hostname| and |port| are required,
   // the rest are optional (and have reasonable defaults).
@@ -179,7 +179,7 @@ class HostResolver : public base::RefCountedThreadSafe<HostResolver> {
   virtual HostResolverImpl* GetAsHostResolverImpl() { return NULL; }
 
  protected:
-  friend class base::RefCountedThreadSafe<HostResolver>;
+  friend class base::RefCounted<HostResolver>;
 
   HostResolver() { }
 
diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index 1f89af4..f3bc5c1 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -755,6 +755,8 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
                               CompletionCallback* callback,
                               RequestHandle* out_req,
                               const BoundNetLog& net_log) {
+  DCHECK(CalledOnValidThread());
+
   if (shutdown_)
     return ERR_UNEXPECTED;
 
@@ -840,6 +842,7 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
 // See OnJobComplete(Job*) for why it is important not to clean out
 // cancelled requests from Job::requests_.
 void HostResolverImpl::CancelRequest(RequestHandle req_handle) {
+  DCHECK(CalledOnValidThread());
   if (shutdown_) {
     // TODO(eroman): temp hack for: http://crbug.com/18373
     // Because we destroy outstanding requests during Shutdown(),
@@ -869,10 +872,12 @@ void HostResolverImpl::CancelRequest(RequestHandle req_handle) {
 }
 
 void HostResolverImpl::AddObserver(HostResolver::Observer* observer) {
+  DCHECK(CalledOnValidThread());
   observers_.push_back(observer);
 }
 
 void HostResolverImpl::RemoveObserver(HostResolver::Observer* observer) {
+  DCHECK(CalledOnValidThread());
   ObserversList::iterator it =
       std::find(observers_.begin(), observers_.end(), observer);
 
@@ -883,18 +888,21 @@ void HostResolverImpl::RemoveObserver(HostResolver::Observer* observer) {
 }
 
 void HostResolverImpl::SetDefaultAddressFamily(AddressFamily address_family) {
+  DCHECK(CalledOnValidThread());
   ipv6_probe_monitoring_ = false;
   DiscardIPv6ProbeJob();
   default_address_family_ = address_family;
 }
 
 void HostResolverImpl::ProbeIPv6Support() {
+  DCHECK(CalledOnValidThread());
   DCHECK(!ipv6_probe_monitoring_);
   ipv6_probe_monitoring_ = true;
   OnIPAddressChanged();  // Give initial setup call.
 }
 
 void HostResolverImpl::Shutdown() {
+  DCHECK(CalledOnValidThread());
   shutdown_ = true;
 
   // Cancel the outstanding jobs.
@@ -907,6 +915,7 @@ void HostResolverImpl::Shutdown() {
 void HostResolverImpl::SetPoolConstraints(JobPoolIndex pool_index,
                                           size_t max_outstanding_jobs,
                                           size_t max_pending_requests) {
+  DCHECK(CalledOnValidThread());
   CHECK_GE(pool_index, 0);
   CHECK_LT(pool_index, POOL_COUNT);
   CHECK(jobs_.empty()) << "Can only set constraints during setup";
diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h
index 417cc77..a0e34ca 100644
--- a/net/base/host_resolver_impl.h
+++ b/net/base/host_resolver_impl.h
@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/non_thread_safe.h"
 #include "base/scoped_ptr.h"
 #include "net/base/capturing_net_log.h"
 #include "net/base/host_cache.h"
@@ -49,6 +50,7 @@ namespace net {
 // Requests are ordered in the queue based on their priority.
 
 class HostResolverImpl : public HostResolver,
+                         public NonThreadSafe,
                          public NetworkChangeNotifier::Observer {
  public:
   // The index into |job_pools_| for the various job pools. Pools with a higher
diff --git a/net/proxy/sync_host_resolver_bridge.cc b/net/proxy/sync_host_resolver_bridge.cc
index 31a326d..caf4549 100644
--- a/net/proxy/sync_host_resolver_bridge.cc
+++ b/net/proxy/sync_host_resolver_bridge.cc
@@ -6,50 +6,123 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/lock.h"
 #include "base/message_loop.h"
+#include "base/waitable_event.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
 namespace net {
 
-// SyncHostResolverBridge -----------------------------------------------------
+// SyncHostResolverBridge::Core ----------------------------------------------
 
-SyncHostResolverBridge::SyncHostResolverBridge(HostResolver* host_resolver,
-                                               MessageLoop* host_resolver_loop)
+class SyncHostResolverBridge::Core
+    : public base::RefCountedThreadSafe<SyncHostResolverBridge::Core> {
+ public:
+  Core(HostResolver* resolver, MessageLoop* host_resolver_loop);
+
+  int ResolveSynchronously(const HostResolver::RequestInfo& info,
+                           AddressList* addresses);
+
+  // Returns true if Shutdown() has been called.
+  bool HasShutdown() const {
+    AutoLock l(lock_);
+    return HasShutdownLocked();
+  }
+
+  // Called on |host_resolver_loop_|.
+  void Shutdown();
+
+ private:
+  friend class base::RefCountedThreadSafe<SyncHostResolverBridge::Core>;
+
+  bool HasShutdownLocked() const {
+    return has_shutdown_;
+  }
+
+  // Called on |host_resolver_loop_|.
+  void StartResolve(const HostResolver::RequestInfo& info,
+                    AddressList* addresses);
+
+  // Called on |host_resolver_loop_|.
+  void OnResolveCompletion(int result);
+
+  // Not called on |host_resolver_loop_|.
+  int WaitForResolveCompletion();
+
+  const scoped_refptr<HostResolver> host_resolver_;
+  MessageLoop* const host_resolver_loop_;
+  net::CompletionCallbackImpl<Core> callback_;
+  // The result from the current request (set on |host_resolver_loop_|).
+  int err_;
+  // The currently outstanding request to |host_resolver_|, or NULL.
+  HostResolver::RequestHandle outstanding_request_;
+
+  // Event to notify completion of resolve request.  We always Signal() on
+  // |host_resolver_loop_| and Wait() on a different thread.
+  base::WaitableEvent event_;
+
+  // True if Shutdown() has been called. Must hold |lock_| to access it.
+  bool has_shutdown_;
+
+  // Mutex to guard accesses to |has_shutdown_|.
+  mutable Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(Core);
+};
+
+SyncHostResolverBridge::Core::Core(HostResolver* host_resolver,
+                                   MessageLoop* host_resolver_loop)
     : host_resolver_(host_resolver),
       host_resolver_loop_(host_resolver_loop),
-      event_(true, false),
       ALLOW_THIS_IN_INITIALIZER_LIST(
-          callback_(this, &SyncHostResolverBridge::OnResolveCompletion)),
+          callback_(this, &Core::OnResolveCompletion)),
+      err_(0),
       outstanding_request_(NULL),
-      has_shutdown_(false) {
-  DCHECK(host_resolver_loop_);
-}
-
-SyncHostResolverBridge::~SyncHostResolverBridge() {
-  DCHECK(HasShutdown());
-}
-
-int SyncHostResolverBridge::Resolve(const RequestInfo& info,
-                                    AddressList* addresses,
-                                    CompletionCallback* callback,
-                                    RequestHandle* out_req,
-                                    const BoundNetLog& net_log) {
-  DCHECK(!callback);
-  DCHECK(!out_req);
+      event_(true, false),
+      has_shutdown_(false) {}
 
+int SyncHostResolverBridge::Core::ResolveSynchronously(
+    const HostResolver::RequestInfo& info,
+    net::AddressList* addresses) {
   // Otherwise start an async resolve on the resolver's thread.
   host_resolver_loop_->PostTask(
       FROM_HERE,
-      NewRunnableMethod(this, &SyncHostResolverBridge::StartResolve,
+      NewRunnableMethod(this, &Core::StartResolve,
                         info, addresses));
 
-  // Wait for the resolve to complete in the resolver's thread.
+  return WaitForResolveCompletion();
+}
+
+void SyncHostResolverBridge::Core::StartResolve(
+    const HostResolver::RequestInfo& info,
+    net::AddressList* addresses) {
+  DCHECK_EQ(MessageLoop::current(), host_resolver_loop_);
+  DCHECK(!outstanding_request_);
+
+  if (HasShutdown())
+    return;
+
+  int error = host_resolver_->Resolve(
+      info, addresses, &callback_, &outstanding_request_, BoundNetLog());
+  if (error != ERR_IO_PENDING)
+    OnResolveCompletion(error);  // Completed synchronously.
+}
+
+void SyncHostResolverBridge::Core::OnResolveCompletion(int result) {
+  DCHECK_EQ(MessageLoop::current(), host_resolver_loop_);
+  err_ = result;
+  outstanding_request_ = NULL;
+  event_.Signal();
+}
+
+int SyncHostResolverBridge::Core::WaitForResolveCompletion() {
+  DCHECK_NE(MessageLoop::current(), host_resolver_loop_);
   event_.Wait();
 
   {
     AutoLock l(lock_);
-    if (has_shutdown_)
+    if (HasShutdownLocked())
       return ERR_ABORTED;
     event_.Reset();
   }
@@ -57,19 +130,7 @@ int SyncHostResolverBridge::Resolve(const RequestInfo& info,
   return err_;
 }
 
-void SyncHostResolverBridge::CancelRequest(RequestHandle req) {
-  NOTREACHED();
-}
-
-void SyncHostResolverBridge::AddObserver(Observer* observer) {
-  NOTREACHED();
-}
-
-void SyncHostResolverBridge::RemoveObserver(Observer* observer) {
-  NOTREACHED();
-}
-
-void SyncHostResolverBridge::Shutdown() {
+void SyncHostResolverBridge::Core::Shutdown() {
   DCHECK_EQ(MessageLoop::current(), host_resolver_loop_);
 
   if (outstanding_request_) {
@@ -77,32 +138,54 @@ void SyncHostResolverBridge::Shutdown() {
     outstanding_request_ = NULL;
   }
 
-  AutoLock l(lock_);
-  has_shutdown_ = true;
+  {
+    AutoLock l(lock_);
+    has_shutdown_ = true;
+  }
 
   // Wake up the PAC thread in case it was waiting for resolve completion.
   event_.Signal();
 }
 
-void SyncHostResolverBridge::StartResolve(const HostResolver::RequestInfo& info,
-                                          net::AddressList* addresses) {
-  DCHECK_EQ(host_resolver_loop_, MessageLoop::current());
-  DCHECK(!outstanding_request_);
+// SyncHostResolverBridge -----------------------------------------------------
 
-  if (HasShutdown())
-    return;
+SyncHostResolverBridge::SyncHostResolverBridge(HostResolver* host_resolver,
+                                               MessageLoop* host_resolver_loop)
+    : host_resolver_loop_(host_resolver_loop),
+      core_(new Core(host_resolver, host_resolver_loop)) {
+  DCHECK(host_resolver_loop_);
+}
 
-  int error = host_resolver_->Resolve(
-      info, addresses, &callback_, &outstanding_request_, BoundNetLog());
-  if (error != ERR_IO_PENDING)
-    OnResolveCompletion(error);  // Completed synchronously.
+SyncHostResolverBridge::~SyncHostResolverBridge() {
+  DCHECK(core_->HasShutdown());
 }
 
-void SyncHostResolverBridge::OnResolveCompletion(int result) {
-  DCHECK_EQ(host_resolver_loop_, MessageLoop::current());
-  err_ = result;
-  outstanding_request_ = NULL;
-  event_.Signal();
+int SyncHostResolverBridge::Resolve(const RequestInfo& info,
+                                    AddressList* addresses,
+                                    CompletionCallback* callback,
+                                    RequestHandle* out_req,
+                                    const BoundNetLog& net_log) {
+  DCHECK(!callback);
+  DCHECK(!out_req);
+
+  return core_->ResolveSynchronously(info, addresses);
+}
+
+void SyncHostResolverBridge::CancelRequest(RequestHandle req) {
+  NOTREACHED();
+}
+
+void SyncHostResolverBridge::AddObserver(Observer* observer) {
+  NOTREACHED();
+}
+
+void SyncHostResolverBridge::RemoveObserver(Observer* observer) {
+  NOTREACHED();
+}
+
+void SyncHostResolverBridge::Shutdown() {
+  DCHECK_EQ(MessageLoop::current(), host_resolver_loop_);
+  core_->Shutdown();
 }
 
 // SingleThreadedProxyResolverUsingBridgedHostResolver -----------------------
diff --git a/net/proxy/sync_host_resolver_bridge.h b/net/proxy/sync_host_resolver_bridge.h
index d082043..4d25b4d 100644
--- a/net/proxy/sync_host_resolver_bridge.h
+++ b/net/proxy/sync_host_resolver_bridge.h
@@ -5,9 +5,7 @@
 #ifndef NET_PROXY_SYNC_HOST_RESOLVER_BRIDGE_H_
 #define NET_PROXY_SYNC_HOST_RESOLVER_BRIDGE_H_
 
-#include "base/lock.h"
 #include "base/scoped_ptr.h"
-#include "base/waitable_event.h"
 #include "net/base/host_resolver.h"
 #include "net/proxy/single_threaded_proxy_resolver.h"
 
@@ -40,39 +38,10 @@ class SyncHostResolverBridge : public HostResolver {
   void Shutdown();
 
  private:
-  // Called on |host_resolver_loop_|.
-  void StartResolve(const HostResolver::RequestInfo& info,
-                    net::AddressList* addresses);
+  class Core;
 
-  // Called on |host_resolver_loop_|.
-  void OnResolveCompletion(int result);
-
-  // Returns true if Shutdown() has been called.
-  bool HasShutdown() const {
-    AutoLock l(lock_);
-    return has_shutdown_;
-  }
-
-  scoped_refptr<HostResolver> host_resolver_;
-  MessageLoop* host_resolver_loop_;
-
-  // Event to notify completion of resolve request.
-  base::WaitableEvent event_;
-
-  // Callback for when the resolve completes on host_resolver_loop_.
-  net::CompletionCallbackImpl<SyncHostResolverBridge> callback_;
-
-  // The result from the current request (set on |host_resolver_loop_|).
-  int err_;
-
-  // The currently outstanding request to |host_resolver_|, or NULL.
-  HostResolver::RequestHandle outstanding_request_;
-
-  // True if Shutdown() has been called. Must hold |lock_| to access it.
-  bool has_shutdown_;
-
-  // Mutex to guard accesses to |has_shutdown_|.
-  mutable Lock lock_;
+  MessageLoop* const host_resolver_loop_;
+  scoped_refptr<Core> core_;
 };
 
 // Subclass of SingleThreadedProxyResolver that additionally calls
@@ -94,4 +63,3 @@ class SingleThreadedProxyResolverUsingBridgedHostResolver
 }  // namespace net
 
 #endif  // NET_PROXY_SYNC_HOST_RESOLVER_BRIDGE_H_
-
diff --git a/net/proxy/sync_host_resolver_bridge_unittest.cc b/net/proxy/sync_host_resolver_bridge_unittest.cc
index 916252b..42dbacc 100644
--- a/net/proxy/sync_host_resolver_bridge_unittest.cc
+++ b/net/proxy/sync_host_resolver_bridge_unittest.cc
@@ -5,6 +5,7 @@
 #include "net/proxy/sync_host_resolver_bridge.h"
 
 #include "base/thread.h"
+#include "base/waitable_event.h"
 #include "net/base/address_list.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
@@ -116,16 +117,20 @@ class SyncProxyResolver : public ProxyResolver {
 // network stack.
 class IOThread : public base::Thread {
  public:
-  explicit IOThread(HostResolver* async_resolver)
-      : base::Thread("IO-thread"), async_resolver_(async_resolver) {
-  }
+  IOThread() : base::Thread("IO-thread") {}
 
   virtual ~IOThread() {
     Stop();
   }
 
+  const scoped_refptr<BlockableHostResolver>& async_resolver() {
+    return async_resolver_;
+  }
+
  protected:
   virtual void Init() {
+    async_resolver_ = new BlockableHostResolver();
+
     // Create a synchronous host resolver that operates the async host
     // resolver on THIS thread.
     scoped_refptr<SyncHostResolverBridge> sync_resolver =
@@ -149,10 +154,19 @@ class IOThread : public base::Thread {
 
     // Delete the single threaded proxy resolver.
     proxy_resolver_.reset();
+
+    // During the teardown sequence of the single threaded proxy resolver,
+    // the outstanding host resolve should have been cancelled.
+    EXPECT_TRUE(async_resolver_->was_request_cancelled());
+
+    async_resolver_ = NULL;
   }
 
  private:
-  HostResolver* async_resolver_;
+  // This (async) host resolver will outlive the thread that is operating it
+  // synchronously.
+  scoped_refptr<BlockableHostResolver> async_resolver_;
+
   scoped_ptr<ProxyResolver> proxy_resolver_;
 
   // Data for the outstanding request to the single threaded proxy resolver.
@@ -165,35 +179,21 @@ class IOThread : public base::Thread {
 // is outstanding on the SyncHostResolverBridge.
 // This is a regression test for http://crbug.com/41244.
 TEST(SingleThreadedProxyResolverWithBridgedHostResolverTest, ShutdownDeadlock) {
-  // This (async) host resolver will outlive the thread that is operating it
-  // synchronously.
-  scoped_refptr<BlockableHostResolver> host_resolver =
-      new BlockableHostResolver();
-
-  {
-    IOThread io_thread(host_resolver.get());
-    base::Thread::Options options;
-    options.message_loop_type = MessageLoop::TYPE_IO;
-    ASSERT_TRUE(io_thread.StartWithOptions(options));
-
-    // Wait until the host resolver receives a request (this means that the
-    // PAC thread is now blocked, waiting for the async response from the
-    // host resolver.
-    host_resolver->WaitUntilRequestIsReceived();
-
-    // Now upon exitting this scope, the IOThread is destroyed -- this will
-    // stop the IOThread, which will in turn delete the
-    // SingleThreadedProxyResolver, which in turn will stop its internal
-    // PAC thread (which is currently blocked waiting on the host resolve which
-    // is running on IOThread).
-  }
-
-  // During the teardown sequence of the single threaded proxy resolver,
-  // the outstanding host resolve should have been cancelled.
-  EXPECT_TRUE(host_resolver->was_request_cancelled());
+  IOThread io_thread;
+  base::Thread::Options options;
+  options.message_loop_type = MessageLoop::TYPE_IO;
+  ASSERT_TRUE(io_thread.StartWithOptions(options));
+
+  io_thread.async_resolver()->WaitUntilRequestIsReceived();
+
+  // Now upon exitting this scope, the IOThread is destroyed -- this will
+  // stop the IOThread, which will in turn delete the
+  // SingleThreadedProxyResolver, which in turn will stop its internal
+  // PAC thread (which is currently blocked waiting on the host resolve which
+  // is running on IOThread).  The IOThread::Cleanup() will verify that after
+  // the PAC thread is stopped, it cancels the request on the HostResolver.
 }
 
 }  // namespace
 
 }  // namespace net
-
diff --git a/webkit/appcache/appcache_update_job_unittest.cc b/webkit/appcache/appcache_update_job_unittest.cc
index 7ea664f..a10f53f 100644
--- a/webkit/appcache/appcache_update_job_unittest.cc
+++ b/webkit/appcache/appcache_update_job_unittest.cc
@@ -308,6 +308,36 @@ std::string HttpHeadersRequestTestJob::expect_if_none_match_;
 bool HttpHeadersRequestTestJob::saw_if_none_match_ = false;
 bool HttpHeadersRequestTestJob::already_checked_ = false;
 
+namespace {
+
+class IOThread : public base::Thread {
+ public:
+  IOThread(const char* name) : base::Thread(name) {
+  }
+
+  ~IOThread() {
+    // We cannot rely on our base class to stop the thread since we want our
+    // CleanUp function to run.
+    Stop();
+  }
+
+  const scoped_refptr<URLRequestContext>& request_context() {
+    return request_context_;
+  }
+
+  virtual void Init() {
+    request_context_ = new TestURLRequestContext();
+  }
+
+  virtual void CleanUp() {
+    request_context_ = NULL;
+  }
+
+  scoped_refptr<URLRequestContext> request_context_;
+};
+
+}  // namespace
+
 class AppCacheUpdateJobTest : public testing::Test,
                               public AppCacheGroup::UpdateObserver {
  public:
@@ -358,15 +388,13 @@ class AppCacheUpdateJobTest : public testing::Test,
   }
 
   static void SetUpTestCase() {
-    io_thread_ = new base::Thread("AppCacheUpdateJob IO test thread");
+    io_thread_ = new IOThread("AppCacheUpdateJob IO test thread");
     base::Thread::Options options(MessageLoop::TYPE_IO, 0);
     io_thread_->StartWithOptions(options);
 
     http_server_ = HTTPTestServer::CreateServer(
         kDocRoot, io_thread_->message_loop()).release();
     ASSERT_TRUE(http_server_);
-    request_context_ = new TestURLRequestContext();
-    request_context_->AddRef();
   }
 
   static base::WaitableEvent* io_thread_shutdown_event_;
@@ -377,10 +405,6 @@ class AppCacheUpdateJobTest : public testing::Test,
       http_server_->Release();
       http_server_ = NULL;
     }
-    if (request_context_) {
-      request_context_->Release();
-      request_context_ = NULL;
-    }
     io_thread_shutdown_event_->Signal();
   }
 
@@ -2630,7 +2654,7 @@ class AppCacheUpdateJobTest : public testing::Test,
 
   void MakeService() {
     service_.reset(new MockAppCacheService());
-    service_->set_request_context(request_context_);
+    service_->set_request_context(io_thread_->request_context());
     service_->set_appcache_policy(&policy_);
   }
 
@@ -2915,9 +2939,8 @@ class AppCacheUpdateJobTest : public testing::Test,
     PENDING_MASTER_NO_UPDATE,
   };
 
-  static base::Thread* io_thread_;
+  static IOThread* io_thread_;
   static HTTPTestServer* http_server_;
-  static TestURLRequestContext* request_context_;
 
   scoped_ptr<MockAppCacheService> service_;
   scoped_refptr<AppCacheGroup> group_;
@@ -2950,9 +2973,8 @@ class AppCacheUpdateJobTest : public testing::Test,
 };
 
 // static
-base::Thread* AppCacheUpdateJobTest::io_thread_ = NULL;
+IOThread* AppCacheUpdateJobTest::io_thread_ = NULL;
 HTTPTestServer* AppCacheUpdateJobTest::http_server_ = NULL;
-TestURLRequestContext* AppCacheUpdateJobTest::request_context_ = NULL;
 base::WaitableEvent* AppCacheUpdateJobTest::io_thread_shutdown_event_ = NULL;
 
 
diff --git a/webkit/support/test_webkit_client.cc b/webkit/support/test_webkit_client.cc
index 21648f0..389a188 100644
--- a/webkit/support/test_webkit_client.cc
+++ b/webkit/support/test_webkit_client.cc
@@ -116,8 +116,7 @@ TestWebKitClient::TestWebKitClient() : url_loader_factory_(NULL) {
 
   // Initializing with a default context, which means no on-disk cookie DB,
   // and no support for directory listings.
-  SimpleResourceLoaderBridge::Init(
-      new TestShellRequestContext(FilePath(), cache_mode, true));
+  SimpleResourceLoaderBridge::Init(FilePath(), cache_mode, true);
 
   // Test shell always exposes the GC.
   webkit_glue::SetJavaScriptFlags(L" --expose-gc");
diff --git a/webkit/tools/test_shell/node_leak_test.cc b/webkit/tools/test_shell/node_leak_test.cc
index 40d19ff..6d303f1 100644
--- a/webkit/tools/test_shell/node_leak_test.cc
+++ b/webkit/tools/test_shell/node_leak_test.cc
@@ -52,8 +52,7 @@ class NodeLeakTest : public TestShellTest {
     net::HttpCache::Mode mode =
         parsed_command_line.HasSwitch(test_shell::kPlaybackMode) ?
         net::HttpCache::PLAYBACK : net::HttpCache::NORMAL;
-    SimpleResourceLoaderBridge::Init(
-        new TestShellRequestContext(cache_path, mode, false));
+    SimpleResourceLoaderBridge::Init(cache_path, mode, false);
 
     TestShellTest::SetUp();
   }
diff --git a/webkit/tools/test_shell/simple_resource_loader_bridge.cc b/webkit/tools/test_shell/simple_resource_loader_bridge.cc
index 690f169..ffa0d81 100644
--- a/webkit/tools/test_shell/simple_resource_loader_bridge.cc
+++ b/webkit/tools/test_shell/simple_resource_loader_bridge.cc
@@ -32,7 +32,11 @@
 
 #include "webkit/tools/test_shell/simple_resource_loader_bridge.h"
 
+#include "base/file_path.h"
 #include "base/message_loop.h"
+#if defined(OS_WIN)
+#include "base/nss_util.h"
+#endif
 #include "base/ref_counted.h"
 #include "base/time.h"
 #include "base/timer.h"
@@ -44,9 +48,13 @@
 #include "net/base/net_util.h"
 #include "net/base/static_cookie_policy.h"
 #include "net/base/upload_data.h"
+#include "net/http/http_cache.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
 #include "net/proxy/proxy_service.h"
+#if defined(OS_WIN)
+#include "net/socket/ssl_client_socket_nss_factory.h"
+#endif
 #include "net/url_request/url_request.h"
 #include "webkit/appcache/appcache_interfaces.h"
 #include "webkit/glue/resource_loader_bridge.h"
@@ -60,10 +68,26 @@ using net::HttpResponseHeaders;
 
 namespace {
 
-//-----------------------------------------------------------------------------
+struct TestShellRequestContextParams {
+  TestShellRequestContextParams(
+      const FilePath& in_cache_path,
+      net::HttpCache::Mode in_cache_mode,
+      bool in_no_proxy)
+      : cache_path(in_cache_path),
+        cache_mode(in_cache_mode),
+        no_proxy(in_no_proxy),
+        accept_all_cookies(false) {}
+
+  FilePath cache_path;
+  net::HttpCache::Mode cache_mode;
+  bool no_proxy;
+  bool accept_all_cookies;
+};
 
-URLRequestContext* request_context = NULL;
-base::Thread* io_thread = NULL;
+TestShellRequestContextParams* g_request_context_params = NULL;
+URLRequestContext* g_request_context = NULL;
+
+//-----------------------------------------------------------------------------
 
 class IOThread : public base::Thread {
  public:
@@ -77,19 +101,44 @@ class IOThread : public base::Thread {
   }
 
   virtual void Init() {
-    SimpleAppCacheSystem::InitializeOnIOThread(request_context);
-    SimpleSocketStreamBridge::InitializeOnIOThread(request_context);
+    if (g_request_context_params) {
+      g_request_context = new TestShellRequestContext(
+          g_request_context_params->cache_path,
+          g_request_context_params->cache_mode,
+          g_request_context_params->no_proxy);
+      SetAcceptAllCookies(g_request_context_params->accept_all_cookies);
+      delete g_request_context_params;
+      g_request_context_params = NULL;
+    } else {
+      g_request_context = new TestShellRequestContext();
+      SetAcceptAllCookies(false);
+    }
+
+    g_request_context->AddRef();
+
+    SimpleAppCacheSystem::InitializeOnIOThread(g_request_context);
+    SimpleSocketStreamBridge::InitializeOnIOThread(g_request_context);
   }
 
   virtual void CleanUp() {
     SimpleSocketStreamBridge::Cleanup();
-    if (request_context) {
-      request_context->Release();
-      request_context = NULL;
+    if (g_request_context) {
+      g_request_context->Release();
+      g_request_context = NULL;
     }
   }
+
+  void SetAcceptAllCookies(bool accept_all_cookies) {
+    StaticCookiePolicy::Type policy_type = accept_all_cookies ?
+        StaticCookiePolicy::ALLOW_ALL_COOKIES :
+        StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES;
+    static_cast<StaticCookiePolicy*>(g_request_context->cookie_policy())->
+        set_type(policy_type);
+  }
 };
 
+IOThread* g_io_thread = NULL;
+
 //-----------------------------------------------------------------------------
 
 struct RequestParams {
@@ -128,13 +177,13 @@ class RequestProxy : public URLRequest::Delegate,
     owner_loop_ = MessageLoop::current();
 
     // proxy over to the io thread
-    io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+    g_io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
         this, &RequestProxy::AsyncStart, params));
   }
 
   void Cancel() {
     // proxy over to the io thread
-    io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+    g_io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
         this, &RequestProxy::AsyncCancel));
   }
 
@@ -144,7 +193,7 @@ class RequestProxy : public URLRequest::Delegate,
   virtual ~RequestProxy() {
     // If we have a request, then we'd better be on the io thread!
     DCHECK(!request_.get() ||
-           MessageLoop::current() == io_thread->message_loop());
+           MessageLoop::current() == g_io_thread->message_loop());
   }
 
   // --------------------------------------------------------------------------
@@ -159,7 +208,7 @@ class RequestProxy : public URLRequest::Delegate,
     if (peer_ && peer_->OnReceivedRedirect(new_url, info,
                                            &has_new_first_party_for_cookies,
                                            &new_first_party_for_cookies)) {
-      io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+      g_io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
           this, &RequestProxy::AsyncFollowDeferredRedirect,
           has_new_first_party_for_cookies, new_first_party_for_cookies));
     } else {
@@ -188,7 +237,7 @@ class RequestProxy : public URLRequest::Delegate,
     // peer could generate new requests in reponse to the received data, which
     // when run on the io thread, could race against this function in doing
     // another InvokeLater.  See bug 769249.
-    io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+    g_io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
         this, &RequestProxy::AsyncReadData));
 
     peer_->OnReceivedData(buf_copy.get(), bytes_read);
@@ -221,7 +270,7 @@ class RequestProxy : public URLRequest::Delegate,
     request_->SetExtraRequestHeaders(headers);
     request_->set_load_flags(params->load_flags);
     request_->set_upload(params->upload.get());
-    request_->set_context(request_context);
+    request_->set_context(g_request_context);
     SimpleAppCacheSystem::SetExtraRequestInfo(
         request_.get(), params->appcache_host_id, params->request_type);
 
@@ -504,7 +553,7 @@ class ResourceLoaderBridgeImpl : public ResourceLoaderBridge {
     if (proxy_) {
       proxy_->DropPeer();
       // Let the proxy die on the IO thread
-      io_thread->message_loop()->ReleaseSoon(FROM_HERE, proxy_);
+      g_io_thread->message_loop()->ReleaseSoon(FROM_HERE, proxy_);
     }
   }
 
@@ -591,8 +640,8 @@ class ResourceLoaderBridgeImpl : public ResourceLoaderBridge {
 class CookieSetter : public base::RefCountedThreadSafe<CookieSetter> {
  public:
   void Set(const GURL& url, const std::string& cookie) {
-    DCHECK(MessageLoop::current() == io_thread->message_loop());
-    request_context->cookie_store()->SetCookie(url, cookie);
+    DCHECK(MessageLoop::current() == g_io_thread->message_loop());
+    g_request_context->cookie_store()->SetCookie(url, cookie);
   }
 
  private:
@@ -607,7 +656,7 @@ class CookieGetter : public base::RefCountedThreadSafe<CookieGetter> {
   }
 
   void Get(const GURL& url) {
-    result_ = request_context->cookie_store()->GetCookies(url);
+    result_ = g_request_context->cookie_store()->GetCookies(url);
     event_.Signal();
   }
 
@@ -641,11 +690,11 @@ ResourceLoaderBridge* ResourceLoaderBridge::Create(
 // Issue the proxy resolve request on the io thread, and wait
 // for the result.
 bool FindProxyForUrl(const GURL& url, std::string* proxy_list) {
-  DCHECK(request_context);
+  DCHECK(g_request_context);
 
   scoped_refptr<net::SyncProxyServiceHelper> sync_proxy_service(
-      new net::SyncProxyServiceHelper(io_thread->message_loop(),
-      request_context->proxy_service()));
+      new net::SyncProxyServiceHelper(g_io_thread->message_loop(),
+      g_request_context->proxy_service()));
 
   net::ProxyInfo proxy_info;
   int rv = sync_proxy_service->ResolveProxy(url, &proxy_info,
@@ -662,27 +711,32 @@ bool FindProxyForUrl(const GURL& url, std::string* proxy_list) {
 //-----------------------------------------------------------------------------
 
 // static
-void SimpleResourceLoaderBridge::Init(TestShellRequestContext* context) {
+void SimpleResourceLoaderBridge::Init(
+    const FilePath& cache_path,
+    net::HttpCache::Mode cache_mode,
+    bool no_proxy) {
   // Make sure to stop any existing IO thread since it may be using the
   // current request context.
   Shutdown();
 
-  if (context) {
-    request_context = context;
-  } else {
-    request_context = new TestShellRequestContext();
-  }
-  request_context->AddRef();
-  SimpleResourceLoaderBridge::SetAcceptAllCookies(false);
+  DCHECK(!g_request_context_params);
+  DCHECK(!g_request_context);
+  DCHECK(!g_io_thread);
+
+  g_request_context_params = new TestShellRequestContextParams(
+      cache_path, cache_mode, no_proxy);
 }
 
 // static
 void SimpleResourceLoaderBridge::Shutdown() {
-  if (io_thread) {
-    delete io_thread;
-    io_thread = NULL;
+  if (g_io_thread) {
+    delete g_io_thread;
+    g_io_thread = NULL;
 
-    DCHECK(!request_context) << "should have been nulled by thread dtor";
+    DCHECK(!g_request_context) << "should have been nulled by thread dtor";
+  } else {
+    delete g_request_context_params;
+    g_request_context_params = NULL;
   }
 }
 
@@ -698,7 +752,7 @@ void SimpleResourceLoaderBridge::SetCookie(const GURL& url,
   }
 
   scoped_refptr<CookieSetter> cookie_setter = new CookieSetter();
-  io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+  g_io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
       cookie_setter.get(), &CookieSetter::Set, url, cookie));
 }
 
@@ -714,7 +768,7 @@ std::string SimpleResourceLoaderBridge::GetCookies(
 
   scoped_refptr<CookieGetter> getter = new CookieGetter();
 
-  io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+  g_io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
       getter.get(), &CookieGetter::Get, url));
 
   return getter->GetResult();
@@ -722,23 +776,31 @@ std::string SimpleResourceLoaderBridge::GetCookies(
 
 // static
 bool SimpleResourceLoaderBridge::EnsureIOThread() {
-  if (io_thread)
+  if (g_io_thread)
     return true;
 
-  if (!request_context)
-    SimpleResourceLoaderBridge::Init(NULL);
+#if defined(OS_WIN)
+  // Use NSS for SSL on Windows.  TODO(wtc): this should eventually be hidden
+  // inside DefaultClientSocketFactory::CreateSSLClientSocket.
+  net::ClientSocketFactory::SetSSLClientSocketFactory(
+      net::SSLClientSocketNSSFactory);
+  // We want to be sure to init NSPR on the main thread.
+  base::EnsureNSPRInit();
+#endif
 
-  io_thread = new IOThread();
+  g_io_thread = new IOThread();
   base::Thread::Options options;
   options.message_loop_type = MessageLoop::TYPE_IO;
-  return io_thread->StartWithOptions(options);
+  return g_io_thread->StartWithOptions(options);
 }
 
 // static
 void SimpleResourceLoaderBridge::SetAcceptAllCookies(bool accept_all_cookies) {
-  StaticCookiePolicy::Type policy_type = accept_all_cookies ?
-      StaticCookiePolicy::ALLOW_ALL_COOKIES :
-      StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES;
-  static_cast<StaticCookiePolicy*>(request_context->cookie_policy())->
-      set_type(policy_type);
+  if (g_request_context_params) {
+    g_request_context_params->accept_all_cookies = accept_all_cookies;
+    DCHECK(!g_request_context);
+    DCHECK(!g_io_thread);
+  } else {
+    g_io_thread->SetAcceptAllCookies(accept_all_cookies);
+  }
 }
diff --git a/webkit/tools/test_shell/simple_resource_loader_bridge.h b/webkit/tools/test_shell/simple_resource_loader_bridge.h
index 6d1d822..1bcfe07 100644
--- a/webkit/tools/test_shell/simple_resource_loader_bridge.h
+++ b/webkit/tools/test_shell/simple_resource_loader_bridge.h
@@ -6,23 +6,23 @@
 #define WEBKIT_TOOLS_TEST_SHELL_SIMPLE_RESOURCE_LOADER_BRIDGE_H__
 
 #include <string>
+#include "base/file_path.h"
+#include "net/http/http_cache.h"
 
 class GURL;
 class TestShellRequestContext;
 
 class SimpleResourceLoaderBridge {
  public:
-  // Call this function to initialize the simple resource loader bridge.  If
-  // the given context is null, then a default TestShellRequestContext will be
-  // instantiated.  Otherwise, a reference is taken to the given request
-  // context, which will be released when Shutdown is called.  The caller
-  // should not hold another reference to the request context!  It is safe to
-  // call this function multiple times.
+  // Call this function to initialize the simple resource loader bridge.
+  // It is safe to call this function multiple times.
   //
   // NOTE: If this function is not called, then a default request context will
   // be initialized lazily.
   //
-  static void Init(TestShellRequestContext* context);
+  static void Init(const FilePath& cache_path,
+                   net::HttpCache::Mode cache_mode,
+                   bool no_proxy);
 
   // Call this function to shutdown the simple resource loader bridge.
   static void Shutdown();
diff --git a/webkit/tools/test_shell/test_shell_main.cc b/webkit/tools/test_shell/test_shell_main.cc
index bcd4dfd..dc46850 100644
--- a/webkit/tools/test_shell/test_shell_main.cc
+++ b/webkit/tools/test_shell/test_shell_main.cc
@@ -164,8 +164,7 @@ int main(int argc, char* argv[]) {
 
   // Initializing with a default context, which means no on-disk cookie DB,
   // and no support for directory listings.
-  SimpleResourceLoaderBridge::Init(
-      new TestShellRequestContext(cache_path, cache_mode, layout_test_mode));
+  SimpleResourceLoaderBridge::Init(cache_path, cache_mode, layout_test_mode);
 
   // Load ICU data tables
   icu_util::Initialize();
diff --git a/webkit/tools/test_shell/test_shell_request_context.cc b/webkit/tools/test_shell/test_shell_request_context.cc
index 4c98f28..d76c6c5 100644
--- a/webkit/tools/test_shell/test_shell_request_context.cc
+++ b/webkit/tools/test_shell/test_shell_request_context.cc
@@ -7,9 +7,6 @@
 #include "build/build_config.h"
 
 #include "base/file_path.h"
-#if defined(OS_WIN)
-#include "base/nss_util.h"
-#endif
 #include "net/base/cookie_monster.h"
 #include "net/base/host_resolver.h"
 #include "net/base/ssl_config_service.h"
@@ -19,9 +16,6 @@
 #include "net/proxy/proxy_config_service.h"
 #include "net/proxy/proxy_config_service_fixed.h"
 #include "net/proxy/proxy_service.h"
-#if defined(OS_WIN)
-#include "net/socket/ssl_client_socket_nss_factory.h"
-#endif
 #include "webkit/glue/webkit_glue.h"
 
 TestShellRequestContext::TestShellRequestContext() : cache_thread_("cache") {
@@ -69,15 +63,6 @@ void TestShellRequestContext::Init(
 
   http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
 
-#if defined(OS_WIN)
-  // Use NSS for SSL on Windows.  TODO(wtc): this should eventually be hidden
-  // inside DefaultClientSocketFactory::CreateSSLClientSocket.
-  net::ClientSocketFactory::SetSSLClientSocketFactory(
-      net::SSLClientSocketNSSFactory);
-  // We want to be sure to init NSPR on the main thread.
-  base::EnsureNSPRInit();
-#endif
-
   if (!cache_path.empty())
     CHECK(cache_thread_.StartWithOptions(
               base::Thread::Options(MessageLoop::TYPE_IO, 0)));