From 269fceddf25ccfd131a819e17d4f5237f5744f02 Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 13 Apr 2017 08:18:07 -0700 Subject: [PATCH] cleanup around posting alerts; skip back-stack Had to disable use of the back-stack for DialogFragments, though that means I can't prevent duplicates from stacking up (esp. in the pathological robot-vs-robot case, but also just when I rotate the device while a "rematch" alert is up.) The problem seems to be in dismiss actions being handled too late. One easy-to-duplicate case is the tile-picker. When it's enabled and you commit a turn I post first the confirmation and then, in response to a "yes", the picker. But the picker gets added to the stack moments before the confirmation is removed, and it's the nature of stack removal that everything above what's being removed gets pulled too. So you never see the picker. Simply post()ing a runnable to put the picker up later fixes this one case, but a similar trick doesn't work elsewhere, so I'm punting until I have time to root-cause the problem. --- .../eehouse/android/xw4/BoardDelegate.java | 21 ++++++++++--- .../org/eehouse/android/xw4/DelegateBase.java | 7 +++-- .../eehouse/android/xw4/DlgDelegateAlert.java | 4 +-- .../org/eehouse/android/xw4/MainActivity.java | 5 ++- .../org/eehouse/android/xw4/XWActivity.java | 31 ++++++++++++------- .../eehouse/android/xw4/XWDialogFragment.java | 7 +++-- 6 files changed, 50 insertions(+), 25 deletions(-) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java index 97dd89415..46a685ebc 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java @@ -1747,11 +1747,17 @@ public class BoardDelegate extends DelegateBase // This is supposed to be called from the jni thread @Override - public void notifyPickTileBlank( int playerNum, int col, int row, String[] texts ) + public void notifyPickTileBlank( int playerNum, int col, int row, + String[] texts ) { - TilePickAlert.TilePickState tps = + final TilePickAlert.TilePickState tps = new TilePickAlert.TilePickState( playerNum, texts, col, row ); - show( TilePickAlert.newInstance( Action.BLANK_PICKED, tps ) ); + runOnUiThread( new Runnable() { + @Override + public void run() { + show( TilePickAlert.newInstance( Action.BLANK_PICKED, tps ) ); + } + } ); } @Override @@ -1759,10 +1765,15 @@ public class BoardDelegate extends DelegateBase int playerNum, int nToPick, String[] texts, int[] counts ) { - TilePickAlert.TilePickState tps + final TilePickAlert.TilePickState tps = new TilePickAlert.TilePickState( isInitial, playerNum, nToPick, texts, counts ); - show( TilePickAlert.newInstance( Action.TRAY_PICKED, tps ) ); + runOnUiThread( new Runnable() { + @Override + public void run() { + show( TilePickAlert.newInstance( Action.TRAY_PICKED, tps ) ); + } + } ); } @Override diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DelegateBase.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DelegateBase.java index cf141743a..bfc2420dd 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DelegateBase.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DelegateBase.java @@ -436,8 +436,9 @@ public class DelegateBase implements DlgClickNotify, return null; } - protected void showDialogFragment( DlgID dlgID, Object... params ) + protected void showDialogFragment( DlgID dlgID, final Object... params ) { + DbgUtils.assertOnUIThread(); show( DBAlert.newInstance( dlgID, params ) ); } @@ -469,6 +470,7 @@ public class DelegateBase implements DlgClickNotify, protected void show( XWDialogFragment df ) { + DbgUtils.assertOnUIThread(); if ( m_activity instanceof XWActivity ) { ((XWActivity)m_activity).show( df ); } else if ( m_activity instanceof PrefsActivity ) { @@ -668,8 +670,7 @@ public class DelegateBase implements DlgClickNotify, public boolean onPosButton( Action action, Object[] params ) { boolean handled = true; - Log.d( TAG, "%s.posButtonClicked(%s)", getClass().getSimpleName(), - action.toString() ); + Log.d( TAG, "%s.onPosButton(%s)", getClass().getSimpleName(), action ); switch( action ) { case ENABLE_SMS_ASK: showSMSEnableDialog( Action.ENABLE_SMS_DO ); diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DlgDelegateAlert.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DlgDelegateAlert.java index 0c7401047..e5bb31839 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DlgDelegateAlert.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DlgDelegateAlert.java @@ -74,12 +74,12 @@ abstract class DlgDelegateAlert extends XWDialogFragment { @Override public void onDismiss( DialogInterface dif ) { + super.onDismiss( dif ); Activity activity = getActivity(); if ( activity instanceof DlgClickNotify ) { ((DlgClickNotify)activity) .onDismissed( m_state.m_action, m_state.m_params ); } - super.onDismiss( dif ); } @Override @@ -108,7 +108,7 @@ abstract class DlgDelegateAlert extends XWDialogFragment { @Override public void onClick( DialogInterface dlg, int button ) { checkNotAgainCheck( m_state, naView ); - XWActivity xwact = (XWActivity)getActivity(); + DlgClickNotify xwact = (DlgClickNotify)getActivity(); xwact.onPosButton( pair.action, m_state.m_params ); } }; diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/MainActivity.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/MainActivity.java index 9953b76ec..91ff5a72a 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/MainActivity.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/MainActivity.java @@ -130,6 +130,7 @@ public class MainActivity extends XWActivity if ( m_safeToCommit ) { handled = dispatchNewIntentImpl( intent ); } else { + DbgUtils.assertOnUIThread(); m_runWhenSafe.add( new Runnable() { @Override public void run() { @@ -300,7 +301,7 @@ public class MainActivity extends XWActivity protected void finishFragment() { // Assert.assertTrue( fragment instanceof XWFragment ); - // DbgUtils.logf( "MainActivity.finishFragment(%s)", fragment.toString() ); + // Log.d( TAG, "finishFragment()" ); getSupportFragmentManager().popBackStack/*Immediate*/(); } @@ -465,6 +466,7 @@ public class MainActivity extends XWActivity if ( m_safeToCommit ) { safeAddFragment( fragment, parentName ); } else { + DbgUtils.assertOnUIThread(); m_runWhenSafe.add( new Runnable() { @Override public void run() { @@ -516,6 +518,7 @@ public class MainActivity extends XWActivity private void setSafeToRun() { + DbgUtils.assertOnUIThread(); m_safeToCommit = true; for ( Runnable proc : m_runWhenSafe ) { proc.run(); diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWActivity.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWActivity.java index e388416ff..5d9ee2c49 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWActivity.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWActivity.java @@ -285,19 +285,26 @@ public class XWActivity extends FragmentActivity { FragmentManager fm = getSupportFragmentManager(); String tag = df.getFragTag(); - Log.d( TAG, "%s.show(%s) called; tag: %s", getClass().getSimpleName(), - df.getClass().getSimpleName(), tag ); - FragmentTransaction trans = fm.beginTransaction(); - Fragment prev = fm.findFragmentByTag( tag ); - if ( null != prev && prev instanceof DialogFragment ) { - Log.d( TAG, "removing %s (tag %s)", - prev.getClass().getSimpleName(), tag ); - ((DialogFragment)prev).dismiss(); - } - trans.addToBackStack( tag ); + if ( true ) { + df.show( fm, tag ); + } else { + Log.d( TAG, "%s.show(%s) called; tag: %s", getClass().getSimpleName(), + df.getClass().getSimpleName(), tag ); + FragmentTransaction trans = fm.beginTransaction(); - // Create and show the dialog. show() commits the transaction - df.show( trans, tag ); + Fragment prev = fm.findFragmentByTag( tag ); + if ( null != prev && prev instanceof DialogFragment ) { + Log.d( TAG, "show(): removing %s (tag %s)", + prev.getClass().getSimpleName(), tag ); + ((DialogFragment)prev).dismiss(); + } else { + Log.d( TAG, "show(): NOT removing or didn't find for tag %s)", tag ); + } + trans.addToBackStack( tag ); + + // Create and show the dialog. show() commits the transaction + df.show( trans, tag ); + } } protected Dialog makeDialog( DBAlert alert, Object[] params ) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWDialogFragment.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWDialogFragment.java index ab5be9fe2..b694487a4 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWDialogFragment.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWDialogFragment.java @@ -72,19 +72,22 @@ abstract class XWDialogFragment extends DialogFragment { @Override public void onCancel( DialogInterface dialog ) { + super.onCancel( dialog ); + // Log.d( TAG, "%s.onCancel() called", getClass().getSimpleName() ); if ( null != m_onCancel ) { m_onCancel.onCancelled( this ); } - super.onCancel( dialog ); } @Override public void onDismiss( DialogInterface dif ) { + // Log.d( TAG, "%s.onDismiss() called", getClass().getSimpleName() ); + super.onDismiss( dif ); + if ( null != m_onDismiss ) { m_onDismiss.onDismissed( this ); } - super.onDismiss( dif ); } abstract String getFragTag();