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): Resuming with intent "FLAG_ACTIVITY_MULTIPLE_TASK" can hang the app #11080

Merged
merged 4 commits into from Aug 20, 2019

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Jul 26, 2019

JIRA:
https://jira.appcelerator.org/browse/TIMOB-27271

Summary:

  • Modified to effectively restart app when the following intent flags are set:
    • FLAG_ACTIVITY_MULTIPLE_TASK
    • FLAG_ACTIVITY_NEW_DOCUMENT
  • Above flags make it impossible to resume existing root activity. Android OS will always create a new activity instance for matching intent when using these flags.
  • Modified "resume" intent to use main/launcher flags.
    • We don't want to resume with FLAG_ACTIVITY_CLEAR_TOP or other destructive flags because they will destroy existing app's child activities.

Test:

  1. Create a classic Titanium app.
  2. Set project name to "IntentTest". (This is <name/> in "tiapp.xml".)
  3. Set project's "Application Id" to "com.appc.intent.test". (This is <id/> in "tiapp.xml".)
  4. Set up the "app.js" with the below code.
  5. Build and run on Android.
  6. Open the Mac "Terminal".
  7. CD (change directory) to: ~/Library/Android/sdk/platform-tools
  8. In the terminal, enter the following...
    ./adb shell am start -n com.appc.intent.test/.IntenttestActivity -a android.intent.action.VIEW -d https://www.appcelerator.com -f 0x08000000
  9. Verify that the app restarts.
  10. Verify displayed "Launch Intent" has "data" value set to "https://www.appcelerator.com".
  11. In the terminal, enter the following...
    ./adb shell am start -n com.appc.intent.test/.IntenttestActivity -a android.intent.action.VIEW -d https://www.google.com
  12. Verify that "Launch Intent" label changes to "New Intent". (App should not restart.)
  13. Verify "New Intent" has "data" value set to "https://www.google.com".
  14. Tap on the Android "Home" button. (Do NOT press Back button.)
  15. In the terminal, enter the following...
    ./adb shell am start -n com.appc.intent.test/.IntenttestActivity -a android.intent.action.VIEW -d https://www.axway.com
  16. Verify that the app is resumed and in the foreground.
  17. Verify displayed "New Intent" has "data" value set to "https://www.axway.com".
Ti.Android.rootActivity.addEventListener("newintent", function(e) {
	label.text = "New Intent:\n" + JSON.stringify(e.intent, null, 4);
	Ti.API.info("@@@ newintent: " + JSON.stringify(e.intent));
});

var window = Ti.UI.createWindow();
var scrollView = Ti.UI.createScrollView({
	scrollType: "vertical",
	width: Ti.UI.FILL,
	height: Ti.UI.FILL,
});
var label = Ti.UI.createLabel({
	text: "Launch Intent:\n" + JSON.stringify(Ti.Android.rootActivity.intent, null, 4),
	width: Ti.UI.FILL,
	height: Ti.UI.SIZE,
});
scrollView.add(label);
window.add(scrollView);
window.open();

…hang the app

- Modified to effectively restart the app when the following intent flags are set:
  * FLAG_ACTIVITY_MULTIPLE_TASK
  * FLAG_ACTIVITY_NEW_DOCUMENT
- Modified resume code to use main/launch intent flags.
  * We don't want flags such as FLAG_ACTIVITY_CLEAR_TOP to destroy child activities when resuming app.
@jquick-axway jquick-axway added this to the 8.2.0 milestone Jul 26, 2019
@build build requested a review from a team July 26, 2019 02:06
@build
Copy link
Contributor

build commented Jul 26, 2019

Warnings
⚠️

Commit f90dfd39205bb84135c8d21fa0903c4db402b009 has a message "fix(android): Resuming with intent "FLAG_ACTIVITY_MULTIPLE_TASK" can hang the app

  • Modified to effectively restart the app when the following intent flags are set:
    • FLAG_ACTIVITY_MULTIPLE_TASK
    • FLAG_ACTIVITY_NEW_DOCUMENT
  • Modified resume code to use main/launch intent flags.
    • We don't want flags such as FLAG_ACTIVITY_CLEAR_TOP to destroy child activities when resuming app." giving 2 errors:
  • header must not be longer than 72 characters, current length is 81
  • subject must not be sentence-case, start-case, pascal-case, upper-case
⚠️

Commit 62de13fd224a3cbec2f0de400538ac727988c56d has a message "fix(android): Updated code comments for [TIMOB-27271] change" giving 1 errors:

  • subject must not be sentence-case, start-case, pascal-case, upper-case
Messages
📖

💾 Here's the generated SDK zipfile.

📖

✅ All tests are passing
Nice one! All 3728 tests are passing.
(There are 470 tests skipped)

📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

Generated by 🚫 dangerJS against 19e4f65

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

@keerthi1032
Copy link
Contributor

FR Passed. NO hanging when resuming app app with an intent having FLAG_ACTIVITY_MULTIPLE_TASK. Works as Expected. Waiting for the 8.1.0 GA release to merge this PR

Test Environment:
Operating System
Name = Mac OS X
Version = 10.14.5
Architecture = 64bit
Node.js
Node.js Version = 10.16.0
npm Version = 6.9.0
Titanium CLI
CLI Version = 5.2.1
Titanium SDK
SDK Version =local sdk 8.1.0.v20190725194646 and 8.2.0.v20190729101453
Device -Samsung s5 android 6, Oneplus 5T android 9
Emulator - 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

5 participants