fix: only delete render_frame_host_ after OnDialogDone completes #30916
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codebytere
approved these changes
Sep 13, 2021
|
Release Notes Persisted
|
|
I have automatically backported this PR to "15-x-y", please check out #30927 |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line
2E31
can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Fixes #30858
There seems to be a race condition with two functions,
OnFilesSelectedandOnOpenDialogDone, when opening or attaching a file from the file dialog. On Windows and Linux, the execution goes something like this:OnFilesSelectedruns.render_frame_host_is set tonullptrandthisis deletedOnOpenDialogDoneruns.render_frame_host_still exists when the function begins, and thenullptrchecks don't trigger. However, therender_frame_host_andthisare then deleted inOnFilesSelected. When we then try to accessrender_frame_host_->GetProcess()at the end of the function, the program hard crashes.It seems as though the
render_frame_host_deletion inOnFilesSelectedwas being done primarily for a different function,OnSaveDialogDone. This PR removes the nullptr reassignment fromOnFilesSelectedand instead callsRunFileChooserEndin both[-]DialogDonefunctions, to make this behavior more predictable and dispose of therender_frame_host_andweb_contents_properly.NB: This file has been refactored and more thoroughly improved in @codebytere's PR here: #30663 This smaller fix will address the immediate crash behavior and is meant to be backported into 15-x-y, so we don't need to backport the refactor PR if needed.
Checklist
npm testpassesRelease Notes
Notes: Fixed a crash when selecting files in a native file dialog on Windows and Linux.