Xamarin.Forms
Xamarin.Forms copied to clipboard
[Core] Rework navigation task handling
Description of Change
To synchronize navigation actions, the variable CurrentNavigationTask is used to track the current navigation and await it if necessary. But this variable is never reset and can cause a memory leak. So I added a new variable in the navigation methods which contains the navigation task created by the method. At the end I compare it with the CurrentNavigationTask, if it is equal we can reset it, if not, it is that another navigation overwritten it, in this case it's this method which will do the reset.
I removed the try catch introduced in 2873, because I don't know if it is still necessary and I can't test on iOS. If the tests fails I will re-add it.
Issues Resolved
- fixes #1429
API Changes
- None
Platforms Affected
- Core (all platforms)
Behavioral/Visual Changes
None
Before/After Screenshots
Not applicable
Testing Procedure
- Run the NavigationUnitTest
- Run the reproduction case in #1429 with the profiler, then :
- Go to Page2
- Go back
- Make a snapshot
- Go to Live Objects
- Search for Page2
- If it succeed no Page2 should be present
PR Checklist
- [ ] Targets the correct branch
- [ ] Tests are passing (or failures are unrelated)
Can you check test Issue9355Test_apple_iphone ?
@rmarinho Sorry, unfortunately I don't have an Apple device to test. I don't know if it is related or not but there is a lot of "Bad Gateway" response from the test server in the failed tests attachment.
@kvpt, @samhouts Can u point out to a xf-ci nightly build nuget package for us to try out ur fix. I dont see 4.6.0.607+125-pr.9411-sha.8ef9ab9d3-azdo.13494 in VS nuget package manager, I have xf-ci package source set to this https://aka.ms/xf-ci/index.json Thanks
@ngalicoco I don't think such build exist. What you can do is to clone the branch and build the package yourself. The procedure is documented here. If you can't do that I will try to find some time this week to create a custom nuget package from the branch.
@kvpt thanks for the info, any eta for this fix to go into standard xamarin nuget package
I think at this point it's safe to say that we won't be taking this for Xamarin.Forms anymore. Thank you all so much for your time and effort on this. It hasn't gone unnoticed!