TIMOB-16889 update: activity lifecycle calls must be synchronous

[mokesmokes]

Strange things happen if the Android Activity lifecycle calls (onCreate, onResume, onCreateOptionsMenu, etc. ) are async: the calls into Javascript happen much later than they should - sometimes disastrously later. For example, if a window is created and opened as the screen is locked, Android will quickly cycle through create, start, resume, pause, stop - and I have seen window.activity.onResume, onCreateOptionsMenu etc called after the activity was stopped! This of course creates issues if the code tries to do something with the activity when the activity's state is inappropriate for the action. This should be merged into master as well as 3_4_X

[mokesmokes]

Please merge ASAP, this module needs this PR, and I assume (hope) it will be used by numerous people: https://github.com/mokesmokes/titanium-android-facebook

[ingo]

We're reviewing this now, but unfortunately there is no way we can take this as part of 3.4.0. It will have to wait for 3.4.1.

[mokesmokes]

@ingo that's OK, as long as it is in master ASAP, thanks!

[pingwang2011]

@mokesmokes sync calls will intercept the activity lifecycle therefore it may cause problems during the app bootstrap process. We have discussed this in your last PR:
"Sync functions are implemented using blocking messages which suspend the UI thread and wait for those functions to return. If those functions cannot return in time, it will have ANR errors. That's why we remove the sync lifescycle events (https://jira.appcelerator.org/browse/TIMOB-16279)."
So we don't think it's a good idea to use sync calls here.

If you want to implement a fragment in your module, maybe you can take a look at TiUIFragment.java (https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/view/TiUIFragment.java) and the ti.map implementation (https://github.com/appcelerator-modules/ti.map/blob/stable/android/src/ti/map/TiUIMapView.java).

[mokesmokes]

@pingwang2011 yes they are blocking but that is the whole point. Plenty of SDKs such as Facebook, Chromecast, etc require functionality precisely at these events, and not "sometime later". These events should be used responsibly of course, and perhaps that should be noted in the docs. But it's no different from native android development which sometimes requires extra code during these events. What is the meaning of these functions if they are async?
As to my usage of fragment, that is a workaround to the issue that the module proxy has no access to the activity lifecycle. So I create a fragment and attach it to the activity that is current when its onResume is called. This fragment has no UI, I just use it since it follows the activity lifecycle (and of course Facebook supports following fragments as well as activities).
Bottom line - if they're async there is very little utility to app lifecycle callbacks. They lose their meaning almost entirely. Don't prevent developers who know what they're doing from using them.

[pingwang2011]

It's very probably generating a lot of issues if we allow general users to intercept the activity lifecycle.

But for the module developers, we have the TiLifecycle.OnLifecycleEvent API to access to the activity lifecycle. This API is a sync call without sending any message. So we don't need to worry about the thread-blocking issue. Here is where it's called in TiBaseActivity.java (https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L1045) and this is an example how to use it (https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/media/src/java/ti/modules/titanium/media/AudioPlayerProxy.java).

[mokesmokes]

Yes, I am aware of this interface. Two main issues with it:

1. No access to onCreate
2. The "initActivity" call entirely depends upon what getCurrentActivity returns, which could be totally arbitrary. It could depend on when the proxy was created, before or after opening the window, if screen was locked suddenly, etc. the only reliable way currently to set the right activity is to call during onResume from JavaScript, but only if onResume is sync of course.
   I really don't get the hesitation about opening this API to all developers. Why is it any different from coding in onCreate during native android development? you can't save everyone from silly coding mistakes if you want to enable serious developers.

On Sep 25, 2014, at 10:38 PM, pingwang2011 notifications@github.com wrote:

It's very probably generating a lot of issues if we allow general users to intercept the activity lifecycle.

But for the module developers, we have the TiLifecycle.OnLifecycleEvent API to access to the activity lifecycle. This API is a sync call without sending any message. So we don't need to worry about the thread-blocking issue. Here is where it's called in TiBaseActivity.java (https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L1045) and this is an example how to use it (https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/media/src/java/ti/modules/titanium/media/AudioPlayerProxy.java).

—
Reply to this email directly or view it on GitHub.

[mokesmokes]

If you would enable the proxy to set the activity intelligently (e.g. passing window.activity in the proxy creation dictionary), and if that interface included all lifecycle events (i.e add onCreate) then we would need a lot less of the activity lifecycle callbacks in JS. Still, to avoid unforeseen requirements, I still believe those callbacks should be sync just as they are in native Android development.

[pingwang2011]

Coding in onCreate during native android development does not have a thread issue because everything you put in onCreate will be run in the main thread. But exposing this sync API in JS means you will suspend the main thread and execute some code in KrollRuntime thread. It has a risk that the main thread hangs there forever. This is the main difference and root problem.

As to the two issues you mentioned above:

1. Is that true that the thing you want to do can only be done in onCreate()? Is it possible to do that in onResume or onStart?
2. I agree it is tricky and important to refer to the right activity. How about documenting that this proxy can only be created after the window opens, eg. in the "open" event listener, so it will always refer to the window activity.

About the screen-locking problem, I am not sure the exact issue but maybe you can take a look at TiLifecycle.OnWindowFocusChangedEvent and TiBaseActivity.isInForeground().

[mokesmokes]

In my case all I really need is to get the correct activity and then internally in the module proxy I use the fragment lifecycle and don't bother with the Titanium activity lifecycle which as I said is problematic. Does the open event for windows and tab groups guarantee that getCurrentActivity return their activity?
As to the activity lifecycle: since I have seen the JAvascript onStart and onResume called after onStop was actually called by Android, these async events are misleading and buggy. So I propose you either make them sync and warn developers to use them carefully, or deprecate them since they don't do what the developer thinks they do. They're quite useless currently. I spent a few hours debugging why Android complained that I can't commit a fragment after onSaveInstanceState was called, when I was doing that during the JavaScript onResume

[mokesmokes]

Actually, even if the open event guarantees getCurrentActivity returns the right activity, that is still not enough. The developer needs freedom to access the activity in its relevant states. How do I guarantee in my module that the activity is (for example) resumed so I can do stuff pertinent for onResume? We really sometimes need all the events, and we need them in time and not late.

[mokesmokes]

@pingwang2011, is the KrollRuntime thread issue relevant for all lifecycle events or just onCreate? In any case, I can't view the Ti code base today, but as I assume the open event is also async the activity may well be stopped by the time it is called (e.g. if the screen locks)

[mokesmokes]

@pingwang2011 note the new PR #6179 for https://jira.appcelerator.org/browse/TIMOB-15443 . Perhaps this is the cleanest solution. All proxies can now get lifecycle events. Please accept that PR, or this one, or both - we need this for the Facebook module and for other modules as well. As to this particular PR - in my opinion if the Javascript lifecycle events are async, then we should just remove that API since it's very confusing. The functionality can be kept by the use of the new PR in the module.

[mokesmokes]

Since sync events to Javascript are problematic we can close this, however we need the full lifecycle available to proxies so please merge #6179 ASAP.