Skip to content

Commit

Permalink
[shell] [windows] fix NRE when clearing shell items (#15220)
Browse files Browse the repository at this point in the history
* [shell] [windows] fix NRE when clearing shell items

Fixes: #13775

You can cause MAUI to throw an NRE by doing:

1. Debug/run, have XAML hot reload enabled

2. Comment out the `ShellContent` in `AppShell.xaml`, hot reload

3. Uncomment the `ShellContent` in `AppShell.xaml`, hot reload

Crashes with:

    System.NullReferenceException: Object reference not set to an instance of an object.
    at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.UpdateCurrentItem()
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.MapCurrentItem(ShellItemHandler handler, ShellItem item)
    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.PropertyMapper.UpdateProperties(IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.Handlers.ElementHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Platform.ShellView.CreateShellItemView()
    at Microsoft.Maui.Controls.Platform.ShellView.SwitchShellItem(ShellItem newItem, Boolean animate)
    at Microsoft.Maui.Controls.Handlers.ShellHandler.MapCurrentItem(ShellHandler handler, Shell view)
    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.PropertyMapper.UpdateProperty(IElementHandler viewHandler, IElement virtualView, String property)
    at Microsoft.Maui.Handlers.ElementHandler.UpdateValue(String property)
    at Microsoft.Maui.Controls.Element.OnPropertyChanged(String propertyName)
    at Microsoft.Maui.Controls.Shell.OnPropertyChanged(String propertyName)
    at Microsoft.Maui.Controls.BindableObject.SetValueActual(BindableProperty property, BindablePropertyContext context, Object value, Boolean currentlyApplying, SetValueFlags attributes, Boolean silent)
    at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes)
    at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes)
    at Microsoft.Maui.Controls.Element.SetValueFromRenderer(BindableProperty property, Object value)
    at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
    at Microsoft.Maui.Controls.Shell.<Initialize>g__SetCurrentItem|172_1()
    at Microsoft.Maui.Controls.Shell.<Initialize>b__172_0(Object s, NotifyCollectionChangedEventArgs e)
    at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
    at Microsoft.UI.Dispatching.DispatcherQueueSynchronizationContext.<>c__DisplayClass2_0.<Post>b__0()

I could reproduce this in a test by simply doing:

    shell.CurrentItem = new ShellContent { Content = page };
    shell.Items.Clear();
    shell.CurrentItem = new ShellContent { Content = page };

I added appropriate null checking and the problem appears to be solved
now.

* Update test, call RemoveAppearanceObserver more reliably
  • Loading branch information
jonathanpeppers authored and rmarinho committed May 30, 2023
1 parent 8e25916 commit ebe3496
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public partial class ShellSectionHandler : ElementHandler<ShellSection, WFrame>,
};

StackNavigationManager? _navigationManager;
WeakReference? _lastShell;

public ShellSectionHandler() : base(Mapper, CommandMapper)
{
Expand All @@ -45,7 +46,12 @@ public override void SetVirtualView(Maui.IElement view)
if (_shellSection != null)
{
((IShellSectionController)_shellSection).NavigationRequested -= OnNavigationRequested;
((IShellController)_shellSection.FindParentOfType<Shell>()!).RemoveAppearanceObserver(this);

if (_lastShell?.Target is IShellController shell)
{
shell.RemoveAppearanceObserver(this);
}
_lastShell = null;
}

// If we've already connected to the navigation manager
Expand All @@ -68,7 +74,13 @@ public override void SetVirtualView(Maui.IElement view)
if (_shellSection != null)
{
((IShellSectionController)_shellSection).NavigationRequested += OnNavigationRequested;
((IShellController)_shellSection.FindParentOfType<Shell>()!).AddAppearanceObserver(this, _shellSection);

var shell = _shellSection.FindParentOfType<Shell>() as IShellController;
if (shell != null)
{
_lastShell = new WeakReference(shell);
shell.AddAppearanceObserver(this, _shellSection);
}
}
}

Expand Down
27 changes: 26 additions & 1 deletion src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Maui;
Expand Down Expand Up @@ -1026,6 +1027,30 @@ await CreateHandlerAndAddToWindow<IWindowHandler>(shell, async (handler) =>
});
}

[Fact(DisplayName = "Can Clear ShellContent")]
public async Task CanClearShellContent()
{
SetupBuilder();
var page = new ContentPage();
var shell = await CreateShellAsync(shell =>
{
shell.CurrentItem = new ShellContent { Content = page };
});

var appearanceObserversField = shell.GetType().GetField("_appearanceObservers", BindingFlags.Instance | BindingFlags.NonPublic);
Assert.NotNull(appearanceObserversField);
var appearanceObservers = appearanceObserversField.GetValue(shell) as IList;
Assert.NotNull(appearanceObservers);

await CreateHandlerAndAddToWindow<IWindowHandler>(shell, _ =>
{
int count = appearanceObservers.Count;
shell.Items.Clear();
shell.CurrentItem = new ShellContent { Content = page };
Assert.Equal(count, appearanceObservers.Count); // Count doesn't increase
});
}

protected Task<Shell> CreateShellAsync(Action<Shell> action) =>
InvokeOnMainThreadAsync(() =>
{
Expand Down

0 comments on commit ebe3496

Please sign in to comment.