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

[net9.0] Merge main to net9.0 #22984

Merged
merged 34 commits into from
Jun 13, 2024
Merged

[net9.0] Merge main to net9.0 #22984

merged 34 commits into from
Jun 13, 2024

Conversation

rmarinho
Copy link
Member

Description of Change

Bring latest changes from main to net9.0 branch

MartyIX and others added 30 commits June 1, 2024 22:21
* Update project & solution templates

* Feedback
The OS does not specify a background so we need to make sure there is
a transparent background to get the ripple and border.
* [ios/catalyst] fix leak in NavigationPage

Fixes: #20119

PR #13833 fixed a leak in child pages of a `NavigationPage`, but it
appears there are several cycles that would prevent the `NavigationPage`
itself from going away.

So, if you did something like this:

    // The original NavigationPage & children leak
    Application.Current.MainPage = new NavigationPage(new Page1());
    Application.Current.MainPage = new Page2();

I could reproduce the same problem in a new device test.

The cycles (and solutions) are:

1. `NavigationPage` -> `NavigationRenderer` -> `ParentingViewController` -> `NavigationPage`

    * Solution: make `_child` a `WeakReference`

2. `NavigationPage` -> `MenuItemTracker` -> `NavigationPage`

    * Solution: make `_target` a `WeakReference`, and `_additionalTargets` a `List<WeakReference>`

3. `NavigationPage` -> `MenuItemTracker.CollectionChanged` -> `NavigationPage`

    * Solution: subscribe/unsubscribe in `WillMoveToParentViewController`

After these changes the sample app works, and the new device test passes.

* Skip test on Windows for now
* Revert changes to fix the issue

* Added repro sample

* Added UITest

* More samples

* Added new UITest

* Fix the issue

* Removed unnecessary changes

* Fixed test description

* Adding snapshots

* More snapshots
…s` (#22781)

### Description of Change

The idea of the PR is simple. There two sets of pointer event
subscriptions:


https://github.com/dotnet/maui/blob/fcabff1fe3e551e22558cba74dd3a86e00323b69/src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Windows.cs#L792-L796

and


https://github.com/dotnet/maui/blob/fcabff1fe3e551e22558cba74dd3a86e00323b69/src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Windows.cs#L824-L827

Consequently, `_container.PointerPressed`, `_container.PointerExited`,
and `_container.PointerReleased` are subscribed twice. And this PR
removes these subscriptions.

 

### Performance impact

* MAIN:
[Maui.Controls.Sample.Sandbox.exe_20240601_222830.speedscope.MAIN.json](https://github.com/user-attachments/files/15523262/Maui.Controls.Sample.Sandbox.exe_20240601_222830.speedscope.MAIN.json)
* PR:
[Maui.Controls.Sample.Sandbox.exe_20240601_223130.speedscope.PR.json](https://github.com/user-attachments/files/15523263/Maui.Controls.Sample.Sandbox.exe_20240601_223130.speedscope.PR.json)


![image](https://github.com/dotnet/maui/assets/203266/fd77fd77-9c62-414f-b7a6-d9ceec6b3621)

-> ~25% improvement

### Issues Fixed

Contributes to #21787
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2064274

In debugging a sample with a `<CollectionView/>` with large images
in each row, we found memory leaking only for certain objects:

* `Microsoft.Maui.Controls.Handlers.Items.MauiCollectionView`
* `Microsoft.Maui.Controls.Handlers.Items.VerticalCell`
* `Microsoft.Maui.Platform.MauiLabel`
* `Microsoft.Maui.Platform.MauiImageView`

It appears all of these types of objects were going away appropriately:

* `Microsoft.Maui.Controls.ContentPage`
* `Microsoft.Maui.Controls.CollectionView`
* `Microsoft.Maui.Controls.Label`
* `Microsoft.Maui.Controls.ImageView`

In #21850, #15831, and #14329 we fixed memory leaks in other various ways
related to `CollectionView`. But the above UIKit objects remain!

After a combination of using `dotnet-gcdump` and Instruments, we found:

* `ReorderableItemsViewController` referenced by
  * `EventHandler<EventArgs>` referenced by
    * `VerticalCell`

And so every `ReorderableItemsViewController` subscribes to:

    cell.ContentSizeChanged += CellContentSizeChanged;
    cell.LayoutAttributesChanged += CellLayoutAttributesChanged;

Creating a cycle:

    ReorderableItemsViewController -> VerticalCell -> ReorderableItemsViewController

This makes every `VerticalCell` never get collected, and so any
child platform views are also never collected. However, any MAUI-views
were already being collected successfully.

After some trial and error, I was able to reproduce this problem in
an existing device test.

The fix was to make both of the `ContentSizeChanged` and `LayoutAttributesChanged`
events backed by `WeakEventManager`.

I tried overriding `WillMoveToParentViewController` to unsubscribe, but
I didn't have a reference to the original cell.
* Move docs to wiki

* add title
* Don't update CV Position while Device is rotating

* - wire up to already added teardown/setup

* - fix CarouselViewLoopManager

* - fix test
…22782)

* Typos

* Font size and font family Optimizations

* Remove not working code
* Clean up / Linq removal

Cleaned up the methods and added more up to date .NET syntax as well as removing the usage of LINQ.

* Removed ToLowerInvariant

As the switch makes use of nameof the usage of ToLowerInvariant is redundant.

* Fix return value / further improvements

The value now returns an easing instead of a string (oops!), I have also implemented a local method to check case sensitive strings for the switch statement allowing the same functionality as previous.

* Tests

* Update EasingTypeConverter.cs

---------

Co-authored-by: Matthew Leibowitz <[email protected]>
* Remove IgnoreIfPlatforms usage on UITests

* Changes in failing test

* Re-enabled tests in some platforms

* Moved comment

* Changes based on PR feedback

* Fixed test errors

* Fix build errors

* Ignore golden tests on macOS for now

* Updated test
Fixes: #22814

`ToolbarItem` has a circular reference that causes it to live forever:

* `ToolbarItem` -> `ToolbarItemExtensions.PrimaryToolbarItem` -> `ToolbarItem`

And then if you have a `Page` such as:

    class LeakyShellPage : ContentPage
    {
        public LeakyShellPage()
        {
            var item = new ToolbarItem { Text = "Primary Item" };
            item.Clicked += OnToolbarItemClicked;
            ToolbarItems.Add(item);
        }

        // Needs to be an instance method
        void OnToolbarItemClicked(object sender, EventArgs e) { }
    }

Then the `ToolbarItem` (that leaks), would also cause the entire `Page`
to leak as it has a strong reference to it. I could reproduce this
problem by updating a tests in `ShellTests.cs` to add a `ToolbarItem`.

To solve this problem, I changed the `_item` field in both
`PrimaryToolbarItem` and `SecondaryToolbarItem` to be a `WeakReference`.

While writing tests, I found there was a mismatched `#if` and `#endif`.

I think part of `ShellTests.cs` were not running at all on `MACCATALYST`
by accident. So, I hope they still pass?
* [Android] Border with RoundRectangle (#21173)

* Added a UiTest (#21173)

* Added pending snapshots

* Updated snapshots

* Updated test

* Added UITest for Issue19877

* Added snapshot

* Fix tests

---------

Co-authored-by: Javier Suárez <[email protected]>
Co-authored-by: Matthew Leibowitz <[email protected]>
* Use corrected index for CellDisplayingEnded

* - update tests

* - ignore all tests on windows

* - remove duplicated code

* - fix test

* Update CarouselViewUITests.cs
* Update ddd DateTimeFormatter

* Update test
* Ignore orientation CarV test for Mac

* - don't throw if appium closing code fails

* - remove tests causing noise
…40604.1 (#22950)

Microsoft.DotNet.XHarness.CLI , Microsoft.DotNet.XHarness.TestRunners.Common , Microsoft.DotNet.XHarness.TestRunners.Xunit
 From Version 9.0.0-prerelease.24277.1 -> To Version 9.0.0-prerelease.24304.1

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
PureWeen and others added 4 commits June 10, 2024 17:10
### Description of Change

PopLifeCycle occasionally fails in CI and we'll see if it's caused by
this extra call
* Update arcade

* Update dotnet-tools.json

* Update xharness
# Conflicts:
#	NuGet.config
#	eng/Version.Details.xml
#	eng/Versions.props
#	eng/common/templates-official/job/source-index-stage1.yml
#	eng/common/templates/job/source-index-stage1.yml
#	global.json
#	src/Core/src/Converters/EasingTypeConverter.cs
@rmarinho rmarinho added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Jun 12, 2024
@rmarinho rmarinho requested review from a team as code owners June 12, 2024 00:20
@rmarinho rmarinho merged commit 26dea64 into net9.0 Jun 13, 2024
43 of 51 checks passed
@rmarinho rmarinho deleted the merge-main-net9 branch June 13, 2024 21:51
@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions fixed-in-9.0.0-preview.6.24327.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.