[Android] Don't crash if Control is accessed in Effect OnDetached when Page is Dispos...
authorSamantha Houts <samantha@teamredwall.com>
Fri, 3 Mar 2017 12:41:04 +0000 (04:41 -0800)
committerRui Marinho <me@ruimarinho.net>
Fri, 3 Mar 2017 12:41:04 +0000 (12:41 +0000)
* Add repro for 51505

* [Android] Don't dispose of EffectControlProvider

* Oops

Xamarin.Forms.ControlGallery.Android/Activity1.cs
Xamarin.Forms.ControlGallery.WindowsUniversal/MainPage.xaml.cs
Xamarin.Forms.ControlGallery.iOS/AppDelegate.cs
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51505.cs [new file with mode: 0644]
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
Xamarin.Forms.Platform.Android/VisualElementRenderer.cs

index cfd8350..6f2b22d 100644 (file)
@@ -43,6 +43,7 @@ namespace Xamarin.Forms.ControlGallery.Android
 
                protected override void OnDetached ()
                {
+                       Control.SetBackgroundColor(global::Android.Graphics.Color.Beige);
                }
 
                protected override void OnElementPropertyChanged (PropertyChangedEventArgs args)
index 744e4d1..5f4ea08 100644 (file)
@@ -8,34 +8,59 @@ using Windows.UI.ViewManagement;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Controls;
 using Windows.UI.Xaml.Media;
+using Xamarin.Forms;
+using Xamarin.Forms.ControlGallery.WindowsUniversal;
 using Xamarin.Forms.Controls;
 using Xamarin.Forms.Platform.UWP;
 
+[assembly: ExportEffect(typeof(BorderEffect), "BorderEffect")]
 namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 {
+       public class BorderEffect : PlatformEffect
+       {
+               protected override void OnAttached()
+               {
+                       var control = Control as Control;
+                       if (control == null)
+                               return;
+
+                       control.Background = new SolidColorBrush(Windows.UI.Colors.Aqua);
+               }
+
+               protected override void OnDetached()
+               {
+                       var control = Control as Control;
+                       if (control == null)
+                               return;
+
+                       control.Background = new SolidColorBrush(Windows.UI.Colors.Beige);
+               }
+       }
+
        /// <summary>
        /// An empty page that can be used on its own or navigated to within a Frame.
        /// </summary>
        public sealed partial class MainPage
-    {
-        public MainPage()
-        {
-            InitializeComponent();
+       {
+               public MainPage()
+               {
+                       InitializeComponent();
 
-                       var app = new Controls.App ();
+                       var app = new Controls.App();
 
                        // When the native control gallery loads up, it'll let us know so we can add the nested native controls
                        MessagingCenter.Subscribe<NestedNativeControlGalleryPage>(this, NestedNativeControlGalleryPage.ReadyForNativeControlsMessage, AddNativeControls);
-                                       
+
                        // When the native binding gallery loads up, it'll let us know so we can set up the native bindings
-                       MessagingCenter.Subscribe<NativeBindingGalleryPage >(this, NativeBindingGalleryPage.ReadyForNativeBindingsMessage, AddNativeBindings);
+                       MessagingCenter.Subscribe<NativeBindingGalleryPage>(this, NativeBindingGalleryPage.ReadyForNativeBindingsMessage, AddNativeBindings);
 
-                       LoadApplication (app);
-        }
+                       LoadApplication(app);
+               }
 
-               void AddNativeControls (NestedNativeControlGalleryPage page)
+               void AddNativeControls(NestedNativeControlGalleryPage page)
                {
-                       if (page.NativeControlsAdded) {
+                       if (page.NativeControlsAdded)
+                       {
                                return;
                        }
 
@@ -43,54 +68,59 @@ namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 
                        // Create and add a native TextBlock
                        var originalText = "I am a native TextBlock";
-                       var textBlock = new TextBlock {
+                       var textBlock = new TextBlock
+                       {
                                Text = originalText,
                                FontSize = 14,
-                               FontFamily = new FontFamily ("HelveticaNeue")
+                               FontFamily = new FontFamily("HelveticaNeue")
                        };
 
-                       sl?.Children.Add (textBlock);
+                       sl?.Children.Add(textBlock);
 
                        // Create and add a native Button 
                        var button = new Windows.UI.Xaml.Controls.Button { Content = "Toggle Font Size", Height = 80 };
                        button.Click += (sender, args) => { textBlock.FontSize = textBlock.FontSize == 14 ? 24 : 14; };
 
-                       sl?.Children.Add (button.ToView ());
+                       sl?.Children.Add(button.ToView());
 
                        // Create a control which we know doesn't behave correctly with regard to measurement
-                       var difficultControl = new BrokenNativeControl {
+                       var difficultControl = new BrokenNativeControl
+                       {
                                Text = "Not Sized/Arranged Properly"
                        };
 
-                       var difficultControl2 = new BrokenNativeControl {
+                       var difficultControl2 = new BrokenNativeControl
+                       {
                                Text = "Fixed"
                        };
 
                        // Add the misbehaving controls, one with a custom delegate for ArrangeOverrideDelegate
-                       sl?.Children.Add (difficultControl);
-                       sl?.Children.Add (difficultControl2,
-                               arrangeOverrideDelegate: (renderer, finalSize) => {
-                                       if (finalSize.Width <= 0 || double.IsInfinity (finalSize.Width)) {
+                       sl?.Children.Add(difficultControl);
+                       sl?.Children.Add(difficultControl2,
+                               arrangeOverrideDelegate: (renderer, finalSize) =>
+                               {
+                                       if (finalSize.Width <= 0 || double.IsInfinity(finalSize.Width))
+                                       {
                                                return null;
                                        }
 
                                        FrameworkElement frameworkElement = renderer.Control;
 
-                                       frameworkElement.Measure (finalSize);
+                                       frameworkElement.Measure(finalSize);
 
                                        // The broken control always tries to size itself to the screen width
                                        // So figure that out and we'll know how far off it's laying itself out
-                                       Rect bounds = ApplicationView.GetForCurrentView ().VisibleBounds;
-                                       double scaleFactor = DisplayInformation.GetForCurrentView ().RawPixelsPerViewPixel;
-                                       var screenWidth = new Size (bounds.Width * scaleFactor, bounds.Height * scaleFactor);
-                                       
+                                       Rect bounds = ApplicationView.GetForCurrentView().VisibleBounds;
+                                       double scaleFactor = DisplayInformation.GetForCurrentView().RawPixelsPerViewPixel;
+                                       var screenWidth = new Size(bounds.Width * scaleFactor, bounds.Height * scaleFactor);
+
                                        // We can re-center it by offsetting it during the Arrange call
                                        double diff = Math.Abs(screenWidth.Width - finalSize.Width) / -2;
-                                       frameworkElement.Arrange (new Rect (diff, 0, finalSize.Width - diff, finalSize.Height));
+                                       frameworkElement.Arrange(new Rect(diff, 0, finalSize.Width - diff, finalSize.Height));
 
                                        // Arranging the control to the left will make it show up past the edge of the stack layout
                                        // We can fix that by clipping it manually
-                                       var clip = new RectangleGeometry { Rect = new Rect (-diff, 0, finalSize.Width, finalSize.Height) };
+                                       var clip = new RectangleGeometry { Rect = new Rect(-diff, 0, finalSize.Width, finalSize.Height) };
                                        frameworkElement.Clip = clip;
 
                                        return finalSize;
@@ -107,12 +137,14 @@ namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 
                        StackLayout sl = page.Layout;
 
-                       var txbLabel = new TextBlock {
+                       var txbLabel = new TextBlock
+                       {
                                FontSize = 14,
                                FontFamily = new FontFamily("HelveticaNeue")
                        };
 
-                       var txbBox = new TextBox {
+                       var txbBox = new TextBox
+                       {
                                FontSize = 14,
                                FontFamily = new FontFamily("HelveticaNeue")
                        };
index 4ea9b30..d310773 100644 (file)
@@ -17,8 +17,22 @@ using Xamarin.Forms.Platform.iOS;
 [assembly: Dependency(typeof(CacheService))]
 [assembly: ExportRenderer(typeof(DisposePage), typeof(DisposePageRenderer))]
 [assembly: ExportRenderer(typeof(DisposeLabel), typeof(DisposeLabelRenderer))]
+[assembly: ExportEffect(typeof(BorderEffect), "BorderEffect")]
 namespace Xamarin.Forms.ControlGallery.iOS
 {
+       public class BorderEffect : PlatformEffect
+       {
+               protected override void OnAttached()
+               {
+                       Control.BackgroundColor = UIColor.Blue;
+               }
+
+               protected override void OnDetached()
+               {
+                       Control.BackgroundColor = UIColor.Brown;
+               }
+       }
+
        public class CacheService : ICacheService
        {
                public void ClearImageCache()
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51505.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51505.cs
new file mode 100644 (file)
index 0000000..2851009
--- /dev/null
@@ -0,0 +1,56 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+// Apply the default category of "Issues" to all of the tests in this assembly
+// We use this as a catch-all for tests which haven't been individually categorized
+#if UITEST
+[assembly: NUnit.Framework.Category("Issues")]
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+       [Preserve(AllMembers = true)]
+       [Issue(IssueTracker.Bugzilla, 51505, "ObjectDisposedException On Effect detachment.", PlatformAffected.Android)]
+       public class Bugzilla51505 : TestContentPage
+       {
+               const string ButtonId = "button";
+
+               protected override void Init()
+               {
+                       var effect = Effect.Resolve("XamControl.BorderEffect");
+
+                       var button = new Button { Text = "Click me", AutomationId = ButtonId };
+                       button.Clicked += async (sender, e) =>
+                       {
+                               await Navigation.PopAsync();
+                       };
+                       button.Effects.Add(effect);
+
+                       Content = new StackLayout
+                       {
+                               Children = 
+                               {
+                                       new Label 
+                                       {
+                                               Text = "The following Button has an Effect applied to it that should attempt to access the Control when it is Detached. When you click the Button, this page should be popped. If the app crashes, this test has failed."
+                                       },
+                                       button
+                               }
+                       };
+               }
+
+#if UITEST
+               [Test]
+               public void Bugzilla51505Test()
+               {
+                       RunningApp.WaitForElement(q => q.Marked(ButtonId));
+                       Assert.DoesNotThrow(() => RunningApp.Tap(q => q.Marked(ButtonId)), "Accessing the Control when an Effect is detached should not throw");
+               }
+#endif
+       }
+}
\ No newline at end of file
index 0deca6b..72c310f 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla37431.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla44777.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51503.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51505.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla52533.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)_Template.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1028.cs" />
index 46bab02..68f5d7d 100644 (file)
@@ -280,8 +280,6 @@ namespace Xamarin.Forms.Platform.Android
                                        if (Platform.GetRenderer(Element) == this)
                                                Platform.SetRenderer(Element, null);
 
-                                       (Element as IElementController).EffectControlProvider = null;
-
                                        Element = null;
                                }
                        }