Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

@icecream17
Copy link
Contributor

Although crashed has recently been updated to render-process-gone, the new params don't actually have to be used. In fact maybe allowing some of the other crash-ishes could result in weird invalid state and things happening. So basically I agree with #23132 (comment)

Maybe there should be a different message for the killed reason though

Although `crashed` has recently been updated to `render-process-gone`, the new params don't actually have to be used. In fact maybe allowing some of the other crash-ishes could result in weird invalid state and things happening. So basically I agree with #23132 (comment)

Maybe there should be a different message for the `killed` reason though
@DeeDeeG
Copy link
Contributor

DeeDeeG commented Nov 7, 2021

This PR gets a 👍 from me (of course, it was my suggestion), because this restores the crash handling behavior to how it was before #23132.

Without this PR, there are several reasons/scenarios for why the renderer process could be gone, that are basically not handled at all. Which is a departure from the old way (before #23132) of running crash handling regardless of why the renderer process is gone.

Maybe we can use the reason info some way, perhaps give a different dialog message based on the reason the renderer process is gone, I'm not against that. But I believe it should be thought through carefully so we handle all scenarios thoroughly enough.

@sadick254 sadick254 merged commit 9ae0cef into atom:master Nov 18, 2021
@icecream17 icecream17 deleted the patch-8 branch November 18, 2021 21:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants