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

4 files changed, 56 insertions, 13 deletions

diff --git a/net/base/run_all_unittests.cc b/net/base/run_all_unittests.cc
index 57a18b8..f0e8176 100644
--- a/net/base/run_all_unittests.cc
+++ b/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();
 }
diff --git a/net/proxy/proxy_resolver_perftest.cc b/net/proxy/proxy_resolver_perftest.cc
index 4eae4c6..baa305f 100644
--- a/net/proxy/proxy_resolver_perftest.cc
+++ b/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();
 }
-
diff --git a/net/proxy/proxy_resolver_v8.cc b/net/proxy/proxy_resolver_v8.cc
index 935072b..72254b3 100644
--- a/net/proxy/proxy_resolver_v8.cc
+++ b/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
diff --git a/net/proxy/proxy_resolver_v8.h b/net/proxy/proxy_resolver_v8.h
index 823e93a..326b5a4 100644
--- a/net/proxy/proxy_resolver_v8.h
+++ b/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().