Timob 9285 Android: Message "An application restart is required" fires incorrectly. #2651
Conversation
…/5277: * New tiapp.xml property: ti.android.bug2373.finishfalseroot. We can't just gut the existing behavior, so a new option really was necessary. * If an Titanium developer sets ti.android.bug2373.finishfalseroot to true, then: * * Play Store launches (and other launches coming directly from the installer) will occur without interruptions (no message to restart.) * * If user then puts the app in the background and later goes back to it via the application menu (the steps which would exhibit 2373 behavior), the newly-created (unwanted) TiLaunchActivity will recognize it is not the task root (isTaskRoot() == false) and finish itself immediately. I.e., it'll just go away before it has a chance to display anything, and meanwhile the application -- with its activity stack still in place -- will have come forward. The goal here being that with this new option the user shouldn't notice *anything*.
…ent flags when launching from history, take those into consideration when determining whether to show the Android bug 2373 warnings.
…s not trying to be launched as a launch activity. Verstehst?
|
Hi Bill, FYI, there is a delay between updating the wiki guides and publishing to the docs site. Please update the new wiki space: https://wiki.appcelerator.org/display/guides2/tiapp.xml+and+timodule.xml+Reference If you make changes to the old wiki space: https://wiki.appcelerator.org/display/guides/tiapp.xml+and+timodule.xml+Reference They will not get propagated to the docs site. |
| ((flags & Intent.FLAG_ACTIVITY_RESET_TASK_IF_NEEDED) != Intent.FLAG_ACTIVITY_RESET_TASK_IF_NEEDED) | ||
| && | ||
| ((flags & Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY) != Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY) | ||
| ); | ||
| } |
There was a problem hiding this comment.
This code might start to get ugly if we add more flags in the future. It's also a bit hard to parse.
Maybe store the set of valid launch flags as a static final in the class then just test if any of these
bit flags are set on the intent in checkInvalidLaunch(). This way we just need to append later to this static
final flag set if we need to add more flags.
Define this in the class as a static final:
// Any activity launched with one of these flags will be considered "valid".
VALID_LAUNCH_FLAGS = Intent.FLAG_ACTIVITY_RESET_TASK_IF_NEEDED | Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY;
Goes into checkInvalidLaunch() for testing the intent flags.
// Check if one of the valid launch flags is set.
invalidLaunchDetected = (flags & VALID_LAUNCH_FLAGS) == 0
|
Just one code review comment to address. Will start functional testing once this is done. |
|
I removed the "do not merge" comment from the description of this PR, since, as Arthur pointed out, I actually have a chance to edit the docs before they're actually live. |
|
Change made, ready for re-review. |
|
Seeing issues running KitchenSink. Does not appear to start up properly |
|
I'm not successful trying to reproduce it so far, but Allen also thinks it's the same bug as he's working on, so it makes me think it's probably not related to my PR, but I want to test more. Apparently the "new" KS is quite different now. Are you using that one or the one still under demos/? (The new one: https://github.com/appcelerator-developer-relations/KitchenSink ) Also, I've merged master in to my branch, just in case a recent change buried in there somewhere helps. Can you also try to re-create again? |
|
Never mind, just got it too... Can't remember precisely which sequence of events led to it. :) So if I can now recreate it with master (not my branch), i think it's safe to declare it's not my PR causing it. |
|
Here it is happening on the master branch (b2782e9, which is the current head of master): This wasn't immediately on clicking "open" from the installer, but after a few out-and-ins. |
|
I was able to re-create what Josh mentions when packaging the app. If I packaged the app with master, it didn't happen, but of course there is a huge difference in that case: KS on the master branch (without any special bug 2373 tiapp settings) will force you to re-start the app immediately upon opening. So I added one of our old properties: which, at startup, has the same effect as the new "finishfalseroot". I compiled for distribution using master and that option in the tiapp.xml. And the problem occurs as Josh described. So it's not something specific to my branch. Of course it's something we need to investigate, but the problem is it only happens on demand with a "distribute" build, and the KS build takes so incredibly long, this is really a pain to debug. I made a "smaller KS" with less JS files, and the bug didn't happen. :) Allen's PR re the TabActivity and NPE won't help, because that is only relevant when we back out of the "stuck" KS (stuck meaning an empty tabgroup shows.) It looks more like a race condition (latch logic?) I'll end up creating a new ticket. Meantime, I don't believe this should hold up this ticket/PR. |
|
Happens in 2.1.1 too. :) |
|
Hi Bill. I have updated mobile SDK 2.1.2.v20120731170109-osx downloaded from this link http://builds.appcelerator.com.s3.amazonaws.com/mobile/2_1_X/mobilesdk-2.1.2.v20120731170109-osx.zip and added this property in tiapp.xml < property name="ti.android.bug2373.disableDetection" type="bool">true< / property > But I still can able to see "Application restart required" Dialog after installing app. So if I add following property also in tiapp.xml , , Will this work to avoid "Application restart required" Dialog? < property name="ti.android.bug2373.finishfalseroot" type="bool">true< / property > I have to release my app in Google Play in coming Tuesday. Please reply ASAP. Will this Cause any issues (Like Crash or this restart dialog ) if I download and install from Google Play Market? |
|
Functional tested with a simple test app. Accepted |
Timob 9285 Android: Message "An application restart is required" fires incorrectly.
|
Thanks Saamyce ! It is < property name="ti.android.bug2373.finishfalseroot" type="bool">true< / property > also works for me. |
See Testing Notes. The comment above the Testing Notes might also be useful for background info.