diff --git a/atom/app/node_main.cc b/atom/app/node_main.cc index 179b8e50fe1f..66a173c2ae36 100644 --- a/atom/app/node_main.cc +++ b/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); } diff --git a/atom/browser/atom_browser_main_parts.cc b/atom/browser/atom_browser_main_parts.cc index d6393d63f8cb..ccd3c82b9d71 100644 --- a/atom/browser/atom_browser_main_parts.cc +++ b/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(); diff --git a/atom/browser/javascript_environment.cc b/atom/browser/javascript_environment.cc index d6418d1c3d46..b27ceb27fe8f 100644 --- a/atom/browser/javascript_environment.cc +++ b/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::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::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 diff --git a/atom/browser/javascript_environment.h b/atom/browser/javascript_environment.h index da4122c54665..2d5bc4dc17e5 100644 --- a/atom/browser/javascript_environment.h +++ b/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 context() const { return v8::Local::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 context_; + v8::Global 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_ diff --git a/patches/common/chromium/.patches.yaml b/patches/common/chromium/.patches.yaml index 7ea17f2bf788..b5cd0a9a4f33 100644 --- a/patches/common/chromium/.patches.yaml +++ b/patches/common/chromium/.patches.yaml @@ -467,6 +467,6 @@ patches: author: Samuel Attard 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. diff --git a/patches/common/chromium/isolate_holder.patch b/patches/common/chromium/isolate_holder.patch index e4ebeabb0743..86a2bb0077ce 100644 --- a/patches/common/chromium/isolate_holder.patch +++ b/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 task_runner) -- : IsolateHolder(std::move(task_runner), AccessMode::kSingleThread) {} -+ scoped_refptr task_runner, -+ IsolateAllocater* isolate_allocator) -+ : IsolateHolder(std::move(task_runner), AccessMode::kSingleThread, isolate_allocator) {} - - IsolateHolder::IsolateHolder( - scoped_refptr 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 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 task_runner); -+ scoped_refptr task_runner, -+ IsolateAllocater* isolate_allocator = nullptr); - IsolateHolder(scoped_refptr task_runner, -- AccessMode access_mode); -+ AccessMode access_mode, -+ IsolateAllocater* isolate_allocator = nullptr); - IsolateHolder( +@@ -66,7 +66,8 @@ class GIN_EXPORT IsolateHolder { scoped_refptr 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