fix(android): override user interface style

[AbdullahFaqeir]

• A PR fixes Android: overrideUserInterfaceStyle makes issues changing it multiple times #13825

Fixes an unexpected behaviour when trying to change the dark/light mode multiple times which causes the screen to go blank and made the change between both animated.

[m1ga]

The last changes make the app restart correctly now 👍 Which makes me wonder: are the other changes needed at all? Do we need a default windowAnimationStyle ?

At some point we'll need to find the source why it is behaving like this after 4-5 clicks but at least it will show the app again.

Would love to have some community members to test this as there are some overrideUserInterfaceStyle users out there.

[AbdullahFaqeir]

I'll explain why it's happening when I get on my laptop

[AbdullahFaqeir]

@m1ga
The whole story begins here

titanium-sdk/android/titanium/src/java/org/appcelerator/titanium/TiRootActivity.java

Line 171 in abbd387

if (this.isDuplicateInstance) {

At some point while clicking, the activity gets duplicated, which is the root activity of the app, when going inside this condition, we lose the state of the activity.

[AbdullahFaqeir]

@m1ga Here's what's happening.

I assumed that since you keep clicking on the button and internally the activity is being recreated which in turn starts the while life cycle of the activity which is the root activity, at some point, the root activity gets duplicated. which leads us to the following

titanium-sdk/android/titanium/src/java/org/appcelerator/titanium/TiRootActivity.java

Lines 154 to 174 in 50d8604

	// Determine if a Titanium root activity already exists.
	// Only 1 root activity is allowed at a time to host the one and only Titanium JavaScript runtime.
	// Note: Android will create a new activity instance if intent is different than last activity's intent.
	TiApplication tiApp = getTiApp();
	TiRootActivity rootActivity = tiApp.getRootActivity();
	this.isDuplicateInstance = (rootActivity != null);
	// Determine if this activity is being restored. This will happen when:
	// - Developer option "Don't keep activities" is enabled and user navigated back to this activity.
	// - The OS forced-quit this app due to low memory or system's "Background process limit" was exceeded.
	boolean isNotRestoringActivity = (savedInstanceState == null);
	// Determine if this activity was created via startActivityForResult().
	// In this case, this activity needs to respond with setResult() and finish().
	boolean isActivityForResult = (getCallingActivity() != null);
	// Handle the duplicate root activity instance case. (Only 1 is allowed at a time.)
	if (this.isDuplicateInstance) {
	// Call this instance's Activity.onCreate() method, bypassing TiBaseActivity.onCreate() method.
	activityOnCreate(savedInstanceState);

if you go over the comments, you'll see at the very last line before calling activityOnCreate() that is bypassing the TiBaseActivity.onCreate() thus, after jumping to specially the TiBaseActivity.onCreate(), we'll see the real effect of what happened.

See below:

titanium-sdk/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java

Lines 685 to 696 in 50d8604

	// If activity is being recreated/restored, then copy last saved intent extras.
	// This is needed to acquire the Ti.UI.Window proxy ID this activity should be assigned to.
	if ((this.launchIntent != null) && (savedInstanceState != null)) {
	Bundle oldExtras = savedInstanceState.getBundle("tiLaunchIntentExtras");
	if (oldExtras != null) {
	Bundle newExtras = this.launchIntent.getExtras();
	if (newExtras != null) {
	oldExtras.putAll(newExtras);
	}
	this.launchIntent.putExtras(oldExtras);
	}
	}

By bypassing the TiBaseActivity.onCreate() we are stepping over the bunch of code that actually recreate the window and brings back whatever views the window had in it.

We ara stepping over this as well

titanium-sdk/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java

Lines 722 to 725 in 50d8604

	// Create the root content layout, if not done already.
	if (layout == null) {
	layout = createLayout();
	}

And I believe many other things that would cause the window/activity to lose it's state.

I don't know if all of this makes since to you!, let me know

[AbdullahFaqeir]

@m1ga any updates on this one?

[m1ga]

@AbdullahFaqeir as mentioned on Slack: I'll take a look at it again at the weekend.

And I think we should just use the one line from the last commit as that was fixing the issue, not the other default parts. But I'll test all at the weekend again and give more feedback

[m1ga]

Ok, I'll tested it again and I think we can reduce this PR to just one line:
just adding getTiApp().setRootActivity(null); before:
https://github.com/tidev/titanium-sdk/blob/master/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L1242

fixes the issue already. That would reduces the possible site effects as we don't change the XML or anything else in the BaseActivity besides the part inside the if condition (which is only triggered when you change the interface style)

[AbdullahFaqeir]

@m1ga
I've created a new method to handle the recreation because there's another places where the activity is recreated for the style https://github.com/tidev/titanium-sdk/blob/e1212e9fccd79f5c302a8380157288df356081da/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L1259C4-L1259C33

[m1ga]

I saw that but it didn't go into that method so it wasn't needed there in my tests. It was only needed in the one place.

[AbdullahFaqeir]

@m1ga it would in other cases, I'm not only fixing your case, I'm doing something to handle all cases in a proper way.

[m1ga]

I totally understand that 👍 Just want to make sure that the setTheme parts or the default transition will not introduce any change will make a default app behave different with 12.6.0 compared to 12.5.0.GA. E.g. in the onCreate you set setTheme(selectedTheme); which is R.style.Theme_Titanium_Light. But what if the user sets a custom theme in tiapp.xml or js? Will that work correctly? I didn't notice any difference with or without it, so if you have a test case where it is needed I would be very happy to test it.

The updateActivity() part works great