refactor: node::Environment self-cleanup #39604
Merged
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 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
NodeBindings::CreateEnvironment()
now returns astd::shared_ptr<node::Environment>
instead of a raw pointer. The new shared pointer has a custom deleter that knows how to cleanup both thenode::Environment
and itsnode::IsolateData
.ElectronRenderClient
,WebWorkerObserver
, andNodeEnvironment
, which all had code similar to the new deleter.NodeEnvironment
.kElectronContextEmbedderDataIndex
, make it a private field.The initial motivation for this PR was to fix a dangling
raw_ptr
bug inNodeEnvironment
; but when investigating the code, it looks like there's an opportunity to reduce coupling across the above classes and remove some code duplication.note: I'd particularly like revewer 馃憖 on the changes in
NodeBindings.cc
, e.g. is thev8::Context
being handled correctly when it's passed to the deleter lambda?Checklist
npm test
passesRelease Notes
Notes: none.