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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use raw_ref in RootView #42114

Merged
merged 3 commits into from May 15, 2024
Merged

Conversation

ckerr
Copy link
Member

@ckerr ckerr commented May 10, 2024

Description of Change

Make the ownership / nullability contract for RootView fields a little stricter:

  • Use raw_ref<T> instead of raw_ptr<T> for class fields that can never be nullptr
  • Aggregate the ViewTracker directly since RootView already owns it, and its lifecycle is identical to the RootView, and the RootView header was already including the ViewTracker header

All reviews welcomed. CC @nornagon as most recent committer and @miniak for general C++ tidying 馃樃

Checklist

Release Notes

Notes: none.

ckerr added 3 commits May 9, 2024 21:22
The Chromium C++ style guide says "prefer const raw_ref<T> whenever
the held pointer will never be null," so let's do that.
> The Chromium C++ style guide says "prefer const raw_ref<T> whenever
> the held pointer will never be null," so let's do that.
RootView already owns it, so aggregate it
@ckerr ckerr added semver/patch backwards-compatible bug fixes no-backport labels May 10, 2024
@electron-cation electron-cation bot added new-pr 馃尡 PR opened in the last 24 hours and removed new-pr 馃尡 PR opened in the last 24 hours labels May 10, 2024
@jkleinsc jkleinsc merged commit c67744a into main May 15, 2024
21 checks passed
@jkleinsc jkleinsc deleted the refactor/use-raw_ref-in-RootView branch May 15, 2024 18:45
Copy link

release-clerk bot commented May 15, 2024

No Release Notes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants