From ec1d05b2ba38b7b74c67a57c0ef71fe3a23c49f8 Mon Sep 17 00:00:00 2001 From: nmilcoff Date: Fri, 30 Mar 2018 16:50:49 -0300 Subject: [PATCH 1/6] Make MvxSetupSingleton don't block initialization when starting from a SplashScreen --- MvvmCross/Core/MvxSetupSingleton.cs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/MvvmCross/Core/MvxSetupSingleton.cs b/MvvmCross/Core/MvxSetupSingleton.cs index 64b1aa7d0b..9be3c1d0b7 100644 --- a/MvvmCross/Core/MvxSetupSingleton.cs +++ b/MvvmCross/Core/MvxSetupSingleton.cs @@ -63,18 +63,21 @@ public virtual void EnsureInitialized() { if (_initialized) return; - - if (IsInitialisedTaskCompletionSource == null) + if (IsInitialisedTaskCompletionSource != null) { - IsInitialisedTaskCompletionSource = StartSetupInitialization(); + + // Only wait for the result here if nobody's monitoring the state + if (_currentMonitor == null) + { + IsInitialisedTaskCompletionSource.Task.GetAwaiter().GetResult(); + MvxLog.Instance.Trace("EnsureInitialized has already been called so now waiting for completion"); + } } else { - MvxLog.Instance.Trace("EnsureInitialized has already been called so now waiting for completion"); + StartSetupInitialization(); } } - - IsInitialisedTaskCompletionSource.Task.GetAwaiter().GetResult(); } public virtual void InitializeAndMonitor(IMvxSetupMonitor setupMonitor) @@ -92,8 +95,8 @@ public virtual void InitializeAndMonitor(IMvxSetupMonitor setupMonitor) { return; } - - IsInitialisedTaskCompletionSource = StartSetupInitialization(); + + StartSetupInitialization(); } } @@ -136,17 +139,16 @@ protected override void Dispose(bool isDisposing) base.Dispose(isDisposing); } - private TaskCompletionSource StartSetupInitialization() + private void StartSetupInitialization() { - var completionSource = new TaskCompletionSource(); + IsInitialisedTaskCompletionSource = new TaskCompletionSource(); _setup.InitializePrimary(); Task.Run(() => { _setup.InitializeSecondary(); lock (LockObject) { - completionSource.SetResult(true); - _initialized = true; + IsInitialisedTaskCompletionSource.SetResult(true); var dispatcher = Mvx.GetSingleton(); dispatcher.RequestMainThreadAction(() => { @@ -157,8 +159,6 @@ private TaskCompletionSource StartSetupInitialization() }); } }); - - return completionSource; } } } From 21c6d5d1e37ec9ebc2a56bb3bad9ea863b24167d Mon Sep 17 00:00:00 2001 From: nmilcoff Date: Fri, 30 Mar 2018 16:51:35 -0300 Subject: [PATCH 2/6] Add some optimizations to MvxSetupSingleton: Remove bool member and cleanup logic --- MvvmCross/Core/MvxSetupSingleton.cs | 40 ++++++++++++----------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/MvvmCross/Core/MvxSetupSingleton.cs b/MvvmCross/Core/MvxSetupSingleton.cs index 9be3c1d0b7..a425348105 100644 --- a/MvvmCross/Core/MvxSetupSingleton.cs +++ b/MvvmCross/Core/MvxSetupSingleton.cs @@ -16,18 +16,12 @@ public abstract class MvxSetupSingleton private static readonly object LockObject = new object(); private static TaskCompletionSource IsInitialisedTaskCompletionSource; private IMvxSetup _setup; - private bool _initialized; private IMvxSetupMonitor _currentMonitor; - protected virtual IMvxSetup Setup - { - get - { - return _setup; - } - } + protected virtual IMvxSetup Setup => _setup; - public virtual TMvxSetup PlatformSetup() where TMvxSetup : IMvxSetup + public virtual TMvxSetup PlatformSetup() + where TMvxSetup : IMvxSetup { try { @@ -61,10 +55,10 @@ public virtual void EnsureInitialized() { lock (LockObject) { - if (_initialized) - return; if (IsInitialisedTaskCompletionSource != null) { + if (IsInitialisedTaskCompletionSource.Task.IsCompleted) + return; // Only wait for the result here if nobody's monitoring the state if (_currentMonitor == null) @@ -85,14 +79,17 @@ public virtual void InitializeAndMonitor(IMvxSetupMonitor setupMonitor) lock (LockObject) { _currentMonitor = setupMonitor; - if (_initialized) - { - _currentMonitor?.InitializationComplete(); - return; - } - + + // if the tcs is not null, it means the initialization is running if (IsInitialisedTaskCompletionSource != null) { + // If the task is already completed at this point, let the monitor know it has finished. + // but don't do it otherwise because it's done elsewhere + if(IsInitialisedTaskCompletionSource.Task.IsCompleted) + { + _currentMonitor?.InitializationComplete(); + } + return; } @@ -104,17 +101,14 @@ public virtual void CancelMonitor(IMvxSetupMonitor setupMonitor) { lock (LockObject) { - if (setupMonitor == _currentMonitor) + if (setupMonitor != _currentMonitor) { - _currentMonitor = null; + throw new MvxException("The specified IMvxSetupMonitor is not the one registered in MvxSetupSingleton"); } + _currentMonitor = null; } } - protected MvxSetupSingleton() - { - } - protected virtual void CreateSetup() { try From 31e1087a7085cd668d5d668ead2b1d0ca633f433 Mon Sep 17 00:00:00 2001 From: nmilcoff Date: Fri, 30 Mar 2018 16:51:59 -0300 Subject: [PATCH 3/6] Android SplashScreens: Do nothing on RunAppStart to avoid multiple AppStart runs --- .../V7.AppCompat/MvxSplashScreenAppCompatActivity.cs | 6 +++++- .../Platforms/Android/Views/MvxSplashScreenActivity.cs | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/MvvmCross.Android.Support/V7.AppCompat/MvxSplashScreenAppCompatActivity.cs b/MvvmCross.Android.Support/V7.AppCompat/MvxSplashScreenAppCompatActivity.cs index c6d6ac44ec..51be9638c5 100644 --- a/MvvmCross.Android.Support/V7.AppCompat/MvxSplashScreenAppCompatActivity.cs +++ b/MvvmCross.Android.Support/V7.AppCompat/MvxSplashScreenAppCompatActivity.cs @@ -7,7 +7,6 @@ using Android.Views; using MvvmCross.Core; using MvvmCross.Platforms.Android.Core; -using MvvmCross.Platforms.Android.Views; using MvvmCross.ViewModels; namespace MvvmCross.Droid.Support.V7.AppCompat @@ -86,5 +85,10 @@ protected virtual void TriggerFirstNavigate() if (!startup.IsStarted) startup.Start(); } + + // do nothing on this override, as initial navigation is managed by TriggerFirstNavigate + protected override void RunAppStart(Bundle bundle) + { + } } } diff --git a/MvvmCross/Platforms/Android/Views/MvxSplashScreenActivity.cs b/MvvmCross/Platforms/Android/Views/MvxSplashScreenActivity.cs index 95ed4a891e..65c1f18319 100644 --- a/MvvmCross/Platforms/Android/Views/MvxSplashScreenActivity.cs +++ b/MvvmCross/Platforms/Android/Views/MvxSplashScreenActivity.cs @@ -85,5 +85,10 @@ protected virtual void TriggerFirstNavigate() if (!startup.IsStarted) startup.Start(); } + + // do nothing on this override, as initial navigation is managed by TriggerFirstNavigate + protected override void RunAppStart(Bundle bundle) + { + } } } From 8c20f090bdb0e1bd22da135cedaa7a2a92e1a4b8 Mon Sep 17 00:00:00 2001 From: nmilcoff Date: Fri, 30 Mar 2018 16:52:20 -0300 Subject: [PATCH 4/6] iOS / tvOS AppDelegates: Set windows to virtual --- MvvmCross/Platforms/Ios/Core/MvxApplicationDelegate.cs | 2 +- MvvmCross/Platforms/Tvos/Core/MvxApplicationDelegate.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MvvmCross/Platforms/Ios/Core/MvxApplicationDelegate.cs b/MvvmCross/Platforms/Ios/Core/MvxApplicationDelegate.cs index e5c3d52ce1..bad49534f8 100644 --- a/MvvmCross/Platforms/Ios/Core/MvxApplicationDelegate.cs +++ b/MvvmCross/Platforms/Ios/Core/MvxApplicationDelegate.cs @@ -15,7 +15,7 @@ public abstract class MvxApplicationDelegate : UIApplicationDelegate, IMvxApplic /// /// UIApplicationDelegate.Window doesn't really exist / work. It was added by Xamarin.iOS templates /// - public new UIWindow Window { get; set; } + public new virtual UIWindow Window { get; set; } public MvxApplicationDelegate() : base() { diff --git a/MvvmCross/Platforms/Tvos/Core/MvxApplicationDelegate.cs b/MvvmCross/Platforms/Tvos/Core/MvxApplicationDelegate.cs index ea45eb87c5..6bac51a92c 100644 --- a/MvvmCross/Platforms/Tvos/Core/MvxApplicationDelegate.cs +++ b/MvvmCross/Platforms/Tvos/Core/MvxApplicationDelegate.cs @@ -15,7 +15,7 @@ public abstract class MvxApplicationDelegate : UIApplicationDelegate, IMvxApplic /// /// UIApplicationDelegate.Window doesn't really exist / work. It was added by Xamarin.iOS templates /// - public new UIWindow Window { get; set; } + public new virtual UIWindow Window { get; set; } public MvxApplicationDelegate() : base() { From 9c9d14e382382c2c3bf2b63f350e7b8b1bdfb970 Mon Sep 17 00:00:00 2001 From: nmilcoff Date: Fri, 30 Mar 2018 16:52:46 -0300 Subject: [PATCH 5/6] Fix icon crazyness on Playground.Droid and set the splashscreen to appcompat version --- .../Playground/Playground.Droid/MainApplication.cs | 1 - .../Playground.Droid/Playground.Droid.csproj | 10 +++++----- .../Resources/layout/SplashScreen.axml | 3 ++- Projects/Playground/Playground.Droid/SplashScreen.cs | 9 +++++++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Projects/Playground/Playground.Droid/MainApplication.cs b/Projects/Playground/Playground.Droid/MainApplication.cs index b115369641..03cf7fab34 100644 --- a/Projects/Playground/Playground.Droid/MainApplication.cs +++ b/Projects/Playground/Playground.Droid/MainApplication.cs @@ -6,7 +6,6 @@ using Android.App; using Android.Runtime; using MvvmCross.Droid.Support.V7.AppCompat; -using MvvmCross.Platforms.Android.Views; using Playground.Core; namespace Playground.Droid diff --git a/Projects/Playground/Playground.Droid/Playground.Droid.csproj b/Projects/Playground/Playground.Droid/Playground.Droid.csproj index a9ff6bfe8b..7132503331 100644 --- a/Projects/Playground/Playground.Droid/Playground.Droid.csproj +++ b/Projects/Playground/Playground.Droid/Playground.Droid.csproj @@ -114,11 +114,11 @@ Designer - - - - - + + + + + diff --git a/Projects/Playground/Playground.Droid/Resources/layout/SplashScreen.axml b/Projects/Playground/Playground.Droid/Resources/layout/SplashScreen.axml index 60edacd234..23a17cc0f3 100644 --- a/Projects/Playground/Playground.Droid/Resources/layout/SplashScreen.axml +++ b/Projects/Playground/Playground.Droid/Resources/layout/SplashScreen.axml @@ -2,7 +2,8 @@ + android:layout_height="match_parent" + android:background="@color/colorAccent"> Date: Sat, 31 Mar 2018 09:47:52 +1100 Subject: [PATCH 6/6] Simplifying setup and forcing EnsureInitialize to block until seutp complete --- MvvmCross/Core/MvxSetupSingleton.cs | 97 +++++++++++++++++------------ 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/MvvmCross/Core/MvxSetupSingleton.cs b/MvvmCross/Core/MvxSetupSingleton.cs index a425348105..539dae6e28 100644 --- a/MvvmCross/Core/MvxSetupSingleton.cs +++ b/MvvmCross/Core/MvxSetupSingleton.cs @@ -14,6 +14,7 @@ public abstract class MvxSetupSingleton : MvxSingleton { private static readonly object LockObject = new object(); + private static readonly object CompletionLockObject = new object(); private static TaskCompletionSource IsInitialisedTaskCompletionSource; private IMvxSetup _setup; private IMvxSetupMonitor _currentMonitor; @@ -53,53 +54,57 @@ protected static TMvxSetupSingleton EnsureSingletonAvailable public virtual void EnsureInitialized() { - lock (LockObject) - { - if (IsInitialisedTaskCompletionSource != null) - { - if (IsInitialisedTaskCompletionSource.Task.IsCompleted) - return; + // This method is safe to call multiple times - will only run init once + StartSetupInitialization(); - // Only wait for the result here if nobody's monitoring the state - if (_currentMonitor == null) - { - IsInitialisedTaskCompletionSource.Task.GetAwaiter().GetResult(); - MvxLog.Instance.Trace("EnsureInitialized has already been called so now waiting for completion"); - } - } - else - { - StartSetupInitialization(); - } - } + // Return immediately if the task is completed + if (IsInitialisedTaskCompletionSource.Task.IsCompleted) + return; + + // Block waiting on Initialize to be completed + IsInitialisedTaskCompletionSource.Task.GetAwaiter().GetResult(); } public virtual void InitializeAndMonitor(IMvxSetupMonitor setupMonitor) { - lock (LockObject) + // Grab the lock for the setupMonitor to make sure + // InitializationComplete isn't attempted whilst we're + // checking to see if Initialize has completed + lock (CompletionLockObject) { - _currentMonitor = setupMonitor; - - // if the tcs is not null, it means the initialization is running - if (IsInitialisedTaskCompletionSource != null) + var hasCompleted = false; + // Grab the lock for setting the completion state after Initialize is complete + lock (LockObject) { - // If the task is already completed at this point, let the monitor know it has finished. - // but don't do it otherwise because it's done elsewhere - if(IsInitialisedTaskCompletionSource.Task.IsCompleted) + hasCompleted = IsInitialisedTaskCompletionSource?.Task.IsCompleted ?? false; + // At this point if the completion state isn't completed, we know + // that we can set the _currentMonitor, and that InitializationComplete will get invoked + if (!hasCompleted) { - _currentMonitor?.InitializationComplete(); + _currentMonitor = setupMonitor; } + } + // At this point, if completion state is completed, we should + // NOT set the _currentMonitor (as there is a risk InitializationComplete + // has already been invoked). Instead we should just call InitializationComplete + // directly, and return. + if (hasCompleted) + { + setupMonitor.InitializationComplete(); return; } - - StartSetupInitialization(); } + + // If we get here, we should attempt to start Initialize because all we + // know is that Initialize hasn't completed. StartSetupInitialization is + // clever enough to know if it needs to run Initialize, or if it's already running + StartSetupInitialization(); } public virtual void CancelMonitor(IMvxSetupMonitor setupMonitor) { - lock (LockObject) + lock (CompletionLockObject) { if (setupMonitor != _currentMonitor) { @@ -125,7 +130,7 @@ protected override void Dispose(bool isDisposing) { if (isDisposing) { - lock (LockObject) + lock (CompletionLockObject) { _currentMonitor = null; } @@ -135,23 +140,35 @@ protected override void Dispose(bool isDisposing) private void StartSetupInitialization() { - IsInitialisedTaskCompletionSource = new TaskCompletionSource(); + // Do double-test to detect if Initialize has started + if (IsInitialisedTaskCompletionSource != null) return; + lock (LockObject) + { + if (IsInitialisedTaskCompletionSource != null) return; + + // At this point we know Initialize hasn't started, so create + // the completion source and kick of Initialize + IsInitialisedTaskCompletionSource = new TaskCompletionSource(); + } + + // InitializePrimary should be only init methods that needs to be done on the UI thread _setup.InitializePrimary(); Task.Run(() => { + // InitializeSecondary should be the bulk of init methods (and done on non-UI thread) _setup.InitializeSecondary(); lock (LockObject) { IsInitialisedTaskCompletionSource.SetResult(true); - var dispatcher = Mvx.GetSingleton(); - dispatcher.RequestMainThreadAction(() => - { - if (_currentMonitor != null) - { - _currentMonitor?.InitializationComplete(); - } - }); } + var dispatcher = Mvx.GetSingleton(); + dispatcher.RequestMainThreadAction(() => + { + lock (CompletionLockObject) + { + _currentMonitor?.InitializationComplete(); + } + }); }); } }