Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(android): Refactored activity/fragment restore handling #10829

Merged
merged 19 commits into from Apr 26, 2019

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Apr 6, 2019

JIRA:

Summary:

  • TIMOB-26966 Fixed bug where back navigating from child window causes app to exit when "Don't keep activities" is enabled as of 7.2.1.
  • TIMOB-26914 Fixed 8.0.0 regression where an OS forced quit app displays a blank window upon relaunch. (Was incorrectly restoring child activity.)
  • TIMOB-17089 Resolved view ID conflicts causes app crashes when activity/fragment is restored.
  • TIMOB-26964 Fixed bug where TabGroup would be blank (no tabs) when calling child window's close() method while "Don't keep activities" is enabled.
  • TIMOB-26890 Fixed bug where tapping 4th tab in TabGroup can cause a crash due to view ID conflict during fragment restore.
  • TIMOB-26975 Fixed bug where LiveView does not restart app if "Don't keep activities" is enabled as of 8.0.0.
  • TIMOB-26978 Added several safety mechanisms to AlertDialog and ProgressIndicator dialog code to avoid crashes when activity is destroyed, but not finished. (Can happen when "Don't keep activities" is enabled.)
  • TIMOB-15829 Fixed crash caused by progress indicator set up on the status bar if shown after hiding twice.
  • TIMOB-26989 Fixed crash caused by restoring an activity having a TableView with a SearchBar or SearchView attached to it. (Proxies should not be released unless activity is finished.)
  • TIMOB-26996 Fixed bug where Window/TabGroup close() method call was ignored until "open" event was fired.
  • Increased TabGroup's ViewPager offscreen page limit from 1 to 128 to avoid tab fragments from being destroyed/restored.

Test Restore After OS Force Quit:

  1. On Android, go to its "Developer Options" screen.
  2. Tap on the "Background process limit" row. (It's near the bottom of the list.)
  3. Tap on "No background processes". (This tells the OS to force quit the last displayed app when launching another app.)
  4. Build and run the below code on the same Android device.
  5. In the app, tap on the "Show Child Window" button.
  6. Press the Android "Home" button.
  7. Tap on another app such as "Gmail".
  8. Press the Android "Home" button.
  9. Resume the Titanium app.
  10. Verify that the app restarts, starting with the splash screen. (It will launch with a blank window, but quickly restart. Unfortunately, we are stuck with this behavior.)
  11. Change device's "Background process limit" setting back to "Standard".
var window = Ti.UI.createWindow();
var openButton = Ti.UI.createButton({ title: "Show Child Window" });
openButton.addEventListener("click", function(e) {
	var childWindow = Ti.UI.createWindow({ backgroundColor: "orange" });
	var closeButton = Ti.UI.createButton({ title: "Close Me" });
	closeButton.addEventListener("click", function(e) {
		childWindow.close();
	});
	childWindow.add(closeButton);
	childWindow.open();
});
window.add(openButton);
window.open();

Test Intent ACTION_VIEW with "Don't keep activities":

  1. Go to the Android device's main "Settings" screen.
  2. Tap on "System" under "Settings.
  3. Tap on "Developer options" under "System" settings.
  4. Enable "Don't keep activities", which should be near the bottom of the list.
  5. Build and run the below code on the above Android device.
  6. Tap the "Send Intent" button.
  7. Verify that a screenshot of the "Send Intent" button is shown in a new window.
  8. Tap the Android "Home" button.
  9. Resume the app.
  10. Verify that the screenshot is still shown.
  11. Tap the Android "Back" button.
  12. Verify that the it navigated back to the Titanium app with the "Send Intent" button. (Should no longer exit the app, which was the bug.)
  13. Tap the Android "Back" button.
  14. Verify that it exits the app. You should not see a splash screen.
var window = Ti.UI.createWindow();
var button = Ti.UI.createButton({
	title: "Send Intent",
});
button.addEventListener("click", function(e){
	button.toImage(function(imageBlob) {
		if (!imageBlob) {
			return;
		}
		var targetFile = Ti.Filesystem.getFile(Ti.Filesystem.applicationDataDirectory, "image.png");
		targetFile.write(imageBlob, false);
		var intent = Ti.Android.createIntent({
			action: Ti.Android.ACTION_VIEW,
			data: targetFile.nativePath,
			type: "image/png",
		});
		intent = Ti.Android.createIntentChooser(intent, "Choose");
		Ti.Android.currentActivity.startActivity(intent);
	});
});
window.add(button);
window.open();

Test TabGroup with "Don't keep activities":

  1. Go to the Android device's main "Settings" screen.
  2. Tap on "System" under "Settings.
  3. Tap on "Developer options" under "System" settings.
  4. Enable "Don't keep activities", which should be near the bottom of the list.
  5. Build and run the below code on the above Android device.
  6. Tap on the "Open Child Window" button.
  7. Tap on the "Close Window" button.
  8. Verify that you are returned back to a 3 tab window. (Should not be blank, which was the bug.)
function createTab(title) {
	var window = Ti.UI.createWindow({ title: title });
	window.add(Ti.UI.createLabel({
		text: title + " View",
		top: "25%",
	}));
	var openButton = Ti.UI.createButton({
		title: "Open Child Window",
		top: "50%",
	});
	openButton.addEventListener("click", function() {
		var childWindow = Ti.UI.createWindow({
			title: title + " Child Window",
			backgroundColor: "white",
		});
		var closeButton = Ti.UI.createButton({
			title: "Close Window",
		});
		closeButton.addEventListener("click", function() {
			childWindow.close();
		});
		childWindow.add(closeButton);
		childWindow.open();
	});
	window.add(openButton);
	var tab = Ti.UI.createTab({
		title: title,
		window: window,
	});
	return tab;
}

var tabGroup = Ti.UI.createTabGroup({
	title: "TabGroup Parent Test",
	tabs: [createTab("Tab 1"), createTab("Tab 2"), createTab("Tab 3")],
});
tabGroup.open();

Test LiveView Restarts with "Don't keep activities":

  1. Go to the Android device's main "Settings" screen.
  2. Tap on "System" under "Settings.
  3. Tap on "Developer options" under "System" settings.
  4. Enable "Don't keep activities", which should be near the bottom of the list.
  5. Build and run the below code on the above Android device.
  6. Edit the "app.js" file and save your changes.
  7. Verify that the app restarts and that the below Ti.API.info() message is logged again.
Ti.API.info("### The 'app.js' file was loaded.");
var window = Ti.UI.createWindow();
window.open();

- [TIMOB-26966] Fixed bug where back navigating from child window causes app to exit when "Don't keep activities" is enabled as of 7.2.1.
- [TIMOB-26914] Fixed 8.0.0 regression where an OS forced quit app displays a blank window upon relaunch. (Was incorrectly restoring child activity.)
- [TIMOB-17089] Resolved view ID conflicts causes app crashes when activity/fragment is restored.
- [TIMOB-26964] Fixed bug where TabGroup would be blank (no tabs) when calling child window's close() method while "Don't keep activities" is enabled.
- [TIMOB-26890] Fixed bug where tapping 3rd tab in TabGroup can cause a crash due to view ID conflict during fragment restore.
- Increased TabGroup's ViewPager offscreen page limit from 1 to 128 to avoid tab fragment from being destroyed/restored.
- Added several safety mechanisms to AlertDialog and ProgressIndicator dialog code to avoid crashes.
@build
Copy link
Contributor

build commented Apr 6, 2019

Fails
🚫 Tests have failed, see below for more information.
Messages
📖

💾 Here's the generated SDK zipfile.

📖 ❌ 1 tests have failed There are 1 tests failing and 462 skipped out of 3647 total tests.

Tests:

Classname Name Time Error
android.emulator.Titanium.UI.WebView .zoomLevel 1.244 expected 0.25 to equal 1

Generated by 🚫 dangerJS against da04a2f

jquick-axway and others added 7 commits April 5, 2019 22:36
…hin a fragment such as "ti.map" view

- Re-added ID assigned to TiUIFragment's view, but it's now guaranteed to be assigned a unique ID.
…f "Don't keep activities" is enabled as of 8.0.0
- [TIMOB-15829] Fixed crash caused by progress indicator set to status bar that happens after hiding twice and then opening.
@ypbnv
Copy link
Contributor

ypbnv commented Apr 12, 2019

@jquick-axway TabGroup changes look good to me!

@build
Copy link
Contributor

build commented Apr 15, 2019

Messages
📖

💾 Here's the generated SDK zipfile.

📖

✅ All tests are passing
Nice one! All 3797 tests are passing.
(There are 466 tests skipped)

Generated by 🚫 dangerJS against 94fc91c

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a minor comment above.

Since onBackPressed has been refactored, QE should also run tests from these tickets again just to be safe:
https://jira.appcelerator.org/browse/TIMOB-24531
https://jira.appcelerator.org/browse/TIMOB-23969
https://jira.appcelerator.org/browse/TIMOB-23989

@@ -1726,6 +1684,22 @@ protected boolean shouldFinishRootActivity()
return TiBaseActivity.canFinishRoot && isIntentRequestingFinishRoot;
}

@Override
public void finishAfterTransition()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this override necessary, does't finishAfterTransition call finish?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The override is needed to call the removeFromActivityStack(this) like we do in the finish() method... because I removed the method call from the WindowProxy class. Now all of the addToActivityStack() and removeFromActivityStack() calls are done in the same place, the TiBaseActivity class. The old code wants to remove the activity from the stack (before the onDestroy() happens) so that the next child Ti.UI.Window that the code might open won't use its context.

@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented Apr 24, 2019

@jquick-axway , For https://jira.appcelerator.org/browse/TIMOB-24531.
Test code here : #8906 (comment)
When I open the second window with exitOnClose set to true & press back I see the first window for like a second.
With 8.0.0 I don't see the first window at all. It directly quits.

All of the other tests seem to be fine.

@garymathews
Copy link
Contributor

@jquick-axway To address the issue @lokeshchdhry found, should we change the direction we terminate the activity stack?

https://github.com/appcelerator/titanium_mobile/pull/10829/files#diff-0d6c63e01d569acf469b905d84a1dacbR188

Terminate the child activities before the current?

- [TIMOB-26966] Fixed bug where Window/TabGroup close()method call is ignored until "open" event has been fired.
  * Resolved by modifying proxy's handleClose() to call TiActivityWindows.remove() immediately.
- Fixed bug where dynamically changing "exitOnClose" would be ignored.
  * Flag was only being read by activity's "launchIntent" in 8.0.0.
  * Now reads directly from proxy, which is the best solution. Especially if onNewIntent() was called. (Old code would have had a problem too.)
- Re-added support for "exitOnClose" in child windows. (Was only supported by root window.)
- Removed window exit animation if its "exitOnClose" was set true. (Prevents parent window briefly appearing upon exit.)
- Added more dialog safe-guards.
  * Would crash if failed to create dialog builder if there are no activities available.
  * Can happen if "Don't keep activities" is enabled, you press the Home button, and code attempts to show dialog in the background.
@jquick-axway
Copy link
Contributor Author

jquick-axway commented Apr 26, 2019

Updated PR.

@garymathews, I resolved the issue @lokeshchdhry found in WindowProxy.handleClose(). The reason the parent window was being shown for a brief moment was because the window was doing an exit animation. The trick was not to call finishAfterTransition() if "exitOnClose" is set true.

@ssjsamir
Copy link
Contributor

@jquick-axway Please could you look at the merge conflict?

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: PASS

@lokeshchdhry
Copy link
Contributor

FR Passed.

The activity/fragment refactor looks good.

Studio Ver: 5.1.2.201903111843
SDK Ver: 8.1.0 local build
OS Ver: 10.14
Xcode Ver: Xcode 10.1
Appc NPM: 4.2.13
Appc CLI: 7.0.10
Daemon Ver: 1.1.3
Ti CLI Ver: 5.1.1
Alloy Ver: 1.13.9
Node Ver: 8.15.1
NPM Ver: 6.4.1
Java Ver: 10.0.2
Devices: ⇨ google Nexus 5 (Android 6.0.1)
⇨ google Pixel (Android 9)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants