diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index a0a77d17f522..4679ab95cf2f 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -2023,16 +2023,21 @@ SkRegion* WebContents::draggable_region() { void WebContents::DidStartNavigation( content::NavigationHandle* navigation_handle) { + base::AutoReset resetter(&is_safe_to_delete_, false); EmitNavigationEvent("did-start-navigation", navigation_handle); } void WebContents::DidRedirectNavigation( content::NavigationHandle* navigation_handle) { + base::AutoReset resetter(&is_safe_to_delete_, false); EmitNavigationEvent("did-redirect-navigation", navigation_handle); } void WebContents::ReadyToCommitNavigation( content::NavigationHandle* navigation_handle) { + base::AutoReset resetter(&is_safe_to_delete_, false); + EmitNavigationEvent("-ready-to-commit-navigation", navigation_handle); + // Don't focus content in an inactive window. if (!owner_window()) return; @@ -2375,7 +2380,7 @@ void WebContents::LoadURL(const GURL& url, // http://crbug.com/347742. auto& ctrl_impl = static_cast( web_contents()->GetController()); - if (ctrl_impl.in_navigate_to_pending_entry()) { + if (!is_safe_to_delete_ || ctrl_impl.in_navigate_to_pending_entry()) { Emit("did-fail-load", static_cast(net::ERR_FAILED), net::ErrorToShortString(net::ERR_FAILED), url.possibly_invalid_spec(), true); diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 390d0e37c5fd..e0daa212ee26 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -858,6 +858,9 @@ class WebContents final : public ExclusiveAccessContext, const scoped_refptr print_task_runner_; #endif + // Track navigation state in order to avoid potential re-entrancy crashes. + bool is_safe_to_delete_ = true; + // Stores the frame that's currently in fullscreen, nullptr if there is none. raw_ptr fullscreen_frame_ = nullptr; diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index 4d0a3eebda20..442743cb45d7 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -484,7 +484,7 @@ describe('webContents module', () => { } }); - it('fails if loadURL is called inside a non-reentrant critical section', (done) => { + it('fails if loadURL is called inside did-start-loading', (done) => { w.webContents.once('did-fail-load', (_event, _errorCode, _errorDescription, validatedURL) => { expect(validatedURL).to.contain('blank.html'); done(); @@ -497,6 +497,49 @@ describe('webContents module', () => { w.loadURL('data:text/html,

HELLO

'); }); + it('fails if loadurl is called after the navigation is ready to commit', () => { + w.webContents.once('did-fail-load', (_event, _errorCode, _errorDescription, validatedURL) => { + expect(validatedURL).to.contain('blank.html'); + }); + + // @ts-expect-error internal-only event. + w.webContents.once('-ready-to-commit-navigation', () => { + w.loadURL(`file://${fixturesPath}/pages/blank.html`); + }); + + w.loadURL('data:text/html,

HELLO

'); + }); + + it('fails if loadURL is called inside did-redirect-navigation', (done) => { + const server = http.createServer((req, res) => { + if (req.url === '/302') { + res.statusCode = 302; + res.setHeader('Location', '/200'); + res.end(); + } else if (req.url === '/200') { + res.end('ok'); + } else { + res.end(); + } + }); + + w.webContents.once('did-fail-load', (_event, _errorCode, _errorDescription, validatedURL) => { + expect(validatedURL).to.contain('blank.html'); + server.close(); + done(); + }); + + listen(server).then(({ url }) => { + w.webContents.once('did-redirect-navigation', () => { + w.loadURL(`file://${fixturesPath}/pages/blank.html`); + }); + w.loadURL(`${url}/302`); + }).catch(e => { + server.close(); + done(e); + }); + }); + it('sets appropriate error information on rejection', async () => { let err: any; try {