[Android] Fix first navigation race condition with main page affectation

[kvpt]

Description of Change

Wait for initial page to be loaded before push new pages inside the navigation stack.

Issues Resolved

• Fix [Android] Navigation dont wait initial pages to be loaded on AppCompat #4729
• Fix [Android] GetFragment(Page, bool, bool); System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. Parameter name: index #3528
• Fix [Feedback] Xamarin Forms 3.2 Android entry cursor jumps back and forth #3923

Platforms Affected

• Android

PR Checklist

• Has automated tests => Only manuel test is possible
• Rebased on top of the target branch at time of PR
• Changes adhere to coding standard

[kvpt]

Since I have provided the reproduction case for my other PR, I made one for this one too.
issue4729.zip

[kvpt]

This one need more work.
It cause a regression,
The navigation, if made in quick successions, are not ordered correctly which cause IndexOutOfRange if combined with RemovePageAction.
I need to try another approach.

[kvpt]

I think now that the fix is right.
I will test it in depth in my application to confirm that.
I also need to test if it doesn't reintroduce #2338 and if yes how to correct it.

[kvpt]

After some tests in my app, no regression found.
I also run the test case 2338 and all the cases succeeded on my emulator (API 27).
So this can be reviewed on your side.

[kvpt]

After several unsuccessful attempts I had the idea to try with the FormsApplicationActivity instead of AppCompat. And the issue was not present it.
So I compared the platforms ways to handle this scenario and found that the difference was in AppCompat the pages are pushed in OnAttachedToWindow and in FormsApplicationActivity the pages are pushed in OnElementChanged.

My attempt was to synchronize OnAttachedToWindow method call with SwitchContentAsync method call, but the root cause was that pages must not be push during OnAttachedToWindow because it's a native call that we dont control when it is called by the native platform below.

So I made a blame and found that this code was changed in AppCompat in the PR #1999.
I partially revert it, to match the FormsApplicationActivity way, that fixed my issue.
A test was provided with the PR #1999, so I runned it to test that it wasn't break and it pass with success.

[kvpt]

Hi @rmarinho,

I will have some freetime in the coming days, I can help to fix the failing tests.
I tried to see what tests failed but it not visible without rights.
If you can list them I can try to fix them.

Thanks.

[jfversluis]

Hey @kvpt I will step over my embarrassment here about how long this has been in here and I would love to work with you to see if this is still needed and how we can make it happen.

[jfversluis]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kvpt]

Hi @jfversluis,

Yes this fix is still needed.
If you want to navigate during the app init (like to implement an auto login feature), either the app crash or is not in the correct page after init without this change.

I updated the reproduction case with the latest version of the framework source code to confirm it still reproducible.
issue4729_v5.zip
In this reproduction case the app will throw an exception on start and if you add an delay of one second in the OnStart method of the App class you will found that it doesn't crash anymore but now the stack is in the wrong order.

I regularly rebase this fix above the framework latest version and used it in my apps for almost 3 years now without any issue.

Like I say earlier if this fix introduce some issue or break some tests, please let me know.
I will do my best to fix it because I really want this PR to be fixed and I think i'm not the only one affected by this issue.

[jfversluis]

Perfect! That sounds like it's been tested pretty well. I will do some more testing as well, could you try out the NuGet that resulted from this build as well? Instructions for that are here.

[kvpt]

Sure, I will do that.

[kvpt]

I confirm that the PR release version work as expected.
NavIssueRepro_PR.zip

[jfversluis]

Awesome, thanks for the confirmation! I will have to see if I want to merge this now... Tbh I don't think so. I pretty much got a release ready. I don't want to risk putting in a last-minute breaking change. But I will make sure to merge it for the next one which should then be released in a month or so.

Is this the only thing that makes you keep a fork around?

[kvpt]

No problem I understand, I was no longer waiting for the PR to be merged so I can easily wait one more version.

My main problem with Xamarin Forms was stability issues, so I made a lot of PR to fix the issues I was having in my apps.
All of them were merged, except this one, it's the only modification that force me to use a fork.

[jfversluis]

I'm honored to be the one that will finally get you get rid of this fork :)

Sorry it took so long though. Keep your eye on this one!