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

Fixed ITopLevelImpl API misuse, added validating layer #7369

Merged
merged 3 commits into from
Jan 16, 2022

Conversation

kekekeks
Copy link
Member

Some platform backends like Win32 silently swallow errors when IXXXImpl APIs are called after window has been destroyed. X11 and macOS will either crash with a managed exception or just segfault, because it's not valid to use an object after it has been disposed.

Since quite a lot of developers only check their changes on Windows, this PR introduces a platform backend equalizer that would just throw ObjectDisposedException before even calling into said backend.

Currently it's only enabled in Debug configuration, will enable for Release in a month or two.

This PR also fixes a problem with:
#7323
#7272
#7245
#7367

@maxkatz6 maxkatz6 enabled auto-merge January 16, 2022 17:29
@maxkatz6 maxkatz6 merged commit 936882a into master Jan 16, 2022
@maxkatz6 maxkatz6 deleted the feature/validating-toplevel-impl branch January 16, 2022 17:45
@grokys
Copy link
Member

grokys commented Jan 17, 2022

What's the advantage of this over simply nulling out the platform impl on dispose and enabling nullable reference types?

@kekekeks
Copy link
Member Author

This PR nulls out the platform impl on dispose. The validating layer is there to make sure that it's actually not used afterwards.

grokys added a commit that referenced this pull request Jan 31, 2022
#7369 introduced a validating layer over `WindowBase.PlatformImpl` that is only enabled in debug mode. This validating layer was causing unit tests to fail in debug mode but not on CI which runs tests in release mode.

Fix the problems:

- `PopupRoot` was not correctly disposing itself on `Dispose` - make it call `HandleClosed` in order to perform the same steps as when the popup is closed via other means
- Some unit tests try to access the `PlatformImpl` to get hold of a mock. Added `ValidatingWindowImpl.Unwrap` to allow this
- `ValidatingWindowBaseImpl.Activated` was setting the wrong property on the wrapped `PlatformImpl`.
grokys added a commit that referenced this pull request Feb 24, 2022
The validating layer introduced in #7369 has done its job and caught a few bugs, but was causing difficulties in certain places so removing it now.

Fixes #7573
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants