Repository: electron/electron
Pull request: fix: revert enabling WASM trap handlers in all Node.js processes

Backport of #48973

See that PR for details.


Notes: Fixed an issue where some apps would throw an exception on launch.


Changed files:
filename: shell/browser/electron_browser_main_parts.cc
patch: @@ -62,7 +62,6 @@
 #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"
@@ -275,10 +274,6 @@ 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,7 +8,6 @@
 #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"
@@ -18,15 +17,6 @@
 #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 "base/debug/stack_trace.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 {
@@ -250,51 +240,6 @@ 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,8 +30,6 @@ 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,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/containers/contains.h"
 #include "content/public/renderer/render_frame.h"
@@ -17,7 +18,6 @@
 #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"
@@ -26,6 +26,13 @@
 #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()
@@ -240,9 +247,45 @@ void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
 }
 
 void ElectronRendererClient::SetUpWebAssemblyTrapHandler() {
-  // content/renderer layer already takes care of the feature flag detection
-  // so no need to check for features::kWebAssemblyTrapHandler here.
-  electron::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)
 }
 
 node::Environment* ElectronRendererClient::GetEnvironment(

filename: shell/services/node/node_service.cc
patch: @@ -11,7 +11,6 @@
 #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"
@@ -23,7 +22,6 @@
 #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()
@@ -132,11 +130,6 @@ 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(