fix: make child windows not crash when ipc messages are received (#19553)

lib/browser/guest-window-manager.js

@@ -34,7 +34,7 @@ const mergeOptions = function (child, parent, visited) {
     if (key in child && key !== 'webPreferences') continue
 
     const value = parent[key]
-    if (typeof value === 'object') {
+    if (typeof value === 'object' && !Array.isArray(value)) {
       child[key] = mergeOptions(child[key] || {}, value, visited)
     } else {
       child[key] = value

shell/common/options_switches.cc

@@ -234,6 +234,7 @@ const char kNativeWindowOpen[] = "native-window-open";
 const char kWebviewTag[] = "webview-tag";
 const char kDisableElectronSiteInstanceOverrides[] =
     "disable-electron-site-instance-overrides";
+const char kEnableNodeLeakageInRenderers[] = "enable-node-leakage-in-renderers";
 
 // Command switch passed to renderer process to control nodeIntegration.
 const char kNodeIntegrationInWorker[] = "node-integration-in-worker";

shell/common/options_switches.h

@@ -118,6 +118,7 @@ extern const char kNodeIntegrationInWorker[];
 extern const char kWebviewTag[];
 extern const char kNodeIntegrationInSubFrames[];
 extern const char kDisableElectronSiteInstanceOverrides[];
+extern const char kEnableNodeLeakageInRenderers[];
 
 extern const char kWidevineCdmPath[];
 extern const char kWidevineCdmVersion[];

shell/renderer/atom_render_frame_observer.cc

@@ -69,16 +69,25 @@ void AtomRenderFrameObserver::DidCreateScriptContext(
   if (ShouldNotifyClient(world_id))
     renderer_client_->DidCreateScriptContext(context, render_frame_);
 
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+
   bool use_context_isolation = renderer_client_->isolated_world();
+  // This logic matches the EXPLAINED logic in atom_renderer_client.cc
+  // to avoid explaining it twice go check that implementation in
+  // DidCreateScriptContext();
   bool is_main_world = IsMainWorld(world_id);
   bool is_main_frame = render_frame_->IsMainFrame();
-  bool is_not_opened = !render_frame_->GetWebFrame()->Opener();
+  bool reuse_renderer_processes_enabled =
+      command_line->HasSwitch(switches::kDisableElectronSiteInstanceOverrides);
+  bool is_not_opened =
+      !render_frame_->GetWebFrame()->Opener() ||
+      command_line->HasSwitch(switches::kEnableNodeLeakageInRenderers);
   bool allow_node_in_sub_frames =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kNodeIntegrationInSubFrames);
+      command_line->HasSwitch(switches::kNodeIntegrationInSubFrames);
   bool should_create_isolated_context =
       use_context_isolation && is_main_world &&
-      (is_main_frame || allow_node_in_sub_frames) && is_not_opened;
+      (is_main_frame || allow_node_in_sub_frames) &&
+      (is_not_opened || reuse_renderer_processes_enabled);
 
   if (should_create_isolated_context) {
     CreateIsolatedWorldContext();

shell/renderer/atom_renderer_client.cc

@@ -77,14 +77,26 @@ void AtomRendererClient::DidCreateScriptContext(
   // TODO(zcbenz): Do not create Node environment if node integration is not
   // enabled.
 
-  // Do not load node if we're aren't a main frame or a devtools extension
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+
+  // Only load node if we are a main frame or a devtools extension
   // unless node support has been explicitly enabled for sub frames
-  bool is_main_frame =
-      render_frame->IsMainFrame() && !render_frame->GetWebFrame()->Opener();
+  bool reuse_renderer_processes_enabled =
+      command_line->HasSwitch(switches::kDisableElectronSiteInstanceOverrides);
+  // Consider the window not "opened" if it does not have an Opener, or if a
+  // user has manually opted in to leaking node in the renderer
+  bool is_not_opened =
+      !render_frame->GetWebFrame()->Opener() ||
+      command_line->HasSwitch(switches::kEnableNodeLeakageInRenderers);
+  // Consider this the main frame if it is both a Main Frame and it wasn't
+  // opened.  We allow an opened main frame to have node if renderer process
+  // reuse is enabled as that will correctly free node environments prevent a
+  // leak in child windows.
+  bool is_main_frame = render_frame->IsMainFrame() &&
+                       (is_not_opened || reuse_renderer_processes_enabled);
   bool is_devtools = IsDevToolsExtension(render_frame);
   bool allow_node_in_subframes =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kNodeIntegrationInSubFrames);
+      command_line->HasSwitch(switches::kNodeIntegrationInSubFrames);
   bool should_load_node =
       (is_main_frame || is_devtools || allow_node_in_subframes) &&
       !IsWebViewFrame(renderer_context, render_frame);
@@ -112,7 +124,6 @@ void AtomRendererClient::DidCreateScriptContext(
   DCHECK(!context.IsEmpty());
   node::Environment* env =
       node_bindings_->CreateEnvironment(context, nullptr, true);
-  auto* command_line = base::CommandLine::ForCurrentProcess();
   // If we have disabled the site instance overrides we should prevent loading
   // any non-context aware native module
   if (command_line->HasSwitch(switches::kDisableElectronSiteInstanceOverrides))

shell/renderer/electron_api_service_impl.cc

@@ -39,7 +39,10 @@ v8::Local<v8::Object> GetIpcObject(v8::Local<v8::Context> context) {
   auto global_object = context->Global();
   auto value =
       global_object->GetPrivate(context, private_binding_key).ToLocalChecked();
-  DCHECK(!value.IsEmpty() && value->IsObject());
+  if (value.IsEmpty() || !value->IsObject()) {
+    LOG(ERROR) << "Attempted to get the 'ipcNative' object but it was missing";
+    return v8::Local<v8::Object>();
+  }
   return value->ToObject(context).ToLocalChecked();
 }
 
@@ -50,6 +53,8 @@ void InvokeIpcCallback(v8::Local<v8::Context> context,
   auto* isolate = context->GetIsolate();
 
   auto ipcNative = GetIpcObject(context);
+  if (ipcNative.IsEmpty())
+    return;
 
   // Only set up the node::CallbackScope if there's a node environment.
   // Sandboxed renderers don't have a node environment.