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.

Sign up for GitHub

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

Jump to bottom

TIMOB-16889: fix Android activity lifecycle events #5701

Merged
merged 2 commits into from
Jul 24, 2014
Merged

TIMOB-16889: fix Android activity lifecycle events #5701

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abba556
TIMOB-16889: fix Android activity lifecycle events
mokesmokes2 May 19, 2014
379aac5
change activity lifecycle events to async callbacks
Jul 24, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 43 additions & 1 deletion android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,6 @@ protected void onCreate(Bundle savedInstanceState)

// create the activity proxy here so that it is accessible from the activity in all cases
activityProxy = new ActivityProxy(this);


// Increment the reference count so we correctly clean up when all of our activities have been destroyed
KrollRuntime.incrementActivityRefCount();
Expand Down Expand Up @@ -517,6 +516,13 @@ protected void onCreate(Bundle savedInstanceState)

windowCreated();

if (activityProxy != null) {
KrollFunction onCreate = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_CREATE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use callPropertySync instead of KrollFunction to avoid circular reference. (refer to this PR #2687).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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().

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

if (onCreate != null) {
onCreate.call(activityProxy.getKrollObject(), new Object[] { null });
}
}

if (activityProxy != null) {
activityProxy.fireEvent(TiC.EVENT_CREATE, null);
}
Expand Down Expand Up @@ -931,6 +937,12 @@ public void onWindowFocusChanged(boolean hasFocus)
protected void onPause()
{
inForeground = false;
if (activityProxy != null) {
KrollFunction onPause = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_PAUSE);
if (onPause != null) {
onPause.call(activityProxy.getKrollObject(), new Object[] { null });
}
}
super.onPause();
isResumed = false;

Expand Down Expand Up @@ -989,6 +1001,12 @@ protected void onPause()
protected void onResume()
{
inForeground = true;
if (activityProxy != null) {
KrollFunction onResume = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_RESUME);
if (onResume != null) {
onResume.call(activityProxy.getKrollObject(), new Object[] { null });
}
}
super.onResume();
if (isFinishing()) {
return;
Expand Down Expand Up @@ -1044,6 +1062,12 @@ protected void onResume()
protected void onStart()
{
inForeground = true;
if (activityProxy != null) {
KrollFunction onStart = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_START);
if (onStart != null) {
onStart.call(activityProxy.getKrollObject(), new Object[] { null });
}
}
super.onStart();
if (isFinishing()) {
return;
Expand Down Expand Up @@ -1102,6 +1126,12 @@ protected void onStart()
protected void onStop()
{
inForeground = false;
if (activityProxy != null) {
KrollFunction onStop = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_STOP);
if (onStop != null) {
onStop.call(activityProxy.getKrollObject(), new Object[] { null });
}
}
super.onStop();

Log.d(TAG, "Activity " + this + " onStop", Log.DEBUG_MODE);
Expand Down Expand Up @@ -1138,6 +1168,12 @@ protected void onStop()
protected void onRestart()
{
inForeground = true;
if (activityProxy != null) {
KrollFunction onRestart = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_RESTART);
if (onRestart != null) {
onRestart.call(activityProxy.getKrollObject(), new Object[] { null });
}
}
super.onRestart();

Log.d(TAG, "Activity " + this + " onRestart", Log.DEBUG_MODE);
Expand Down Expand Up @@ -1196,6 +1232,12 @@ protected void onUserLeaveHint()
protected void onDestroy()
{
Log.d(TAG, "Activity " + this + " onDestroy", Log.DEBUG_MODE);
if (activityProxy != null) {
KrollFunction onDestroy = (KrollFunction) activityProxy.getProperty(TiC.PROPERTY_ON_DESTROY);
if (onDestroy != null) {
onDestroy.call(activityProxy.getKrollObject(), new Object[] { null });
}
}

inForeground = false;
TiApplication tiApp = getTiApp();
Expand Down
35 changes: 35 additions & 0 deletions android/titanium/src/java/org/appcelerator/titanium/TiC.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1662,6 +1662,41 @@ public class TiC
*/
public static final String PROPERTY_ON_CREATE_OPTIONS_MENU = "onCreateOptionsMenu";

/**
* @module.api
*/
public static final String PROPERTY_ON_CREATE = "onCreate";

/**
* @module.api
*/
public static final String PROPERTY_ON_START = "onStart";

/**
* @module.api
*/
public static final String PROPERTY_ON_RESUME = "onResume";

/**
* @module.api
*/
public static final String PROPERTY_ON_RESTART = "onRestart";

/**
* @module.api
*/
public static final String PROPERTY_ON_PAUSE = "onPause";

/**
* @module.api
*/
public static final String PROPERTY_ON_STOP = "onStop";

/**
* @module.api
*/
public static final String PROPERTY_ON_DESTROY = "onDestroy";

/**
* @module.api
*/
Expand Down
9 changes: 8 additions & 1 deletion android/titanium/src/java/org/appcelerator/titanium/proxy/ActivityProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@

@Kroll.proxy(propertyAccessors = {
TiC.PROPERTY_ON_CREATE_OPTIONS_MENU,
TiC.PROPERTY_ON_PREPARE_OPTIONS_MENU
TiC.PROPERTY_ON_PREPARE_OPTIONS_MENU,
TiC.PROPERTY_ON_CREATE,
TiC.PROPERTY_ON_START,
TiC.PROPERTY_ON_RESTART,
TiC.PROPERTY_ON_RESUME,
TiC.PROPERTY_ON_PAUSE,
TiC.PROPERTY_ON_STOP,
TiC.PROPERTY_ON_DESTROY
})
/**
* This is a proxy representation of the Android Activity type.
Expand Down