fix: Use the new isolate initialization api

https://chromium-review.googlesource.com/c/chromium/src/+/1015020

atom/app/node_main.cc

@@ -51,7 +51,7 @@ int NodeMain(int argc, char* argv[]) {
     base::TaskScheduler::CreateAndStartWithDefaultParams("Electron");
 
     // Initialize gin::IsolateHolder.
-    JavascriptEnvironment gin_env;
+    JavascriptEnvironment gin_env(loop);
 
     // Explicitly register electron's builtin modules.
     NodeBindings::RegisterBuiltinModules();
@@ -100,6 +100,7 @@ int NodeMain(int argc, char* argv[]) {
     node::RunAtExit(env);
     gin_env.platform()->DrainTasks(env->isolate());
     gin_env.platform()->CancelPendingDelayedTasks(env->isolate());
+    gin_env.platform()->UnregisterIsolate(env->isolate());
 
     node::FreeEnvironment(env);
   }

atom/browser/atom_browser_main_parts.cc

@@ -140,10 +140,9 @@ void AtomBrowserMainParts::PostEarlyInitialization() {
 
   // The ProxyResolverV8 has setup a complete V8 environment, in order to
   // avoid conflicts we only initialize our V8 environment after that.
-  js_env_.reset(new JavascriptEnvironment);
+  js_env_.reset(new JavascriptEnvironment(node_bindings_->uv_loop()));
 
   node_bindings_->Initialize();
-
   // Create the global environment.
   node::Environment* env = node_bindings_->CreateEnvironment(
       js_env_->context(), js_env_->platform());
@@ -218,8 +217,6 @@ void AtomBrowserMainParts::ToolkitInitialized() {
 }
 
 void AtomBrowserMainParts::PreMainMessageLoopRun() {
-  js_env_->OnMessageLoopCreated();
-
   // Run user's main script before most things get initialized, so we can have
   // a chance to setup everything.
   node_bindings_->PrepareMessageLoop();

atom/browser/javascript_environment.cc

@@ -19,29 +19,22 @@
 
 namespace atom {
 
-JavascriptEnvironment::JavascriptEnvironment()
-    : initialized_(Initialize()),
+JavascriptEnvironment::JavascriptEnvironment(uv_loop_t* event_loop)
+    : isolate_(Initialize(event_loop)),
       isolate_holder_(base::ThreadTaskRunnerHandle::Get(),
-                      new AtomIsolateAllocator(this)),
-      isolate_(isolate_holder_.isolate()),
+                      gin::IsolateHolder::kSingleThread,
+                      gin::IsolateHolder::kAllowAtomicsWait,
+                      gin::IsolateHolder::IsolateCreationMode::kNormal,
+                      isolate_),
       isolate_scope_(isolate_),
       locker_(isolate_),
       handle_scope_(isolate_),
       context_(isolate_, v8::Context::New(isolate_)),
-      context_scope_(v8::Local<v8::Context>::New(isolate_, context_)) {
-  // The assumption is made here that the isolate_holder_ holds a single isolate
-  // if this is not the case or changes in the constructor above the logic
-  // below must be rewritten
-  node::InitializePrivatePropertiesOnIsolateData(isolate_data_);
-}
+      context_scope_(v8::Local<v8::Context>::New(isolate_, context_)) {}
 
 JavascriptEnvironment::~JavascriptEnvironment() = default;
 
-void JavascriptEnvironment::OnMessageLoopCreated() {}
-
-void JavascriptEnvironment::OnMessageLoopDestroying() {}
-
-bool JavascriptEnvironment::Initialize() {
+v8::Isolate* JavascriptEnvironment::Initialize(uv_loop_t* event_loop) {
   auto* cmd = base::CommandLine::ForCurrentProcess();
 
   // --js-flags.
@@ -56,12 +49,21 @@ bool JavascriptEnvironment::Initialize() {
   platform_ = node::CreatePlatform(
       base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0),
       tracing_controller);
+
   v8::V8::InitializePlatform(platform_);
   gin::IsolateHolder::Initialize(
       gin::IsolateHolder::kNonStrictMode, gin::IsolateHolder::kStableV8Extras,
       gin::ArrayBufferAllocator::SharedInstance(),
       nullptr /* external_reference_table */, false /* create_v8_platform */);
-  return true;
+
+  v8::Isolate* isolate = v8::Isolate::Allocate();
+  platform_->RegisterIsolate(isolate, event_loop);
+
+  return isolate;
+}
+
+void JavascriptEnvironment::OnMessageLoopDestroying() {
+  platform_->UnregisterIsolate(isolate_);
 }
 
 NodeEnvironment::NodeEnvironment(node::Environment* env) : env_(env) {}
@@ -70,16 +72,4 @@ NodeEnvironment::~NodeEnvironment() {
   node::FreeEnvironment(env_);
 }
 
-AtomIsolateAllocator::AtomIsolateAllocator(JavascriptEnvironment* env)
-    : env_(env) {}
-
-v8::Isolate* AtomIsolateAllocator::Allocate() {
-  v8::Isolate* isolate = v8::Isolate::Allocate();
-  // This is a cheatsy way to add the Isolate and it's IsolateData to the node
-  // platform before it is ready
-  env_->set_isolate_data(node::CreateIsolateData(
-      isolate, uv_default_loop(), env_->platform(), true /* only_register */));
-  return isolate;
-}
-
 }  // namespace atom

atom/browser/javascript_environment.h

@@ -7,11 +7,11 @@
 
 #include "base/macros.h"
 #include "gin/public/isolate_holder.h"
+#include "uv.h"  // NOLINT(build/include)
 
 namespace node {
 class Environment;
 class MultiIsolatePlatform;
-class IsolateData;
 }  // namespace node
 
 namespace atom {
@@ -19,10 +19,9 @@ namespace atom {
 // Manage the V8 isolate and context automatically.
 class JavascriptEnvironment {
  public:
-  JavascriptEnvironment();
+  explicit JavascriptEnvironment(uv_loop_t* event_loop);
   ~JavascriptEnvironment();
 
-  void OnMessageLoopCreated();
   void OnMessageLoopDestroying();
 
   node::MultiIsolatePlatform* platform() const { return platform_; }
@@ -30,23 +29,19 @@ class JavascriptEnvironment {
   v8::Local<v8::Context> context() const {
     return v8::Local<v8::Context>::New(isolate_, context_);
   }
-  void set_isolate_data(node::IsolateData* data) { isolate_data_ = data; }
 
  private:
-  bool Initialize();
-
+  v8::Isolate* Initialize(uv_loop_t* event_loop);
   // Leaked on exit.
   node::MultiIsolatePlatform* platform_;
 
-  bool initialized_;
-  gin::IsolateHolder isolate_holder_;
   v8::Isolate* isolate_;
+  gin::IsolateHolder isolate_holder_;
   v8::Isolate::Scope isolate_scope_;
   v8::Locker locker_;
   v8::HandleScope handle_scope_;
-  v8::UniquePersistent<v8::Context> context_;
+  v8::Global<v8::Context> context_;
   v8::Context::Scope context_scope_;
-  node::IsolateData* isolate_data_;
 
   DISALLOW_COPY_AND_ASSIGN(JavascriptEnvironment);
 };
@@ -63,18 +58,6 @@ class NodeEnvironment {
   DISALLOW_COPY_AND_ASSIGN(NodeEnvironment);
 };
 
-class AtomIsolateAllocator : public gin::IsolateAllocater {
- public:
-  explicit AtomIsolateAllocator(JavascriptEnvironment* env);
-
-  v8::Isolate* Allocate() override;
-
- private:
-  JavascriptEnvironment* env_;
-
-  DISALLOW_COPY_AND_ASSIGN(AtomIsolateAllocator);
-};
-
 }  // namespace atom
 
 #endif  // ATOM_BROWSER_JAVASCRIPT_ENVIRONMENT_H_

patches/common/chromium/.patches.yaml

@@ -467,6 +467,6 @@ patches:
   author: Samuel Attard <samuel.r.attard@gmail.com>
   file: isolate_holder.patch
   description: |
-    Expose a hook that allows embedders to intercept allocated isolates
-    before they are initialized or used so they can be added to the correct
-    platform.  (In our case, node_platform)
+    Pass pre allocated isolate for initialization, node platform
+    needs to register on an isolate so that it can be used later
+    down in the initialization process of an isolate.

patches/common/chromium/isolate_holder.patch

@@ -1,94 +1,41 @@
 diff --git a/gin/isolate_holder.cc b/gin/isolate_holder.cc
-index e099fd3f03e5..46b5ea629d3a 100644
+index e099fd3f03e5..4b362843e4f8 100644
 --- a/gin/isolate_holder.cc
 +++ b/gin/isolate_holder.cc
-@@ -30,23 +30,31 @@ v8::ArrayBuffer::Allocator* g_array_buffer_allocator = nullptr;
- const intptr_t* g_reference_table = nullptr;
- }  // namespace
- 
-+v8::Isolate* IsolateAllocater::Allocate() {
-+    return nullptr;
-+}
-+
- IsolateHolder::IsolateHolder(
--    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
--    : IsolateHolder(std::move(task_runner), AccessMode::kSingleThread) {}
-+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-+    IsolateAllocater* isolate_allocator)
-+    : IsolateHolder(std::move(task_runner), AccessMode::kSingleThread, isolate_allocator) {}
- 
- IsolateHolder::IsolateHolder(
-     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
--    AccessMode access_mode)
-+    AccessMode access_mode,
-+    IsolateAllocater* isolate_allocator)
-     : IsolateHolder(std::move(task_runner),
-                     access_mode,
-                     kAllowAtomicsWait,
--                    IsolateCreationMode::kNormal) {}
-+                    IsolateCreationMode::kNormal,
-+                    isolate_allocator) {}
- 
- IsolateHolder::IsolateHolder(
+@@ -46,7 +46,8 @@ IsolateHolder::IsolateHolder(
      scoped_refptr<base::SingleThreadTaskRunner> task_runner,
      AccessMode access_mode,
      AllowAtomicsWaitMode atomics_wait_mode,
 -    IsolateCreationMode isolate_creation_mode)
 +    IsolateCreationMode isolate_creation_mode,
-+    IsolateAllocater* isolate_allocator)
++    v8::Isolate* isolate)
      : access_mode_(access_mode) {
    DCHECK(task_runner);
    DCHECK(task_runner->BelongsToCurrentThread());
-@@ -54,7 +62,11 @@ IsolateHolder::IsolateHolder(
+@@ -54,7 +55,11 @@ IsolateHolder::IsolateHolder(
    v8::ArrayBuffer::Allocator* allocator = g_array_buffer_allocator;
    CHECK(allocator) << "You need to invoke gin::IsolateHolder::Initialize first";
  
 -  isolate_ = v8::Isolate::Allocate();
-+  if (isolate_allocator) {
-+    isolate_ = isolate_allocator->Allocate();
-+  } else {
++  if (!isolate) {
 +    isolate_ = v8::Isolate::Allocate();
++  } else {
++    isolate_ = isolate;
 +  }
    isolate_data_.reset(
        new PerIsolateData(isolate_, allocator, access_mode_, task_runner));
    if (isolate_creation_mode == IsolateCreationMode::kCreateSnapshot) {
 diff --git a/gin/public/isolate_holder.h b/gin/public/isolate_holder.h
-index 84cf66e6e9cd..2d5ebb76f050 100644
+index 84cf66e6e9cd..cc0d65e4e4be 100644
 --- a/gin/public/isolate_holder.h
 +++ b/gin/public/isolate_holder.h
-@@ -22,6 +22,14 @@ namespace gin {
- class PerIsolateData;
- class V8IsolateMemoryDumpProvider;
- 
-+class GIN_EXPORT IsolateAllocater {
-+  public:
-+    explicit IsolateAllocater() {};
-+    virtual ~IsolateAllocater() {};
-+
-+    virtual v8::Isolate* Allocate();
-+};
-+
- // To embed Gin, first initialize gin using IsolateHolder::Initialize and then
- // create an instance of IsolateHolder to hold the v8::Isolate in which you
- // will execute JavaScript. You might wish to subclass IsolateHolder if you
-@@ -59,14 +67,17 @@ class GIN_EXPORT IsolateHolder {
-   };
- 
-   explicit IsolateHolder(
--      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
-+      scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-+      IsolateAllocater* isolate_allocator = nullptr);
-   IsolateHolder(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
--                AccessMode access_mode);
-+                AccessMode access_mode,
-+                IsolateAllocater* isolate_allocator = nullptr);
-   IsolateHolder(
+@@ -66,7 +66,8 @@ class GIN_EXPORT IsolateHolder {
        scoped_refptr<base::SingleThreadTaskRunner> task_runner,
        AccessMode access_mode,
        AllowAtomicsWaitMode atomics_wait_mode,
 -      IsolateCreationMode isolate_creation_mode = IsolateCreationMode::kNormal);
 +      IsolateCreationMode isolate_creation_mode = IsolateCreationMode::kNormal,
-+      IsolateAllocater* isolate_allocator = nullptr);
++      v8::Isolate* isolate = nullptr);
    ~IsolateHolder();
  
    // Should be invoked once before creating IsolateHolder instances to