Disconnect old handler after setting new handler (#8781)

* Disconnect old handler after setting new handler

* - add check for disconnect call

* - more disconnect

* - fix up order of setting handlers

* - fix cast check

* Update TestNavigationPage.cs

* - change to xunit

* - fix namespaces

src/Controls/src/Core/HandlerImpl/Element/Element.Impl.cs

@@ -60,6 +60,12 @@ void SetHandler(IElementHandler newHandler)
 
  _handler = newHandler;
 
+ // Only call disconnect if the previous handler is still connected to this virtual view.
+ // If a handler is being reused for a different VirtualView then the virtual
+ // view would have already rolled 
+ if (_previousHandler?.VirtualView == this)
+ _previousHandler?.DisconnectHandler();
+
  if (_handler?.VirtualView != this)
  _handler?.SetVirtualView(this);
 

src/Controls/src/Core/HandlerImpl/Shell.Impl.cs

@@ -28,14 +28,5 @@ protected override void OnPropertyChanged([CallerMemberName] string propertyName
  if (propertyName == Shell.FlyoutIsPresentedProperty.PropertyName)
  Handler?.UpdateValue(nameof(IFlyoutView.IsPresented));
  }
-
-#if ANDROID
- protected override void OnHandlerChanging(HandlerChangingEventArgs args)
- {
- base.OnHandlerChanging(args);
- args.OldHandler?.DisconnectHandler();
- }
-
-#endif
  }
 }

src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt

@@ -17,4 +17,3 @@ override Microsoft.Maui.Controls.TemplatedView.MeasureOverride(double widthConst
 *REMOVED*override Microsoft.Maui.Controls.FlexLayout.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
 static readonly Microsoft.Maui.Controls.VisualElement.ZIndexProperty -> Microsoft.Maui.Controls.BindableProperty!
 *REMOVED*override Microsoft.Maui.Controls.Platform.ControlsAccessibilityDelegate.OnInitializeAccessibilityNodeInfo(Android.Views.View? host, AndroidX.Core.View.Accessibility.AccessibilityNodeInfoCompat? info) -> void
-~override Microsoft.Maui.Controls.Shell.OnHandlerChanging(Microsoft.Maui.Controls.HandlerChangingEventArgs args) -> void

src/Controls/tests/Core.UnitTests/HandlerLifeCycleTests.cs

@@ -2,8 +2,11 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
+using Microsoft.Maui.Controls.Hosting;
 using Microsoft.Maui.Controls.Internals;
 using Microsoft.Maui.Graphics;
+using Microsoft.Maui.Hosting;
+using Microsoft.Maui.Platform;
 using Xunit;
 using Xunit.Sdk;
 
@@ -12,6 +15,58 @@ namespace Microsoft.Maui.Controls.Core.UnitTests
 
  public class HandlerLifeCycleTests : BaseTestFixture
  {
+ [Fact]
+ public void SettingHandlerToNullDisconnectsHandlerFromVirtualView()
+ {
+ var mauiApp1 = MauiApp.CreateBuilder()
+ .UseMauiApp<ApplicationStub>()
+ .ConfigureMauiHandlers(handlers => handlers.AddHandler<LifeCycleButton, HandlerStub>())
+ .Build();
+
+ LifeCycleButton button = new LifeCycleButton();
+
+ MauiContext mauiContext1 = new MauiContext(mauiApp1.Services);
+ var handler1 = button.ToHandler(mauiContext1);
+ button.Handler = null;
+ Assert.NotEqual(button, handler1.VirtualView);
+ }
+
+ [Fact]
+ public void SettingNewHandlerDisconnectsOldHandler()
+ {
+ var mauiApp1 = MauiApp.CreateBuilder()
+ .UseMauiApp<ApplicationStub>()
+ .ConfigureMauiHandlers(handlers => handlers.AddHandler<LifeCycleButton, HandlerStub>())
+ .Build();
+
+ LifeCycleButton button = new LifeCycleButton();
+
+ MauiContext mauiContext1 = new MauiContext(mauiApp1.Services);
+ var handler1 = button.ToHandler(mauiContext1);
+
+ var mauiApp2 = MauiApp.CreateBuilder()
+ .UseMauiApp<ApplicationStub>()
+ .ConfigureMauiHandlers(handlers => handlers.AddHandler<LifeCycleButton, HandlerStub>())
+ .Build();
+
+ MauiContext mauiContext2 = new MauiContext(mauiApp2.Services);
+
+ List<HandlerChangingEventArgs> changingArgs = new List<HandlerChangingEventArgs>();
+ button.HandlerChanging += (s, a) =>
+ {
+ Assert.Equal(handler1, a.OldHandler);
+ Assert.NotNull(a.NewHandler);
+
+ changingArgs.Add(a);
+ };
+
+ var handler2 = button.ToHandler(mauiContext2);
+
+ Assert.NotEqual(button, handler1.VirtualView);
+ Assert.Equal(button, handler2.VirtualView);
+ Assert.Single(changingArgs);
+ }
+
  [Fact]
  public void ChangingAndChangedBothFireInitially()
  {

src/Controls/tests/Core.UnitTests/TestClasses/TestNavigationPage.cs

@@ -62,8 +62,9 @@ public void CompleteCurrentNavigation()
 
  var newStack = CurrentNavigationRequest.NavigationStack.ToList();
  CurrentNavigationRequest = null;
- (VirtualView as IStackNavigation)
- .NavigationFinished(newStack);
+
+ if ((this as IElementHandler).VirtualView is IStackNavigation sn)
+ sn.NavigationFinished(newStack);
  }
 
  async void RequestNavigation(NavigationRequest navigationRequest)

src/Core/src/Handlers/Element/ElementHandler.cs

@@ -43,8 +43,6 @@ public virtual void SetVirtualView(IElement view)
  return;
 
  var oldVirtualView = VirtualView;
- if (oldVirtualView?.Handler != null)
- oldVirtualView.Handler = null;
 
  bool setupPlatformView = oldVirtualView == null;
 
@@ -54,6 +52,13 @@ public virtual void SetVirtualView(IElement view)
  if (VirtualView.Handler != this)
  VirtualView.Handler = this;
 
+ // We set the previous virtual view to null after setting it on the incoming virtual view.
+ // This makes it easier for the incoming virtual view to have influence
+ // on how the exchange of handlers happens.
+ // We will just set the handler to null ourselves as a last resort cleanup
+ if (oldVirtualView?.Handler != null)
+ oldVirtualView.Handler = null;
+
  if (setupPlatformView)
  {
  ConnectHandler(PlatformView);