[Android] Crash when switching MainPage from MDP to Page in Init/Appearing/ctor

[samhouts]

Description

When the MasterDetailContainer Dispose runs, it crashes if you're switching the MainPage immediately on a page load.

Steps to Reproduce

1. Run B40005 test in Control Gallery/
   https://github.com/Schinwinkwinsky/DroidFragmentCrash

Expected Behavior

Test loads

Actual Behavior

Test immediately crashes

Basic Information

• Version with issue: 3.0.0,2.5.1
• Last known good version: ?

[jalbertSyncroTech]

@PureWeen I'm currently dealing with an issue that may be resolved by your changes, but I can't tell from the code:

I have an Android app that remains running in the background after the user closes the activity. I go through the full Xamarin.Forms.Forms.Init() every time the user re-opens the app since it creates a new activity.

I'm currently cleaning up Xamarin.Forms by calling Application.ClearCurrent(), but I think it's leaving some of the old renderers around, in particular I think it's reusing the MasterDetailPageRender from the previous instance because it's never calling OnAttachedToWindow() when I re-open the app.

So, I've got two questions:

1. Is there a proscribed way to cleanup Xamarin.Forms?
2. In this pull request you added MarkedForDispose, but I can't find any code that actually disposes objects with that property set. The only code I could find that actually checks the value is IsAttachedToRoot()

[PureWeen]

@jalbertSyncroTech

The change will be apart of the next service release for 3.1

Is there a proscribed way to cleanup Xamarin.Forms?

This kind of depends on your scenario. I've never used ClearCurrent in my applications. That's already called by base.OnCreate I currently just keep a static reference to my pages and then in OnCreate I look to see if that exists. If it does then I use that for setting main page other wise I recreate.

In this pull request you added MarkedForDispose,

Marked For Dispose is just a way to indicate that it will be disposed eventually so just don't do anything more with this. That dispose essentially happens here

Xamarin.Forms/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs

Line 261 in 11a1e70

void SetPageInternal(Page newRoot)

The actual dispose gets queue'd on the UI thread because if we just do everything immediately you run into Fragment issues when swapping out the main page in weird ways.

I think it's reusing the MasterDetailPageRender from the previous instance because it's never calling OnAttachedToWindow() when I re-open the app

If you could put together a repro that would be helpful so I can see how you're handling things and how this isn't being called. I did a basic test with the Xamarin.Forms.ControlGallery.Android and detachwindow is called there when I background/resume an app

[jalbertSyncroTech]

@PureWeen Thanks for the update! I tracked down my issue:

I have a PlatformEffect attached to my NavigationPage, which is the Detail of my MasterDetailPage. I'm using the new UpdateMenuItemIcon() of the NavigationPageRenderer to modify the ToolbarItems according to the effect.

When the effect is first attached, the NavigationPage's renderer doesn't exist yet, so I was listening to the Appearing event to know when the renderer had been created. However, the startup Appearing event had actually been triggered already, and it was just a lucky side-effect that I was manually calling SendAppearing() later in my code for a different feature and that the renderer existed at that point. I wasn't calling SendAppearing() on the second run, so it looked like I was never getting the Appearing event.

I changed my effect to instead listen for a property changed event on "Renderer", based on the attached property here, and everything's working as intended.

Xamarin.Forms/Xamarin.Forms.Platform.Android/Platform.cs

Line 28 in c9ef843

internal static readonly BindableProperty RendererProperty = BindableProperty.CreateAttached("Renderer", typeof(IVisualElementRenderer), typeof(Platform), default(IVisualElementRenderer),