fix(android): improved handling for nested frames (#10713)

packages/core/ui/core/view-base/index.ts

@@ -117,12 +117,13 @@ export interface ShowModalOptions {
  * @param criterion - The type of ancestor view we are looking for. Could be a string containing a class name or an actual type.
  * Returns an instance of a view (if found), otherwise undefined.
  */
-export function getAncestor(view: ViewBaseDefinition, criterion: string | { new () }): ViewBaseDefinition {
-	let matcher: (view: ViewBaseDefinition) => boolean = null;
+export function getAncestor<T extends ViewBaseDefinition = ViewBaseDefinition>(view: T, criterion: string | { new () }): T {
+	let matcher: (view: ViewBaseDefinition) => view is T;
+
 	if (typeof criterion === 'string') {
-		matcher = (view: ViewBaseDefinition) => view.typeName === criterion;
+		matcher = (view: ViewBaseDefinition): view is T => view.typeName === criterion;
 	} else {
-		matcher = (view: ViewBaseDefinition) => view instanceof criterion;
+		matcher = (view: ViewBaseDefinition): view is T => view instanceof criterion;
 	}
 
 	for (let parent = view.parent; parent != null; parent = parent.parent) {

packages/core/ui/frame/callbacks/activity-callbacks.ts

@@ -5,8 +5,6 @@ import { AndroidActivityBackPressedEventData, AndroidActivityNewIntentEventData,
 import { Trace } from '../../../trace';
 import { View } from '../../core/view';
 
-import { _clearEntry, _clearFragment, _getAnimatedEntries, _reverseTransitions, _setAndroidFragmentTransitions, _updateTransitions } from '../fragment.transitions';
-
 import { profile } from '../../../profiling';
 import { isEmbedded, setEmbeddedView } from '../../embedding';
 

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

@@ -1,5 +1,5 @@
 // Definitions.
-import { NavigationType } from './frame-common';
+import { NavigationType, TransitionState } from './frame-common';
 import { NavigationTransition, BackstackEntry } from '.';
 
 // Types.
@@ -152,7 +152,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran
 			setupCurrentFragmentExplodeTransition(navigationTransition, currentEntry);
 		}
 	} else if (name.indexOf('flip') === 0) {
-		const direction = name.substr('flip'.length) || 'right'; //Extract the direction from the string
+		const direction = name.substring('flip'.length) || 'right'; //Extract the direction from the string
 		const flipTransition = new FlipTransition(direction, navigationTransition.duration, navigationTransition.curve);
 
 		setupNewFragmentCustomTransition(navigationTransition, newEntry, flipTransition);
@@ -282,23 +282,28 @@ export function _getAnimatedEntries(frameId: number): Set<BackstackEntry> {
 
 export function _updateTransitions(entry: ExpandedEntry): void {
 	const fragment = entry.fragment;
+
+	if (!fragment) {
+		return;
+	}
+
 	const enterTransitionListener = entry.enterTransitionListener;
-	if (enterTransitionListener && fragment) {
+	if (enterTransitionListener) {
 		fragment.setEnterTransition(enterTransitionListener.transition);
 	}
 
 	const exitTransitionListener = entry.exitTransitionListener;
-	if (exitTransitionListener && fragment) {
+	if (exitTransitionListener) {
 		fragment.setExitTransition(exitTransitionListener.transition);
 	}
 
 	const reenterTransitionListener = entry.reenterTransitionListener;
-	if (reenterTransitionListener && fragment) {
+	if (reenterTransitionListener) {
 		fragment.setReenterTransition(reenterTransitionListener.transition);
 	}
 
 	const returnTransitionListener = entry.returnTransitionListener;
-	if (returnTransitionListener && fragment) {
+	if (returnTransitionListener) {
 		fragment.setReturnTransition(returnTransitionListener.transition);
 	}
 }
@@ -428,6 +433,16 @@ function addToWaitingQueue(entry: ExpandedEntry): void {
 	entries.add(entry);
 }
 
+function cloneExpandedTransitionListener(expandedTransitionListener: ExpandedTransitionListener) {
+	if (!expandedTransitionListener) {
+		return null;
+	}
+
+	const cloneTransition = expandedTransitionListener.transition.clone();
+
+	return addNativeTransitionListener(expandedTransitionListener.entry, cloneTransition);
+}
+
 function clearExitAndReenterTransitions(entry: ExpandedEntry, removeListener: boolean): void {
 	const fragment: androidx.fragment.app.Fragment = entry.fragment;
 	const exitListener = entry.exitTransitionListener;
@@ -469,15 +484,56 @@ function clearExitAndReenterTransitions(entry: ExpandedEntry, removeListener: bo
 	}
 }
 
+export function _getTransitionState(entry: ExpandedEntry): TransitionState {
+	let transitionState: TransitionState;
+
+	if (entry.enterTransitionListener && entry.exitTransitionListener) {
+		transitionState = {
+			enterTransitionListener: cloneExpandedTransitionListener(entry.enterTransitionListener),
+			exitTransitionListener: cloneExpandedTransitionListener(entry.exitTransitionListener),
+			reenterTransitionListener: cloneExpandedTransitionListener(entry.reenterTransitionListener),
+			returnTransitionListener: cloneExpandedTransitionListener(entry.returnTransitionListener),
+			transitionName: entry.transitionName,
+			entry,
+		};
+	} else {
+		transitionState = null;
+	}
+
+	return transitionState;
+}
+
+export function _restoreTransitionState(snapshot: TransitionState): void {
+	const entry = snapshot.entry as ExpandedEntry;
+
+	if (snapshot.enterTransitionListener) {
+		entry.enterTransitionListener = snapshot.enterTransitionListener;
+	}
+
+	if (snapshot.exitTransitionListener) {
+		entry.exitTransitionListener = snapshot.exitTransitionListener;
+	}
+
+	if (snapshot.reenterTransitionListener) {
+		entry.reenterTransitionListener = snapshot.reenterTransitionListener;
+	}
+
+	if (snapshot.returnTransitionListener) {
+		entry.returnTransitionListener = snapshot.returnTransitionListener;
+	}
+
+	entry.transitionName = snapshot.transitionName;
+}
+
 export function _clearFragment(entry: ExpandedEntry): void {
-	clearEntry(entry, false);
+	clearTransitions(entry, false);
 }
 
 export function _clearEntry(entry: ExpandedEntry): void {
-	clearEntry(entry, true);
+	clearTransitions(entry, true);
 }
 
-function clearEntry(entry: ExpandedEntry, removeListener: boolean): void {
+function clearTransitions(entry: ExpandedEntry, removeListener: boolean): void {
 	clearExitAndReenterTransitions(entry, removeListener);
 
 	const fragment: androidx.fragment.app.Fragment = entry.fragment;
@@ -569,7 +625,7 @@ function setReturnTransition(navigationTransition: NavigationTransition, entry:
 
 function setupNewFragmentSlideTransition(navTransition: NavigationTransition, entry: ExpandedEntry, name: string): void {
 	setupCurrentFragmentSlideTransition(navTransition, entry, name);
-	const direction = name.substr('slide'.length) || 'left'; //Extract the direction from the string
+	const direction = name.substring('slide'.length) || 'left'; //Extract the direction from the string
 	switch (direction) {
 		case 'left':
 			setEnterTransition(navTransition, entry, new androidx.transition.Slide(android.view.Gravity.RIGHT));
@@ -594,7 +650,7 @@ function setupNewFragmentSlideTransition(navTransition: NavigationTransition, en
 }
 
 function setupCurrentFragmentSlideTransition(navTransition: NavigationTransition, entry: ExpandedEntry, name: string): void {
-	const direction = name.substr('slide'.length) || 'left'; //Extract the direction from the string
+	const direction = name.substring('slide'.length) || 'left'; //Extract the direction from the string
 	switch (direction) {
 		case 'left':
 			setExitTransition(navTransition, entry, new androidx.transition.Slide(android.view.Gravity.LEFT));

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

@@ -1,4 +1,4 @@
-import { NavigationTransition, BackstackEntry } from '.';
+import { NavigationTransition, BackstackEntry, TransitionState } from '.';
 
 /**
  * @private
@@ -20,6 +20,14 @@ export function _updateTransitions(entry: BackstackEntry): void;
  * Reverse transitions from entry to fragment if any.
  */
 export function _reverseTransitions(previousEntry: BackstackEntry, currentEntry: BackstackEntry): boolean;
+/**
+ * @private
+ */
+export function _getTransitionState(entry: BackstackEntry): TransitionState;
+/**
+ * @private
+ */
+export function _restoreTransitionState(snapshot: TransitionState): void;
 /**
  * @private
  * Called when entry is removed from backstack (either back navigation or

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

@@ -76,13 +76,13 @@ export class FrameBase extends CustomLayoutView {
 			return true;
 		} else if (top) {
 			let parentFrameCanGoBack = false;
-			let parentFrame = <FrameBase>getAncestor(top, 'Frame');
+			let parentFrame = getAncestor(top, 'Frame');
 
 			while (parentFrame && !parentFrameCanGoBack) {
 				if (parentFrame && parentFrame.canGoBack()) {
 					parentFrameCanGoBack = true;
 				} else {
-					parentFrame = <FrameBase>getAncestor(parentFrame, 'Frame');
+					parentFrame = getAncestor(parentFrame, 'Frame');
 				}
 			}
 
@@ -122,7 +122,6 @@ export class FrameBase extends CustomLayoutView {
 	@profile
 	public onLoaded() {
 		super.onLoaded();
-
 		this._processNextNavigationEntry();
 	}
 
@@ -323,9 +322,10 @@ export class FrameBase extends CustomLayoutView {
 	}
 
 	private isNestedWithin(parentFrameCandidate: FrameBase): boolean {
-		let frameAncestor: FrameBase = this;
+		let frameAncestor = this as FrameBase;
+
 		while (frameAncestor) {
-			frameAncestor = <FrameBase>getAncestor(frameAncestor, FrameBase);
+			frameAncestor = getAncestor(frameAncestor, FrameBase);
 			if (frameAncestor === parentFrameCandidate) {
 				return true;
 			}

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

@@ -11,7 +11,7 @@ import { Trace } from '../../trace';
 import { View } from '../core/view';
 import { _stack, FrameBase, NavigationType } from './frame-common';
 
-import { _clearEntry, _clearFragment, _getAnimatedEntries, _reverseTransitions, _setAndroidFragmentTransitions, _updateTransitions, addNativeTransitionListener } from './fragment.transitions';
+import { _clearEntry, _clearFragment, _getAnimatedEntries, _getTransitionState, _restoreTransitionState, _reverseTransitions, _setAndroidFragmentTransitions, _updateTransitions, addNativeTransitionListener } from './fragment.transitions';
 
 import { profile } from '../../profiling';
 import { android as androidUtils } from '../../utils/native-helper';
@@ -28,6 +28,7 @@ const FRAMEID = '_frameId';
 const CALLBACKS = '_callbacks';
 
 const ownerSymbol = Symbol('_owner');
+const isPendingDetachSymbol = Symbol('_isPendingDetach');
 
 let navDepth = -1;
 let fragmentId = -1;
@@ -58,6 +59,12 @@ function getAttachListener(): android.view.View.OnAttachStateChangeListener {
 				if (owner) {
 					owner._onDetachedFromWindow();
 				}
+
+				if (view[isPendingDetachSymbol]) {
+					delete view[isPendingDetachSymbol];
+					view.removeOnAttachStateChangeListener(this);
+					view[ownerSymbol] = null;
+				}
 			},
 		});
 
@@ -73,7 +80,10 @@ export class Frame extends FrameBase {
 	private _containerViewId = -1;
 	private _tearDownPending = false;
 	private _attachedToWindow = false;
-	private _wasReset = false;
+	/**
+	 * This property indicates that the view is to be reused as a root view or has been previously disposed.
+	 */
+	private _isReset = false;
 	private _cachedTransitionState: TransitionState;
 	private _frameCreateTimeout: NodeJS.Timeout;
 
@@ -143,8 +153,9 @@ export class Frame extends FrameBase {
 		}
 
 		this._attachedToWindow = true;
-		this._wasReset = false;
+		this._isReset = false;
 		this._processNextNavigationEntry();
+		this._ensureEntryFragment();
 	}
 
 	_onDetachedFromWindow(): void {
@@ -162,12 +173,12 @@ export class Frame extends FrameBase {
 			return;
 		}
 
-		// in case the activity is "reset" using resetRootView we must wait for
+		// in case the activity is "reset" using resetRootView or disposed we must wait for
 		// the attachedToWindow event to make the first navigation or it will crash
 		// https://github.com/NativeScript/NativeScript/commit/9dd3e1a8076e5022e411f2f2eeba34aabc68d112
 		// though we should not do it on app "start"
 		// or it will create a "flash" to activity background color
-		if (this._wasReset && !this._attachedToWindow) {
+		if (this._isReset && !this._attachedToWindow) {
 			return;
 		}
 
@@ -194,7 +205,7 @@ export class Frame extends FrameBase {
 			// simulated navigation (NoTransition, zero duration animator) and thus the fragment immediately disappears;
 			// the user only sees the animation of the entering fragment as per its specific enter animation settings.
 			// NOTE: we are restoring the animation settings in Frame.setCurrent(...) as navigation completes asynchronously
-			const cachedTransitionState = getTransitionState(this._currentEntry);
+			const cachedTransitionState = _getTransitionState(this._currentEntry);
 
 			if (cachedTransitionState) {
 				this._cachedTransitionState = cachedTransitionState;
@@ -228,7 +239,7 @@ export class Frame extends FrameBase {
 	public _onRootViewReset(): void {
 		super._onRootViewReset();
 		// used to handle the "first" navigate differently on first run and on reset
-		this._wasReset = true;
+		this._isReset = true;
 		// call this AFTER the super call to ensure descendants apply their rootview-reset logic first
 		// i.e. in a scenario with nested frames / frame with tabview let the descendandt cleanup the inner
 		// fragments first, and then cleanup the parent fragments
@@ -241,6 +252,33 @@ export class Frame extends FrameBase {
 			this.backgroundColor = this._originalBackground;
 			this._originalBackground = null;
 		}
+
+		this._ensureEntryFragment();
+		super.onLoaded();
+	}
+
+	onUnloaded() {
+		super.onUnloaded();
+
+		if (typeof this._frameCreateTimeout === 'number') {
+			clearTimeout(this._frameCreateTimeout);
+			this._frameCreateTimeout = null;
+		}
+	}
+
+	/**
+	 * TODO: Check if this fragment precaution is still needed
+	 */
+	private _ensureEntryFragment(): void {
+		// in case the activity is "reset" using resetRootView or disposed we must wait for
+		// the attachedToWindow event to make the first navigation or it will crash
+		// https://github.com/NativeScript/NativeScript/commit/9dd3e1a8076e5022e411f2f2eeba34aabc68d112
+		// though we should not do it on app "start"
+		// or it will create a "flash" to activity background color
+		if (this._isReset && !this._attachedToWindow) {
+			return;
+		}
+
 		this._frameCreateTimeout = setTimeout(() => {
 			// there's a bug with nested frames where sometimes the nested fragment is not recreated at all
 			// so we manually check on loaded event if the fragment is not recreated and recreate it
@@ -255,17 +293,9 @@ export class Frame extends FrameBase {
 					transaction.commitAllowingStateLoss();
 				}
 			}
-		}, 0);
 
-		super.onLoaded();
-	}
-
-	onUnloaded() {
-		super.onUnloaded();
-		if (typeof this._frameCreateTimeout === 'number') {
-			clearTimeout(this._frameCreateTimeout);
 			this._frameCreateTimeout = null;
-		}
+		}, 0);
 	}
 
 	private disposeCurrentFragment(): void {
@@ -352,7 +382,7 @@ export class Frame extends FrameBase {
 
 		// restore cached animation settings if we just completed simulated first navigation (no animation)
 		if (this._cachedTransitionState) {
-			restoreTransitionState(this._currentEntry, this._cachedTransitionState);
+			_restoreTransitionState(this._cachedTransitionState);
 			this._cachedTransitionState = null;
 		}
 
@@ -391,7 +421,7 @@ export class Frame extends FrameBase {
 	// HACK: This @profile decorator creates a circular dependency
 	// HACK: because the function parameter type is evaluated with 'typeof'
 	@profile
-	public _navigateCore(newEntry: any) {
+	public _navigateCore(newEntry: BackstackEntry) {
 		// should be (newEntry: BackstackEntry)
 		super._navigateCore(newEntry);
 
@@ -487,19 +517,19 @@ export class Frame extends FrameBase {
 
 		if (removed.fragment) {
 			_clearEntry(removed);
+			removed.fragment = null;
 		}
 
-		removed.fragment = null;
 		removed.viewSavedState = null;
 	}
 
 	protected _disposeBackstackEntry(entry: BackstackEntry): void {
 		if (entry.fragment) {
 			_clearFragment(entry);
+			entry.fragment = null;
 		}
 
 		entry.recreated = false;
-		entry.fragment = null;
 
 		super._disposeBackstackEntry(entry);
 	}
@@ -518,9 +548,12 @@ export class Frame extends FrameBase {
 	public initNativeView(): void {
 		super.initNativeView();
 		const listener = getAttachListener();
-		this.nativeViewProtected.addOnAttachStateChangeListener(listener);
-		this.nativeViewProtected[ownerSymbol] = this;
-		this._android.rootViewGroup = this.nativeViewProtected;
+		const nativeView = this.nativeViewProtected as android.view.ViewGroup;
+
+		nativeView.addOnAttachStateChangeListener(listener);
+		nativeView[ownerSymbol] = this;
+
+		this._android.rootViewGroup = nativeView;
 		if (this._containerViewId < 0) {
 			this._containerViewId = android.view.View.generateViewId();
 		}
@@ -528,12 +561,23 @@ export class Frame extends FrameBase {
 	}
 
 	public disposeNativeView() {
+		const nativeView = this.nativeViewProtected as android.view.ViewGroup;
 		const listener = getAttachListener();
-		this.nativeViewProtected.removeOnAttachStateChangeListener(listener);
-		this.nativeViewProtected[ownerSymbol] = null;
+
+		// There are cases like root view when detach listener is not called upon removing view from view-tree
+		// so mark those views as pending and remove listener once the view is detached
+		if (nativeView.isAttachedToWindow()) {
+			nativeView[isPendingDetachSymbol] = true;
+		} else {
+			nativeView.removeOnAttachStateChangeListener(listener);
+			nativeView[ownerSymbol] = null;
+		}
+
 		this._tearDownPending = !!this._executingContext;
+
 		const current = this._currentEntry;
 		const executingEntry = this._executingContext ? this._executingContext.entry : null;
+
 		this.backStack.forEach((entry) => {
 			// Don't destroy current and executing entries or UI will look blank.
 			// We will do it in setCurrent.
@@ -546,6 +590,12 @@ export class Frame extends FrameBase {
 			this._disposeBackstackEntry(current);
 		}
 
+		// Dispose cached transition and store it again if view ever gets re-used
+		this._cachedTransitionState = null;
+
+		// Mark as reset in order to properly re-initialize fragments if view ever gets re-used
+		this._isReset = true;
+
 		this._android.rootViewGroup = null;
 		this._removeFromFrameStack();
 		super.disposeNativeView();
@@ -603,55 +653,6 @@ export function reloadPage(context?: ModuleContext): void {
 // attach on global, so it can be overwritten in NativeScript Angular
 global.__onLiveSyncCore = Frame.reloadPage;
 
-function cloneExpandedTransitionListener(expandedTransitionListener: any) {
-	if (!expandedTransitionListener) {
-		return null;
-	}
-
-	const cloneTransition = expandedTransitionListener.transition.clone();
-
-	return addNativeTransitionListener(expandedTransitionListener.entry, cloneTransition);
-}
-
-function getTransitionState(entry: BackstackEntry): TransitionState {
-	const expandedEntry = <any>entry;
-	const transitionState = <TransitionState>{};
-
-	if (expandedEntry.enterTransitionListener && expandedEntry.exitTransitionListener) {
-		transitionState.enterTransitionListener = cloneExpandedTransitionListener(expandedEntry.enterTransitionListener);
-		transitionState.exitTransitionListener = cloneExpandedTransitionListener(expandedEntry.exitTransitionListener);
-		transitionState.reenterTransitionListener = cloneExpandedTransitionListener(expandedEntry.reenterTransitionListener);
-		transitionState.returnTransitionListener = cloneExpandedTransitionListener(expandedEntry.returnTransitionListener);
-		transitionState.transitionName = expandedEntry.transitionName;
-		transitionState.entry = entry;
-	} else {
-		return null;
-	}
-
-	return transitionState;
-}
-
-function restoreTransitionState(entry: BackstackEntry, snapshot: TransitionState): void {
-	const expandedEntry = <any>entry;
-	if (snapshot.enterTransitionListener) {
-		expandedEntry.enterTransitionListener = snapshot.enterTransitionListener;
-	}
-
-	if (snapshot.exitTransitionListener) {
-		expandedEntry.exitTransitionListener = snapshot.exitTransitionListener;
-	}
-
-	if (snapshot.reenterTransitionListener) {
-		expandedEntry.reenterTransitionListener = snapshot.reenterTransitionListener;
-	}
-
-	if (snapshot.returnTransitionListener) {
-		expandedEntry.returnTransitionListener = snapshot.returnTransitionListener;
-	}
-
-	expandedEntry.transitionName = snapshot.transitionName;
-}
-
 let framesCounter = 0;
 const framesCache = new Array<WeakRef<AndroidFrame>>();
 

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

@@ -97,7 +97,7 @@ export class Frame extends FrameBase {
 	// !!! THIS PROFILE DECORATOR CREATES A CIRCULAR DEPENDENCY
 	// !!! BECAUSE THE PARAMETER TYPE IS EVALUATED WITH TYPEOF
 	@profile
-	public _navigateCore(backstackEntry: any) {
+	public _navigateCore(backstackEntry: BackstackEntry) {
 		super._navigateCore(backstackEntry);
 
 		const viewController: UIViewController = backstackEntry.resolvedPage.ios;
@@ -507,7 +507,7 @@ class UINavigationControllerImpl extends UINavigationController {
 		}
 	}
 
-	private animateWithDuration(navigationTransition: NavigationTransition, nativeTransition: UIViewAnimationTransition, transitionType: string, baseCallback: Function): void {
+	private animateWithDuration(navigationTransition: NavigationTransition, nativeTransition: UIViewAnimationTransition, transitionType: string, baseCallback: () => void): void {
 		const duration = navigationTransition.duration ? navigationTransition.duration / 1000 : CORE_ANIMATION_DEFAULTS.duration;
 		const curve = _getNativeCurve(navigationTransition);