TIMOB-16889: fix Android activity lifecycle events #5701
Conversation
| @@ -518,6 +517,13 @@ protected void onCreate(Bundle savedInstanceState) | |||
| windowCreated(); | |||
|
|
|||
| if (activityProxy != null) { | |||
| KrollFunction onCreate = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_CREATE); | |||
There was a problem hiding this comment.
Use callPropertySync instead of KrollFunction to avoid circular reference. (refer to this PR #2687).
There was a problem hiding this comment.
Ping, I was just following the existing pattern for onCreateOptionsMenu and onPrepareOptionsMenu in TiMenuSupport.java
So it's buggy there too? And I'm noting that the codebase has very few usages of callProperty, and tons of usages of KrollFunction. So these are all causing leaks?
Thanks.
There was a problem hiding this comment.
It does not always cause a leak. But I think it's always safe to avoid circular references.
You used KrollFunction.call() which executes a function synchronously, so I supposed you wanted a synchronous function call. Do you really mean to use that? If you want an async call, you should use KrollFunction.callAsync() or callPropertyAsync().
There was a problem hiding this comment.
Sync calls in the activity's lifecycle functions may cause some other issues. 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 can you elaborate why you need sync calls here?
|
Code reviewed and functionally tested. Accepted |
TIMOB-16889: fix Android activity lifecycle events
|
@pingwang2011 Please update this fix with this pull request - the activity properties must be called synchronously |
|
I agree lifecycle events should be communicated in sync. Even more when indeed @mokesmokes's comment in #6160 is true they might even be communicated in the wrong order, not even just late. |
|
@FokkeZB the events are not communicated in wrong order - just very late. Too late in some cases. For example, in my Facebook module I need to attach a fragment to a specific activity. I know when that is the current activity when that activity's onResume function is called. However - this must be done when the activity is indeed in the resumed state. With async calls I have seen onResume be called sometimes after the activity was stopped.... This is a no-no of course. This happens very easily if the activity is created while the screen is locked - Android will quickly cycle from onCreate, onStart, onResume, onPause, onStop in such cases and then Android is like WTF when I try to do stuff thinking I am resumed. |
See explanation and test app in JIRA issue:
https://jira.appcelerator.org/browse/TIMOB-16889