Skip to content
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

fix: crash when BrowserView with destroyed webcontents is reused #36414

Conversation

CezaryKulakowski
Copy link
Contributor

Fixed #36371.

Description of Change

Checklist

Release Notes

Notes: fixed crash when BrowserView with destroyed webcontents is reused

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Nov 20, 2022
@codebytere codebytere changed the title fix: fixed crash when BrowserView with destroyed webcontents is reused fix: rash when BrowserView with destroyed webcontents is reused Nov 21, 2022
@codebytere codebytere changed the title fix: rash when BrowserView with destroyed webcontents is reused fix: crash when BrowserView with destroyed webcontents is reused Nov 21, 2022
Copy link
Member

@codebytere codebytere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a regression test to this!

Additionally - this does prevent a full crash but what also seems to be happening is that the BrowserView webContents is being destroyed when maybe it shouldn't be. This is happening as a result of #35509 - specifically this change cc @nornagon

We should consider whether that's the destruction order we want and fix it there as opposed to catching the destroyed webContents as is being done in this change.

@codebytere codebytere added the semver/patch backwards-compatible bug fixes label Nov 21, 2022
@CezaryKulakowski
Copy link
Contributor Author

I've pushed fixup with test. It crashes without this fix.

@CezaryKulakowski CezaryKulakowski requested review from codebytere and removed request for nornagon November 21, 2022 12:28
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Nov 21, 2022
nornagon added a commit that referenced this pull request Nov 21, 2022
@nornagon
Copy link
Member

Thanks for the new test, pulled the test into #35658, which threw a JS exception in this case, which is now fixed.

This PR will be obsoleted if #35658 lands.

@codebytere
Copy link
Member

Handled in #37420 - apologies for not getting to this sooner before i realized it.

@codebytere codebytere closed this Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: browser process crashes when BrowserView with invalidated web contents is used
3 participants