8000 fix(core): safety-checks to prevent potential navigation exceptions (… · NativeScript/NativeScript@03cca58 · GitHub
[go: up one dir, main page]
Skip to content

Commit 03cca58

Browse files
CatchABusCatchABus
authored
fix(core): safety-checks to prevent potential navigation exceptions (#10683)
Plus coding conventions and notes updates. [skip ci]
1 parent 9bd147c commit 03cca58

File tree

5 files changed

+166
-146
lines changed

5 files changed

+166
-146
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran
119119
curve: transition.getCurve(),
120120
},
121121
newEntry,
122-
transition
122+
transition,
123123
);
124124
if (currentFragmentNeedsDifferentAnimation) {
125125
setupCurrentFragmentCustomTransition(
@@ -128,7 +128,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran
128128
curve: transition.getCurve(),
129129
},
130130
currentEntry,
131-
transition
131+
transition,
132132
);
133133
}
134134
} else if (name === 'default') {
@@ -356,7 +356,10 @@ function getTransitionListener(entry: ExpandedEntry, transition: androidx.transi
356356
@Interfaces([(<any>androidx).transition.Transition.TransitionListener])
357357
class TransitionListenerImpl extends java.lang.Object implements androidx.transition.Transition.TransitionListener {
358358
public backEntry?: BackstackEntry;
359-
constructor(public entry: ExpandedEntry, public transition: androidx.transition.Transition) {
359+
constructor(
360+
public entry: ExpandedEntry,
361+
public transition: androidx.transition.Transition,
362+
) {
360363
super();
361364

362365
return global.__native(this);
@@ -702,7 +705,7 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta
702705
if (entries.size === 0) {
703706
// We have 0 or 1 entry per frameId in completedEntries
704707
// So there is no need to make it to Set like waitingQueue
705-
const previousCompletedAnimationEntry = completedEntries.get(frameId);
708+
const previousCompletedEntry = completedEntries.get(frameId);
706709
completedEntries.delete(frameId);
707710
waitingQueue.delete(frameId);
708711

@@ -716,8 +719,8 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta
716719
const navigationContext = frame._executingContext || {
717720
navigationType: NavigationType.back,
718721
};
719-
let current = frame.isCurrent(entry) ? previousCompletedAnimationEntry : entry;
720-
current = current || entry;
722+
const current = frame.isCurrent(entry) && previousCompletedEntry ? previousCompletedEntry : entry;
723+
721724
// Will be null if Frame is shown modally...
722725
// transitionOrAnimationCompleted fires again (probably bug in android).
723726
if (current) {

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

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export class FrameBase extends CustomLayoutView {
4141
private _animated: boolean;
4242
private _transition: NavigationTransition;
4343
private _backStack = new Array<BackstackEntry>();
44-
_navigationQueue = new Array<NavigationContext>();
44+
private _navigationQueue = new Array<NavigationContext>();
4545

4646
public actionBarVisibility: 'auto' | 'never' | 'always';
4747
public _currentEntry: BackstackEntry;
@@ -300,7 +300,14 @@ export class FrameBase extends CustomLayoutView {
300300
this._backStack.pop();
301301
} else if (!isReplace) {
302302
if (entry.entry.clearHistory) {
303-
this._backStack.forEach((e) => this._removeEntry(e));
303+
this._backStack.forEach((e) => {
304+
if (e !== entry) {
305+
this._removeEntry(e);
306+
} else {
307+
// This case is extremely rare but can occur when fragment resumes
308+
Trace.write(`Failed to dispose backstack entry ${entry}. This entry is the one frame is navigating to.`, Trace.categories.Navigation, Trace.messageType.warn);
309+
}
310+
});
304311
this._backStack.length = 0;
305312
} else if (FrameBase._isEntryBackstackVisible(current)) {
306313
this._backStack.push(current);
@@ -370,6 +377,16 @@ export class FrameBase extends CustomLayoutView {
370377
return entry;
371378
}
372379

380+
public getNavigationQueueContextByEntry(entry: BackstackEntry): NavigationContext {
381+
for (const context of this._navigationQueue) {
382+
if (context.entry === entry) {
383+
return context;
384+
}
385+
}
386+
387+
return null;
388+
}
389+
373390
public navigationQueueIsEmpty(): boolean {
374391
return this._navigationQueue.length === 0;
375392
}
@@ -429,16 +446,18 @@ export class FrameBase extends CustomLayoutView {
429446

430447
@profile
431448
performGoBack(navigationContext: NavigationContext) {
432-
let backstackEntry = navigationContext.entry;
433449
const backstack = this._backStack;
434-
if (!backstackEntry) {
435-
backstackEntry = backstack[backstack.length - 1];
450+
const backstackEntry = navigationContext.entry || backstack[backstack.length - 1];
451+
452+
if (backstackEntry) {
436453
navigationContext.entry = backstackEntry;
437-
}
438454

439-
this._executingContext = navigationContext;
440-
this._onNavigatingTo(backstackEntry, true);
441-
this._goBackCore(backstackEntry);
455+
this._executingContext = navigationContext;
456+
this._onNavigatingTo(backstackEntry, true);
457+
this._goBackCore(backstackEntry);
458+
} else {
459+
Trace.write('Frame.performGoBack: No backstack entry found to navigate back to', Trace.categories.Navigation, Trace.messageType.warn);
460+
}
442461
}
443462

444463
public _goBackCore(backstackEntry: BackstackEntry) {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,10 @@ export class Frame extends FrameBase {
200200
* @param navigationType
201201
*/
202202
setCurrent(entry: BackstackEntry, navigationType: NavigationType): void;
203+
/**
204+
* @private
205+
*/
206+
getNavigationQueueContextByEntry(entry: BackstackEntry): NavigationContext;
203207
/**
204208
* @private
205209
*/

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const DELEGATE = '_delegate';
1818
const TRANSITION = '_transition';
1919
const NON_ANIMATED_TRANSITION = 'non-animated';
2020

21-
function isBackNavigationTo(page: Page, entry): boolean {
21+
function isBackNavigationTo(page: Page, entry: BackstackEntry): boolean {
2222
const frame = page.frame;
2323
if (!frame) {
2424
return false;
@@ -37,14 +37,8 @@ function isBackNavigationTo(page: Page, entry): boolean {
3737
return true;
3838
}
3939

40-
const navigationQueue = (<any>frame)._navigationQueue;
41-
for (let i = 0; i < navigationQueue.length; i++) {
42-
if (navigationQueue[i].entry === entry) {
43-
return navigationQueue[i].navigationType === NavigationType.back;
44-
}
45-
}
46-
47-
return false;
40+
const queueContext = frame.getNavigationQueueContextByEntry(entry);
41+
return queueContext && queueContext.navigationType === NavigationType.back;
4842
}
4943

5044
function isBackNavigationFrom(controller: UIViewControllerImpl, page: Page): boolean {
@@ -121,7 +115,7 @@ class UIViewControllerImpl extends UIViewController {
121115
}
122116

123117
const frame: Frame = this.navigationController ? (<any>this.navigationController).owner : null;
124-
const newEntry = this[ENTRY];
118+
const newEntry: BackstackEntry = this[ENTRY];
125119

126120
// Don't raise event if currentPage was showing modal page.
127121
if (!owner._presentedViewController && newEntry && (!frame || frame.currentPage !== owner)) {

0 commit comments

Comments
 (0)
0