fix(core): safety-checks to prevent potential navigation exceptions (#10683)

Plus coding conventions and notes updates. [skip ci]

packages/core/ui/frame/fragment.transitions.android.ts

@@ -119,7 +119,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran
 				curve: transition.getCurve(),
 			},
 			newEntry,
-			transition
+			transition,
 		);
 		if (currentFragmentNeedsDifferentAnimation) {
 			setupCurrentFragmentCustomTransition(
@@ -128,7 +128,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran
 					curve: transition.getCurve(),
 				},
 				currentEntry,
-				transition
+				transition,
 			);
 		}
 	} else if (name === 'default') {
@@ -356,7 +356,10 @@ function getTransitionListener(entry: ExpandedEntry, transition: androidx.transi
 		@Interfaces([(<any>androidx).transition.Transition.TransitionListener])
 		class TransitionListenerImpl extends java.lang.Object implements androidx.transition.Transition.TransitionListener {
 			public backEntry?: BackstackEntry;
-			constructor(public entry: ExpandedEntry, public transition: androidx.transition.Transition) {
+			constructor(
+				public entry: ExpandedEntry,
+				public transition: androidx.transition.Transition,
+			) {
 				super();
 
 				return global.__native(this);
@@ -702,7 +705,7 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta
 	if (entries.size === 0) {
 		// We have 0 or 1 entry per frameId in completedEntries
 		// So there is no need to make it to Set like waitingQueue
-		const previousCompletedAnimationEntry = completedEntries.get(frameId);
+		const previousCompletedEntry = completedEntries.get(frameId);
 		completedEntries.delete(frameId);
 		waitingQueue.delete(frameId);
 
@@ -716,8 +719,8 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta
 		const navigationContext = frame._executingContext || {
 			navigationType: NavigationType.back,
 		};
-		let current = frame.isCurrent(entry) ? previousCompletedAnimationEntry : entry;
+		const current = frame.isCurrent(entry) && previousCompletedEntry ? previousCompletedEntry : entry;
-		current = current || entry;
+
 		// Will be null if Frame is shown modally...
 		// transitionOrAnimationCompleted fires again (probably bug in android).
 		if (current) {

packages/core/ui/frame/frame-common.ts

@@ -41,7 +41,7 @@ export class FrameBase extends CustomLayoutView {
 	private _animated: boolean;
 	private _transition: NavigationTransition;
 	private _backStack = new Array<BackstackEntry>();
-	_navigationQueue = new Array<NavigationContext>();
+	private _navigationQueue = new Array<NavigationContext>();
 
 	public actionBarVisibility: 'auto' | 'never' | 'always';
 	public _currentEntry: BackstackEntry;
@@ -300,7 +300,14 @@ export class FrameBase extends CustomLayoutView {
 			this._backStack.pop();
 		} else if (!isReplace) {
 			if (entry.entry.clearHistory) {
-				this._backStack.forEach((e) => this._removeEntry(e));
+				this._backStack.forEach((e) => {
+					if (e !== entry) {
+						this._removeEntry(e);
+					} else {
+						// This case is extremely rare but can occur when fragment resumes
+						Trace.write(`Failed to dispose backstack entry ${entry}. This entry is the one frame is navigating to.`, Trace.categories.Navigation, Trace.messageType.warn);
+					}
+				});
 				this._backStack.length = 0;
 			} else if (FrameBase._isEntryBackstackVisible(current)) {
 				this._backStack.push(current);
@@ -370,6 +377,16 @@ export class FrameBase extends CustomLayoutView {
 		return entry;
 	}
 
+	public getNavigationQueueContextByEntry(entry: BackstackEntry): NavigationContext {
+		for (const context of this._navigationQueue) {
+			if (context.entry === entry) {
+				return context;
+			}
+		}
+
+		return null;
+	}
+
 	public navigationQueueIsEmpty(): boolean {
 		return this._navigationQueue.length === 0;
 	}
@@ -429,16 +446,18 @@ export class FrameBase extends CustomLayoutView {
 
 	@profile
 	performGoBack(navigationContext: NavigationContext) {
-		let backstackEntry = navigationContext.entry;
 		const backstack = this._backStack;
-		if (!backstackEntry) {
+		const backstackEntry = navigationContext.entry || backstack[backstack.length - 1];
-			backstackEntry = backstack[backstack.length - 1];
+
+		if (backstackEntry) {
 			navigationContext.entry = backstackEntry;
-		}
 
 			this._executingContext = navigationContext;
 			this._onNavigatingTo(backstackEntry, true);
 			this._goBackCore(backstackEntry);
+		} else {
+			Trace.write('Frame.performGoBack: No backstack entry found to navigate back to', Trace.categories.Navigation, Trace.messageType.warn);
+		}
 	}
 
 	public _goBackCore(backstackEntry: BackstackEntry) {

packages/core/ui/frame/index.d.ts

@@ -200,6 +200,10 @@ export class Frame extends FrameBase {
 	 * @param navigationType
 	 */
 	setCurrent(entry: BackstackEntry, navigationType: NavigationType): void;
+	/**
+	 * @private
+	 */
+	getNavigationQueueContextByEntry(entry: BackstackEntry): NavigationContext;
 	/**
 	 * @private
 	 */

packages/core/ui/page/index.ios.ts

@@ -18,7 +18,7 @@ const DELEGATE = '_delegate';
 const TRANSITION = '_transition';
 const NON_ANIMATED_TRANSITION = 'non-animated';
 
-function isBackNavigationTo(page: Page, entry): boolean {
+function isBackNavigationTo(page: Page, entry: BackstackEntry): boolean {
 	const frame = page.frame;
 	if (!frame) {
 		return false;
@@ -37,14 +37,8 @@ function isBackNavigationTo(page: Page, entry): boolean {
 		return true;
 	}
 
-	const navigationQueue = (<any>frame)._navigationQueue;
+	const queueContext = frame.getNavigationQueueContextByEntry(entry);
-	for (let i = 0; i < navigationQueue.length; i++) {
+	return queueContext && queueContext.navigationType === NavigationType.back;
-		if (navigationQueue[i].entry === entry) {
-			return navigationQueue[i].navigationType === NavigationType.back;
-		}
-	}
-
-	return false;
 }
 
 function isBackNavigationFrom(controller: UIViewControllerImpl, page: Page): boolean {
@@ -121,7 +115,7 @@ class UIViewControllerImpl extends UIViewController {
 		}
 
 		const frame: Frame = this.navigationController ? (<any>this.navigationController).owner : null;
-		const newEntry = this[ENTRY];
+		const newEntry: BackstackEntry = this[ENTRY];
 
 		// Don't raise event if currentPage was showing modal page.
 		if (!owner._presentedViewController && newEntry && (!frame || frame.currentPage !== owner)) {

tools/notes/CodingConvention.md

@@ -2,11 +2,11 @@
 
 ## Tabs vs Spaces
 
-Use 4 spaces indentation.
+Use tab width 2 indentation.
 
 ## Line length
 
-Try to limit your lines to 80 characters.
+Try to limit your lines to 600 characters.
 
 ## Semicolons, statement Termination
 
@@ -26,18 +26,18 @@ let x = 1
 
 ## Quotes
 
-Use double quotes for strings:
+Use single quotes for strings:
 
 *Right:*
 
 ```TypeScript
-let foo = "bar";
+let foo = 'bar';
 ```
 
 *Wrong:*
 
 ```TypeScript
-let foo = 'bar';
+let foo = "bar";
 ```
 
 ## Braces
@@ -48,7 +48,7 @@ Your opening braces go on the same line as the statement.
 
 ```TypeScript
 if (true) {
-    console.log("winning");
+  console.log('winning');
 }
 ```
 
@@ -57,7 +57,7 @@ if (true) {
 ```TypeScript
 if (true)
 {
-    console.log("losing");
+  console.log('losing');
 }
 ```
 
@@ -69,9 +69,9 @@ Follow the JavaScript convention of stacking `else/catch` clauses on the same li
 
 ```TypeScript
 if (i % 2 === 0) {
-    console.log("even");
+  console.log('even');
 } else {
-    console.log("odd");
+  console.log('odd');
 }
 ```
 
@@ -79,10 +79,10 @@ if (i % 2 === 0) {
 
 ```TypeScript
 if (i % 2 === 0) {
-    console.log("even");
+  console.log('even');
 }
 else {
-    console.log("odd");
+  console.log('odd');
 }
 ```
 
@@ -120,13 +120,13 @@ uncommon abbreviations should generally be avoided unless it is something well k
 *Right:*
 
 ```TypeScript
-let adminUser = db.query("SELECT * FROM users ...");
+let adminUser = db.query('SELECT * FROM users ...');
 ```
 
 *Wrong:*
 
 ```TypeScript
-let admin_user = db.query("SELECT * FROM users ...");
+let admin_user = db.query('SELECT * FROM users ...');
 ```
 
 [camelcase]: https://en.wikipedia.org/wiki/camelCase#Variations_and_synonyms
@@ -139,7 +139,7 @@ Type names should be capitalized using [upper camel case][camelcase].
 
 ```TypeScript
 class UserAccount() {
-  this.field = "a";
+  this.field = 'a';
 }
 ```
 
@@ -147,7 +147,7 @@ class UserAccount() {
 
 ```TypeScript
 class userAccount() {
-  this.field = "a";
+  this.field = 'a';
 }
 ```
 
@@ -176,10 +176,10 @@ keys when your interpreter complains:
 *Right:*
 
 ```TypeScript
-let a = ["hello", "world"];
+let a = ['hello', 'world'];
 let b = {
-  good: "code",
+  good: 'code',
-  "is generally": "pretty",
+  'is generally': 'pretty',
 };
 ```
 
@@ -187,23 +187,23 @@ let b = {
 
 ```TypeScript
 let a = [
-  "hello", "world"
+  'hello', 'world'
 ];
-let b = {"good": "code"
+let b = {'good': 'code'
-        , is generally: "pretty"
+        , is generally: 'pretty'
         };
 ```
 
 ## Equality operator
 
-Use the [strict comparison operators][comparisonoperators]. The triple equality operator helps to maintain data type integrity throughout the code.
+Use the [strict comparison operators][comparisonoperators] when needed. The triple equality operator helps to maintain data type integrity throughout the code.
 
 *Right:*
 
 ```TypeScript
 let a = 0;
-if (a === "") {
+if (a === '') {
-    console.log("winning");
+  console.log('winning');
 }
 
 ```
@@ -212,8 +212,8 @@ if (a === "") {
 
 ```TypeScript
 let a = 0;
-if (a == "") {
+if (a == '') {
-    console.log("losing");
+  console.log('losing');
 }
 ```
 
@@ -227,8 +227,8 @@ Try to avoid short-hand operators except in very simple scenarios.
 
 ```TypeScript
 let default = x || 50;
-let extraLarge = "xxl";
+let extraLarge = 'xxl';
-let small = "s"
+let small = 's'
 let big = (x > 10) ? extraLarge : small;
 ```
 
@@ -247,7 +247,7 @@ Always use curly braces even in the cases of one line conditional operations.
 
 ```TypeScript
 if (a) {
-    return "winning";
+  return 'winning';
 }
 
 ```
@@ -257,9 +257,9 @@ if (a) {
 ```TypeScript
 
 if (a)
-    return "winning";
+  return 'winning';
 
-if (a) return "winning";
+if (a) return 'winning';
 ```
 
 ## Boolean comparisons
@@ -271,11 +271,11 @@ if (a) return "winning";
 ```TypeScript
 
 if (condition) {
-    console.log("winning");
+  console.log('winning');
 }
 
 if (!condition) {
-    console.log("winning");
+  console.log('winning');
 }
 
 ```
@@ -285,15 +285,15 @@ if (!condition) {
 ```TypeScript
 
 if (condition === true) {
-    console.log("losing");
+  console.log('losing');
 }
 
 if (condition !== true) {
-    console.log("losing");
+  console.log('losing');
 }
 
 if (condition !== false) {
-    console.log("losing");
+  console.log('losing');
 }
 
 ```
@@ -306,7 +306,7 @@ Do not use the **Yoda Conditions** when writing boolean expressions:
 ```TypeScript
 let num;
 if (num >= 0) {
-    console.log("winning");
+  console.log('winning');
 }
 ```
 
@@ -315,14 +315,14 @@ if (num >= 0) {
 ```TypeScript
 let num;
 if (0 <= num) {
-    console.log("losing");
+  console.log('losing');
 }
 ```
 
 **NOTE** It is OK to use constants on the left when comparing for a range.
 ```TypeScript
 if (0 <= num && num <= 100) {
-    console.log("winning");
+  console.log('winning');
 }
 ```
 
@@ -392,7 +392,7 @@ Use arrow functions over anonymous function expressions. Typescript will take ca
 *Right:*
 
 ```TypeScript
-req.on("end", () => {
+req.on('end', () => {
   exp1();
   exp2();
   this.doSomething();
@@ -403,7 +403,7 @@ req.on("end", () => {
 
 ```TypeScript
 let that = this;
-req.on("end", function () {
+req.on('end', function () {
   exp1();
   exp2();
   that.doSomething();