Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[iOS] Fixed crash updating ToolbarItem icon (already disposed) #14749

Merged
merged 3 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="utf-8" ?>
<controls:TestContentPage
xmlns:controls="clr-namespace:Xamarin.Forms.Controls"
xmlns="http://xamarin.com/schemas/2014/forms"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Xamarin.Forms.Controls.Issues.Issue6387"
Title="Issue 6387">
<ContentPage.Content>
<StackLayout>
<Label
Padding="12"
Text="Tap the ToolBarItem with an Icon. Without exceptions, the test has passed." />
</StackLayout>
</ContentPage.Content>
</controls:TestContentPage>
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;
using System.Reflection;

#if UITEST
using Xamarin.UITest;
using NUnit.Framework;
using Xamarin.Forms.Core.UITests;
#endif

namespace Xamarin.Forms.Controls.Issues
{
#if UITEST
[Category(UITestCategories.ToolbarItem)]
#endif
[Preserve(AllMembers = true)]
[Issue(IssueTracker.Github, 6387,
"[Bug] [iOS] Crash When the First Toolbar Item Has an Icon and a Second Item Gets Added",
PlatformAffected.iOS)]
public partial class Issue6387 : TestContentPage
{
#if APP
readonly ToolbarItem _item0;
readonly ToolbarItem _item1;
#endif
public Issue6387()
{
#if APP
InitializeComponent();

_item0 = new ToolbarItem("Item 0", null, ClearAndAddToolbarItems)
{
IconImageSource = ImageSource.FromResource("Xamarin.Forms.Controls.GalleryPages.crimson.jpg", typeof(Issue6387).GetTypeInfo().Assembly)
};
_item1 = new ToolbarItem("Item 1", null, ClearAndAddToolbarItems)
{
// It doesn't matter if this item has an image or not.
};

ClearAndAddToolbarItems();
#endif
}

protected override void Init()
{

}
#if APP
void ClearAndAddToolbarItems()
{
ToolbarItems.Clear();

ToolbarItems.Add(_item0);
ToolbarItems.Add(_item1);
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Issue13577.xaml.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue14505.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue14505-II.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue6387.xaml.cs" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">
Expand Down Expand Up @@ -2279,6 +2280,9 @@
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue13577.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue6387.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla27417Xaml.xaml">
Expand Down Expand Up @@ -2799,7 +2803,7 @@
<ItemGroup>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue13037.xaml">
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue2447.xaml">
<SubType>Designer</SubType>
Expand Down
33 changes: 25 additions & 8 deletions Xamarin.Forms.Platform.iOS/Extensions/ToolbarItemExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.ComponentModel;
using System.Threading.Tasks;
using CoreGraphics;
using UIKit;
using PointF = CoreGraphics.CGPoint;
Expand All @@ -23,6 +24,7 @@ public static UIBarButtonItem ToUIBarButtonItem(this ToolbarItem item, bool forc

sealed class PrimaryToolbarItem : UIBarButtonItem
{
bool _disposed;
readonly bool _forceName;
readonly ToolbarItem _item;

Expand All @@ -32,7 +34,7 @@ public PrimaryToolbarItem(ToolbarItem item, bool forceName)
_item = item;

if (item.IconImageSource != null && !item.IconImageSource.IsEmpty && !forceName)
UpdateIconAndStyle();
Device.BeginInvokeOnMainThread(async () => { await UpdateIconAndStyleAsync(); });
else
UpdateTextAndStyle();
UpdateIsEnabled();
Expand All @@ -50,11 +52,15 @@ public PrimaryToolbarItem(ToolbarItem item, bool forceName)
protected override void Dispose(bool disposing)
{
if (disposing)
{
_item.PropertyChanged -= OnPropertyChanged;
_disposed = true;
}

base.Dispose(disposing);
}

void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
async void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName)
UpdateIsEnabled();
Expand All @@ -68,7 +74,7 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
if (!_forceName)
{
if (_item.IconImageSource != null && !_item.IconImageSource.IsEmpty)
UpdateIconAndStyle();
await UpdateIconAndStyleAsync();
else
UpdateTextAndStyle();
}
Expand All @@ -79,8 +85,11 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
this.SetAccessibilityLabel(_item);
}

async void UpdateIconAndStyle()
async Task UpdateIconAndStyleAsync()
{
if (_disposed)
return;

Image = await _item.IconImageSource.GetNativeImageAsync();
Style = UIBarButtonItemStyle.Plain;
}
Expand All @@ -100,13 +109,14 @@ void UpdateTextAndStyle()

sealed class SecondaryToolbarItem : UIBarButtonItem
{
bool _disposed;
readonly ToolbarItem _item;

public SecondaryToolbarItem(ToolbarItem item) : base(new SecondaryToolbarItemContent())
{
_item = item;
UpdateText();
UpdateIcon();
Device.BeginInvokeOnMainThread(async () => { await UpdateIconAsync(); });
UpdateIsEnabled();

((SecondaryToolbarItemContent)CustomView).TouchUpInside += (sender, e) => ((IMenuItemController)_item).Activate();
Expand All @@ -122,16 +132,20 @@ public SecondaryToolbarItem(ToolbarItem item) : base(new SecondaryToolbarItemCon
protected override void Dispose(bool disposing)
{
if (disposing)
{
_item.PropertyChanged -= OnPropertyChanged;
_disposed = true;
}

base.Dispose(disposing);
}

void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
async void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == MenuItem.TextProperty.PropertyName)
UpdateText();
else if (e.PropertyName == MenuItem.IconImageSourceProperty.PropertyName)
UpdateIcon();
await UpdateIconAsync();
else if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName)
UpdateIsEnabled();
else if (e.PropertyName == AutomationProperties.HelpTextProperty.PropertyName)
Expand All @@ -140,8 +154,11 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
this.SetAccessibilityLabel(_item);
}

async void UpdateIcon()
async Task UpdateIconAsync()
{
if (_disposed)
return;

UIImage image = null;
if (_item.IconImageSource != null && !_item.IconImageSource.IsEmpty)
{
Expand Down