[net9.0] Merge main net9.0 bring arcade #22447
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Add unit test * Simplify TargetType=x:Type before setting resources
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:  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.
* [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>
* Code style * Remove unused line
Co-authored-by: Edward Miller <symbiogenisis@outlook.com>
…tton doesn't have a width or height set
* 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
…21919) * Fix for continuous listener always null * Added test --------- Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
* just trigger SizeChanged when Window size changes * adding unit test * adjusted test to match new behavior * implement way to notify if handler should be aware of PC * adjust SetPropertyChange method to accept the flag * add the missing attribute Co-authored-by: Shane Neuville <shane94@hotmail.com> * update Window cs to match changes on main * revert Element.cs to main * Revert changes on SetPropertyChanged method Co-authored-by: Shane Neuville <shane94@hotmail.com> * move Window class to old impl. and override UpdateHandlerValues * add flag to trigger size changed * add handle if the value changes during PC event --------- Co-authored-by: Shane Neuville <shane94@hotmail.com>
Fixes: #21777 (comment) When creating a new .NET MAUI project, the first design-time build can randomly fail with: C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.95\tools\Xamarin.Android.Common.targets(613,3): error XA1018: Specified AndroidManifest file does not exist: D:\source\MauiApp3\MauiApp3\AndroidManifest.xml. This can happen because: 1. NuGet restore isn't complete 2. `Microsoft.Maui.Controls.SingleProject.targets` is not imported 3. `$(AndroidManifest)` has not been moved to `Platforms/Android/AndroidManifest.xml` This means the Android workload would just look for `AndroidManifest.xml` in the root of the project, which doesn't exist. To fix this, let's check for the non-existence of `$(AndroidManifest)` in the workload and set it. After testing this change in: * `C:\Program Files\dotnet\packs\Microsoft.Maui.Sdk\8.0.21\Sdk\Microsoft.Maui.Sdk.After.targets` I don't get a design-time build failure anymore.
Co-authored-by: Edward Miller <symbiogenisis@outlook.com>
--------- Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
* Added repro sample * Added UI Test * Fix the issue * Updated test * Renamed Listener
* Only the library projects updates build numbers * ok * better logging * ' * why tho * that is why * wow * metadata * PreReleaseVersionLabel * this is a bit better
* Update appium/drivers versions * Use newer .NET SDK * Try to uninstall appium / cleanup first * Invoke appium -v in try/catch * Log npm into LogDirectory and verbose * Use newer appium doctor commands The separate doctor package is deprecated and now appium contains doctor commands for drivers and plugins: * Remove added artifact stage These will get picked up by virtue of being placed in $(LogDirectory) now * Fix logic syntax * Fix ps command for create dir * Fix path combine for logs * Fix logging options * Try ignoring appium doctor exit code * Try driver uninstall directly with npm * Try using update/install * Use same npm command appium does to install driver * Install drivers based on platform * Remove clearing .appium user folder * Try setting APPIUM_HOME to a local folder * Move variables * Remove old appium cleanup We now are setting APPIUM_HOME which will point appium to where we want our drivers/plugins saved, and this directory will be inside the CI/CD working directory so it should be 'clean' every time we run this. We shouldn't ever see the uninstall driver commands run, and we shouldn't need to touch any ~/.appium* folders from other pipelines. * Install explicit versions * List out appium home dir, not npm logs
… system service ref, MAUI cannot recover (#22281) Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
* Fix the issue * Added repro sample * Added UITest
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Merge main and bring arcade