Skip to content

Commit f67ee37

Browse files
edusperoniedusperoni
committed
fix(android): stop the FragmentManager driving fragments with a dead entry
A fragment can outlive the BackstackEntry it is bound to. Navigation only evicts fragments through transaction.replace(containerViewId, ...), so any fragment whose container no longer matches the frame's current container - left over from an activity recreation, a frame reset, or an interrupted navigation - stays added to the FragmentManager. _removeEntry then clears resolvedPage and drops entry.fragment without telling the FragmentManager, and the next transaction happily drives that fragment back through onCreateView. Every callback on that path either reported the condition with Trace.error - which routes to the error handler, rethrows, and becomes a fatal exception across the JNI boundary - or dereferenced the missing page directly, so the stale fragment took the app down instead of being discarded. - _removeEntry: detach the fragment's callbacks and remove it from the FragmentManager when it is still added, so it can neither be driven again nor resurrect the torn down page - findPageForFragment: discard an unclaimed restored fragment instead of throwing, and widen the entry lookup through _findEntryForTag so fragments restored from the backstack or the navigation queue are matched instead of treated as orphans - onDestroy: report a missing entry without throwing - onResume: bail out when the entry or page is gone rather than reading entry.resolvedPage.frame - onPause/onDestroyView: tolerate a missing frame
1 parent b8d613b commit f67ee37

2 files changed

Lines changed: 68 additions & 8 deletions

File tree

‎packages/core/ui/frame/frame-helper-for-android.ts‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ function findPageForFragment(fragment: androidx.fragment.app.Fragment, frame: Fr
3737
entry = current;
3838
} else if (executingContext && executingContext.entry && executingContext.entry.fragmentTag === fragmentTag) {
3939
entry = executingContext.entry;
40+
} else {
41+
// Android also restores fragments that were only in the backstack or still queued, so
42+
// widen the lookup before treating this fragment as an orphan.
43+
entry = frame._findEntryForTag(fragmentTag);
4044
}
4145

4246
let page: Page;
@@ -52,7 +56,41 @@ function findPageForFragment(fragment: androidx.fragment.app.Fragment, frame: Fr
5256
entry.fragment = fragment;
5357
_updateTransitions(entry);
5458
} else {
55-
throw new Error(`Could not find a page for ${fragmentTag}.`);
59+
// Android restored a fragment no live entry owns anymore - the frame navigated on (or was
60+
// reset) while the activity was being recreated. Throwing is fatal across the JNI boundary
61+
// and this fragment can never produce a view, so discard it instead.
62+
Trace.write(`Could not find a page for ${fragmentTag}. Discarding orphaned fragment.`, Trace.categories.NativeLifecycle, Trace.messageType.error);
63+
removeFragmentIfAdded(fragment);
64+
}
65+
}
66+
67+
/**
68+
* Drops a fragment the JS side no longer owns from its FragmentManager, so the manager stops
69+
* driving it through the lifecycle. The removal is always deferred - callers may be inside a
70+
* FragmentManager transaction, where commitNow throws "already executing transactions".
71+
*/
72+
export function removeFragmentIfAdded(fragment: androidx.fragment.app.Fragment): void {
73+
if (!fragment.isAdded()) {
74+
return;
75+
}
76+
77+
const manager = fragment.getParentFragmentManager();
78+
if (manager.isDestroyed()) {
79+
return;
80+
}
81+
82+
manager.beginTransaction().remove(fragment).commitAllowingStateLoss();
83+
}
84+
85+
/**
86+
* Breaks the fragment -> BackstackEntry link so a fragment the FragmentManager is still holding
87+
* cannot bring a discarded entry (and its torn down page) back into the frame.
88+
*/
89+
export function detachFragmentCallbacks(fragment: androidx.fragment.app.Fragment): void {
90+
const callbacks: FragmentCallbacksImplementation = fragment[CALLBACKS];
91+
if (callbacks) {
92+
callbacks.entry = null;
93+
callbacks.frame = null;
5694
}
5795
}
5896

@@ -214,7 +252,7 @@ export class FragmentCallbacksImplementation implements AndroidFragmentCallbacks
214252
const hasRemovingParent = fragment.getRemovingParentFragment();
215253

216254
if (hasRemovingParent) {
217-
const nativeFrameView = this.frame.nativeViewProtected;
255+
const nativeFrameView = this.frame?.nativeViewProtected;
218256
if (nativeFrameView) {
219257
const bitmapDrawable = new android.graphics.drawable.BitmapDrawable(getNativeApp<android.app.Application>().getApplicationContext().getResources(), this.backgroundBitmap);
220258
this.frame._originalBackground = this.frame.backgroundColor || new Color('White');
@@ -237,7 +275,10 @@ export class FragmentCallbacksImplementation implements AndroidFragmentCallbacks
237275

238276
const entry = this.entry;
239277
if (!entry) {
240-
Trace.error(`${fragment}.onDestroy: entry is null or undefined`);
278+
// A fragment that was never bound to an entry, or whose entry has already been
279+
// discarded, is still destroyed by the FragmentManager. Trace.error routes to the error
280+
// handler, which rethrows and turns this teardown into a fatal exception.
281+
Trace.write(`${fragment}.onDestroy: entry is null or undefined`, Trace.categories.NativeLifecycle, Trace.messageType.error);
241282

242283
return null;
243284
}
@@ -270,7 +311,7 @@ export class FragmentCallbacksImplementation implements AndroidFragmentCallbacks
270311
const hasRemovingParent = fragment.getRemovingParentFragment();
271312

272313
if (hasRemovingParent) {
273-
this.backgroundBitmap = this.loadBitmapFromView(this.frame.nativeViewProtected);
314+
this.backgroundBitmap = this.loadBitmapFromView(this.frame?.nativeViewProtected);
274315
}
275316
} finally {
276317
superFunc.call(fragment);
@@ -279,7 +320,16 @@ export class FragmentCallbacksImplementation implements AndroidFragmentCallbacks
279320

280321
@profile
281322
public onResume(fragment: org.nativescript.widgets.FragmentBase, superFunc: Function): void {
282-
const frame = this.entry.resolvedPage.frame;
323+
const frame = this.entry?.resolvedPage?.frame;
324+
if (!frame) {
325+
// Stale fragment the FragmentManager is still driving after its entry (or page) was
326+
// discarded: there is no navigation left to complete, and dereferencing the missing page
327+
// would throw across the JNI boundary.
328+
superFunc.call(fragment);
329+
330+
return;
331+
}
332+
283333
// on some cases during the first navigation on nested frames the animation doesn't trigger
284334
// we depend on the animation (even None animation) to set the entry as the current entry
285335
// animation should start between start and resume, so if we have an executing navigation here it probably means the animation was skipped
@@ -288,7 +338,7 @@ export class FragmentCallbacksImplementation implements AndroidFragmentCallbacks
288338
const weakRef = new WeakRef(this);
289339
setTimeout(() => {
290340
const owner = weakRef.get();
291-
if (!owner) {
341+
if (!owner || !owner.entry) {
292342
return;
293343
}
294344
if (!owner.entry.isAnimationRunning) {

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { getAppMainEntry } from '../../application/helpers-common';
1616
import { AndroidActivityBackPressedEventData, AndroidActivityNewIntentEventData, AndroidActivityRequestPermissionsEventData, AndroidActivityResultEventData } from '../../application/application-interfaces';
1717
import { Application } from '../../application/application';
1818
import { isEmbedded, setEmbeddedView } from '../embedding';
19-
import { CALLBACKS, FRAMEID, framesCache, setFragmentCallbacks } from './frame-helper-for-android';
19+
import { CALLBACKS, detachFragmentCallbacks, FRAMEID, framesCache, removeFragmentIfAdded, setFragmentCallbacks } from './frame-helper-for-android';
2020
import { SDK_VERSION } from '../../utils';
2121

2222
export * from './frame-common';
@@ -459,8 +459,18 @@ export class Frame extends FrameBase {
459459
public _removeEntry(removed: BackstackEntry): void {
460460
super._removeEntry(removed);
461461

462-
if (removed.fragment) {
462+
const fragment = removed.fragment;
463+
if (fragment) {
463464
_clearEntry(removed);
465+
466+
// The entry is gone now - its page was torn down and resolvedPage cleared - but the
467+
// fragment can still be sitting in the FragmentManager: navigation only evicts fragments
468+
// sharing the frame's current container, so anything left over from an activity
469+
// recreation or a frame reset survives. Cut it loose here, or the FragmentManager keeps
470+
// driving it through the lifecycle against a dead entry.
471+
detachFragmentCallbacks(fragment);
472+
removeFragmentIfAdded(fragment);
473+
464474
removed.fragment = null;
465475
}
466476

0 commit comments

Comments
 (0)