XF 3.4 FlexLayout.OnChildPropertyChanged null ref when changing control height

[Liam2349]

Description

When setting the HeightRequest of a control within a FlexLayout, on the WPF platform, a null reference exception can be thrown from Xamarin.Forms.FlexLayout.OnChildPropertyChanged(Object sender, PropertyChangedEventArgs e):347.

This may also be occurring on macOS.

Steps to Reproduce

This is quite difficult for me to reproduce. It doesn't occur reliably. For about two weeks, I was under the impression that I had resolved the problem, as it had stopped occurring. It was only today that I noticed it was coming from the Xamarin.Forms.Core assembly, so I stepped in and implemented a small fix.

Expected Behavior

Not throwing a null ref.

Actual Behavior

Throws a null ref, due to "item" being null.

Basic Information

• Version with issue: 3.4, looks like the current version of FlexLayout will exhibit the same issue.
• Last known good version: Unknown, potentially none.
• IDE: VS 15.9.4 Community
• Platform Target Frameworks:
  • WPF on Windows 10 1803, with Xamarin.Forms.Platform.WPF 3.4.0.1008975
  • macOS on macOS 10.14

I do not believe that I have encountered this in my Android app, however, it may be occurring in my macOS app. I'm not able to verify this at the moment, but I believe macOS is likely exhibiting the same issue, as it produces the same result in my layout and occurs in a similar area of the code.

StackTrace

at Xamarin.Forms.FlexLayout.OnChildPropertyChanged(Object sender, PropertyChangedEventArgs e) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\FlexLayout.cs:line 347
at Xamarin.Forms.BindableObject.OnPropertyChanged(String propertyName) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\BindableObject.cs:line 150
at Xamarin.Forms.Element.OnPropertyChanged(String propertyName) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\Element.cs:line 370
at Xamarin.Forms.BindableObject.SetValueActual(BindableProperty property, BindablePropertyContext context, Object value, Boolean currentlyApplying, SetValueFlags attributes, Boolean silent) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\BindableObject.cs:line 623
at Xamarin.Forms.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\BindableObject.cs:line 417
at Xamarin.Forms.BindableObject.SetValue(BindableProperty property, Object value, Boolean fromStyle, Boolean checkAccess) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\BindableObject.cs:line 573
at Xamarin.Forms.BindableObject.SetValue(BindableProperty property, Object value) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\BindableObject.cs:line 99
at Xamarin.Forms.VisualElement.set_HeightRequest(Double value) in C:\Repos\Xamarin.Forms-Forked\Xamarin.Forms.Core\VisualElement.cs:line 291
at ClipboardEverywhere.UserControls.FileTransferControl_Compact.SetProgress() in C:\Repos\UC\ClipboardEverywhere\ClipboardEverywhere\UserControls\FileTransferControl_Compact.xaml.cs:line 140
at ClipboardEverywhere.UserControls.FileTransferControl_Compact.Bind(FileTransfer fileTransfer, Boolean inUse) in C:\Repos\UC\ClipboardEverywhere\ClipboardEverywhere\UserControls\FileTransferControl_Compact.xaml.cs:line 70
at ClipboardEverywhere.Pages.Files.AddFileTransfersToLayoutAndGetCombinedHeight(List1 transfersToAdd, FlexLayout flexLayout, List1 cache) in C:\Repos\UC\ClipboardEverywhere\ClipboardEverywhere\Pages\Files.xaml.cs:line 665
at ClipboardEverywhere.Pages.Files.<>c__DisplayClass40_1.<b__4>d.MoveNext() in C:\Repos\UC\ClipboardEverywhere\ClipboardEverywhere\Pages\Files.xaml.cs:line 591

Proposed Solution

Please see the FlexLayout.cs changes from this commit to my fork: Liam2349@50701ae

I have resolved this by adding a null check. It looks like null checks were already in place for 3 of the 4 "if" statements, so I'm unsure why one of them was left without a null check, but that's why this issue is occurring.

Since the "item" variable is used in each of the "if" statements, I also chose to define it at the beginning of the method, along with one null check, to reduce code duplication.

Please let me know what you think, and thanks for reading.

Liam Foot