Fix stack overflow with two way x:Bind to NumberBox.Value (#1905)

dev/Generated/NumberBox.properties.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
 // DO NOT EDIT! This file was generated by CustomTasks.DependencyPropertyCodeGen

dev/NumberBox/InteractionTests/NumberBoxTests.cs

@@ -426,6 +426,12 @@ public void ValueChangedTest()
                 Verify.AreEqual("NaN", newValueTextBlock.GetText());
                 Verify.AreEqual("100", oldValueTextBlock.GetText());
 
+                Log.Comment("Verify that setting two way bound value to NaN doesn't cause a crash");
+                Button twoWayBindNanbutton = FindElement.ByName<Button>("SetTwoWayBoundValueNaNButton");
+                twoWayBindNanbutton.InvokeAndWait();
+                TextBlock twoWayBoundNumberBoxValue = FindElement.ByName<TextBlock>("TwoWayBoundNumberBoxValue");
+                Verify.AreEqual("NaN", twoWayBoundNumberBoxValue.GetText());
+
                 Log.Comment("Verify that setting value to NaN doesn't have any effect");
                 Button nanbutton = FindElement.ByName<Button>("SetValueNaNButton");
                 nanbutton.InvokeAndWait();

dev/NumberBox/NumberBox.cpp

@@ -49,6 +49,24 @@ NumberBox::NumberBox()
     SetDefaultStyleKey(this);
 }
 
+void NumberBox::Value(double value)
+{
+    // When using two way bindings to Value using x:Bind, we could end up with a stack overflow because
+    // nan != nan. However in this case, we are using nan as a value to represent value not set (cleared)
+    // and that can happen quite often. We can avoid the stack overflow by breaking the cycle here. This is possible
+    // for x:Bind since the generated code goes through this property setter. This is not the case for Binding
+    // unfortunately. x:Bind is recommended over Binding anyway due to its perf and debuggability benefits.
+    if (!std::isnan(value) || !std::isnan(Value()))
+    {
+        static_cast<NumberBox*>(this)->SetValue(s_ValueProperty, ValueHelper<double>::BoxValueIfNecessary(value));
+    }
+}
+
+double NumberBox::Value()
+{
+    return ValueHelper<double>::CastOrUnbox(static_cast<NumberBox*>(this)->GetValue(s_ValueProperty));
+}
+
 winrt::AutomationPeer NumberBox::OnCreateAutomationPeer()
 {
     return winrt::make<NumberBoxAutomationPeer>(*this);

dev/NumberBox/NumberBox.h

@@ -33,6 +33,9 @@ class NumberBox :
 
     NumberBox();
 
+    void Value(double value);
+    double Value();
+
     // IUIElement
     virtual winrt::AutomationPeer OnCreateAutomationPeer();
 

dev/NumberBox/TestUI/NumberBoxPage.xaml

@@ -57,8 +57,10 @@
             <Button x:Name="SetTextButton" AutomationProperties.Name="SetTextButton" Content="Set text to 15" Click="SetTextButton_Click" Margin="0,4,0,0"/>
 
             <Button x:Name="SetValueButton" AutomationProperties.Name="SetValueButton" Content="Set value to 42" Click="SetValueButton_Click" Margin="0,4,0,0"/>
-            
+
             <Button x:Name="SetValueNaNButton" AutomationProperties.Name="SetValueNaNButton" Content="Set value to NaN" Click="SetNaNButton_Click" Margin="0,4,0,0"/>
+
+            <Button x:Name="SetTwoWayBoundValueNaNButton" AutomationProperties.Name="SetTwoWayBoundValueNaNButton" Content="Set two way bound value to NaN" Click="SetTwoWayBoundNaNButton_Click" Margin="0,4,0,0"/>
         </StackPanel>
 
         <Grid Grid.Column="1">
@@ -113,6 +115,13 @@
                     <TextBlock x:Name="VerticalOffsetDisplayBlock" Text="0"
                             AutomationProperties.Name="VerticalOffsetDisplayBlock"/>
                 </StackPanel>
+
+                <!-- NumberBox two way binding with x:Bind -->
+                <StackPanel>
+                    <controls:NumberBox Header="TwoWayBinding" x:Name="TwoWayBoundNumberBox"
+                              Value="{x:Bind DataModelWithINPC.Value, Mode=TwoWay}" />
+                    <TextBlock x:Name="TwoWayBoundNumberBoxValue" AutomationProperties.Name="TwoWayBoundNumberBoxValue" />
+                </StackPanel>
             </StackPanel>
         </Grid>
 

dev/NumberBox/TestUI/NumberBoxPage.xaml.cs

@@ -1,18 +1,21 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
+using Microsoft.UI.Xaml.Controls;
 using System;
+using System.Collections.Generic;
+using System.ComponentModel;
+using Windows.Globalization.NumberFormatting;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Controls;
-using Microsoft.UI.Xaml.Controls;
-using Windows.Globalization.NumberFormatting;
-using System.Collections.Generic;
 
 namespace MUXControlsTestApp
 {
     [TopLevelTestPage(Name = "NumberBox")]
     public sealed partial class NumberBoxPage : TestPage
     {
+        public DataModelWithINPC DataModelWithINPC { get; set; } = new DataModelWithINPC();
+
         public NumberBoxPage()
         {
             this.InitializeComponent();
@@ -115,6 +118,12 @@ private void SetNaNButton_Click(object sender, RoutedEventArgs e)
             TestNumberBox.Value = Double.NaN;
         }
 
+        private void SetTwoWayBoundNaNButton_Click(object sender, RoutedEventArgs e)
+        {
+            DataModelWithINPC.Value = Double.NaN;
+            TwoWayBoundNumberBoxValue.Text = TwoWayBoundNumberBox.Value.ToString();
+        }
+
         private void TextPropertyChanged(DependencyObject o, DependencyProperty p)
         {
             TextTextBox.Text = TestNumberBox.Text;
@@ -125,4 +134,29 @@ private void ScrollviewerWithScroll_ViewChanged(object sender, ScrollViewerViewC
             VerticalOffsetDisplayBlock.Text = (sender as Windows.UI.Xaml.Controls.ScrollViewer).VerticalOffset.ToString();
         }
     }
+
+    public class DataModelWithINPC : INotifyPropertyChanged
+    {
+        private double _value;
+
+        public event PropertyChangedEventHandler PropertyChanged;
+
+        protected virtual void OnPropertyChanged(string propertyName)
+        {
+            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+        }
+
+        public double Value
+        {
+            get => _value;
+            set
+            {
+                if (value != _value)
+                {
+                    _value = value;
+                    OnPropertyChanged(nameof(this.Value));
+                }
+            }
+        }
+    }
 }