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

[net8.0] Merge main to net8.0 #22219

Merged
merged 55 commits into from
May 6, 2024
Merged

[net8.0] Merge main to net8.0 #22219

merged 55 commits into from
May 6, 2024

Conversation

rmarinho
Copy link
Member

@rmarinho rmarinho commented May 6, 2024

Description of Change

Bring the latest changes from main to net8.0 branch

jsuarezruiz and others added 30 commits April 15, 2024 11:44
* Added repro sample

* Fix the issue

* Added UI Test

* Updated csproj

* More changes

* Removed sample and test

* More changes

* Removed unnecesary changes
…40408.1 (#21839)

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

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Fix the issue

* Added device test
* Add test

* Add special case for resolving x:DataType for the  binding context property
* Add unit test

* Simplify TargetType=x:Type before setting resources
…tory (#21797)

#21750
---------
Co-authored-by: Eilon Lipton <Eilon@users.noreply.github.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>
Fixes: #20710
Context: https://github.com/marco-skizza/MauiCollectionView

Testing the sample above, you can see a memory leak when setting up a
`CollectionView`'s `DataTemplate` in XAML, but it appears to work just
fine with C#.

Using `dotnet-gcdump` with a `Release` build of the app, I see:

![image](https://github.com/dotnet/maui/assets/840039/6b4b8682-2989-4108-8726-daf46da146e4)

You can cause the C# version to leak, if you make the lambda an instance
method:

* jonathanpeppers/iOSMauiCollectionView@3e5fb02

XAML just *happens* to use an instance method, no matter if XamlC is on/off.

I was able to reproduce this in `CollectionViewTests.iOS.cs` by making the
`CollectionView` look like a "user control" and create an instance method.

There is a "cycle" that causes the problem here:

* `Microsoft.Maui.Controls.Handlers.Items.VerticalCell` (note this is a `UICollectionViewCell`) ->
* `DataTemplate` ->
* `Func<object>` ->
* `PageXamlLeak` (or `PageCsOk` w/ my change) ->
* `CollectionView` ->
* `Microsoft.Maui.Controls.Handlers.Items.VerticalCell`

The solution being to make `TemplatedCell` (which is a subclass of `VerticalCell`)
hold only a `WeakReference` to the `DataTemplate`.

With this change in place, the test passes.

~~ Notes about Catalyst ~~

1. The new test passes on Catalyst (with the `#if` removed), if you run it by itself
2. If you run *all* the tests, it seems like there is a `Window` -> `Page` -> `CollectionView` that makes the test fail.
3. This seems like it's all related to the test setup, and not a bug.

It seems like what is here might be OK for now?

If Catalyst leaks, it would probably leak on iOS as well and the test passes there.
Context: #21725

In #21725 we noticed the source generator unit tests (for `*.xaml.g.cs`) don't run on CI.

Whoops!
* [C] Propagate resources when reparenting

- fixes a bug when the theme is changed while the control/page isn't
  parented. Should fix @LeDahu22 reported issue of #21744

* move logic to resourcesextensions
* [iOS] Fix crash closing Popup with WebView (#21718)

* Added repro sample

* Fix the issue

* Added UI Test

* Updated csproj

* More changes

* Removed sample and test

* More changes

* Removed unnecesary changes

* Added UITest

* Update Issue21846.xaml.cs

* Update Issue21846Modal.xaml.cs

* Update Issue21846.cs

---------

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
* Bump the Locker action version
Refer to microsoft/vscode-github-triage-actions#210

* Restrict the Locker action to dotnet org
The PR trivially attempts to decrease the number of interops, using a local variable.

Contributes to #21787
…es (#21928)

Context: https://github.com/AdamEssenmacher/MemoryToolkit.Maui/tree/main/samples

Testing the above sample, I saw some odd results in a `.gcdump`
related to `UriImageSource` inside a `CollectionView`:

    AsyncTaskMethodBuilder+AsyncStateMachineBox<VoidTaskResult, Microsoft.Maui.Controls.ImageElement+<CancelOldValue>d__10>, Count: 182
        Dictionary+Entry<Int32, Task>[], Count: 182
            Dictionary<Int32, Task> [Static variable Task.s_currentActiveTasks], Count: 1

It appears that we `await` a `Task` that never completes, which is:

https://github.com/dotnet/maui/blob/8e4450cbc14932a6c74aeb8b7bfee9c94eca18b0/src/Controls/src/Core/ImageElement.cs#L129

The `Task` also holds onto the `ImageSource` and whatever memory it
happened to use. That would be ok if the `Task` completed.

I could reproduce the problem in a test:

    [Fact]
    public async Task CancelCompletes()
    {
        var imageSource = new StreamImageSource
        {
            Stream = _ => Task.FromResult<Stream>(new MemoryStream())
        };
        await ((IStreamImageSource)imageSource).GetStreamAsync();
        await imageSource.Cancel(); // This should complete!
    }

This non-completing `Task` can occur when changing `Image.Source`,
which would certainly be bad while scrolling a `CollectionView`!

To fix, I refactored the code slightly and had the problematic case
return:

    return Task.FromResult(false);
* Use better IndexOf where possible

* apply similar performance improvement to the predicate-based IndexOf()

* use built-in IndexOfChild on Android

* add braces as per new editorconfig rule

* use TryGetValue to avoid double lookup

---------

Co-authored-by: Edward Miller <symbiogenisis@outlook.com>
* [ios/catalyst] fix memory leak in gestures

Related: #21089
Context: jonathanpeppers/iOSMauiCollectionView@547b7fa

Doing something like this and running on iOS or Catalyst:

    <CollectionView.ItemTemplate>
        <DataTemplate>
            <Label Text="{Binding Name}">
                <Label.GestureRecognizers>
                    <TapGestureRecognizer Command="{Binding BindingContext.Command}" />
                </Label.GestureRecognizers>
            </Label>
        </DataTemplate>
    </CollectionView.ItemTemplate>

Causes a memory leak.

I could reproduce this in a test that is a bit more elaborate than #21089,
showing issues for different types of gestures:

* Usage of `ShouldReceiveTouch` creates a cycle:

  * `GesturePlatformManager` -> `UIGestureRecognizer.ShouldReceiveTouch` -> `GesturePlatformManager`

  * We can move `ShouldReceiveTouch` to a "proxy" class, and avoid the cycle.

* `PanGestureRecognizer` has closures that cause the same cycle:

  * `this.PlatformView` -> `eventTracker?.PlatformView`

  * `panRecognizer` -> `((PanGestureRecognizer)panGestureRecognizer)`

  * These already had a `WeakReference` to use instead, but we could just make use of them.

There *may* be a general problem with `CollectionView` in how it doesn't
tear down `GesturePlatformManager` in the same way for children. But
in either case, the problems above are cycles that should be broken.

* Fix test on Windows
* Just added a couple of tests

* Focus on Android and iOS (for now)

* Added legacy tests to the pipeline

* More changes

* More changes

* Fixed build

* Created ui-tests-legacy-steps.yml

* More changes

* More changes

* Updated Appium to RC7

* Update ui-tests.yml

* Fixed test on Android

* Fixed test

* More changes

* More changes

* Updated snapshot

* More fixes

* Clean code

* Run test only on iOS

* Added pending constant

* Fix build error

* More changes

* Update ControlGallery.iOS.Appium.UITests.csproj

* Update ControlGallery.Shared.Appium.UITests.csproj

* Trying to run only on iOS

* This is the fix, I feel it

* Fix merge error

---------

Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>
* Update EnumerableExtensions.cs

* Update EnumerableExtensions.cs
Co-authored-by: Edward Miller <symbiogenisis@outlook.com>
* Add s/triaged label for issues opened by (core) team

Alternate approach to #21775 as that clearly didn't work. Hoping this _will_ work!

From the original PR:

> Adds a rule to the bot that adds the s/triaged label to an issue which is opened by the core team (someone with write permissions on the repo). A lot of times these issues are tasks and/or very cryptic to the triage team. This way, the triage team will skip these issues and we assume the issues are valid ones and/or not issues that need triage.

* Update resourceManagement.yml
…bViewSource (#21892)

### Description of Change

This PR removes the use of a 2nd "hidden" WebView2 that was used to
parse and add a HTML `base` tag to the `head` tag when setting the HTML
source of a WebView to a string.

This was done by appending the `base` tag script to the start of the
user's HTML string, which the WebView then adds into the `head` element.
While this is technically not valid HTML, all current browsers correct
this behavior.

This is a work-around for the lack of being able to set the base URL
when navigating to a string using WebView2
(MicrosoftEdge/WebView2Feedback#530).

As a bonus, using `HtmlWebViewSource` should now be 2x faster 😅

### Issues Fixed

Fixes #21631
PureWeen and others added 15 commits April 26, 2024 14:39
Context: https://cs.android.com/android/platform/superproject/main/+/main:libcore/ojluni/annotations/hiddenapi/java/lang/Boolean.java;l=109?q=boolean.java

Some internal APIs in MAUI were using `java.lang.Boolean` when they
could just use `boolean` instead.

Java `boolean` is a primitive type, while `java.lang.Boolean` is an
object (that can be null). This is similar to `bool` vs `bool?` in C#,
except Java does not have `struct`.

This avoids:

* JNI field lookup of `Boolean.TRUE` and `Boolean.FALSE` on startup

* JNI field reads of `Boolean.TRUE` and `Boolean.FALSE` on every
  `UriImageSource` creation

* Creating a C# instance of `Java.Lang.Boolean`, and the bookkeeping
  for reusing C# instances

I was also going to change `ImageLoaderCallback.onComplete`, but it is
listed as a public API.

This is a small improvement, but should help all `UriImageSource` on
Android.
Co-authored-by: Edward Miller <symbiogenisis@outlook.com>
* Improve error logging for failed resizetizering

* Fix tests
* Implement PressEnter method on appium

* IsKeyboardShown not implemented on Windows

* Updated test
* Fix TabbedPage title displaying incorrectly

Presently, when a navigation page is created and a tabbed page is added to it with children, the navigation page uses the tabbed page's selected child title as its title. This behavior is unexpected when using standard tabs (tabs on the top) in a navigation page on Android.

Without a custom tabbed page, this behavior makes sense, especially for bottom tabs; however, when a custom tabbed page is defined, it seems that the title should not be overridden.

Fixes #8577

* Add device tests

* Add unit tests

* Updated test

* More tests

* Fixed test not being able to fail

* Changed GetTitle to work with more than just TabbedPages

* Removed tests that deal with Child page titles replacing the Toolbar title

---------

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: Dustin Wojciechowski <dustin.wojciechowski@microsoft.com>
Co-authored-by: Dustin Wojciechowski <dustinwo@microsoft.com>
…40424.1 (#22119)

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

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Fix #19786

* Added a Ui test (#19786)

* Update MauiCarouselRecyclerView.cs
* [iOS] Shell page title fix (#20199)

* Added a UiTest (#20199)

* Added pending snapshot

* Shell page title fix

---------

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Context https://developercommunity.visualstudio.com/t/Android-mipmapappicon-failing-with-APT/10622494

Users are reporting this error quite allot. During
a build it fails with the following error

```
 APT2260 resource mipmap/appicon (aka xxx:mipmap/appicon) not found.
```

Further investigation this only appears to happen during a full build
of all the platforms. Specifying `-f net8.0-android` on the build
this does not seem to happen at all.

The problem is the build gets into a weird situation where the `mauiimage.stamp` file
is present but none of the generated images are. Considering that the
stamp file gets generated AFTER the images its kinda difficult to see
how this even occurs.

The work around for this is to write the list of generated image files to
`mauiimage.outputs`. We can then read this list back on subsequent builds
and use that list for the `Outputs` of the `ResizetizeImages` target.
This means the target will run if either the stamp file or any of the
expected images are not present.
# Conflicts:
#	eng/Version.Details.xml
@rmarinho rmarinho requested a review from a team as a code owner May 6, 2024 10:13
@rmarinho rmarinho requested review from PureWeen and jsuarezruiz May 6, 2024 10:13
@rmarinho rmarinho changed the base branch from main to net8.0 May 6, 2024 10:14
@rmarinho rmarinho requested a review from a team as a code owner May 6, 2024 10:14
@rmarinho rmarinho merged commit 9c766ab into net8.0 May 6, 2024
48 of 50 checks passed
@rmarinho rmarinho deleted the merge-main-net80 branch May 6, 2024 13:40
rmarinho added a commit that referenced this pull request May 6, 2024
rmarinho added a commit that referenced this pull request May 6, 2024
@rmarinho rmarinho restored the merge-main-net80 branch May 6, 2024 13:40
@rmarinho rmarinho deleted the merge-main-net80 branch May 6, 2024 13:41
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2024
@samhouts samhouts added the fixed-in-net8.0-nightly This may be available in a nightly release! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-net8.0-nightly This may be available in a nightly release! t/housekeeping ♻︎
Projects
None yet
Development

Successfully merging this pull request may close these issues.