fix(android): prevent BaseActivity to stay with exitOnClose:false

[m1ga]

Fixes #13889

A better attempt to fix the isse where the splashscreens stays open when you close two windows quickly and the root window has exitOnClose:false.

var win1 = Ti.UI.createWindow({
	backgroundColor: "green",
	exitOnClose: false
})

Ti.App.addEventListener("pause", function(){console.log("---pause---");})
Ti.App.addEventListener("resume", function(){console.log("---resume---");})
win1.addEventListener("close", function(){console.log("---close---");})
win1.addEventListener("open", function(){console.log("---open---");})

win1.activity.onCreate = () => { console.log("activity create"); }
win1.activity.onPause = () => { console.log("activity pause"); }
win1.activity.onDestroy = () => { console.log("activity destroy"); }

win1.addEventListener("click", function() {
	var win2 = Ti.UI.createWindow({
		backgroundColor: "red"
	})
	win2.open();
})
win1.open();

The TiActivityWindows.getWindowCount() check is already used in TiBaseActivity to check if the app should be put into the background (search for onBackPressed: suspend to background). In this case we actually need to finish it otherwise it will be in a non resumable state.

12.6.0

Bildschirmaufnahme_20241219_095358.webm

this PR

Bildschirmaufnahme_20241219_095239.webm

Different approach:
The Back button event is already fired before the first "remove from stack" is finished and that is why it still has 2 windows instead of 1 and runs another "remove from stack" instead of putting it to the background.
I can also block the "back event" until https://github.com/tidev/titanium-sdk/blob/master/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L1662 is done and then enable it again. This will stay at the green window when you close both quickly.

Bildschirmaufnahme_20241218_221941.webm

But to me it looks more like a bug if I want to close two windows quickly I want to be closed and not stop at the parent window.

@prashantsaini1: would love to get your feedback on this approach

[prashantsaini1]

@m1ga I assume the changes in this commit are not for the Different approach: you shared, but for the above description. For this change also, can you check if all of window's events close, blur, focus + activity's onPause, onStop, onDestroy are triggered correctly because I feel this change might skip a few of them?

[m1ga]

Yes, this is only the first "close app" case as I think it is more useful instead of blocking the back event.

All events are fired the same way as they are in 12.6.0, I've added some in the example above. Only difference: 12.6.0 will say "activity destroy, --close--" but will stay at the splashscreen. With my PR it still says the same but will actually close the app.

[prashantsaini1]

@m1ga I think this will need some additional condition checks for exitOnClose.

• For exitOnClose: false window, though it prevents to show splash screen, but it will close entire activity stack too which is not intended.
• Also, for exitOnClose: true window, it may not dispose JS runtime properly as it's terminating stack without checking whether JS script is running or not.

We need more testing on this change as it can break the behaviour.

[m1ga]

The idea here is to close the window in the exitOnClose:false state because we've ended up in a place where we shouldn't be. So I don't respect the false part and close the app so it has to restart next time. This is better then it's being stuck at the splashscreen.
If you close the windows in a normal speed it won't end up in that if case.

exitOnClose: true shouldn't change with the PR as it won't go into that if condition.

[prashantsaini1]

• That's exactly I meant in my 1st point in last comment. exitOnClose: false won't work as intended with this PR.

There are likely two scenarios an app would want to do on back-press:

1. Do not exit - we set exitOnClose: false on first window and open home-intent on its back button press.
   • This would surely ignore enableOnBackInvokedCallback which we can improve in future releases.
2. Exit - it's already the default behaviour now since we fixed the exitOnClose default value in this PR.

Or is there any other case I am missing here that you are trying to achieve? 😊

[m1ga]

Currently with exitOnClose: false it will put the app to the background and not close it when you "close" the last window. This works fine unless you close two windows quickly. Then it is not putting the app to the background but stays at the splashscreen/base actitvity. Check the demo code + videos.

This is the whole issue I'm trying to fix here by "force closing" the app when we end up in that state. So it will be the same as exitOnClose:true for this edge case.

The issue is a due to the system close animations and closing it quicker before the system close animation is done and everything is cleaned up.
Normally it will end up in a state where you have 1 window in the stack and Ti will put it to the background because of exitOnClose:false. But when you do it quickly it will end up with a zero window stack and it doesn't know what to do here.

The video under Different approach: will show you another way where I've blocked the "back" until the animation is done. But for me that looks a bit like the app is not doing what you want it do do.

And to test it you have to have normal window animations enabled on your phone and close the 2nd window + 1st window very quickly 😄 I think it's an edge case but since Ti is that performant that you can close them so quickly we should add the fix for it.

[prashantsaini1]

Taking an excerpt from your last comment:

Currently with exitOnClose: false it will put the app to the background and not close it when you "close" the last window. This works fine unless you close two windows quickly. Then it is not putting the app to the background but stays at the splashscreen/base actitvity.

• With your PR, will it still put apps to background without closing when back-press happens at normal speed?
  • If yes, then this PR is probably fine.
  • If not, then it introduce a breaking change as users won't know that their app will be closed in an edge case.

Another solution: Rather terminating the app, we launch the home-intent in this case in SDK itself, because then it still keeps the same purpose of setting exitOnClose: false, the app won't be killed.

[prashantsaini1]

@m1ga Thanks for confirming the androidback.

Your only concern is now that the app didn't close when you called win1.close(), it's because you have to reset its exitOnClose: true again.

Ideally root Ti.Window (win1) won't be closed programmatically just like that. It will be closed if users really want to open another screen, e.g. signing out the app and opening login/signup screen.

• In general if I want to call win1.close(), either:
  • I want to close the app, so I set exitOnClose: true again on win1 before calling it's close().

win2.addEventListener("close", function(){
   win1.exitOnClose = true; // reset it here and app closes fine just like this PR.
   win1.close();
});

• Or I want to open another screen, which does not need any further changes.

win2.addEventListener("close", function(){
	win1.close();

	var win3 = Ti.UI.createWindow({
		backgroundColor: "teal",
		exitOnClose: false // `false` will put app to background again, `true` will close the app.
	})
	win3.open();
})

To summarise, you have to take care of adding/removing androidback and setting exitOnClose depending on what you want to achieve.

If you want, we can jump over a Slack call and test things out together to see if there's really an edge case we cannot handle with current SDK features (obviously after normal working hours 😊).

[prashantsaini1]

@m1ga would love to hear your further updates if there's still any open issue!

[m1ga]

while the JS code works it's just a workaround that I don't want to add in all apps or have other devs find this out and add it.

As a app dev I just want to set exitOnClose:false and no think about switching it or using androidback. So the issue still exists without this PR.

[prashantsaini1]

I understand your use case which is possible when users presses back button too fast.

My whole point is that this PR is changing the meaning of exitOnClose: false in this edge case as apps will exit anyways.

However, if you still want to take this forward, then I would strongly suggest you to verify following scenarios after the app is terminated in this edge case:

1. Clicking on notification should bring the app to foreground.
2. Sending data to app via intent-filters should be delegated to app as expected.

These scenarios are very common ways to invoke the app and should work same as with current SDK.

We should rather find a proper way to put the app to background no matter how fast the back button is pressed. I will try to give this issue a proper fix in coming weeks (not very soon though).

[m1ga]

Thanks for all the infos. Will test those cases 👍

If we find the real lifecycle issue it would be great! No rush here, no user report so far about this edge case. Thank you for taking the time to answer with all the details 👍