Skip to content

Commit

Permalink
Fix potential NRE accessing current application via Page.RealParent (#…
Browse files Browse the repository at this point in the history 
…330)

* Fix potential NRE accessing current application via Page.RealParent

* Update Native Bindings Gallery to use MessagingCenter
  • Loading branch information
@hartez @rmarinho
hartez authored and rmarinho committed Sep 27, 2016
1 parent c83c830 commit 53e1d99
Show file tree
Hide file tree
Showing 15 changed files with 127 additions and 121 deletions.
39 changes: 8 additions & 31 deletions Xamarin.Forms.ControlGallery.Android/Activity1.cs
View file
Expand Up @@ -205,17 +205,8 @@ protected override void OnCreate(Bundle bundle)

var app = new App ();

var mdp = app.MainPage as MasterDetailPage;
var detail = mdp?.Detail as NavigationPage;
if (detail != null) {
detail.Pushed += (sender, args) => {
var nncgPage = args.Page as NestedNativeControlGalleryPage;
if (nncgPage != null) {
AddNativeControls (nncgPage);
}
};
}
// 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);

LoadApplication (app);
}
Expand Down Expand Up @@ -327,29 +318,15 @@ protected override void OnCreate (Bundle bundle)
// uncomment to verify turning off title bar works. This is not intended to be dynamic really.
//Forms.SetTitleBarVisibility (AndroidTitleBarVisibility.Never);

var app = new App ();
var app = new App();

var mdp = app.MainPage as MasterDetailPage;
var detail = mdp?.Detail as NavigationPage;
if (detail != null) {
detail.Pushed += (sender, args) => {
var nncgPage = args.Page as NestedNativeControlGalleryPage;
// 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);

if (nncgPage != null) {
AddNativeControls (nncgPage);
}
// 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);

var nncgPage1 = args.Page as NativeBindingGalleryPage;
if (nncgPage1 != null)
{
AddNativeBindings(nncgPage1);
}
};
}

LoadApplication (app);
LoadApplication(app);
}

public override void OnConfigurationChanged (global::Android.Content.Res.Configuration newConfig)
Expand Down
14 changes: 2 additions & 12 deletions Xamarin.Forms.ControlGallery.WP8/MainPage.xaml.cs
View file
Expand Up @@ -61,18 +61,8 @@ public MainPage()

var app = new Controls.App ();

var mdp = app.MainPage as MasterDetailPage;

var detail = mdp?.Detail as NavigationPage;
if (detail != null) {
detail.Pushed += (sender, args) => {
var nncgPage = args.Page as NestedNativeControlGalleryPage;
if (nncgPage != null) {
AddNativeControls (nncgPage);
}
};
}
// 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);

LoadApplication (app);
}
Expand Down
14 changes: 2 additions & 12 deletions Xamarin.Forms.ControlGallery.Windows/MainPage.xaml.cs
View file
Expand Up @@ -17,18 +17,8 @@ public MainPage ()

var app = new Controls.App ();

var mdp = app.MainPage as MasterDetailPage;

var detail = mdp?.Detail as NavigationPage;
if (detail != null) {
detail.Pushed += (sender, args) => {
var nncgPage = args.Page as NestedNativeControlGalleryPage;
if (nncgPage != null) {
AddNativeControls (nncgPage);
}
};
}
// 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);

LoadApplication (app);
}
Expand Down
14 changes: 2 additions & 12 deletions Xamarin.Forms.ControlGallery.WindowsPhone/MainPage.xaml.cs
View file
Expand Up @@ -23,18 +23,8 @@ public MainPage ()

var app = new Controls.App ();

var mdp = app.MainPage as MasterDetailPage;

var detail = mdp?.Detail as NavigationPage;
if (detail != null) {
detail.Pushed += (sender, args) => {
var nncgPage = args.Page as NestedNativeControlGalleryPage;
if (nncgPage != null) {
AddNativeControls (nncgPage);
}
};
}
// 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);

LoadApplication (app);
}
Expand Down
23 changes: 5 additions & 18 deletions Xamarin.Forms.ControlGallery.WindowsUniversal/MainPage.xaml.cs
View file
Expand Up @@ -24,24 +24,11 @@ public MainPage()

var app = new Controls.App ();

var mdp = app.MainPage as MasterDetailPage;

var detail = mdp?.Detail as NavigationPage;
if (detail != null) {
detail.Pushed += (sender, args) => {
var nncgPage = args.Page as NestedNativeControlGalleryPage;
if (nncgPage != null) {
AddNativeControls (nncgPage);
}
var nncgPage1 = args.Page as NativeBindingGalleryPage;
if (nncgPage1 != null) {
AddNativeBindings(nncgPage1);
}
};
}
// 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);

LoadApplication (app);
}
Expand Down
64 changes: 44 additions & 20 deletions Xamarin.Forms.ControlGallery.iOS/AppDelegate.cs
View file
Expand Up @@ -151,27 +151,12 @@ public override bool FinishedLaunching(UIApplication uiApplication, NSDictionary

var app = new App();

var mdp = app.MainPage as MasterDetailPage;
var detail = mdp?.Detail as NavigationPage;
if (detail != null)
{
detail.Pushed += (sender, args) =>
{
var nncgPage = args.Page as NestedNativeControlGalleryPage;
if (nncgPage != null)
{
AddNativeControls(nncgPage);
}
// 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);
MessagingCenter.Subscribe<Bugzilla40911>(this, Bugzilla40911.ReadyToSetUp40911Test, SetUp40911Test);

var nncgPage1 = args.Page as NativeBindingGalleryPage;
if (nncgPage1 != null)
{
AddNativeBindings(nncgPage1);
}
};
}
// 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);

LoadApplication(app);
return base.FinishedLaunching(uiApplication, launchOptions);
Expand Down Expand Up @@ -321,6 +306,45 @@ void AddNativeBindings(NativeBindingGalleryPage page)
sl?.Children.Add(colorPicker);
page.NativeControlsAdded = true;
}

#region Stuff for repro of Bugzilla case 40911

void SetUp40911Test(Bugzilla40911 page)
{
var button = new Button { Text = "Start" };

button.Clicked += (s, e) =>
{
StartPressed40911();
};

page.Layout.Children.Add(button);
}

public void StartPressed40911 ()
{
var loginViewController = new UIViewController { View = { BackgroundColor = UIColor.White } };
var button = UIButton.FromType (UIButtonType.RoundedRect);
button.SetTitle ("Login", UIControlState.Normal);
button.Frame = new CGRect (20, 100, 200, 44);
loginViewController.View.AddSubview (button);

button.TouchUpInside += (sender, e) => {
Xamarin.Forms.Application.Current.MainPage = new ContentPage {Content = new Label {Text = "40911 Success"} };
loginViewController.DismissViewController (true, null);
};

var window= UIApplication.SharedApplication.KeyWindow;
var vc = window.RootViewController;
while (vc.PresentedViewController != null)
{
vc = vc.PresentedViewController;
}

vc.PresentViewController (loginViewController, true, null);
}

#endregion
}

public class ColorConverter : IValueConverter
Expand Down
41 changes: 41 additions & 0 deletions Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla40911.cs
View file
@@ -0,0 +1,41 @@
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;

#if UITEST
using Xamarin.UITest;
using NUnit.Framework;
#endif
namespace Xamarin.Forms.Controls
{
[Preserve (AllMembers = true)]
[Issue (IssueTracker.Bugzilla, 40911, "NRE with Facebook Login", PlatformAffected.iOS)]
public class Bugzilla40911 : TestContentPage
{
public StackLayout Layout { get; private set; }

public const string ReadyToSetUp40911Test = "ReadyToSetUp40911Test";

protected override void Init ()
{
Layout = new StackLayout();

Layout.Children.Add(new Label{Text = "This is an iOS-specific issue. If you're on another platform, you can ignore this." });

Content = Layout;

MessagingCenter.Send(this, ReadyToSetUp40911Test);
}

#if UITEST && __IOS__
[Test]
public void CanFinishLoginWithoutNRE ()
{
RunningApp.WaitForElement("Start");
RunningApp.Tap("Start");
RunningApp.WaitForElement("Login");
RunningApp.Tap("Login");
RunningApp.WaitForElement("40911 Success");
}
#endif
}
}
1 change: 1 addition & 0 deletions ...ssues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
View file
Expand Up @@ -108,6 +108,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla31806.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla40858.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla40824.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla40911.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla40955.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla41078.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Bugzilla40998.cs" />
Expand Down
8 changes: 8 additions & 0 deletions Xamarin.Forms.Controls/ControlGalleryPages/NativeBindingGalleryPage.cs
View file
Expand Up @@ -10,6 +10,14 @@ public class NativeBindingGalleryPage : ContentPage

NestedNativeViewModel ViewModel { get; set; }

public const string ReadyForNativeBindingsMessage = "ReadyForNativeBindings";

protected override void OnAppearing()
{
base.OnAppearing();
MessagingCenter.Send(this, ReadyForNativeBindingsMessage);
}

public NativeBindingGalleryPage()
{

Expand Down
8 changes: 8 additions & 0 deletions Xamarin.Forms.Controls/ControlGalleryPages/NestedNativeControlGalleryPage.cs
View file
Expand Up @@ -6,6 +6,14 @@ public partial class NestedNativeControlGalleryPage : ContentPage

public bool NativeControlsAdded { get; set; }

public const string ReadyForNativeControlsMessage = "ReadyForNativeControls";

protected override void OnAppearing()
{
base.OnAppearing();
MessagingCenter.Send(this, ReadyForNativeControlsMessage);
}

public NestedNativeControlGalleryPage ()
{
Layout = new StackLayout { Padding = 20, VerticalOptions = LayoutOptions.FillAndExpand };
Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Platform.Android/AppCompat/Platform.cs
View file
Expand Up @@ -238,7 +238,7 @@ internal void SetPage(Page newRoot)
Page.Platform = this;
AddChild(Page, layout);

((Application)Page.RealParent).NavigationProxy.Inner = this;
Application.Current.NavigationProxy.Inner = this;
}

void AddChild(Page page, bool layout = false)
Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Platform.Android/Platform.cs
View file
Expand Up @@ -413,7 +413,7 @@ internal void SetPage(Page newRoot)
Page.Platform = this;
AddChild(Page, layout);

((Application)Page.RealParent).NavigationProxy.Inner = this;
Application.Current.NavigationProxy.Inner = this;

_toolbarTracker.Target = newRoot;

Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Platform.WP8/Platform.cs
View file
Expand Up @@ -437,7 +437,7 @@ internal void SetPage(Page newRoot)
_navModel.PushModal(newRoot);
SetCurrent(newRoot, false, true);

((Application)newRoot.RealParent).NavigationProxy.Inner = this;
Application.Current.NavigationProxy.Inner = this;
}

internal event EventHandler SizeChanged;
Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Platform.WinRT/Platform.cs
View file
Expand Up @@ -100,7 +100,7 @@ internal void SetPage(Page newRoot)
_navModel.Push(newRoot, null);
newRoot.NavigationProxy.Inner = this;
SetCurrent(newRoot, false, true);
((Application)newRoot.RealParent).NavigationProxy.Inner = this;
Application.Current.NavigationProxy.Inner = this;
}

public IReadOnlyList<Page> NavigationStack
Expand Down
14 changes: 2 additions & 12 deletions Xamarin.Forms.Platform.iOS/Platform.cs
View file
Expand Up @@ -149,16 +149,6 @@ internal UIViewController ViewController

Page Page { get; set; }

Application TargetApplication
{
get
{
if (Page == null)
return null;
return Page.RealParent as Application;
}
}

void IDisposable.Dispose()
{
if (_disposed)
Expand Down Expand Up @@ -304,7 +294,7 @@ protected override void OnBindingContextChanged()
internal void DidAppear()
{
_animateModals = false;
TargetApplication.NavigationProxy.Inner = this;
Application.Current.NavigationProxy.Inner = this;
_animateModals = true;
}

Expand Down Expand Up @@ -388,7 +378,7 @@ internal void SetPage(Page newRoot)

Page.DescendantRemoved += HandleChildRemoved;

TargetApplication.NavigationProxy.Inner = this;
Application.Current.NavigationProxy.Inner = this;
}

internal void WillAppear()
Expand Down

0 comments on commit 53e1d99

Please sign in to comment.