Skip to content

Commit 0d451fc

Browse files
herefishyfishherefishyfish
committed
fix(android): limit root view actions to the activity that shows it
resetActivityContent and both back press paths used the cached root view without checking which activity shows it. An activity that lost the view could unload it or run its back navigation while another activity showed it. They now use the same check as onStop. A new activity inherits a root view only from a finishing primary window. A closing secondary window no longer gives its view away. _findSurvivingNativeScriptActivity uses one find over the windows. The specs cover a stale activity that gets other content or a back press, a finishing secondary window, and a new activity that starts before the old one stops. The spec for a root given back to the activity that shows it is removed, because production code cannot reach that state.
1 parent 1985bf8 commit 0d451fc

3 files changed

Lines changed: 56 additions & 24 deletions

File tree

‎packages/core/application/application.android.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -738,10 +738,7 @@ export class AndroidApplication extends ApplicationCommon implements IAndroidApp
738738
return undefined;
739739
}
740740

741-
return this._getWindows()
742-
.filter((nativeWindow) => nativeWindow.state === 'attached')
743-
.map((nativeWindow) => (nativeWindow as AndroidNativeWindow).activity)
744-
.find((candidate) => candidate && candidate !== destroyed && !candidate.isFinishing() && 'isNativeScriptActivity' in candidate);
741+
return (this._getWindows() as AndroidNativeWindow[]).find((w) => w.state === 'attached' && w.activity && w.activity !== destroyed && !w.activity.isFinishing() && 'isNativeScriptActivity' in w.activity)?.activity;
745742
}
746743

747744
/**

‎packages/core/ui/frame/activity-callbacks.android.spec.ts‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ function createRootView(context: object | null, isLoaded = true) {
3030
order.push('setup');
3131
view._context = activity;
3232
},
33+
_onRootViewReset() {
34+
order.push('reset');
35+
},
36+
_getFragmentManager: () => ({ executePendingTransactions() {} }),
3337
_tearDownUI: vi.fn(),
3438
};
3539

@@ -114,6 +118,23 @@ describe('activity callbacks with a root view shared between activities', () =>
114118
expect(order).toEqual(['unloaded', 'setup', 'loaded']);
115119
expect(view._context).toBe(stale);
116120
});
121+
122+
it('leaves the root alone when it is given other content', () => {
123+
const { view, order } = createRootView(current);
124+
const { view: replacement } = createRootView(null, false);
125+
126+
createCallbacks(view).resetActivityContent(stale as any, replacement as any);
127+
128+
expect(order).toEqual([]);
129+
});
130+
131+
it('passes a back press to the system', () => {
132+
const { view } = createRootView(current);
133+
134+
createCallbacks(view).onBackPressed(stale, superFunc);
135+
136+
expect(superFunc).toHaveBeenCalledTimes(1);
137+
});
117138
});
118139

119140
describe('the activity showing the root', () => {
@@ -142,14 +163,6 @@ describe('activity callbacks with a root view shared between activities', () =>
142163

143164
expect(order).toEqual(['loaded']);
144165
});
145-
146-
it('does not unload a root it is given again', () => {
147-
const { view, order } = createRootView(showing);
148-
149-
createCallbacks().resetActivityContent(showing as any, view as any);
150-
151-
expect(order).toEqual(['setup', 'loaded']);
152-
});
153166
});
154167

155168
it('unloads a root still loaded in another activity before setting it up', () => {
@@ -160,6 +173,20 @@ describe('activity callbacks with a root view shared between activities', () =>
160173
expect(order).toEqual(['unloaded', 'setup', 'loaded']);
161174
});
162175

176+
it('keeps the root in the new activity when the old one stops and is destroyed afterwards', () => {
177+
const old = createActivity(true);
178+
const next = createActivity();
179+
const { view, order } = createRootView(old);
180+
181+
createCallbacks(view).onStart(next, superFunc);
182+
createCallbacks(view).onStop(old, superFunc);
183+
createCallbacks(view).onDestroy(old, superFunc);
184+
185+
expect(order).toEqual(['unloaded', 'setup', 'loaded']);
186+
expect(view._tearDownUI).not.toHaveBeenCalled();
187+
expect(view._context).toBe(next);
188+
});
189+
163190
it('passes a back press to the system when the activity has no root yet', () => {
164191
createCallbacks().onBackPressed(createActivity(), superFunc);
165192

@@ -183,22 +210,29 @@ describe('activity callbacks with a root view shared between activities', () =>
183210
const finishing = createActivity(true);
184211
const { view } = createRootView(finishing);
185212

186-
expect(createWith([{ activity: finishing, rootView: view }])).toBe(view);
213+
expect(createWith([{ activity: finishing, rootView: view, isPrimary: true }])).toBe(view);
214+
});
215+
216+
it('leaves the root of a finishing secondary window alone', () => {
217+
const finishing = createActivity(true);
218+
const { view } = createRootView(finishing);
219+
220+
expect(createWith([{ activity: finishing, rootView: view, isPrimary: false }])).toBeUndefined();
187221
});
188222

189223
it('leaves the root of a live window alone', () => {
190224
const live = createActivity();
191225
const { view } = createRootView(live);
192226

193-
expect(createWith([{ activity: live, rootView: view }])).toBeUndefined();
227+
expect(createWith([{ activity: live, rootView: view, isPrimary: true }])).toBeUndefined();
194228
});
195229

196230
it('stays empty when a resolver has taken over the content', () => {
197231
vi.spyOn(Application, 'getWindowContentResolver').mockReturnValue(() => null);
198232
const finishing = createActivity(true);
199233
const { view } = createRootView(finishing);
200234

201-
expect(createWith([{ activity: finishing, rootView: view }])).toBeUndefined();
235+
expect(createWith([{ activity: finishing, rootView: view, isPrimary: true }])).toBeUndefined();
202236
});
203237
});
204238
});

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -751,8 +751,8 @@ if (SDK_VERSION >= 33) {
751751
return;
752752
}
753753

754-
const callbacks: AndroidActivityCallbacks = activity[CALLBACKS];
755-
const view = callbacks?.getRootView();
754+
const callbacks: ActivityCallbacksImplementation = activity[CALLBACKS];
755+
const view = callbacks?.getRootViewShownBy(activity);
756756
let callSuper: boolean = !view;
757757

758758
if (view) {
@@ -1009,7 +1009,7 @@ export class ActivityCallbacksImplementation implements AndroidActivityCallbacks
10091009
return;
10101010
}
10111011

1012-
const view = this._rootView;
1012+
const view = this.getRootViewShownBy(activity);
10131013
if (!view) {
10141014
superFunc.call(activity);
10151015

@@ -1095,7 +1095,7 @@ export class ActivityCallbacksImplementation implements AndroidActivityCallbacks
10951095
Application.android.notify(resultArgs);
10961096
}
10971097

1098-
private getRootViewShownBy(activity: any): View | undefined {
1098+
public getRootViewShownBy(activity: any): View | undefined {
10991099
return this._rootView?._context === activity ? this._rootView : undefined;
11001100
}
11011101

@@ -1107,7 +1107,7 @@ export class ActivityCallbacksImplementation implements AndroidActivityCallbacks
11071107
const finishing = Application.android._getWindows().find((nativeWindow) => {
11081108
const candidate = (nativeWindow as AndroidNativeWindow).activity;
11091109

1110-
return candidate && candidate !== activity && candidate.isFinishing() && nativeWindow.rootView;
1110+
return candidate && candidate !== activity && candidate.isFinishing() && nativeWindow.isPrimary && nativeWindow.rootView;
11111111
});
11121112

11131113
return finishing?.rootView;
@@ -1120,13 +1120,14 @@ export class ActivityCallbacksImplementation implements AndroidActivityCallbacks
11201120
}
11211121

11221122
public resetActivityContent(activity: androidx.appcompat.app.AppCompatActivity, view?: View): void {
1123-
if (this._rootView) {
1124-
const manager = this._rootView._getFragmentManager();
1123+
const shownRootView = this.getRootViewShownBy(activity);
1124+
if (shownRootView) {
1125+
const manager = shownRootView._getFragmentManager();
11251126
manager.executePendingTransactions();
11261127

11271128
// Some flavors reuse the same root view, so unload the view in order to load it successfully when needed
1128-
this._rootView.callUnloaded();
1129-
this._rootView._onRootViewReset();
1129+
shownRootView.callUnloaded();
1130+
shownRootView._onRootViewReset();
11301131
}
11311132
// Delete previously cached root view in order to recreate it.
11321133
this._rootView = null;

0 commit comments

Comments
 (0)