Change wallet defaults and fix race condition. #430
Conversation
| @@ -87,15 +87,15 @@ object PreferencesHelper { | |||
| } | |||
|
|
|||
| fun isConnectionAutoCloseEnabled(): Boolean { | |||
There was a problem hiding this comment.
nit: single line functions where the return type is clear in kotlin are often better written as:
fun isConnectionAutoCloseEnabled() = sharedPreferences.getBoolean(CONNECTION_AUTO_CLOSE, true)
if you wanted to be fancy you could also do this as a var with overridden getter/setters:
var connectionAutoCloseEnabled: Boolean
get() = sharedPreferences.getBoolean(CONNECTION_AUTO_CLOSE, true)
set(enabled) { sharedPreferences.edit { putBoolean(CONNECTION_AUTO_CLOSE, enabled) } }
There was a problem hiding this comment.
Cool. I didn't write that code but I'll keep that in mind (I'm still a Kotlin novice so this is helpful!)
| if (!mInhibitCallbacks) { | ||
| listener.onProgressUpdate(progress, max); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| protected void reportTransportSpecificSessionTermination() { | ||
| final Listener listener = mListener; | ||
| final Executor executor = mListenerExecutor; | ||
| if (!mInhibitCallbacks && listener != null && executor != null) { |
There was a problem hiding this comment.
It is a little weird to check mInhibitCallback twice, rather than just in the execution (it means we don't do callbacks if the boolean is false on initial call or when resolving the callback) Probably, that makes sense but it might be reasonable to only do this in the runnable.
There was a problem hiding this comment.
I agree. I guess I was trying to prematurely optimize not scheduling something on the executor but it's really not worth it and I agree a bit weird/confusing. I'll remove it.
Make the wallet app default to NFC Negotiated Handover and close the connection after sending the response. This change is to more closely mimic wallets already in production using this code. Also add additional checks for callback inhibition when inside the executor to avoid transport callbacks after closing a connection. This happened in the reader app - causing a crash - when the mDL included status 20 in the same SessionData message which contained the result (e.g. using auto-close). Test: Manually tested. Test: All unit tests pass. Signed-off-by: David Zeuthen <[email protected]>
davidz25
force-pushed
the
fix-race-and-change-defaults
branch
from
983336a to
9b2e279
Compare
Make the wallet app default to NFC Negotiated Handover and close the connection after sending the response. This change is to more closely mimic wallets already in production using this code.
Also add additional checks for callback inhibition when inside the executor to avoid transport callbacks after closing a connection. This happened in the reader app - causing a crash - when the mDL included status 20 in the same SessionData message which contained the result (e.g. using auto-close).
Test: Manually tested.
Test: All unit tests pass.