Skip to content

Commit

Permalink
Fix TIMOB-19903 Android: Fix crash due to GC of UI native proxies in …
Browse files Browse the repository at this point in the history 
…hyperloop

- Need to fire on* property callbacks synchronously when run on main thread. Otherwise they fire _after_ we've already released the activity proxy on the native side and we get a fatal crash
  • Loading branch information
@sgtcoolguy
sgtcoolguy committed Nov 16, 2015
1 parent ff13c13 commit 3eafaf5
Showing 1 changed file with 60 additions and 53 deletions.
113 changes: 60 additions & 53 deletions android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java
View file
Expand Up @@ -72,7 +72,7 @@
* The base class for all non tab Titanium activities. To learn more about Activities, see the
* <a href="http://developer.android.com/reference/android/app/Activity.html">Android Activity documentation</a>.
*/
public abstract class TiBaseActivity extends AppCompatActivity
public abstract class TiBaseActivity extends AppCompatActivity
implements TiActivitySupport/*, ITiWindowHandler*/
{
private static final String TAG = "TiBaseActivity";
Expand All @@ -94,7 +94,7 @@ public abstract class TiBaseActivity extends AppCompatActivity

public static KrollObject cameraCallbackContext, contactsCallbackContext, oldCalendarCallbackContext, calendarCallbackContext, locationCallbackContext;
public static KrollFunction cameraPermissionCallback, contactsPermissionCallback, oldCalendarPermissionCallback, calendarPermissionCallback, locationPermissionCallback;

protected View layout;
protected TiActivitySupportHelper supportHelper;
protected int supportHelperId = -1;
Expand All @@ -108,7 +108,7 @@ public abstract class TiBaseActivity extends AppCompatActivity
protected int msgActivityCreatedId = -1;
protected int msgId = -1;
protected static int previousOrientation = -1;
//Storing the activity's dialogs and their persistence
//Storing the activity's dialogs and their persistence
private CopyOnWriteArrayList<DialogWrapper> dialogs = new CopyOnWriteArrayList<DialogWrapper>();
private Stack<TiWindowProxy> windowStack = new Stack<TiWindowProxy>();

Expand All @@ -122,13 +122,13 @@ public class DialogWrapper {
Dialog dialog;

WeakReference<TiBaseActivity> dialogActivity;

public DialogWrapper(Dialog d, boolean persistent, WeakReference<TiBaseActivity> activity) {
isPersistent = persistent;
dialog = d;
dialogActivity = activity;
}

public TiBaseActivity getActivity()
{
if (dialogActivity == null) {
Expand All @@ -146,12 +146,12 @@ public void setActivity(WeakReference<TiBaseActivity> da)
public Dialog getDialog() {
return dialog;
}

public void setDialog(Dialog d) {
dialog = d;
}
public void release()

public void release()
{
dialog = null;
dialogActivity = null;
Expand All @@ -162,7 +162,7 @@ public boolean getPersistent()
return isPersistent;
}

public void setPersistent(boolean p)
public void setPersistent(boolean p)
{
isPersistent = p;
}
Expand Down Expand Up @@ -208,7 +208,7 @@ public TiWindowProxy topWindowOnStack()
}

// could use a normal ConfigurationChangedListener but since only orientation changes are
// forwarded, create a separate interface in order to limit scope and maintain clarity
// forwarded, create a separate interface in order to limit scope and maintain clarity
public static interface OrientationChangedListener
{
public void onOrientationChanged (int configOrientationMode);
Expand Down Expand Up @@ -256,7 +256,7 @@ public void setWindowProxy(TiWindowProxy proxy)

/**
* Sets the proxy for our layout (used for post layout event)
*
*
* @param proxy
*/
public void setLayoutProxy(TiViewProxy proxy)
Expand All @@ -283,14 +283,14 @@ public ActivityProxy getActivityProxy()
return activityProxy;
}

public void addDialog(DialogWrapper d)
public void addDialog(DialogWrapper d)
{
if (!dialogs.contains(d)) {
dialogs.add(d);
}
}
public void removeDialog(Dialog d)

public void removeDialog(Dialog d)
{
for (int i = 0; i < dialogs.size(); i++) {
DialogWrapper p = dialogs.get(i);
Expand Down Expand Up @@ -419,17 +419,17 @@ protected View createLayout()
// set to null for now, this will get set correctly in setWindowProxy()
return new TiCompositeLayout(this, arrangement, null);
}

private void permissionCallback(int[] grantResults, KrollFunction callback, KrollObject context, String permission) {
if (callback == null) {
return;
}
boolean granted = true;
for (int i = 0; i < grantResults.length; ++i) {
if (grantResults[i] == PackageManager.PERMISSION_DENIED) {
granted = false;
break;
}
if (grantResults[i] == PackageManager.PERMISSION_DENIED) {
granted = false;
break;
}
}
if (granted) {
KrollDict response = new KrollDict();
Expand Down Expand Up @@ -486,13 +486,13 @@ protected void windowCreated(Bundle savedInstanceState)
int softInputMode = getIntentInt(TiC.PROPERTY_WINDOW_SOFT_INPUT_MODE, -1);
int windowFlags = getIntentInt(TiC.PROPERTY_WINDOW_FLAGS, 0);
boolean hasSoftInputMode = softInputMode != -1;

setFullscreen(fullscreen);

if (windowFlags > 0) {
getWindow().addFlags(windowFlags);
}

if (modal) {
if (Build.VERSION.SDK_INT < TiC.API_LEVEL_ICE_CREAM_SANDWICH) {
// This flag is deprecated in API 14. On ICS, the background is not blurred but straight black.
Expand Down Expand Up @@ -600,16 +600,16 @@ protected void onCreate(Bundle savedInstanceState)
if (theme != -1) {
this.setTheme(theme);
}

// Set ActionBar into split mode must be done before the decor view has been created
// we need to do this before calling super.onCreate()
if (intent != null && intent.hasExtra(TiC.PROPERTY_SPLIT_ACTIONBAR)) {
getWindow().setUiOptions(ActivityInfo.UIOPTION_SPLIT_ACTION_BAR_WHEN_NARROW);
}


// we only want to set the current activity for good in the resume state but we need it right now.
// save off the existing current activity, set ourselves to be the new current activity temporarily
// save off the existing current activity, set ourselves to be the new current activity temporarily
// so we don't run into problems when we give the proxy the event
Activity tempCurrentActivity = tiApp.getCurrentActivity();
tiApp.setCurrentActivity(this, this);
Expand All @@ -618,7 +618,7 @@ protected void onCreate(Bundle savedInstanceState)
this.requestWindowFeature(Window.FEATURE_PROGRESS);
this.requestWindowFeature(Window.FEATURE_INDETERMINATE_PROGRESS);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
this.requestWindowFeature(Window.FEATURE_ACTIVITY_TRANSITIONS);
this.requestWindowFeature(Window.FEATURE_ACTIVITY_TRANSITIONS);
}
super.onCreate(savedInstanceState);

Expand Down Expand Up @@ -710,7 +710,7 @@ public void run()
}
});
}

protected void handleSendMessage(int messageId)
{
try {
Expand Down Expand Up @@ -751,7 +751,7 @@ public void launchActivityForResult(Intent intent, int code, TiActivityResultHan
{
getSupportHelper().launchActivityForResult(intent, code, resultHandler);
}

/**
* See TiActivitySupport.launchIntentSenderForResult for more details.
*/
Expand Down Expand Up @@ -806,24 +806,24 @@ public void onBackPressed()
}

@Override
public boolean dispatchKeyEvent(KeyEvent event)
public boolean dispatchKeyEvent(KeyEvent event)
{
boolean handled = false;

TiViewProxy window;
if (this.window != null) {
window = this.window;
} else {
window = this.view;
}

if (window == null) {
return super.dispatchKeyEvent(event);
}

switch(event.getKeyCode()) {
case KeyEvent.KEYCODE_BACK : {

if (event.getAction() == KeyEvent.ACTION_UP) {
String backEvent = "android:back";
KrollProxy proxy = null;
Expand All @@ -835,12 +835,12 @@ public boolean dispatchKeyEvent(KeyEvent event)
} else if (window.hasListeners(backEvent)) {
proxy = window;
}

if (proxy != null) {
proxy.fireEvent(backEvent, null);
handled = true;
}

}
break;
}
Expand Down Expand Up @@ -930,7 +930,7 @@ public boolean dispatchKeyEvent(KeyEvent event)
break;
}
}

if (!handled) {
handled = super.dispatchKeyEvent(event);
}
Expand All @@ -941,7 +941,7 @@ public boolean dispatchKeyEvent(KeyEvent event)
@Override
public boolean onCreateOptionsMenu(Menu menu)
{
// If targetSdkVersion is set to 11+, Android will invoke this function
// If targetSdkVersion is set to 11+, Android will invoke this function
// to initialize the menu (since it's part of the action bar). Due
// to the fix for Android bug 2373, activityProxy won't be initialized b/c the
// activity is expected to restart, so we will ignore it.
Expand Down Expand Up @@ -1030,7 +1030,7 @@ public void onConfigurationChanged(Configuration newConfig)
}

@Override
protected void onNewIntent(Intent intent)
protected void onNewIntent(Intent intent)
{
super.onNewIntent(intent);

Expand Down Expand Up @@ -1093,7 +1093,15 @@ private void dispatchCallback(String name, KrollDict data) {

data.put("source", activityProxy);

activityProxy.callPropertyAsync(name, new Object[] { data });
// TIMOB-19903
if (TiApplication.getInstance().runOnMainThread()) {
// We must call this synchornously to ensure it happens before we release the Activity reference on the V8/Native side!
activityProxy.callPropertySync(name, new Object[] { data });
} else {
// This hopefully finishes before we release the reference on the native side?! I have seen it crash because it didn't before though...
// Not sure it's safe to keep this behavior...
activityProxy.callPropertyAsync(name, new Object[] { data });
}
}

private void releaseDialogs(boolean finish)
Expand Down Expand Up @@ -1135,7 +1143,7 @@ public void onWindowFocusChanged(boolean hasFocus)
* When this activity pauses, this method sets the current activity to null, fires a javascript 'pause' event,
* and if the activity is finishing, remove all dialogs associated with it.
*/
protected void onPause()
protected void onPause()
{
inForeground = false;
if (activityProxy != null) {
Expand All @@ -1158,7 +1166,7 @@ protected void onPause()
if (!windowStack.empty()) {
windowStack.peek().onWindowFocusChange(false);
}

TiApplication.updateActivityTransitionState(true);
tiApp.setCurrentActivity(this, null);
TiUIHelper.showSoftKeyboard(getWindow().getDecorView(), false);
Expand Down Expand Up @@ -1219,11 +1227,11 @@ protected void onResume()

if (!windowStack.empty()) {
windowStack.peek().onWindowFocusChange(true);
}
}

tiApp.setCurrentActivity(this, this);
TiApplication.updateActivityTransitionState(false);

if (activityProxy != null) {
activityProxy.fireEvent(TiC.EVENT_RESUME, null);
}
Expand Down Expand Up @@ -1251,7 +1259,7 @@ protected void onResume()
@Override
/**
* When this activity starts, this method updates the current activity to this if necessary and
* fire javascript 'start' and 'focus' events. Focus events will only fire if
* fire javascript 'start' and 'focus' events. Focus events will only fire if
* the activity is not a tab activity.
*/
protected void onStart()
Expand Down Expand Up @@ -1284,7 +1292,7 @@ protected void onStart()

if (activityProxy != null) {
// we only want to set the current activity for good in the resume state but we need it right now.
// save off the existing current activity, set ourselves to be the new current activity temporarily
// save off the existing current activity, set ourselves to be the new current activity temporarily
// so we don't run into problems when we give the proxy the event
Activity tempCurrentActivity = tiApp.getCurrentActivity();
tiApp.setCurrentActivity(this, this);
Expand All @@ -1306,7 +1314,7 @@ protected void onStart()
}
}
// store current configuration orientation
// This fixed bug with double orientation chnage firing when activity starts in landscape
// This fixed bug with double orientation chnage firing when activity starts in landscape
previousOrientation = getWindowManager().getDefaultDisplay().getRotation();
}

Expand Down Expand Up @@ -1363,7 +1371,7 @@ protected void onRestart()
super.onRestart();

Log.d(TAG, "Activity " + this + " onRestart", Log.DEBUG_MODE);

TiApplication tiApp = getTiApp();
if (tiApp.isRestartPending()) {
if (!isFinishing()) {
Expand All @@ -1375,7 +1383,7 @@ protected void onRestart()

if (activityProxy != null) {
// we only want to set the current activity for good in the resume state but we need it right now.
// save off the existing current activity, set ourselves to be the new current activity temporarily
// save off the existing current activity, set ourselves to be the new current activity temporarily
// so we don't run into problems when we give the proxy the event
Activity tempCurrentActivity = tiApp.getCurrentActivity();
tiApp.setCurrentActivity(this, this);
Expand All @@ -1389,7 +1397,7 @@ protected void onRestart()

@Override
/**
* When the activity is about to go into the background as a result of user choice, this method fires the
* When the activity is about to go into the background as a result of user choice, this method fires the
* javascript 'userleavehint' event.
*/
protected void onUserLeaveHint()
Expand All @@ -1409,11 +1417,11 @@ protected void onUserLeaveHint()

super.onUserLeaveHint();
}

@Override
/**
* When this activity is destroyed, this method removes it from the activity stack, performs
* clean up, and fires javascript 'destroy' event.
* clean up, and fires javascript 'destroy' event.
*/
protected void onDestroy()
{
Expand All @@ -1424,7 +1432,7 @@ protected void onDestroy()

inForeground = false;
TiApplication tiApp = getTiApp();
//Clean up dialogs when activity is destroyed.
//Clean up dialogs when activity is destroyed.
releaseDialogs(true);

if (tiApp.isRestartPending()) {
Expand Down Expand Up @@ -1639,4 +1647,3 @@ public static boolean isUnsupportedReLaunch(Activity activity, Bundle savedInsta
return false;
}
}

0 comments on commit 3eafaf5

Please sign in to comment.