From 8104c7908a2e281cdb2e380dd2ec49c3ead7fa3e Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Fri, 26 Jan 2024 19:53:07 +0100
Subject: [PATCH] fix: potential `async_hooks` crash in `NotifyWindowRestore`
 on Windows (#40576)

* fix: potential async_hooks crash in NotifyWindowRestore on Windows

* fix: don't use CallbackScope for Error objects
---
 .../browser/api/electron_api_auto_updater.cc  | 23 +++++++++++---
 .../common/gin_helper/event_emitter_caller.cc |  8 +++++
 shell/renderer/electron_api_service_impl.cc   |  8 ++---
 spec/api-browser-window-spec.ts               | 31 +++++++++++++++++++
 4 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/shell/browser/api/electron_api_auto_updater.cc b/shell/browser/api/electron_api_auto_updater.cc
index 051ce07e47..387fda264d 100644
--- a/shell/browser/api/electron_api_auto_updater.cc
+++ b/shell/browser/api/electron_api_auto_updater.cc
@@ -32,13 +32,26 @@ void AutoUpdater::OnError(const std::string& message) {
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::HandleScope handle_scope(isolate);
   v8::Local<v8::Object> wrapper;
+
+  // We do not use gin::EmitEvent here because we do not want to
+  // put this in its own CallbackScope and delegate to Node.js'
+  // specialized handling for Error objects.
   if (GetWrapper(isolate).ToLocal(&wrapper)) {
     auto error = v8::Exception::Error(gin::StringToV8(isolate, message));
-    gin_helper::EmitEvent(
-        isolate, wrapper, "error",
-        error->ToObject(isolate->GetCurrentContext()).ToLocalChecked(),
-        // Message is also emitted to keep compatibility with old code.
-        message);
+    std::vector<v8::Local<v8::Value>> args = {
+        gin::StringToV8(isolate, "error"),
+        gin::ConvertToV8(
+            isolate,
+            error->ToObject(isolate->GetCurrentContext()).ToLocalChecked()),
+        gin::StringToV8(isolate, message),
+    };
+
+    gin_helper::MicrotasksScope microtasks_scope(
+        isolate, wrapper->GetCreationContextChecked()->GetMicrotaskQueue(),
+        true);
+
+    node::MakeCallback(isolate, wrapper, "emit", args.size(), args.data(),
+                       {0, 0});
   }
 }
 
diff --git a/shell/common/gin_helper/event_emitter_caller.cc b/shell/common/gin_helper/event_emitter_caller.cc
index 468f0d854f..d68c18d781 100644
--- a/shell/common/gin_helper/event_emitter_caller.cc
+++ b/shell/common/gin_helper/event_emitter_caller.cc
@@ -13,6 +13,14 @@ v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
                                         v8::Local<v8::Object> obj,
                                         const char* method,
                                         ValueVector* args) {
+  // Only set up the node::CallbackScope if there's a node environment.
+  std::unique_ptr<node::CallbackScope> callback_scope;
+  if (node::Environment::GetCurrent(isolate)) {
+    v8::HandleScope handle_scope(isolate);
+    callback_scope = std::make_unique<node::CallbackScope>(
+        isolate, v8::Object::New(isolate), node::async_context{0, 0});
+  }
+
   // Perform microtask checkpoint after running JavaScript.
   gin_helper::MicrotasksScope microtasks_scope(
       isolate, obj->GetCreationContextChecked()->GetMicrotaskQueue(), true);
diff --git a/shell/renderer/electron_api_service_impl.cc b/shell/renderer/electron_api_service_impl.cc
index 529de045fb..db5b910242 100644
--- a/shell/renderer/electron_api_service_impl.cc
+++ b/shell/renderer/electron_api_service_impl.cc
@@ -61,12 +61,10 @@ void InvokeIpcCallback(v8::Local<v8::Context> context,
 
   // Only set up the node::CallbackScope if there's a node environment.
   // Sandboxed renderers don't have a node environment.
-  node::Environment* env = node::Environment::GetCurrent(context);
   std::unique_ptr<node::CallbackScope> callback_scope;
-  if (env) {
-    node::async_context async_context = {};
-    callback_scope = std::make_unique<node::CallbackScope>(isolate, ipcNative,
-                                                           async_context);
+  if (node::Environment::GetCurrent(context)) {
+    callback_scope = std::make_unique<node::CallbackScope>(
+        isolate, ipcNative, node::async_context{0, 0});
   }
 
   auto callback_key = gin::ConvertToV8(isolate, callback_name)
diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts
index 764b6e72bb..bc7cc52208 100644
--- a/spec/api-browser-window-spec.ts
+++ b/spec/api-browser-window-spec.ts
@@ -14,6 +14,7 @@ import { closeWindow, closeAllWindows } from './lib/window-helpers';
 import { areColorsSimilar, captureScreen, HexColors, getPixelColor } from './lib/screen-helpers';
 import { once } from 'node:events';
 import { setTimeout } from 'node:timers/promises';
+import { setTimeout as syncSetTimeout } from 'node:timers';
 
 const fixtures = path.resolve(__dirname, 'fixtures');
 const mainFixtures = path.resolve(__dirname, 'fixtures');
@@ -1809,6 +1810,36 @@ describe('BrowserWindow module', () => {
           expect(w.isFullScreen()).to.equal(true);
         });
 
+        it('does not crash if maximized, minimized, then restored to maximized state', (done) => {
+          w.destroy();
+          w = new BrowserWindow({ show: false });
+
+          w.show();
+
+          let count = 0;
+
+          w.on('maximize', () => {
+            if (count === 0) syncSetTimeout(() => { w.minimize(); });
+            count++;
+          });
+
+          w.on('minimize', () => {
+            if (count === 1) syncSetTimeout(() => { w.restore(); });
+            count++;
+          });
+
+          w.on('restore', () => {
+            try {
+              throw new Error('hey!');
+            } catch (e: any) {
+              expect(e.message).to.equal('hey!');
+              done();
+            }
+          });
+
+          w.maximize();
+        });
+
         it('checks normal bounds for maximized transparent window', async () => {
           w.destroy();
           w = new BrowserWindow({