Skip to content

Commit

Permalink
Call disconnect on previous handler with Shell (#8455)
Browse files Browse the repository at this point in the history
* Wire up DisconnectHandler for Shell

* - Add tests for swapping out same window/shell with new context

* - add overrides for windows/ios

* Update ShellRenderer.cs
  • Loading branch information
PureWeen authored Jul 3, 2022
1 parent 670590b commit 2d1e763
Show file tree
Hide file tree
Showing 15 changed files with 210 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
{
public class ShellFlyoutRecyclerAdapter : RecyclerView.Adapter
{
readonly IShellContext _shellContext;
IShellContext _shellContext;
List<AdapterListItem> _listItems;
List<List<Element>> _flyoutGroupings;
Action<Element> _selectedCallback;
Expand Down Expand Up @@ -203,15 +203,22 @@ protected override void Dispose(bool disposing)

if (disposing)
{
((IShellController)Shell).FlyoutItemsChanged -= OnFlyoutItemsChanged;

_listItems = null;
_selectedCallback = null;
Disconnect();
}

base.Dispose(disposing);
}

internal void Disconnect()
{
if (Shell is IShellController scc)
scc.FlyoutItemsChanged -= OnFlyoutItemsChanged;

_listItems = null;
_selectedCallback = null;
_shellContext = null;
}

public class AdapterListItem
{
// This ensures that we have a stable id for each element
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ void IFlyoutBehaviorObserver.OnFlyoutBehaviorChanged(FlyoutBehavior behavior)
#endregion IFlyoutBehaviorObserver

const uint DefaultScrimColor = 0x99000000;
readonly IShellContext _shellContext;
IShellContext _shellContext;
AView _content;
IShellFlyoutContentRenderer _flyoutContent;
int _flyoutWidthDefault;
Expand Down Expand Up @@ -391,24 +391,41 @@ void UpdateScrim(Brush backdrop)
}
}

protected override void Dispose(bool disposing)
internal void Disconnect()
{
if (_disposed)
return;
ShellController?.RemoveAppearanceObserver(this);

_disposed = true;

if (disposing)
{
ShellController.RemoveAppearanceObserver(this);
if (Shell != null)
Shell.PropertyChanged -= OnShellPropertyChanged;

if (this.IsAlive())
{
this.DrawerClosed -= OnDrawerClosed;
this.DrawerSlide -= OnDrawerSlide;
this.DrawerOpened -= OnDrawerOpened;
this.DrawerStateChanged -= OnDrawerStateChanged;
}

ShellController?.RemoveFlyoutBehaviorObserver(this);

if (_flyoutContent is ShellFlyoutTemplatedContentRenderer flyoutTemplatedContentRenderer)
{
flyoutTemplatedContentRenderer.Disconnect();
}

_shellContext = null;
}

protected override void Dispose(bool disposing)
{
if (_disposed)
return;

_disposed = true;

((IShellController)_shellContext.Shell).RemoveFlyoutBehaviorObserver(this);
if (disposing)
{
Disconnect();

RemoveView(_content);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ protected virtual void UpdateFlyoutContent()
UpdateContentPadding();
}


void DisconnectRecyclerView()
{
if (_flyoutContentView.IsAlive() &&
_flyoutContentView is RecyclerViewContainer rvc &&
rvc.GetAdapter() is ShellFlyoutRecyclerAdapter sfra)
{
sfra.Disconnect();
}
}

AView CreateFlyoutContent(ViewGroup rootView)
{
_rootView = rootView;
Expand All @@ -209,6 +220,8 @@ AView CreateFlyoutContent(ViewGroup rootView)
oldContentView.View = null;
}

DisconnectRecyclerView();

var content = ((IShellController)ShellContext.Shell).FlyoutContent;
if (content == null)
{
Expand Down Expand Up @@ -592,6 +605,26 @@ public void OnOffsetChanged(AppBarLayout appBarLayout, int verticalOffset)

}

internal void Disconnect()
{
if (_shellContext?.Shell != null)
_shellContext.Shell.PropertyChanged -= OnShellPropertyChanged;

if (_flyoutHeader != null)
_flyoutHeader.MeasureInvalidated -= OnFlyoutHeaderMeasureInvalidated;

_flyoutHeader = null;

if (_footerView != null)
_footerView.View = null;

_headerView?.Disconnect();
DisconnectRecyclerView();

if (_contentView != null)
_contentView.View = null;
}

protected override void Dispose(bool disposing)
{
if (_disposed)
Expand All @@ -601,10 +634,7 @@ protected override void Dispose(bool disposing)

if (disposing)
{
_shellContext.Shell.PropertyChanged -= OnShellPropertyChanged;

if (_flyoutHeader != null)
_flyoutHeader.MeasureInvalidated -= OnFlyoutHeaderMeasureInvalidated;
Disconnect();

if (_appBar != null)
{
Expand All @@ -627,17 +657,13 @@ protected override void Dispose(bool disposing)
_contentView.View = null;

_flyoutContentView?.Dispose();
_headerView.Dispose();
_headerView?.Dispose();

if (_footerView != null)
_footerView.View = null;

_rootView.Dispose();
_rootView?.Dispose();
_defaultBackgroundColor?.Dispose();
_bgImage?.Dispose();

_contentView = null;
_flyoutHeader = null;
_rootView = null;
_headerView = null;
_shellContext = null;
Expand Down Expand Up @@ -715,17 +741,22 @@ protected override void Dispose(bool disposing)
return;

_isdisposed = true;
if (disposing)
{
if (View != null)
View.PropertyChanged -= OnViewPropertyChanged;
}

View = null;
if (disposing)
Disconnect();

base.Dispose(disposing);
}

internal void Disconnect()
{
if (View != null)
{
View.PropertyChanged -= OnViewPropertyChanged;
View = null;
}
}

internal void SetFlyoutHeaderBehavior(FlyoutHeaderBehavior flyoutHeaderBehavior)
{
if (_flyoutHeaderBehavior == flyoutHeaderBehavior)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,20 @@ public override AView OnCreateView(LayoutInflater inflater, ViewGroup container,
// Handler and MauiContext
// But we want to update the inflater and ChildFragmentManager to match
// the handlers new home
if (_page?.Handler?.MauiContext is MauiContext scopedMauiContext)
if (_page.Handler?.MauiContext is MauiContext scopedMauiContext)
{
scopedMauiContext.AddWeakSpecific(ChildFragmentManager);
scopedMauiContext.AddWeakSpecific(inflater);
mauiContext = scopedMauiContext;
// If this page comes to us from a different activity then don't reuse it
// disconnect the handler so it can recreate against new MauiContext
if (scopedMauiContext.GetActivity() == Context.GetActivity())
{
scopedMauiContext.AddWeakSpecific(ChildFragmentManager);
scopedMauiContext.AddWeakSpecific(inflater);
mauiContext = scopedMauiContext;
}
else
{
_page.Handler.DisconnectHandler();
}
}

mauiContext ??= _mauiContext.MakeScoped(layoutInflater: inflater, fragmentManager: ChildFragmentManager);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public void OnItemsCollectionChanged(object sender, NotifyCollectionChangedEvent

public override Fragment CreateFragment(int position)
{

var shellContent = _items[position];
return new ShellFragmentContainer(shellContent, _mauiContext) { Arguments = Bundle.Empty };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected Page DisplayedPage
}
}

protected IShellContext ShellContext { get; }
protected IShellContext ShellContext { get; private set; }

protected ShellItem ShellItem { get; private set; }

Expand All @@ -84,6 +84,13 @@ protected virtual IShellObservableFragment CreateFragmentForPage(Page page)
return ShellContext.CreateFragmentForPage(page);
}

internal void Disconnect()
{
ShellSection = null;
DisplayedPage = null;
ShellContext = null;
}

void Destroy()
{
foreach (var item in _fragmentMap)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ static Color ResolveThemeColor(Color light, Color dark)
IShellFlyoutRenderer _flyoutView;
FrameLayout _frameLayout;
IMauiContext _mauiContext;
bool _disposed;

event EventHandler<PropertyChangedEventArgs> _elementPropertyChanged;

Expand Down Expand Up @@ -345,6 +346,30 @@ void IElementHandler.Invoke(string command, object args)

void IElementHandler.DisconnectHandler()
{
if (_disposed)
return;

_disposed = true;

Element.PropertyChanged -= OnElementPropertyChanged;
Element.SizeChanged -= OnElementSizeChanged;
((IShellController)Element).RemoveAppearanceObserver(this);

if (_flyoutView is ShellFlyoutRenderer sfr)
sfr.Disconnect();
else
(_flyoutView as IDisposable)?.Dispose();

if (_currentView is ShellItemRendererBase sir)
sir.Disconnect();
else
_currentView.Dispose();

_currentView = null;

Element = null;

_disposed = true;
}

class SplitDrawable : Drawable
Expand Down Expand Up @@ -390,4 +415,4 @@ public override void SetColorFilter(ColorFilter colorFilter)
}

}
}
}
9 changes: 9 additions & 0 deletions src/Controls/src/Core/HandlerImpl/Shell.Impl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,14 @@ 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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ override Microsoft.Maui.Controls.TemplatedView.MeasureOverride(double widthConst
*REMOVED*override Microsoft.Maui.Controls.RadioButton.ArrangeOverride(Microsoft.Maui.Graphics.Rect bounds) -> Microsoft.Maui.Graphics.Size
*REMOVED*override Microsoft.Maui.Controls.RadioButton.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
*REMOVED*override Microsoft.Maui.Controls.FlexLayout.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
~override Microsoft.Maui.Controls.Shell.OnHandlerChanging(Microsoft.Maui.Controls.HandlerChangingEventArgs args) -> void
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
using AndroidX.CoordinatorLayout.Widget;
using AndroidX.Core.View;
using static Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutTemplatedContentRenderer;
using Microsoft.Maui.DeviceTests.Stubs;
using Microsoft.Maui.Handlers;

namespace Microsoft.Maui.DeviceTests
{
Expand Down Expand Up @@ -219,6 +221,44 @@ await CreateHandlerAndAddToWindow<ShellRenderer>(shell, async (handler) =>
});
}


[Fact]
public async Task SwappingOutAndroidContextDoesntCrash()
{
SetupBuilder();

var shell = await CreateShellAsync(shell =>
{
shell.Items.Add(new FlyoutItem() { Route = "FlyoutItem1", Items = { new ContentPage() }, Title = "Flyout Item" });
shell.Items.Add(new FlyoutItem() { Route = "FlyoutItem2", Items = { new ContentPage() }, Title = "Flyout Item" });
});

var window = new Controls.Window(shell);
var mauiContextStub1 = new ContextStub(MauiApp.Services);
var activity = mauiContextStub1.GetActivity();
mauiContextStub1.Context = new ContextThemeWrapper(activity, Resource.Style.Maui_MainTheme_NoActionBar);

await CreateHandlerAndAddToWindow<IWindowHandler>(window, async (handler) =>
{
await OnLoadedAsync(shell.CurrentPage);
await OnNavigatedToAsync(shell.CurrentPage);
await Task.Delay(100);
await shell.GoToAsync("//FlyoutItem2");
}, mauiContextStub1);

var mauiContextStub2 = new ContextStub(MauiApp.Services);
mauiContextStub2.Context = new ContextThemeWrapper(activity, Resource.Style.Maui_MainTheme_NoActionBar);

await CreateHandlerAndAddToWindow<IWindowHandler>(window, async (handler) =>
{
await OnLoadedAsync(shell.CurrentPage);
await OnNavigatedToAsync(shell.CurrentPage);
await Task.Delay(100);
await shell.GoToAsync("//FlyoutItem1");
await shell.GoToAsync("//FlyoutItem2");
}, mauiContextStub2);
}

protected AView GetFlyoutPlatformView(ShellRenderer shellRenderer)
{
var drawerLayout = GetDrawerLayout(shellRenderer);
Expand Down
3 changes: 2 additions & 1 deletion src/Controls/tests/DeviceTests/HandlerTestBase.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ public AView GetSemanticPlatformElement(IViewHandler viewHandler)
}

static Drawable _decorDrawable;
Task SetupWindowForTests<THandler>(IWindow window, Func<Task> runTests)
Task SetupWindowForTests<THandler>(IWindow window, Func<Task> runTests, IMauiContext mauiContext = null)
where THandler : class, IElementHandler
{
mauiContext ??= MauiContext;
return InvokeOnMainThreadAsync(async () =>
{
AViewGroup rootView = MauiContext.Context.GetActivity().Window.DecorView as AViewGroup;
Expand Down
Loading

0 comments on commit 2d1e763

Please sign in to comment.