fix: re-entrancy issues in webContents.loadURL()
(#48043)
This commit is contained in:
parent
cea5034019
commit
a130d4ebfe
3 changed files with 53 additions and 2 deletions
|
@ -2023,16 +2023,21 @@ SkRegion* WebContents::draggable_region() {
|
|||
|
||||
void WebContents::DidStartNavigation(
|
||||
content::NavigationHandle* navigation_handle) {
|
||||
base::AutoReset<bool> resetter(&is_safe_to_delete_, false);
|
||||
EmitNavigationEvent("did-start-navigation", navigation_handle);
|
||||
}
|
||||
|
||||
void WebContents::DidRedirectNavigation(
|
||||
content::NavigationHandle* navigation_handle) {
|
||||
base::AutoReset<bool> resetter(&is_safe_to_delete_, false);
|
||||
EmitNavigationEvent("did-redirect-navigation", navigation_handle);
|
||||
}
|
||||
|
||||
void WebContents::ReadyToCommitNavigation(
|
||||
content::NavigationHandle* navigation_handle) {
|
||||
base::AutoReset<bool> 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<content::NavigationControllerImpl&>(
|
||||
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<int>(net::ERR_FAILED),
|
||||
net::ErrorToShortString(net::ERR_FAILED), url.possibly_invalid_spec(),
|
||||
true);
|
||||
|
|
|
@ -858,6 +858,9 @@ class WebContents final : public ExclusiveAccessContext,
|
|||
const scoped_refptr<base::TaskRunner> 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<content::RenderFrameHost> fullscreen_frame_ = nullptr;
|
||||
|
||||
|
|
|
@ -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,<h1>HELLO</h1>');
|
||||
});
|
||||
|
||||
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,<h1>HELLO</h1>');
|
||||
});
|
||||
|
||||
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 {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue