Repository: electron/electron
Pull request: fix: enable wasm trap handlers in all Node.js  processes

#### Description of Change

Found while investigating https://github.com/microsoft/vscode/issues/273191

Since https://github.com/electron/electron/pull/47186 we had accidentally disabled V8 trap handlers in browser and utility process. This incurs performance cost when executing wasm due to inlined bound checks.

Enable the feature back and align it with upstream based on the feature parameter across all processes [features::kWebAssemblyTrapHandler](https://source.chromium.org/chromium/chromium/src/+/main:content/public/common/content_features.cc;l=1175-1186)

Current matrix of enabled platforms.

```
Linux, Windows, MacOS - x86_64
Linux, MacOS - arm64
```

#### Release Notes

Notes: Reenable V8 trap handlers for wasm in browser and utility process, improves runtime execution of wasm

Changed files:
filename: shell/browser/electron_browser_main_parts.cc
patch: @@ -62,6 +62,7 @@
 #include "shell/common/logging.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
+#include "shell/common/v8_util.h"
 #include "ui/base/idle/idle.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_switches.h"
@@ -274,6 +275,10 @@ void ElectronBrowserMainParts::PostEarlyInitialization() {
   // Initialize field trials.
   InitializeFieldTrials();
 
+  if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
+    electron::SetUpWebAssemblyTrapHandler();
+  }
+
   // Reinitialize logging now that the app has had a chance to set the app name
   // and/or user data directory.
   logging::InitElectronLogging(*base::CommandLine::ForCurrentProcess(),

filename: shell/common/v8_util.cc
patch: @@ -8,6 +8,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/base_switches.h"
 #include "base/memory/raw_ptr.h"
 #include "gin/converter.h"
 #include "shell/common/api/electron_api_native_image.h"
@@ -17,6 +18,14 @@
 #include "ui/gfx/image/image_skia.h"
 #include "v8/include/v8.h"
 
+#if BUILDFLAG(IS_LINUX) && (defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64))
+#define ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX
+#include "base/command_line.h"
+#include "components/crash/core/app/crashpad.h"  // nogncheck
+#include "content/public/common/content_switches.h"
+#include "v8/include/v8-wasm-trap-handler-posix.h"
+#endif
+
 namespace electron {
 
 namespace {
@@ -240,6 +249,51 @@ v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
   return V8Deserializer(isolate, data).Deserialize();
 }
 
+void SetUpWebAssemblyTrapHandler() {
+#if BUILDFLAG(IS_WIN)
+  // On Windows we use the default trap handler provided by V8.
+  v8::V8::EnableWebAssemblyTrapHandler(true);
+#elif BUILDFLAG(IS_MAC)
+  // On macOS, Crashpad uses exception ports to handle signals in a
+  // different process. As we cannot just pass a callback to this other
+  // process, we ask V8 to install its own signal handler to deal with
+  // WebAssembly traps.
+  v8::V8::EnableWebAssemblyTrapHandler(true);
+#elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
+  const bool crash_reporter_enabled =
+      crash_reporter::GetHandlerSocket(nullptr, nullptr);
+
+  if (crash_reporter_enabled) {
+    // If either --enable-crash-reporter or --enable-crash-reporter-for-testing
+    // is enabled it should take care of signal handling for us, use the default
+    // implementation which doesn't register an additional handler.
+    v8::V8::EnableWebAssemblyTrapHandler(false);
+    return;
+  }
+
+  const bool use_v8_default_handler =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ::switches::kDisableInProcessStackTraces);
+
+  if (use_v8_default_handler) {
+    // There is no signal handler yet, but it's okay if v8 registers one.
+    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/true);
+    return;
+  }
+
+  if (base::debug::SetStackDumpFirstChanceCallback(
+          v8::TryHandleWebAssemblyTrapPosix)) {
+    // Crashpad and Breakpad are disabled, but the in-process stack dump
+    // handlers are enabled, so set the callback on the stack dump handlers.
+    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false);
+    return;
+  }
+
+  // As the registration of the callback failed, we don't enable trap
+  // handlers.
+#endif
+}
+
 namespace util {
 
 /**

filename: shell/common/v8_util.h
patch: @@ -30,6 +30,8 @@ v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
 v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
                                         base::span<const uint8_t> data);
 
+void SetUpWebAssemblyTrapHandler();
+
 namespace util {
 
 [[nodiscard]] base::span<uint8_t> as_byte_span(

filename: shell/renderer/electron_renderer_client.cc
patch: @@ -6,7 +6,6 @@
 
 #include <algorithm>
 
-#include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/containers/contains.h"
 #include "base/debug/stack_trace.h"
@@ -19,6 +18,7 @@
 #include "shell/common/node_includes.h"
 #include "shell/common/node_util.h"
 #include "shell/common/options_switches.h"
+#include "shell/common/v8_util.h"
 #include "shell/renderer/electron_render_frame_observer.h"
 #include "shell/renderer/web_worker_observer.h"
 #include "third_party/blink/public/common/web_preferences/web_preferences.h"
@@ -27,13 +27,6 @@
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"  // nogncheck
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"  // nogncheck
 
-#if BUILDFLAG(IS_LINUX) && (defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64))
-#define ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX
-#include "components/crash/core/app/crashpad.h"  // nogncheck
-#include "content/public/common/content_switches.h"
-#include "v8/include/v8-wasm-trap-handler-posix.h"
-#endif
-
 namespace electron {
 
 ElectronRendererClient::ElectronRendererClient()
@@ -248,45 +241,9 @@ void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
 }
 
 void ElectronRendererClient::SetUpWebAssemblyTrapHandler() {
-// See CL:5372409 - copied from ShellContentRendererClient.
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-  // Mac and Windows use the default implementation (where the default v8 trap
-  // handler gets set up).
-  ContentRendererClient::SetUpWebAssemblyTrapHandler();
-  return;
-#elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
-  const bool crash_reporter_enabled =
-      crash_reporter::GetHandlerSocket(nullptr, nullptr);
-
-  if (crash_reporter_enabled) {
-    // If either --enable-crash-reporter or --enable-crash-reporter-for-testing
-    // is enabled it should take care of signal handling for us, use the default
-    // implementation which doesn't register an additional handler.
-    ContentRendererClient::SetUpWebAssemblyTrapHandler();
-    return;
-  }
-
-  const bool use_v8_default_handler =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          ::switches::kDisableInProcessStackTraces);
-
-  if (use_v8_default_handler) {
-    // There is no signal handler yet, but it's okay if v8 registers one.
-    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/true);
-    return;
-  }
-
-  if (base::debug::SetStackDumpFirstChanceCallback(
-          v8::TryHandleWebAssemblyTrapPosix)) {
-    // Crashpad and Breakpad are disabled, but the in-process stack dump
-    // handlers are enabled, so set the callback on the stack dump handlers.
-    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false);
-    return;
-  }
-
-  // As the registration of the callback failed, we don't enable trap
-  // handlers.
-#endif  // defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
+  // content/renderer layer already takes care of the feature flag detection
+  // so no need to check for features::kWebAssemblyTrapHandler here.
+  electron::SetUpWebAssemblyTrapHandler();
 }
 
 node::Environment* ElectronRendererClient::GetEnvironment(

filename: shell/services/node/node_service.cc
patch: @@ -11,6 +11,7 @@
 #include "base/no_destructor.h"
 #include "base/process/process.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/public/common/content_features.h"
 #include "electron/mas.h"
 #include "net/base/network_change_notifier.h"
 #include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
@@ -22,6 +23,7 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
+#include "shell/common/v8_util.h"
 #include "shell/services/node/parent_port.h"
 
 #if !IS_MAS_BUILD()
@@ -130,6 +132,11 @@ void NodeService::Initialize(
   v8::Isolate* const isolate = js_env_->isolate();
   v8::HandleScope scope{isolate};
 
+  // Initialize after setting up the V8 isolate.
+  if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
+    electron::SetUpWebAssemblyTrapHandler();
+  }
+
   node_bindings_->Initialize(isolate, isolate->GetCurrentContext());
 
   network_change_notifier_ = net::NetworkChangeNotifier::CreateIfNeeded(