From 86df4db6f10afbe5872f8ecbcbfe6191374fc1f7 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Tue, 10 Oct 2023 12:46:04 +0200
Subject: [PATCH] fix: crash when calling non-reentrant function in `loadURL`
 (#40143)

---
 shell/browser/api/electron_api_web_contents.cc | 17 +++++++++++++++--
 spec/api-web-contents-spec.ts                  | 13 +++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
index cb6cdaacc7c9..1919f3066089 100644
--- a/shell/browser/api/electron_api_web_contents.cc
+++ b/shell/browser/api/electron_api_web_contents.cc
@@ -33,6 +33,7 @@
 #include "components/security_state/content/content_utils.h"
 #include "components/security_state/core/security_state.h"
 #include "content/browser/renderer_host/frame_tree_node.h"  // nogncheck
+#include "content/browser/renderer_host/navigation_controller_impl.h"  // nogncheck
 #include "content/browser/renderer_host/render_frame_host_manager.h"  // nogncheck
 #include "content/browser/renderer_host/render_widget_host_impl.h"  // nogncheck
 #include "content/browser/renderer_host/render_widget_host_view_base.h"  // nogncheck
@@ -2384,8 +2385,20 @@ void WebContents::LoadURL(const GURL& url,
   params.transition_type = ui::PageTransitionFromInt(
       ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
   params.override_user_agent = content::NavigationController::UA_OVERRIDE_TRUE;
-  // Discard non-committed entries to ensure that we don't re-use a pending
-  // entry
+
+  // It's not safe to start a new navigation or otherwise discard the current
+  // one while the call that started it is still on the stack. See
+  // http://crbug.com/347742.
+  auto& ctrl_impl = static_cast<content::NavigationControllerImpl&>(
+      web_contents()->GetController());
+  if (ctrl_impl.in_navigate_to_pending_entry()) {
+    Emit("did-fail-load", static_cast<int>(net::ERR_FAILED),
+         net::ErrorToShortString(net::ERR_FAILED), url.possibly_invalid_spec(),
+         true);
+    return;
+  }
+
+  // Discard non-committed entries to ensure we don't re-use a pending entry.
   web_contents()->GetController().DiscardNonCommittedEntries();
   web_contents()->GetController().LoadURLWithParams(params);
 
diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts
index 2ecc068babf9..576d5e54971e 100644
--- a/spec/api-web-contents-spec.ts
+++ b/spec/api-web-contents-spec.ts
@@ -431,6 +431,19 @@ describe('webContents module', () => {
       }
     });
 
+    it('fails if loadURL is called inside a non-reentrant critical section', (done) => {
+      w.webContents.once('did-fail-load', (_event, _errorCode, _errorDescription, validatedURL) => {
+        expect(validatedURL).to.contain('blank.html');
+        done();
+      });
+
+      w.webContents.once('did-start-loading', () => {
+        w.loadURL(`file://${fixturesPath}/pages/blank.html`);
+      });
+
+      w.loadURL('data:text/html,<h1>HELLO</h1>');
+    });
+
     it('sets appropriate error information on rejection', async () => {
       let err: any;
       try {