[Bug] Layout bug in a multi-window app

Description

In the implementation of multi-window (for UWP) in #2432, all layout changes are batched in a static list:

https://github.com/xamarin/Xamarin.Forms/blob/52592fac6158c9f3abb9118c2ceca6c348e21c98/Xamarin.Forms.Core/Layout.cs#L62

This fails with multiple windows because they each have their own dispatcher.

Steps to Reproduce

The Control Gallery already has a multi-window sample, so we will add some extra code to repro this bug:

We need an object that will be shared between the windows. For example:

public class FontSizeProvider : INotifyPropertyChanged
{
    public static FontSizeProvider Current { get; } = new FontSizeProvider();

    private double _value = 16;

    public double Value
    {
        get => _value;
        set
        {
            if (_value == value)
                return;
            _value = value;
            OnPropertyChanged();
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

Update the content page that is created for these secondary windows: https://github.com/xamarin/Xamarin.Forms/blob/52592fac6158c9f3abb9118c2ceca6c348e21c98/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue2482.cs#L88 After this line, add:

layout.Children.Add(new Button
{
    Text = "Click me to increase my font size",
    BindingContext = FontSizeProvider.Current,
    Command = new Command(() => FontSizeProvider.Current.Value += 8)
}.Bind(Label.FontSizeProperty, nameof(FontSizeProvider.Value)));

Run the Control Gallery app and click on the "Open Secondary Window" button twice.
Click on the new button.

Expected Behavior

The font size of this button should increase in all windows.

Actual Behavior

The app crashes.

Basic Information

Version with issue: 4.8.0.1269
Platform Target Frameworks:
- iOS: Haven't checked
- Android: Haven't checked
- UWP: 18362

Solution

My solution is to batch the layouts changes per dispatcher.

Replace s_resolutionList with:

private static readonly IDictionary<IDispatcher, ISet<Layout>> ResolutionList = new Dictionary<IDispatcher, ISet<Layout>>();

Replace all this code with:

// End of OnChildMeasureInvalidated() {
    bool newList;
    ISet<Layout> layouts;
    lock (ResolutionList)
    {
        if (newList = !ResolutionList.TryGetValue(Dispatcher, out layouts))
            ResolutionList.Add(Dispatcher, layouts = new HashSet<Layout>());
    }
    layouts.Add(this);
    if (newList)
        Dispatcher.BeginInvokeOnMainThread(() => ResolveLayoutChanges(Dispatcher));
}

private static void ResolveLayoutChanges(IDispatcher dispatcher)
{
    ISet<Layout> layouts;
    lock (ResolutionList)
    {
        layouts = ResolutionList[dispatcher];
        ResolutionList.Remove(dispatcher);
    }

    foreach (var layout in layouts)
    {
        double width = layout.Width, height = layout.Height;
        if (!layout._allocatedFlag && width >= 0 && height >= 0)
        {
            layout.SizeAllocated(width, height);
        }
    }
}

You can also delete the method GetElementDepth() which doesn't seem to be needed.

Note that I use a HashSet of layouts to avoid scheduling the same layout multiple times. I've noticed that this can happen. Small perf detail: It would be better if the IDispatcher implementations were IEquatable<>. This will ensure that the dictionary only contains one dispatcher per window to minimize the number of batches. But most of the time, this should not matter. Also, see this comment...

By the way, I experimented with this solution after fixing the bugs in #11705 and #11880. So, my local X.F doesn't exactly match with 4.8.0.1269, and this solution assumes that those bugs are fixed.

xamarin / Xamarin.Forms