chore: address review feedback

patches/chromium/feat_plumb_node_integration_in_worker_through_workersettings.patch

@@ -34,6 +34,48 @@ index 7ea6daec53a497bf867d799e041bf6ae7191ef7b..15940624940d5c629c40319f45c59282
      agent_group_scheduler_compositor_task_runner =
          execution_context->GetScheduler()
              ->ToFrameScheduler()
+diff --git a/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc b/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
+index 936f5ebe28caa993ed5de0f7de3613fa338e263f..961ac8091aa82128e1cfb8800a7efcb80d100a05 100644
+--- a/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
++++ b/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
+@@ -13,10 +13,12 @@
+ #include "third_party/blink/public/platform/task_type.h"
+ #include "third_party/blink/renderer/core/dom/document.h"
+ #include "third_party/blink/renderer/core/execution_context/security_context.h"
++#include "third_party/blink/renderer/core/exported/web_view_impl.h"
+ #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
+ #include "third_party/blink/renderer/core/frame/local_dom_window.h"
+ #include "third_party/blink/renderer/core/frame/local_frame.h"
+ #include "third_party/blink/renderer/core/frame/local_frame_client.h"
++#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
+ #include "third_party/blink/renderer/core/inspector/thread_debugger_common_impl.h"
+ #include "third_party/blink/renderer/core/loader/worker_fetch_context.h"
+ #include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
+@@ -135,6 +137,14 @@ void ThreadedWorkletMessagingProxy::Initialize(
+   DCHECK(csp);
+ 
+   LocalFrameClient* frame_client = window->GetFrame()->Client();
++  auto worklet_settings =
++      std::make_unique<WorkerSettings>(window->GetFrame()->GetSettings());
++  if (auto* web_local_frame = WebLocalFrameImpl::FromFrame(window->GetFrame())) {
++    if (auto* web_view = web_local_frame->ViewImpl()) {
++      worklet_settings->SetNodeIntegrationInWorker(
++          web_view->GetWebPreferences().node_integration_in_worker);
++    }
++  }
+   auto global_scope_creation_params =
+       std::make_unique<GlobalScopeCreationParams>(
+           window->Url(), mojom::blink::ScriptType::kModule, global_scope_name,
+@@ -147,8 +157,7 @@ void ThreadedWorkletMessagingProxy::Initialize(
+           window->GetHttpsState(), worker_clients,
+           frame_client->CreateWorkerContentSettingsClient(),
+           OriginTrialContext::GetInheritedTrialFeatures(window).get(),
+-          base::UnguessableToken::Create(),
+-          std::make_unique<WorkerSettings>(window->GetFrame()->GetSettings()),
++          base::UnguessableToken::Create(), std::move(worklet_settings),
+           mojom::blink::V8CacheOptions::kDefault, module_responses_map,
+           mojo::NullRemote() /* browser_interface_broker */,
+           window->GetFrame()->Loader().CreateWorkerCodeCacheHost(),
 diff --git a/third_party/blink/renderer/core/workers/worker_settings.cc b/third_party/blink/renderer/core/workers/worker_settings.cc
 index 45680c5f6ea0c7e89ccf43eb88f8a11e3318c02e..3fa3af62f4e7ba8186441c5e3184b1c04fe32d12 100644
 --- a/third_party/blink/renderer/core/workers/worker_settings.cc
@@ -71,3 +113,56 @@ index 45c60dd2c44b05fdd279f759069383479823c7f2..33a2a0337efb9a46293e11d0d09b3fc1
  
    GenericFontFamilySettings generic_font_family_settings_;
  };
+diff --git a/third_party/blink/renderer/core/workers/worklet_global_scope.cc b/third_party/blink/renderer/core/workers/worklet_global_scope.cc
+index b5300dea97f20d72a807543a6da0baf61d21955f..a7030c1ba6851b26c765c7b05cd26e1453866719 100644
+--- a/third_party/blink/renderer/core/workers/worklet_global_scope.cc
++++ b/third_party/blink/renderer/core/workers/worklet_global_scope.cc
+@@ -32,6 +32,7 @@
+ #include "third_party/blink/renderer/core/script/modulator.h"
+ #include "third_party/blink/renderer/core/workers/global_scope_creation_params.h"
+ #include "third_party/blink/renderer/core/workers/worker_reporting_proxy.h"
++#include "third_party/blink/renderer/core/workers/worker_settings.h"
+ #include "third_party/blink/renderer/core/workers/worker_thread.h"
+ #include "third_party/blink/renderer/core/workers/worklet_module_responses_map.h"
+ #include "third_party/blink/renderer/core/workers/worklet_module_tree_client.h"
+@@ -110,6 +111,10 @@ WorkletGlobalScope::WorkletGlobalScope(
+       parent_cross_origin_isolated_capability_(
+           creation_params->cross_origin_isolated_capability),
+       parent_is_isolated_context_(creation_params->parent_is_isolated_context),
++      node_integration_in_worker_(
++          creation_params->worker_settings
++              ? creation_params->worker_settings->NodeIntegrationInWorker()
++              : false),
+       browser_interface_broker_proxy_(this) {
+   DCHECK((thread_type_ == ThreadType::kMainThread && frame_) ||
+          (thread_type_ == ThreadType::kOffMainThread && worker_thread_));
+diff --git a/third_party/blink/renderer/core/workers/worklet_global_scope.h b/third_party/blink/renderer/core/workers/worklet_global_scope.h
+index c7dd62900f0de48ab992a7c99058f5b6d98212cf..47ceea11ec9db6b67cef6945d165f46c868f4ca5 100644
+--- a/third_party/blink/renderer/core/workers/worklet_global_scope.h
++++ b/third_party/blink/renderer/core/workers/worklet_global_scope.h
+@@ -140,6 +140,13 @@ class CORE_EXPORT WorkletGlobalScope : public WorkerOrWorkletGlobalScope {
+   // Returns the WorkletToken that uniquely identifies this worklet.
+   virtual WorkletToken GetWorkletToken() const = 0;
+ 
++  // Electron: returns whether the creator frame had the
++  // `nodeIntegrationInWorker` web preference enabled. Copied from
++  // GlobalScopeCreationParams::worker_settings at construction time so the
++  // value is readable on the worker thread without crossing back to the
++  // main thread.
++  bool NodeIntegrationInWorker() const { return node_integration_in_worker_; }
++
+   // Returns the ExecutionContextToken that uniquely identifies the parent
+   // context that created this worklet. Note that this will always be a
+   // LocalFrameToken.
+@@ -207,6 +214,11 @@ class CORE_EXPORT WorkletGlobalScope : public WorkerOrWorkletGlobalScope {
+   // TODO(crbug.com/1206150): We need a spec for this capability.
+   const bool parent_is_isolated_context_;
+ 
++  // Electron: snapshot of the creator frame's nodeIntegrationInWorker
++  // WebPreference, copied out of GlobalScopeCreationParams::worker_settings
++  // at construction time.
++  const bool node_integration_in_worker_;
++
+   // This is the interface that handles generated code cache
+   // requests both to fetch code cache when loading resources
+   // and to store generated code cache to disk.

shell/common/node_bindings.cc

@@ -56,6 +56,10 @@
 #include "shell/common/crash_keys.h"
 #endif
 
+#if BUILDFLAG(IS_WIN)
+#include <windows.h>
+#endif
+
 #define ELECTRON_BROWSER_BINDINGS(V)      \
   V(electron_browser_app)                 \
   V(electron_browser_auto_updater)        \
@@ -535,6 +539,28 @@ NodeBindings::~NodeBindings() {
 
   WakeupEmbedThread();
 
+#if BUILDFLAG(IS_WIN)
+  // On Windows the embed thread parks in GetQueuedCompletionStatus inside
+  // PollEvents, and the WakeupEmbedThread() call above wakes it via
+  // uv_async_send -> POST_COMPLETION_FOR_REQ. However, libuv's
+  // uv_async_send is a no-op when its internal `async_sent` flag is
+  // already set, and that flag is only cleared by uv_run processing the
+  // pending async. If something has called set_uv_env(nullptr) before
+  // this destructor runs (e.g. WebWorkerObserver::ContextWillDestroy
+  // ahead of a deferred ~NodeBindings on a pooled worklet thread),
+  // UvRunOnce early-returns and the flag is never cleared, so the
+  // wakeup above is silently dropped and uv_thread_join blocks forever.
+  //
+  // Post a dummy completion packet directly to the loop's IOCP so the
+  // embed thread breaks out of GetQueuedCompletionStatus regardless of
+  // libuv's internal state. PollEvents will return with overlapped ==
+  // nullptr, the embed loop will see embed_closed_ and exit cleanly.
+  // Linux/Mac don't need this because uv_async_send writes to a real
+  // file descriptor that epoll/kqueue is watching, so the wakeup is
+  // observable at the OS level regardless of libuv-internal flags.
+  PostQueuedCompletionStatus(uv_loop_->iocp, 0, 0, nullptr);
+#endif
+
   // Wait for everything to be done.
   uv_thread_join(&embed_thread_);
 

shell/renderer/electron_renderer_client.cc

@@ -25,6 +25,7 @@
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"  // nogncheck
 #include "third_party/blink/renderer/core/workers/worker_global_scope.h"  // nogncheck
 #include "third_party/blink/renderer/core/workers/worker_settings.h"  // nogncheck
+#include "third_party/blink/renderer/core/workers/worklet_global_scope.h"  // nogncheck
 
 namespace electron {
 
@@ -206,11 +207,20 @@ bool WorkerHasNodeIntegration(blink::ExecutionContext* ec) {
   // owing to an inability to customize sandbox policies in these workers
   // given that they're run out-of-process.
   // Also avoid creating a Node.js environment for worklet global scope
-  // created on the main thread.
+  // created on the main thread — those share the page's V8 context where
+  // Node is already wired up.
   if (ec->IsServiceWorkerGlobalScope() || ec->IsSharedWorkerGlobalScope() ||
       ec->IsMainThreadWorkletGlobalScope())
     return false;
 
+  // Off-main-thread worklets (AudioWorklet, PaintWorklet, AnimationWorklet,
+  // SharedStorageWorklet) have their own dedicated worker thread but do not
+  // derive from WorkerGlobalScope, so check for them separately and read the
+  // flag from WorkletGlobalScope, which copies it out of the same
+  // WorkerSettings as dedicated workers do.
+  if (auto* wlgs = blink::DynamicTo<blink::WorkletGlobalScope>(ec))
+    return wlgs->NodeIntegrationInWorker();
+
   auto* wgs = blink::DynamicTo<blink::WorkerGlobalScope>(ec);
   if (!wgs)
     return false;
@@ -246,7 +256,7 @@ void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
 
   auto* current = WebWorkerObserver::GetCurrent();
   if (current)
-    current->ContextWillDestroy(context, ec->IsAudioWorkletGlobalScope());
+    current->ContextWillDestroy(context);
 }
 
 void ElectronRendererClient::SetUpWebAssemblyTrapHandler() {

shell/renderer/web_worker_observer.cc

@@ -15,6 +15,7 @@
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
 #include "shell/common/node_util.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"  // nogncheck
 
 namespace electron {
 
@@ -23,6 +24,23 @@ namespace {
 static base::NoDestructor<base::ThreadLocalOwnedPointer<WebWorkerObserver>>
     lazy_tls;
 
+// Returns true if `context` belongs to a worklet that runs on a thread
+// pooled by Blink's WorkletThreadHolder, where the worker thread can be
+// reused for multiple worklet contexts. For these scopes the
+// WebWorkerObserver and its NodeBindings must outlive the v8::Context so
+// the next pooled context can reuse them — Node.js cannot be re-initialized
+// on the same thread (the allocator shim only loads once). See callers of
+// blink::WorkletThreadHolder in third_party/blink for the authoritative
+// list.
+bool IsPooledWorkletContext(v8::Local<v8::Context> context) {
+  auto* ec = blink::ExecutionContext::From(context);
+  if (!ec)
+    return false;
+  return ec->IsAudioWorkletGlobalScope() || ec->IsPaintWorkletGlobalScope() ||
+         ec->IsAnimationWorkletGlobalScope() ||
+         ec->IsSharedStorageWorkletGlobalScope();
+}
+
 }  // namespace
 
 // static
@@ -204,12 +222,13 @@ void WebWorkerObserver::ShareEnvironmentWithContext(
   }
 }
 
-void WebWorkerObserver::ContextWillDestroy(v8::Local<v8::Context> context,
-                                           bool is_audio_worklet) {
+void WebWorkerObserver::ContextWillDestroy(v8::Local<v8::Context> context) {
   node::Environment* env = node::Environment::GetCurrent(context);
   if (!env)
     return;
 
+  const bool is_pooled_worklet = IsPooledWorkletContext(context);
+
   active_context_count_--;
 
   if (active_context_count_ == 0) {
@@ -237,6 +256,20 @@ void WebWorkerObserver::ContextWillDestroy(v8::Local<v8::Context> context,
         emit_v.As<v8::Function>()
             ->Call(ctx, env->process_object(), 1, args)
             .FromMaybe(v8::Local<v8::Value>());
+        // We are mid-teardown and about to destroy the worker's
+        // node::Environment, so we cannot let an exception thrown by an
+        // 'exit' listener propagate back into Blink (it would assert in
+        // V8::FromJustIsNothing on the next call into V8). Log it and
+        // explicitly reset the TryCatch so the destructor doesn't rethrow.
+        if (try_catch.HasCaught()) {
+          if (auto message = try_catch.Message(); !message.IsEmpty()) {
+            std::string str;
+            if (gin::ConvertFromV8(isolate, message->Get(), &str))
+              LOG(ERROR) << "Exception thrown from worker 'exit' handler: "
+                         << str;
+          }
+          try_catch.Reset();
+        }
       }
     }
 
@@ -253,23 +286,24 @@ void WebWorkerObserver::ContextWillDestroy(v8::Local<v8::Context> context,
     // ElectronBindings is tracking node environments.
     electron_bindings_->EnvironmentDestroyed(env);
 
-    // For non-AudioWorklet contexts (e.g., PaintWorklet, dedicated workers)
-    // Blink does not pool the worker thread, so tear down the observer
-    // synchronously while the worker thread is still in a clean state.
-    // Deferring teardown to the thread-exit TLS callback hangs on Windows:
-    // after set_uv_env(nullptr) the embed thread parks in
+    // For non-pooled worker contexts (e.g., dedicated workers) Blink does
+    // not reuse the worker thread, so tear down the observer synchronously
+    // while the worker thread is still in a clean state. Deferring teardown
+    // to the thread-exit TLS callback hangs on Windows: after
+    // set_uv_env(nullptr) the embed thread parks in
     // GetQueuedCompletionStatus and the later WakeupEmbedThread() from
     // ~NodeBindings can be lost, causing uv_thread_join to block forever
     // and the renderer to deadlock during navigation.
     //
-    // For AudioWorklet, threads are pooled (Chromium CL:5270028) and the
-    // same NodeBindings/embed thread must be reused for the next context
-    // on the thread because Node.js cannot be re-initialized on the same
-    // thread (the allocator shim can only be loaded once). In that case
-    // we keep the observer alive and let the next
+    // For pooled worklet contexts (AudioWorklet, PaintWorklet,
+    // AnimationWorklet, SharedStorageWorklet — see
+    // blink::WorkletThreadHolder) the same NodeBindings/embed thread must
+    // be reused for the next context on the thread because Node.js cannot
+    // be re-initialized on the same thread (the allocator shim only loads
+    // once). In that case we keep the observer alive and let the next
     // WorkerScriptReadyForEvaluation call InitializeNewEnvironment on the
     // (now empty) environments_ set.
-    if (!is_audio_worklet) {
+    if (!is_pooled_worklet) {
       lazy_tls->Set(nullptr);  // destroys *this; do not access members below
       return;
     }

shell/renderer/web_worker_observer.h

@@ -37,8 +37,7 @@ class WebWorkerObserver {
   WebWorkerObserver& operator=(const WebWorkerObserver&) = delete;
 
   void WorkerScriptReadyForEvaluation(v8::Local<v8::Context> context);
-  void ContextWillDestroy(v8::Local<v8::Context> context,
-                          bool is_audio_worklet);
+  void ContextWillDestroy(v8::Local<v8::Context> context);
 
  private:
   // Full initialization for the first context on a thread.

spec/chromium-spec.ts

@@ -1625,6 +1625,27 @@ describe('chromium features', () => {
       expect(data).to.equal('function function function function function');
     });
 
+    it('AudioWorklet keeps node integration across pooled worker threads', async () => {
+      // Regression test for https://github.com/electron/electron/issues/41263.
+      // Blink pools the AudioWorklet backing thread (Chromium CL:5270028) so
+      // the Nth+ AudioWorklet on a page reuses the same thread; the page
+      // creates several AudioWorklet contexts in sequence and asserts node
+      // integration is wired up in every one of them.
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true,
+          nodeIntegrationInWorker: true,
+          contextIsolation: false
+        }
+      });
+
+      w.loadURL(`file://${fixturesPath}/pages/audio-worklet.html`);
+      const [, results] = await once(ipcMain, 'audio-worklet-result');
+      expect(results).to.be.an('array').with.length.greaterThan(0);
+      for (const r of results) expect(r).to.equal('ok');
+    });
+
     describe('SharedWorker', () => {
       it('can work', async () => {
         const w = new BrowserWindow({ show: false });

spec/fixtures/pages/audio-worklet.html

@@ -0,0 +1,44 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  const { ipcRenderer } = require('electron');
+
+  // Create a number of AudioContext + AudioWorklet pairs in sequence so
+  // that Blink's WorkletThreadHolder pools and reuses the underlying
+  // worker thread (Chromium CL:5270028). For each context we ask the
+  // worklet to report whether `require` is a function and post that back
+  // via its MessagePort. The bug being guarded is that the Nth+ pooled
+  // worklet would silently lose its Node.js environment, so the test
+  // must run enough iterations to exercise thread reuse.
+  const NUM_CONTEXTS = 6;
+
+  async function runOne(index) {
+    const audioCtx = new AudioContext();
+    try {
+      await audioCtx.audioWorklet.addModule('../workers/audio_worklet_node.js');
+      const node = new AudioWorkletNode(audioCtx, 'node-integration-probe');
+      const reply = new Promise((resolve) => {
+        node.port.onmessage = (e) => resolve(e.data);
+      });
+      node.port.postMessage('probe');
+      node.connect(audioCtx.destination);
+      return await reply;
+    } finally {
+      await audioCtx.close();
+    }
+  }
+
+  (async () => {
+    const results = [];
+    for (let i = 0; i < NUM_CONTEXTS; i++) {
+      try {
+        results.push(await runOne(i));
+      } catch (err) {
+        results.push(`error: ${err && err.message ? err.message : err}`);
+      }
+    }
+    ipcRenderer.send('audio-worklet-result', results);
+  })();
+</script>
+</body>
+</html>

spec/fixtures/workers/audio_worklet_node.js

@@ -0,0 +1,28 @@
+// Reports whether the Node.js environment is wired up inside this
+// AudioWorklet's global scope. Used by spec/fixtures/pages/audio-worklet.html
+// to verify that nodeIntegrationInWorker keeps working when Blink reuses a
+// pooled worker thread for multiple AudioWorklet contexts.
+class NodeIntegrationProbeProcessor extends AudioWorkletProcessor {
+  constructor () {
+    super();
+    this.port.onmessage = () => {
+      let info;
+      try {
+        // require should be a function and `node:timers` should resolve.
+        const ok = typeof require === 'function' &&
+          typeof require('node:timers').setImmediate === 'function' &&
+          typeof process === 'object';
+        info = ok ? 'ok' : 'missing';
+      } catch (err) {
+        info = `throw: ${err && err.message ? err.message : err}`;
+      }
+      this.port.postMessage(info);
+    };
+  }
+
+  process () {
+    return true;
+  }
+}
+
+registerProcessor('node-integration-probe', NodeIntegrationProbeProcessor);