Lazily initialize the v8::Isolate used by ProxyResolverV8 (in the browser process).

Also removes the limitation of ProxyResolverV8 not being available in single-process mode which no longer applies.

BUG=454983

Review URL: https://codereview.chromium.org/957933002

Cr-Commit-Position: refs/heads/master@{#318277}

chrome/browser/net/proxy_service_factory.cc

@@ -104,12 +104,6 @@ net::ProxyService* ProxyServiceFactory::CreateProxyService(
   bool use_v8 = false;
 #else
   bool use_v8 = !command_line.HasSwitch(switches::kWinHttpProxyResolver);
-  if (use_v8 && command_line.HasSwitch(switches::kSingleProcess)) {
-    // See the note about V8 multithreading in net/proxy/proxy_resolver_v8.h
-    // to understand why we have this limitation.
-    LOG(ERROR) << "Cannot use V8 Proxy resolver in single process mode.";
-    use_v8 = false;  // Fallback to non-v8 implementation.
-  }
 #endif  // defined(OS_IOS)
 
   size_t num_pac_threads = 0u;  // Use default number of threads.
@@ -133,8 +127,6 @@ net::ProxyService* ProxyServiceFactory::CreateProxyService(
 #if defined(OS_IOS)
     NOTREACHED();
 #else
-    net::ProxyResolverV8::EnsureIsolateCreated();
-
     net::DhcpProxyScriptFetcher* dhcp_proxy_script_fetcher;
 #if defined(OS_CHROMEOS)
     dhcp_proxy_script_fetcher =

net/proxy/mojo_proxy_resolver_factory_impl.cc

@@ -15,7 +15,6 @@ namespace {
 
 scoped_ptr<ProxyResolver> CreateDefaultProxyResolver(
     HostResolver* host_resolver) {
-  ProxyResolverV8::EnsureIsolateCreated();
   return make_scoped_ptr(
       new ProxyResolverV8Tracing(host_resolver, nullptr, nullptr));
 }

net/proxy/proxy_resolver_perftest.cc

@@ -215,8 +215,6 @@ class MockJSBindings : public net::ProxyResolverV8::JSBindings {
 };
 
 TEST(ProxyResolverPerfTest, ProxyResolverV8) {
-  net::ProxyResolverV8::EnsureIsolateCreated();
-
   MockJSBindings js_bindings;
   net::ProxyResolverV8 resolver;
   resolver.set_js_bindings(&js_bindings);

net/proxy/proxy_resolver_v8.cc

@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/debug/leak_annotations.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
@@ -345,6 +346,57 @@ bool IsPlainHostName(const std::string& hostname_utf8) {
   return !ParseIPLiteralToNumber(hostname_utf8, &unused);
 }
 
+// All instances of ProxyResolverV8 share the same v8::Isolate. This isolate is
+// created lazily the first time it is needed and lives until process shutdown.
+// This creation might happen from any thread, as ProxyResolverV8 is typically
+// run in a threadpool.
+//
+// TODO(eroman): The lazily created isolate is never freed. Instead it should be
+// disposed once there are no longer any ProxyResolverV8 referencing it.
+class SharedIsolateFactory {
+ public:
+  SharedIsolateFactory() : has_initialized_v8_(false) {}
+
+  // Lazily creates a v8::Isolate, or returns the already created instance.
+  v8::Isolate* GetSharedIsolate() {
+    base::AutoLock l(lock_);
+
+    if (!holder_) {
+      // Do one-time initialization for V8.
+      if (!has_initialized_v8_) {
+#ifdef V8_USE_EXTERNAL_STARTUP_DATA
+        gin::IsolateHolder::LoadV8Snapshot();
+#endif
+
+        gin::IsolateHolder::Initialize(
+            gin::IsolateHolder::kNonStrictMode,
+            gin::ArrayBufferAllocator::SharedInstance());
+
+        has_initialized_v8_ = true;
+      }
+
+      holder_.reset(new gin::IsolateHolder);
+    }
+
+    return holder_->isolate();
+  }
+
+  v8::Isolate* GetSharedIsolateWithoutCreating() {
+    base::AutoLock l(lock_);
+    return holder_ ? holder_->isolate() : NULL;
+  }
+
+ private:
+  base::Lock lock_;
+  scoped_ptr<gin::IsolateHolder> holder_;
+  bool has_initialized_v8_;
+
+  DISALLOW_COPY_AND_ASSIGN(SharedIsolateFactory);
+};
+
+base::LazyInstance<SharedIsolateFactory>::Leaky g_isolate_factory =
+    LAZY_INSTANCE_INITIALIZER;
+
 }  // namespace
 
 // ProxyResolverV8::Context ---------------------------------------------------
@@ -789,53 +841,39 @@ int ProxyResolverV8::SetPacScript(
     return ERR_PAC_SCRIPT_FAILED;
 
   // Try parsing the PAC script.
-  scoped_ptr<Context> context(new Context(this, GetDefaultIsolate()));
+  scoped_ptr<Context> context(
+      new Context(this, g_isolate_factory.Get().GetSharedIsolate()));
   int rv = context->InitV8(script_data);
   if (rv == OK)
     context_.reset(context.release());
   return rv;
 }
 
-// static
-void ProxyResolverV8::EnsureIsolateCreated() {
-  if (g_proxy_resolver_isolate_)
-    return;
-  gin::IsolateHolder::Initialize(gin::IsolateHolder::kNonStrictMode,
-                                 gin::ArrayBufferAllocator::SharedInstance());
-  g_proxy_resolver_isolate_ = new gin::IsolateHolder;
-  ANNOTATE_LEAKING_OBJECT_PTR(g_proxy_resolver_isolate_);
-}
-
-// static
-v8::Isolate* ProxyResolverV8::GetDefaultIsolate() {
-  DCHECK(g_proxy_resolver_isolate_)
-      << "Must call ProxyResolverV8::EnsureIsolateCreated() first";
-  return g_proxy_resolver_isolate_->isolate();
-}
-
-gin::IsolateHolder* ProxyResolverV8::g_proxy_resolver_isolate_ = NULL;
-
 // static
 size_t ProxyResolverV8::GetTotalHeapSize() {
-  if (!g_proxy_resolver_isolate_)
+  v8::Isolate* isolate =
+      g_isolate_factory.Get().GetSharedIsolateWithoutCreating();
+  if (!isolate)
     return 0;
 
-  v8::Locker locked(g_proxy_resolver_isolate_->isolate());
-  v8::Isolate::Scope isolate_scope(g_proxy_resolver_isolate_->isolate());
+  v8::Locker locked(isolate);
+  v8::Isolate::Scope isolate_scope(isolate);
   v8::HeapStatistics heap_statistics;
-  g_proxy_resolver_isolate_->isolate()->GetHeapStatistics(&heap_statistics);
+  isolate->GetHeapStatistics(&heap_statistics);
   return heap_statistics.total_heap_size();
 }
 
 // static
 size_t ProxyResolverV8::GetUsedHeapSize() {
-  if (!g_proxy_resolver_isolate_)
+  v8::Isolate* isolate =
+      g_isolate_factory.Get().GetSharedIsolateWithoutCreating();
+  if (!isolate)
     return 0;
 
-  v8::Locker locked(g_proxy_resolver_isolate_->isolate());
-  v8::Isolate::Scope isolate_scope(g_proxy_resolver_isolate_->isolate());
+  v8::Locker locked(isolate);
+  v8::Isolate::Scope isolate_scope(isolate);
   v8::HeapStatistics heap_statistics;
-  g_proxy_resolver_isolate_->isolate()->GetHeapStatistics(&heap_statistics);
+  isolate->GetHeapStatistics(&heap_statistics);
   return heap_statistics.used_heap_size();
 }
 

net/proxy/proxy_resolver_v8.h

@@ -10,35 +10,9 @@
 #include "net/base/net_export.h"
 #include "net/proxy/proxy_resolver.h"
 
-namespace gin {
-class IsolateHolder;
-}  // namespace gin
-
-namespace v8 {
-class HeapStatistics;
-class Isolate;
-}  // namespace v8
-
 namespace net {
 
 // Implementation of ProxyResolver that uses V8 to evaluate PAC scripts.
-//
-// ----------------------------------------------------------------------------
-// !!! Important note on threading model:
-// ----------------------------------------------------------------------------
-// There can be only one instance of V8 running at a time. To enforce this
-// constraint, ProxyResolverV8 holds a v8::Locker during execution. Therefore
-// it is OK to run multiple instances of ProxyResolverV8 on different threads,
-// since only one will be running inside V8 at a time.
-//
-// It is important that *ALL* instances of V8 in the process be using
-// v8::Locker. If not there can be race conditions between the non-locked V8
-// instances and the locked V8 instances used by ProxyResolverV8 (assuming they
-// run on different threads).
-//
-// This is the case with the V8 instance used by chromium's renderer -- it runs
-// on a different thread from ProxyResolver (renderer thread vs PAC thread),
-// and does not use locking since it expects to be alone.
 class NET_EXPORT_PRIVATE ProxyResolverV8 : public ProxyResolver {
  public:
   // Interface for the javascript bindings.
@@ -93,21 +67,12 @@ class NET_EXPORT_PRIVATE ProxyResolverV8 : public ProxyResolver {
   int SetPacScript(const scoped_refptr<ProxyResolverScriptData>& script_data,
                    const net::CompletionCallback& /*callback*/) override;
 
-  // Create an isolate to use for the proxy resolver. If the embedder invokes
-  // this method multiple times, it must be invoked in a thread safe manner,
-  // e.g. always from the same thread.
-  static void EnsureIsolateCreated();
-
-  static v8::Isolate* GetDefaultIsolate();
-
   // Get total/ued heap memory usage of all v8 instances used by the proxy
   // resolver.
   static size_t GetTotalHeapSize();
   static size_t GetUsedHeapSize();
 
  private:
-  static gin::IsolateHolder* g_proxy_resolver_isolate_;
-
   // Context holds the Javascript state for the most recently loaded PAC
   // script. It corresponds with the data from the last call to
   // SetPacScript().

net/test/run_all_unittests.cc

@@ -19,14 +19,6 @@
 #include "url/android/url_jni_registrar.h"
 #endif
 
-#if !defined(OS_IOS)
-#include "net/proxy/proxy_resolver_v8.h"
-#endif
-
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
-#include "gin/public/isolate_holder.h"
-#endif
-
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
 #include "third_party/mojo/src/mojo/edk/embedder/test_embedder.h"
 #endif
@@ -65,14 +57,6 @@ int main(int argc, char** argv) {
   // single-threaded.
   net::EnableSSLServerSockets();
 
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
-  gin::IsolateHolder::LoadV8Snapshot();
-#endif
-
-#if !defined(OS_IOS)
-  net::ProxyResolverV8::EnsureIsolateCreated();
-#endif
-
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
   mojo::embedder::test::InitWithSimplePlatformSupport();
 #endif