Make the v8 Isolate used in the proxy resolver explicit.

For conceptual and performance reasons, the v8 API will require explicit passing
of Isolates in the future instead of relying on the retrieval of one out of thin
air (= either TLS or an evil global variable in v8). This CL makes the Isolate
explicit in ProxyResolverV8::Context.

The default Isolate is retrieved and remembered in the main browser thread and
then passed down to the Context where it is needed in different threads.

Review URL: https://codereview.chromium.org/11959029
Patch from Sven Panne <svenpanne@chromium.org>.

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

chrome/browser/io_thread.cc

@@ -70,6 +70,10 @@
 #include "net/ocsp/nss_ocsp.h"
 #endif
 
+#if !defined(OS_IOS)
+#include "net/proxy/proxy_resolver_v8.h"
+#endif
+
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/proxy_config_service_impl.h"
 #endif  // defined(OS_CHROMEOS)
@@ -369,6 +373,9 @@ IOThread::IOThread(
       sdch_manager_(NULL),
       is_spdy_disabled_by_policy_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
+#if !defined(OS_IOS)
+  net::ProxyResolverV8::RememberDefaultIsolate();
+#endif
   // We call RegisterPrefs() here (instead of inside browser_prefs.cc) to make
   // sure that everything is initialized in the right order.
   //

net/base/run_all_unittests.cc

@@ -16,6 +16,10 @@
 #include "net/android/net_jni_registrar.h"
 #endif
 
+#if !defined(OS_IOS)
+#include "net/proxy/proxy_resolver_v8.h"
+#endif
+
 using net::internal::ClientSocketPoolBaseHelper;
 using net::SpdySession;
 
@@ -43,5 +47,10 @@ int main(int argc, char** argv) {
   // single-threaded.
   net::EnableSSLServerSockets();
 
+#if !defined(OS_IOS)
+  // This has to be done on the main thread.
+  net::ProxyResolverV8::RememberDefaultIsolate();
+#endif
+
   return test_suite.Run();
 }

net/proxy/proxy_resolver_perftest.cc

@@ -215,10 +215,12 @@ class MockJSBindings : public net::ProxyResolverV8::JSBindings {
 };
 
 TEST(ProxyResolverPerfTest, ProxyResolverV8) {
+  // This has to be done on the main thread.
+  net::ProxyResolverV8::RememberDefaultIsolate();
+
   MockJSBindings js_bindings;
   net::ProxyResolverV8 resolver;
   resolver.set_js_bindings(&js_bindings);
   PacPerfSuiteRunner runner(&resolver, "ProxyResolverV8");
   runner.RunAllTests();
 }
-

net/proxy/proxy_resolver_v8.cc

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#define V8_DISABLE_DEPRECATIONS 1
-
 #include "net/proxy/proxy_resolver_v8.h"
 
 #include <algorithm>
@@ -334,15 +332,17 @@ bool IsInNetEx(const std::string& ip_address, const std::string& ip_prefix) {
 
 class ProxyResolverV8::Context {
  public:
-  explicit Context(ProxyResolverV8* parent)
-      : parent_(parent) {
+  Context(ProxyResolverV8* parent, v8::Isolate* isolate)
+      : parent_(parent),
+        isolate_(isolate) {
+    DCHECK(isolate);
   }
 
   ~Context() {
-    v8::Locker locked;
+    v8::Locker locked(isolate_);
 
-    v8_this_.Dispose();
-    v8_context_.Dispose();
+    v8_this_.Dispose(isolate_);
+    v8_context_.Dispose(isolate_);
   }
 
   JSBindings* js_bindings() {
@@ -350,7 +350,7 @@ class ProxyResolverV8::Context {
   }
 
   int ResolveProxy(const GURL& query_url, ProxyInfo* results) {
-    v8::Locker locked;
+    v8::Locker locked(isolate_);
     v8::HandleScope scope;
 
     v8::Context::Scope function_scope(v8_context_);
@@ -401,10 +401,11 @@ class ProxyResolverV8::Context {
   }
 
   int InitV8(const scoped_refptr<ProxyResolverScriptData>& pac_script) {
-    v8::Locker locked;
+    v8::Locker locked(isolate_);
     v8::HandleScope scope;
 
-    v8_this_ = v8::Persistent<v8::External>::New(v8::External::New(this));
+    v8_this_ = v8::Persistent<v8::External>::New(isolate_,
+                                                 v8::External::New(this));
     v8::Local<v8::ObjectTemplate> global_template = v8::ObjectTemplate::New();
 
     // Attach the javascript bindings.
@@ -479,7 +480,7 @@ class ProxyResolverV8::Context {
   }
 
   void PurgeMemory() {
-    v8::Locker locked;
+    v8::Locker locked(isolate_);
     v8::V8::LowMemoryNotification();
   }
 
@@ -669,6 +670,7 @@ class ProxyResolverV8::Context {
 
   mutable base::Lock lock_;
   ProxyResolverV8* parent_;
+  v8::Isolate* isolate_;
   v8::Persistent<v8::External> v8_this_;
   v8::Persistent<v8::Context> v8_context_;
 };
@@ -729,11 +731,30 @@ int ProxyResolverV8::SetPacScript(
     return ERR_PAC_SCRIPT_FAILED;
 
   // Try parsing the PAC script.
-  scoped_ptr<Context> context(new Context(this));
+  scoped_ptr<Context> context(new Context(this, GetDefaultIsolate()));
   int rv = context->InitV8(script_data);
   if (rv == OK)
     context_.reset(context.release());
   return rv;
 }
 
+// static
+void ProxyResolverV8::RememberDefaultIsolate() {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  DCHECK(isolate)
+      << "ProxyResolverV8::RememberDefaultIsolate called on wrong thread";
+  DCHECK(g_default_isolate_ == NULL || g_default_isolate_ == isolate)
+      << "Default Isolate can not be changed";
+  g_default_isolate_ = isolate;
+}
+
+// static
+v8::Isolate* ProxyResolverV8::GetDefaultIsolate() {
+  DCHECK(g_default_isolate_)
+      << "Must call ProxyResolverV8::RememberDefaultIsolate() first";
+  return g_default_isolate_;
+}
+
+v8::Isolate* ProxyResolverV8::g_default_isolate_ = NULL;
+
 }  // namespace net

net/proxy/proxy_resolver_v8.h

@@ -10,6 +10,10 @@
 #include "net/base/net_export.h"
 #include "net/proxy/proxy_resolver.h"
 
+namespace v8 {
+class Isolate;
+}  // namespace v8
+
 namespace net {
 
 // Implementation of ProxyResolver that uses V8 to evaluate PAC scripts.
@@ -85,7 +89,14 @@ class NET_EXPORT_PRIVATE ProxyResolverV8 : public ProxyResolver {
       const scoped_refptr<ProxyResolverScriptData>& script_data,
       const net::CompletionCallback& /*callback*/) OVERRIDE;
 
+  // Remember the default Isolate, must be called from the main thread. This
+  // hack can be removed when the "default Isolate" concept is gone.
+  static void RememberDefaultIsolate();
+  static v8::Isolate* GetDefaultIsolate();
+
  private:
+  static v8::Isolate* g_default_isolate_;
+
   // Context holds the Javascript state for the most recently loaded PAC
   // script. It corresponds with the data from the last call to
   // SetPacScript().