-
Notifications
You must be signed in to change notification settings - Fork 16.4k
fix: re-entrancy issues in webContents.loadurl("")
#48004
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fea6b9f
to
e351bf9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an edge case we could craft a test case for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on the implementation
@dsanders11 added two more cases I think should cover it! |
e351bf9
to
7f0c3e9
Compare
Release Notes Persisted
|
I have automatically backported this PR to "38-x-y", please check out #48043 |
I have automatically backported this PR to "36-x-y", please check out #48044 |
I have automatically backported this PR to "37-x-y", please check out #48045 |
Description of Change
Closes #40809
This fixes a crash possible when calling
webContents.loadurl("")
from a failed call's catch handler. Essentially, a re-entrant navigation triggered from thedid-start-navigation
event causes the in-flightNavigationRequest
to be destroyed before Chromium marks it safe, tripping theis_safe_to_delete_
DCHECK here.Fix this by monitoring potential re-entrancy sites and failing if we try to call
loadURL
again at a dangerous time.Checklist
npm test
passesRelease Notes
Notes: Fixed a crash possible when calling
webContents.loadurl("")
from a failedwebContents.loadurl("")
call's catch handler.