From ba97416397b2e01a04282bccaddfd90429c7a9d7 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 13:23:02 -0400 Subject: [PATCH 1/4] fix(android): process lifecycle, foreground pushes, quick reply off the main thread One lifecycle reporter: AppLifecycleReporter (ProcessLifecycleOwner) talks to Go and JS through a LifecycleBind seam so the mapping runs in JVM tests. MainActivity resume/finishing-destroy go through it too. Pushes always reach Go. In the foreground master returned before calling handleBackgroundNotification, so a foreground push was never unboxed or acked; now Go handles it with a notifier that does not display while the app is in the foreground. In the background Go is held in BACKGROUNDACTIVE around the work, then appDidEnterBackground decides whether to keep running. Quick reply runs on a worker thread under goAsync with a status notification, is not sent when logged out or for a negative message id (Go sends before it checks either and swallows the send's error), and is sent in the foreground too (master skipped it). Also: one unique periodic background sync job (legacy duplicates cancelled once), share intents handed over when JS registers instead of polling, and the Kotlin seen-set capped at 100. Flush audit (calls that run a leveldb flush on master): - Process ON_STOP appDidEnterBackground -> appBeginBackgroundTaskNonblock: unchanged from the parent, no else branch. - appWillExit: MainActivity finishing destroy, now also skipped when isChangingConfigurations (subset of the parent's calls). - Push / quick reply window, background only: appDidEnterBackground -> appBeginBackgroundTaskNonblock, same call master made on the same path; master's else setAppStateBackground (a second flush) is removed. - Foreground push / quick reply: no lifecycle call (master made none). - Quick reply rejected by the pre-checks: no window, no call (master opened one). --- shared/android/app/build.gradle | 2 + .../ossifrage/AppLifecycleForwarder.kt | 35 --- .../keybase/ossifrage/AppLifecycleReporter.kt | 131 ++++++++++ .../ossifrage/ChatBroadcastReceiver.kt | 38 +-- .../keybase/ossifrage/KeybaseLifecycleBind.kt | 22 ++ .../KeybasePushNotificationListenerService.kt | 132 +++++----- .../java/io/keybase/ossifrage/MainActivity.kt | 104 +++----- .../io/keybase/ossifrage/MainApplication.kt | 77 +++++- .../modules/BackgroundSyncSchedule.kt | 29 +++ .../ossifrage/AppLifecycleReporterTest.kt | 228 ++++++++++++++++++ .../ossifrage/WithBackgroundActiveTest.kt | 153 ++++++++++++ .../modules/BackgroundSyncScheduleTest.kt | 64 +++++ 12 files changed, 809 insertions(+), 206 deletions(-) delete mode 100644 shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleForwarder.kt create mode 100644 shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt create mode 100644 shared/android/app/src/main/java/io/keybase/ossifrage/KeybaseLifecycleBind.kt create mode 100644 shared/android/app/src/main/java/io/keybase/ossifrage/modules/BackgroundSyncSchedule.kt create mode 100644 shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt create mode 100644 shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt create mode 100644 shared/android/app/src/test/java/io/keybase/ossifrage/modules/BackgroundSyncScheduleTest.kt diff --git a/shared/android/app/build.gradle b/shared/android/app/build.gradle index 9327e574c2cb..e205353b3d61 100644 --- a/shared/android/app/build.gradle +++ b/shared/android/app/build.gradle @@ -171,6 +171,8 @@ dependencies { implementation 'com.android.installreferrer:installreferrer:2.2' implementation "androidx.lifecycle:lifecycle-common-java8:2.10.0" implementation "androidx.lifecycle:lifecycle-process:2.10.0" + + testImplementation "junit:junit:4.13.2" } // This requires a google-services.json file locally. Drop it in diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleForwarder.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleForwarder.kt deleted file mode 100644 index cc69eaad34fb..000000000000 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleForwarder.kt +++ /dev/null @@ -1,35 +0,0 @@ -package io.keybase.ossifrage - -import android.content.Context -import android.os.Bundle -import androidx.lifecycle.DefaultLifecycleObserver -import androidx.lifecycle.LifecycleOwner -import com.reactnativekb.KbModule -import io.keybase.ossifrage.modules.NativeLogger -import keybase.Keybase - -// Reports the whole process's visibility, not one activity's, to Go and JS -// together, so both see the same state. Process ON_STOP only fires once no -// activity is started, so moving between our own activities never looks like a -// trip to the background. -internal class AppLifecycleForwarder(private val context: Context) : DefaultLifecycleObserver { - override fun onStart(owner: LifecycleOwner) = foreground("onStart") - - override fun onResume(owner: LifecycleOwner) = foreground("onResume") - - override fun onStop(owner: LifecycleOwner) { - NativeLogger.info("AppLifecycleForwarder: process onStop") - // appDidEnterBackground already reports BACKGROUND (and flushes) when it - // returns false; calling setAppStateBackground too would flush twice. - if (Keybase.appDidEnterBackground()) { - Keybase.appBeginBackgroundTaskNonblock(KBPushNotifier(context, Bundle())) - } - KbModule.emitAppLifecycle("background") - } - - private fun foreground(event: String) { - NativeLogger.info("AppLifecycleForwarder: process $event") - Keybase.setAppStateForeground() - KbModule.emitAppLifecycle("active") - } -} diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt new file mode 100644 index 000000000000..e2759ad4145a --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt @@ -0,0 +1,131 @@ +package io.keybase.ossifrage + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +// Go's lifecycle entry points and the JS app-state event. Kept free of Android +// and gomobile calls so the event mapping and the push window run in JVM tests. +internal interface LifecycleBind { + fun setAppStateForeground() + fun setAppStateBackgroundActive() + fun isAppStateForeground(): Boolean + fun appDidEnterBackground(): Boolean + fun appBeginBackgroundTaskNonblock() + fun appWillExit() + fun emitAppLifecycle(state: String) +} + +// Reports the whole process's visibility, not one activity's, to Go and JS +// together, so both see the same state. Process ON_STOP only fires once no +// activity is started, so moving between our own activities, dialogs and +// permission prompts never looks like a trip to the background. +// +// Calls reach Go on the calling thread, before the callback returns. +internal class AppLifecycleReporter( + private val bind: LifecycleBind, + private val log: (String) -> Unit, +) : DefaultLifecycleObserver { + override fun onStart(owner: LifecycleOwner) = foreground("process onStart") + + override fun onResume(owner: LifecycleOwner) = foreground("process onResume") + + override fun onStop(owner: LifecycleOwner) { + report("process onStop") { + // appDidEnterBackground already reports BACKGROUND (and flushes) when + // it returns false; calling setAppStateBackground too would flush twice. + if (bind.appDidEnterBackground()) { + bind.appBeginBackgroundTaskNonblock() + } + } + bind.emitAppLifecycle("background") + } + + fun onMainActivityResume() = foreground("MainActivity onResume") + + // Activity recreation and a task moved to the back are not an exit. + fun onMainActivityDestroy(isFinishing: Boolean, isChangingConfigurations: Boolean) { + if (!isFinishing || isChangingConfigurations) { + return + } + report("MainActivity finishing") { bind.appWillExit() } + bind.emitAppLifecycle("background") + } + + private fun foreground(event: String) { + report(event) { bind.setAppStateForeground() } + bind.emitAppLifecycle("active") + } + + private fun report(event: String, call: () -> Unit) { + log("AppLifecycleReporter: $event") + try { + call() + } catch (e: Exception) { + log("AppLifecycleReporter: $event failed: $e") + } + } +} + +// Sends a notification quick reply. Returns the text for the replied +// notification. +internal fun sendQuickReply( + currentUID: String, + msgId: Long, + error: (String, Throwable?) -> Unit, + send: () -> Unit, +): String { + // Go sends before it checks either, and swallows the send's error. + if (currentUID.isEmpty()) { + error("Quick reply while logged out", null) + return QUICK_REPLY_FAILED + } + if (msgId < 0) { + error("Quick reply to invalid message id $msgId", null) + return QUICK_REPLY_FAILED + } + return try { + send() + QUICK_REPLY_SENT + } catch (e: Exception) { + error("Failed to send quick reply", e) + QUICK_REPLY_FAILED + } +} + +// Runs a receiver's work off the main thread and calls finish exactly once: +// when the work ends or when budgetMs runs out, whichever is first, so the +// broadcast never outlives its limit. Work that overruns keeps going. An +// exception from work is logged, since it would otherwise kill the process. +internal fun runReceiverWork( + budgetMs: Long, + start: (Runnable) -> Unit, + warn: (String) -> Unit, + error: (String, Throwable) -> Unit, + finish: () -> Unit, + work: () -> Unit, +) { + val done = CountDownLatch(1) + start(Runnable { + try { + work() + } catch (e: Exception) { + error("runReceiverWork: work failed", e) + } finally { + done.countDown() + } + }) + start(Runnable { + try { + if (!done.await(budgetMs, TimeUnit.MILLISECONDS)) { + warn("runReceiverWork: still running after ${budgetMs}ms, finishing the broadcast") + } + } finally { + finish() + } + }) +} + +internal const val QUICK_REPLY_SENT = "Replied" +internal const val QUICK_REPLY_FAILED = "Couldn't send reply" diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt index afb2f3a7dd03..50090cc358f0 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt @@ -19,37 +19,37 @@ class ChatBroadcastReceiver : BroadcastReceiver() { } override fun onReceive(context: Context, intent: Intent) { - setupKBRuntime(context, false) val convData = ConvData.fromIntent(intent) val openConv = intent.getParcelableExtra("openConvPendingIntent") - val repliedNotification = NotificationCompat.Builder(context, KeybasePushNotificationListenerService.CHAT_CHANNEL_ID) - .setContentIntent(openConv) - .setTimeoutAfter(1000) - .setSmallIcon(R.drawable.ic_notif) - val notificationManager = NotificationManagerCompat.from(context) val messageBody = getMessageText(intent) - if (messageBody != null) { - try { - val withBackgroundActive: WithBackgroundActive = object : WithBackgroundActive { - override fun task() { + val pendingResult = goAsync() + runReceiverWork(RECEIVER_BUDGET_MS, { Thread(it).start() }, { NativeLogger.warn(it) }, { msg, e -> NativeLogger.error(msg, e) }, + { pendingResult.finish() }) { + val status = if (messageBody == null) { + NativeLogger.error("Message Body in quick reply was null") + "Couldn't send reply - Failed to read input." + } else { + setupKBRuntime(context, false) + sendQuickReply(Keybase.currentUID(), convData.lastMsgId, { msg, e -> NativeLogger.error(msg, e) }) { + withBackgroundActive(KeybaseLifecycleBind(context), null, { NativeLogger.info(it) }) { Keybase.handlePostTextReply(convData.convID, convData.tlfName, convData.lastMsgId, messageBody) } } - withBackgroundActive.whileActive(context) - repliedNotification.setContentText("Replied") - } catch (e: Exception) { - repliedNotification.setContentText("Couldn't send reply") - NativeLogger.error("Failed to send quick reply", e) } - } else { - repliedNotification.setContentText("Couldn't send reply - Failed to read input.") - NativeLogger.error("Message Body in quick reply was null") + val repliedNotification = NotificationCompat.Builder(context, KeybasePushNotificationListenerService.CHAT_CHANNEL_ID) + .setContentIntent(openConv) + .setTimeoutAfter(1000) + .setSmallIcon(R.drawable.ic_notif) + .setContentText(status) + NotificationManagerCompat.from(context).notify(convData.convID, 0, repliedNotification.build()) } - notificationManager.notify(convData.convID, 0, repliedNotification.build()) } companion object { const val KEY_TEXT_REPLY = "key_text_reply" + + // goAsync gives a broadcast 10s; leave margin. + private const val RECEIVER_BUDGET_MS = 9_000L } } diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybaseLifecycleBind.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybaseLifecycleBind.kt new file mode 100644 index 000000000000..fa1909649588 --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybaseLifecycleBind.kt @@ -0,0 +1,22 @@ +package io.keybase.ossifrage + +import android.content.Context +import android.os.Bundle +import com.reactnativekb.KbModule +import keybase.Keybase + +internal class KeybaseLifecycleBind(private val context: Context) : LifecycleBind { + override fun setAppStateForeground() = Keybase.setAppStateForeground() + + override fun setAppStateBackgroundActive() = Keybase.setAppStateBackgroundActive() + + override fun isAppStateForeground() = Keybase.isAppStateForeground() + + override fun appDidEnterBackground() = Keybase.appDidEnterBackground() + + override fun appBeginBackgroundTaskNonblock() = Keybase.appBeginBackgroundTaskNonblock(KBPushNotifier(context, Bundle())) + + override fun appWillExit() = Keybase.appWillExit(KBPushNotifier(context, Bundle())) + + override fun emitAppLifecycle(state: String) = KbModule.emitAppLifecycle(state) +} diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt index 396b0aabe2e9..db96e4523295 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt @@ -5,8 +5,6 @@ import android.app.NotificationManager import android.content.Context import android.os.Build import android.os.Bundle -import android.os.Handler -import android.os.Looper import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat import androidx.core.app.Person @@ -14,7 +12,9 @@ import com.google.firebase.messaging.FirebaseMessagingService import com.google.firebase.messaging.RemoteMessage import io.keybase.ossifrage.MainActivity.Companion.setupKBRuntime import io.keybase.ossifrage.modules.NativeLogger +import keybase.ChatNotification import keybase.Keybase +import keybase.PushNotifier import com.reactnativekb.KbModule import org.json.JSONObject @@ -23,8 +23,12 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { // was notified about to give context to future notifications. private val msgCache = HashMap() - // Avoid ever showing doubles - private val seenChatNotifications = HashSet() + // Go's seen cache dedupes what Go displays, but not the fallback below: a + // redelivered push that Go fails on again would show the fallback twice, + // and each display adds the message to msgCache's history again. + private val seenChatNotifications = object : LinkedHashMap(16, 0.75f, false) { + override fun removeEldestEntry(eldest: MutableMap.MutableEntry?) = size > SEEN_CHAT_NOTIFICATIONS_MAX + } private fun chatNotificationKey(convID: String?, messageId: Int, targetUID: String): String { return "$targetUID|$convID|$messageId" } @@ -112,12 +116,12 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { // Key includes UID so two signed-in accounts in the same chat are not treated as duplicates. if (!dontNotify) { val notificationKey = chatNotificationKey(n.convID, n.messageId, targetUID) - if (seenChatNotifications.contains(notificationKey)) { + if (seenChatNotifications.containsKey(notificationKey)) { NativeLogger.info("KeybasePushNotificationListenerService skipping duplicate notification: $notificationKey") return } // Mark as seen immediately to prevent duplicate processing - seenChatNotifications.add(notificationKey) + seenChatNotifications[notificationKey] = Unit NativeLogger.info("KeybasePushNotificationListenerService marked notification as seen: $notificationKey") } @@ -129,27 +133,18 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { } notifier.setMsgCache(msgCache[n.convID]) try { - val withBackgroundActive: WithBackgroundActive = object : WithBackgroundActive { - override fun task() { - try { - Keybase.handleBackgroundNotification(n.convID, payload, n.serverMessageBody, n.sender, - n.membersType.toLong(), n.displayPlaintext, n.messageId.toLong(), n.pushId, - n.badgeCount.toLong(), n.unixTime, n.soundName, if (dontNotify) null else notifier, true, - targetUID) - goProcessingSucceeded = true - if (!dontNotify) { - seenChatNotifications.add(chatNotificationKey(n.convID, n.messageId, targetUID)) - } - } catch (ex: Exception) { - NativeLogger.error("Go Couldn't handle background notification2: " + ex.message) - throw ex - } - } + withBackgroundActive(KeybaseLifecycleBind(applicationContext), if (dontNotify) null else notifier, + { NativeLogger.info(it) }) { pusher -> + Keybase.handleBackgroundNotification(n.convID, payload, n.serverMessageBody, n.sender, + n.membersType.toLong(), n.displayPlaintext, n.messageId.toLong(), n.pushId, + n.badgeCount.toLong(), n.unixTime, n.soundName, pusher, true, targetUID) + } + goProcessingSucceeded = true + if (!dontNotify) { + seenChatNotifications[chatNotificationKey(n.convID, n.messageId, targetUID)] = Unit } - withBackgroundActive.whileActive(applicationContext) } catch (ex: Exception) { - NativeLogger.error("Failed to process notification (app may not be running): " + ex.message) - goProcessingSucceeded = false + NativeLogger.error("Go couldn't handle background notification: " + ex.message) } } @@ -203,7 +198,7 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { chatNotif.uid = targetUID notifier.displayChatNotification(chatNotif) - seenChatNotifications.add(chatNotificationKey(n.convID, n.messageId, targetUID)) + seenChatNotifications[chatNotificationKey(n.convID, n.messageId, targetUID)] = Unit NativeLogger.info("KeybasePushNotificationListenerService fallback notification displayed successfully") } catch (e: Exception) { NativeLogger.error("Failed to display notification fallback: " + e.message) @@ -281,6 +276,7 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { } companion object { + private const val SEEN_CHAT_NOTIFICATIONS_MAX = 100 const val CHAT_CHANNEL_ID = "kb_chat_channel" const val FOLLOW_CHANNEL_ID = "kb_follow_channel" const val DEVICE_CHANNEL_ID = "kb_device_channel" @@ -395,49 +391,49 @@ internal class NotificationData(type: String, bundle: Bundle) { } } -// Interface to run some task while in backgroundActive. -// If already foreground, ignore -internal interface WithBackgroundActive { - @Throws(Exception::class) - fun task() +// Hands Go the work a push or a quick reply started. In the foreground Go +// handles it as is, so the push is still unboxed and acked, but nothing is +// displayed. Otherwise Go is held in BACKGROUNDACTIVE while the work runs and +// then decides whether it must keep running; appDidEnterBackground reports +// BACKGROUND itself (and flushes) when it returns false, so there is no +// setAppStateBackground here. +internal fun withBackgroundActive( + bind: LifecycleBind, + notifier: PushNotifier?, + log: (String) -> Unit, + work: (PushNotifier?) -> Unit, +) { + val pusher = notifier?.let { ForegroundSuppressingNotifier(it, bind) } + if (bind.isAppStateForeground()) { + log("withBackgroundActive: foreground") + work(pusher) + return + } + bind.setAppStateBackgroundActive() + work(pusher) + if (bind.isAppStateForeground()) { + log("withBackgroundActive: foregrounded during the work") + return + } + if (bind.appDidEnterBackground()) { + bind.appBeginBackgroundTaskNonblock() + } +} - @Throws(Exception::class) - fun whileActive(context: Context?) { - try { - // We are foreground don't show anything - val isForeground = Keybase.isAppStateForeground() - NativeLogger.info("WithBackgroundActive.whileActive isForeground: $isForeground") - if (isForeground) { - NativeLogger.info("WithBackgroundActive.whileActive app is foreground, returning early") - return - } else { - NativeLogger.info("WithBackgroundActive.whileActive setting background active and calling task") - Keybase.setAppStateBackgroundActive() - task() - NativeLogger.info("WithBackgroundActive.whileActive task completed") - - // Check if we are foreground now for some reason. In that case we don't want to go background again - val isForegroundNow = Keybase.isAppStateForeground() - NativeLogger.info("WithBackgroundActive.whileActive isForegroundNow: $isForegroundNow") - if (isForegroundNow) { - NativeLogger.info("WithBackgroundActive.whileActive app became foreground, returning") - return - } - val didEnterBackground = Keybase.appDidEnterBackground() - NativeLogger.info("WithBackgroundActive.whileActive didEnterBackground: $didEnterBackground") - if (didEnterBackground) { - if (context != null) { - NativeLogger.info("WithBackgroundActive.whileActive beginning background task") - Keybase.appBeginBackgroundTaskNonblock(KBPushNotifier(context, Bundle())) - } - } else { - NativeLogger.info("WithBackgroundActive.whileActive setting app state to background") - Keybase.setAppStateBackground() - } - } - } catch (ex: Exception) { - NativeLogger.error("WithBackgroundActive.whileActive exception: " + ex.message) - throw ex +// Checked at display time: the app can come to the foreground while Go works. +private class ForegroundSuppressingNotifier( + private val inner: PushNotifier, + private val bind: LifecycleBind, +) : PushNotifier { + override fun displayChatNotification(notification: ChatNotification?) { + if (bind.isAppStateForeground()) { + return } + inner.displayChatNotification(notification) } + + override fun localNotification( + ident: String?, title: String?, msg: String?, badgeCount: Long, soundName: String?, + convID: String?, typ: String?, uid: String?, + ) = inner.localNotification(ident, title, msg, badgeCount, soundName, convID, typ, uid) } diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt index f17293a9bf95..91eefdd268fa 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt @@ -10,7 +10,6 @@ import android.os.Bundle import android.os.Handler import android.os.Looper import android.provider.MediaStore -import android.provider.Settings import android.util.Log import android.view.KeyEvent import androidx.core.content.IntentCompat @@ -19,10 +18,8 @@ import com.facebook.react.ReactActivity import com.facebook.react.ReactActivityDelegate import com.facebook.react.ReactApplication import com.facebook.react.bridge.Arguments -import com.facebook.react.bridge.ReactContext import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint.fabricEnabled import com.facebook.react.defaults.DefaultReactActivityDelegate -import com.facebook.react.modules.core.PermissionListener import com.reactnativekb.DarkModePreference import com.reactnativekb.IncomingShareCache import com.reactnativekb.KbModule @@ -40,7 +37,6 @@ import java.security.cert.CertificateException import java.util.UUID class MainActivity : ReactActivity() { - private val listener: PermissionListener? = null private var isUsingHardwareKeyboard = false override fun invokeDefaultOnBackPressed() { @@ -75,8 +71,6 @@ class MainActivity : ReactActivity() { super.onCreate(null) KeybasePushNotificationListenerService.createNotificationChannel(this) updateIsUsingHardwareKeyboard() - - scheduleHandleIntent() } override fun onKeyUp(keyCode: Int, event: KeyEvent): Boolean { @@ -85,11 +79,6 @@ class MainActivity : ReactActivity() { } else super.onKeyUp(keyCode, event) } - override fun onRequestPermissionsResult(requestCode: Int, permissions: Array, grantResults: IntArray) { - listener?.onRequestPermissionsResult(requestCode, permissions, grantResults) - super.onRequestPermissionsResult(requestCode, permissions, grantResults) - } - override fun onPause() { NativeLogger.info("Activity onPause") super.onPause() @@ -111,10 +100,10 @@ class MainActivity : ReactActivity() { return filename } - private fun saveFileToCache(reactContext: ReactContext?, uri: Uri, filename: String): File { - val file = IncomingShareCache.file(reactContext!!, filename) + private fun saveFileToCache(context: Context, uri: Uri, filename: String): File { + val file = IncomingShareCache.file(context, filename) try { - reactContext.contentResolver.openInputStream(uri).use { istream -> + context.contentResolver.openInputStream(uri).use { istream -> FileOutputStream(file).use { ostream -> val buf = ByteArray(64 * 1024) var len: Int @@ -129,11 +118,11 @@ class MainActivity : ReactActivity() { return file } - private fun readFileFromUri(reactContext: ReactContext?, uri: Uri?): String? { + private fun readFileFromUri(context: Context, uri: Uri?): String? { if (uri == null) return null var filePath: String? filePath = if (uri.scheme == "content") { - val resolver = reactContext!!.contentResolver + val resolver = context.contentResolver val mimeType = resolver.getType(uri) val extension = MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType) @@ -141,7 +130,7 @@ class MainActivity : ReactActivity() { val filename = getFileNameFromResolver(resolver, uri, extension) // Now load the file itself. - val file = saveFileToCache(reactContext, uri, filename) + val file = saveFileToCache(context, uri, filename) file.path } else { uri.path @@ -152,8 +141,7 @@ class MainActivity : ReactActivity() { override fun onResume() { NativeLogger.info("Activity onResume") super.onResume() - Keybase.setAppStateForeground() - KbModule.emitAppLifecycle("active") + (application as MainApplication).lifecycleReporter.onMainActivityResume() handleIntent() } @@ -165,34 +153,36 @@ class MainActivity : ReactActivity() { override fun onDestroy() { NativeLogger.info("Activity onDestroy") super.onDestroy() - // A configuration change destroys and recreates the activity; only a - // real finish is the app going away. - if (isFinishing) { - Keybase.appWillExit(KBPushNotifier(this, Bundle())) - KbModule.emitAppLifecycle("background") - } + (application as MainApplication).lifecycleReporter.onMainActivityDestroy(isFinishing, isChangingConfigurations) } + // A share or notification intent parks here until JS asks for it. Nothing else is parked: + // deep links go through super.onNewIntent -> RCTLinkingManager, so a plain launch leaves + // this null. private var cachedIntent: Intent? = null private var pendingShareUris: List? = null private var pendingShareSubject: String? = null private var pendingShareText: String? = null - // Snapshot share/notification data out of the intent right away: share URI - // permission grants and clip data are tied to the delivered intent, and JS may - // not be ready to consume them until much later (see tryHandleIntentWithRetry). + // Snapshot share data out of the intent right away: share URI permission grants and clip + // data are tied to the delivered intent, and JS may not be ready to route them until much + // later (see shareListenersRegistered). private fun captureIntent(intent: Intent) { + val bundleFromNotification = intent.getBundleExtra("notification") + if (bundleFromNotification != null) { + KbModule.setInitialNotification(bundleFromNotification.clone() as Bundle) + } + val isShare = Intent.ACTION_SEND == intent.action || Intent.ACTION_SEND_MULTIPLE == intent.action + if (!isShare && bundleFromNotification == null) { + return + } cachedIntent = intent - if (Intent.ACTION_SEND == intent.action || Intent.ACTION_SEND_MULTIPLE == intent.action) { + if (isShare) { pendingShareUris = extractSharedUris(intent) pendingShareSubject = intent.getStringExtra(Intent.EXTRA_SUBJECT) pendingShareText = intent.getStringExtra(Intent.EXTRA_TEXT) } - val bundleFromNotification = intent.getBundleExtra("notification") - if (bundleFromNotification != null) { - KbModule.setInitialNotification(bundleFromNotification.clone() as Bundle) - } } override fun onNewIntent(intent: Intent) { @@ -204,9 +194,11 @@ class MainActivity : ReactActivity() { private var jsIsListening = false + // JS calls this once it is ready to route a share. That is the only signal the parked + // intent waits on. public fun shareListenersRegistered() { jsIsListening = true - tryHandleIntentWithRetry() + handleIntent() } private var handledIntentHash: String? = null @@ -242,37 +234,9 @@ class MainActivity : ReactActivity() { return uris.distinct() } - private var handleIntentRetryCount = 0 - private val maxHandleIntentRetries = 20 // 20 * 500ms = 10s max - - private fun scheduleHandleIntent() { - if (cachedIntent == null) return - handleIntentRetryCount = 0 - tryHandleIntentWithRetry() - } - - private fun tryHandleIntentWithRetry() { - if (cachedIntent == null) return - if (handleIntent()) return - handleIntentRetryCount++ - if (handleIntentRetryCount >= maxHandleIntentRetries) { - NativeLogger.info("MainActivity: giving up on handleIntent after $maxHandleIntentRetries retries") - return - } - NativeLogger.info("MainActivity: scheduling handleIntent retry #$handleIntentRetryCount") - Handler(Looper.getMainLooper()).postDelayed({ tryHandleIntentWithRetry() }, 500) - } - - private fun handleIntent(): Boolean { - val intent = cachedIntent ?: return true - val rc = reactActivityDelegate?.getCurrentReactContext() ?: run { - NativeLogger.info("MainActivity.handleIntent: no react context, will retry") - return false - } - if (!jsIsListening) { - NativeLogger.info("MainActivity.handleIntent: JS not listening yet, will retry") - return false - } + private fun handleIntent() { + val intent = cachedIntent ?: return + if (!jsIsListening) return NativeLogger.info("MainActivity.handleIntent: processing intent action=${intent.action}") // Here we are just reading from the notification bundle. @@ -322,10 +286,11 @@ class MainActivity : ReactActivity() { } else { // Copying out of the content providers can be slow for big files; don't // block the main thread on it. + val context: Context = this Thread { val filePaths = uris.mapNotNull { uri -> try { - readFileFromUri(rc, uri) + readFileFromUri(context, uri) } catch (e: SecurityException) { null } @@ -343,7 +308,6 @@ class MainActivity : ReactActivity() { } cachedIntent = null - return true } private fun emitShareText(text: String) { @@ -439,12 +403,6 @@ class MainActivity : ReactActivity() { } } - // Is this a robot controlled test device? (i.e. pre-launch report?) - fun isTestDevice(context: Context): Boolean { - val testLabSetting = Settings.System.getString(context.contentResolver, "firebase.test.lab") - return "true" == testLabSetting - } - @JvmStatic fun setupKBRuntime(context: Context, shouldCreateDummyFile: Boolean) { try { diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/MainApplication.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/MainApplication.kt index fa1d20abc963..857c636b162a 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/MainApplication.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/MainApplication.kt @@ -6,9 +6,11 @@ import android.content.res.Configuration import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ProcessLifecycleOwner +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.Operation import androidx.work.PeriodicWorkRequest import androidx.work.WorkManager -import androidx.work.WorkRequest +import androidx.work.await import com.bumptech.glide.Glide import com.facebook.react.PackageList import com.facebook.react.ReactApplication @@ -22,9 +24,15 @@ import com.reactnativekb.IncomingShareCache import expo.modules.ApplicationLifecycleDispatcher.onApplicationCreate import expo.modules.ApplicationLifecycleDispatcher.onConfigurationChanged import expo.modules.ExpoReactHostFactory +import io.keybase.ossifrage.modules.BackgroundSyncJobs import io.keybase.ossifrage.modules.BackgroundSyncWorker +import io.keybase.ossifrage.modules.LegacyJobsCleanupFlag import io.keybase.ossifrage.modules.NativeLogger +import io.keybase.ossifrage.modules.scheduleBackgroundSync import keybase.Keybase +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import java.util.concurrent.TimeUnit internal class AppLifecycleListener(private val context: Context?) : @@ -51,12 +59,15 @@ class MainApplication : Application(), ReactApplication { ) } + internal val lifecycleReporter by lazy { + AppLifecycleReporter(KeybaseLifecycleBind(this)) { NativeLogger.info(it) } + } override fun onCreate() { NativeLogger.info("MainApplication created") super.onCreate() // Before any activity starts, so the first process ON_START is seen. - ProcessLifecycleOwner.get().lifecycle.addObserver(AppLifecycleForwarder(this)) + ProcessLifecycleOwner.get().lifecycle.addObserver(lifecycleReporter) try { DefaultNewArchitectureEntryPoint.releaseLevel = ReleaseLevel.valueOf(BuildConfig.REACT_NATIVE_RELEASE_LEVEL.uppercase()) } catch (e: IllegalArgumentException) { @@ -75,15 +86,13 @@ class MainApplication : Application(), ReactApplication { } }.start() - val backgroundSyncRequest: WorkRequest = PeriodicWorkRequest.Builder( - BackgroundSyncWorker::class.java, - 1, TimeUnit.HOURS, - 15, TimeUnit.MINUTES - ) - .build() - WorkManager - .getInstance(this) - .enqueue(backgroundSyncRequest) + Thread { + try { + scheduleBackgroundSync(WorkManagerBackgroundSyncJobs(this), SharedPrefsCleanupFlag(this)) + } catch (e: Exception) { + NativeLogger.warn("MainApplication: error scheduling background sync", e) + } + }.start() } fun onReactContextInitialized(context: ReactContext?) { @@ -102,3 +111,49 @@ class MainApplication : Application(), ReactApplication { super.onLowMemory() } } + +private class WorkManagerBackgroundSyncJobs(context: Context) : BackgroundSyncJobs { + private val workManager = WorkManager.getInstance(context) + + // WorkManager tags every request with its worker's class name. + override fun cancelAll() { + workManager.cancelAllWorkByTag(BackgroundSyncWorker::class.java.name).awaitDone("cancel") + } + + override fun enqueueUnique() { + val request = PeriodicWorkRequest.Builder( + BackgroundSyncWorker::class.java, + 1, TimeUnit.HOURS, + 15, TimeUnit.MINUTES + ).build() + workManager.enqueueUniquePeriodicWork("background_sync", ExistingPeriodicWorkPolicy.KEEP, request).awaitDone("enqueue") + } + + // A stalled WorkManager must not park the scheduling thread forever. + private fun Operation.awaitDone(what: String) { + try { + runBlocking { withTimeout(OPERATION_TIMEOUT_MS) { await() } } + } catch (e: TimeoutCancellationException) { + NativeLogger.warn("MainApplication: background sync $what timed out after ${OPERATION_TIMEOUT_MS}ms") + throw e + } + } + + companion object { + private const val OPERATION_TIMEOUT_MS = 30_000L + } +} + +private class SharedPrefsCleanupFlag(context: Context) : LegacyJobsCleanupFlag { + private val prefs = context.getSharedPreferences("background_sync", Context.MODE_PRIVATE) + + override fun isDone() = prefs.getBoolean(KEY, false) + + override fun markDone() { + prefs.edit().putBoolean(KEY, true).commit() + } + + companion object { + private const val KEY = "legacy_jobs_cancelled" + } +} diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/modules/BackgroundSyncSchedule.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/modules/BackgroundSyncSchedule.kt new file mode 100644 index 000000000000..930c305053f6 --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/modules/BackgroundSyncSchedule.kt @@ -0,0 +1,29 @@ +package io.keybase.ossifrage.modules + +// WorkManager and the persisted flag, behind interfaces so the scheduling +// decision runs in JVM tests. Each call returns once its operation is done and +// throws if it failed. +internal interface BackgroundSyncJobs { + // Cancels every BackgroundSyncWorker job, including ones enqueued without + // a unique name by older versions. + fun cancelAll() + fun enqueueUnique() +} + +internal interface LegacyJobsCleanupFlag { + fun isDone(): Boolean + fun markDone() +} + +// Older versions enqueued a new periodic job on every process start, so +// existing installs can carry many. Clear them once, then keep one unique job +// whose period isn't reset on each launch. +internal fun scheduleBackgroundSync(jobs: BackgroundSyncJobs, cleanup: LegacyJobsCleanupFlag) { + if (cleanup.isDone()) { + jobs.enqueueUnique() + return + } + jobs.cancelAll() + jobs.enqueueUnique() + cleanup.markDone() +} diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt new file mode 100644 index 000000000000..d905af7825a2 --- /dev/null +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt @@ -0,0 +1,228 @@ +package io.keybase.ossifrage + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import java.util.Collections +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +private class FakeBind : LifecycleBind { + val calls: MutableList = Collections.synchronizedList(mutableListOf()) + val threads: MutableSet = Collections.synchronizedSet(mutableSetOf()) + + private fun record(call: String) { + threads.add(Thread.currentThread()) + calls.add(call) + } + + override fun setAppStateForeground() = record("setAppStateForeground") + + override fun setAppStateBackgroundActive() = record("setAppStateBackgroundActive") + + override fun isAppStateForeground() = false + + override fun appDidEnterBackground(): Boolean { + record("appDidEnterBackground") + return false + } + + override fun appBeginBackgroundTaskNonblock() = record("appBeginBackgroundTaskNonblock") + + override fun appWillExit() = record("appWillExit") + + override fun emitAppLifecycle(state: String) {} +} + +private object Owner : LifecycleOwner { + override val lifecycle: Lifecycle + get() = throw UnsupportedOperationException() +} + +class AppLifecycleReporterTest { + private val bind = FakeBind() + private val reporter = AppLifecycleReporter(bind) {} + + private fun launch() { + reporter.onCreate(Owner) + reporter.onStart(Owner) + reporter.onResume(Owner) + } + + private fun stop() { + reporter.onPause(Owner) + reporter.onStop(Owner) + } + + private fun calls(): List = bind.calls.toList() + + @Test + fun processStartAndStopReportEventsInOrder() { + launch() + stop() + reporter.onStart(Owner) + reporter.onResume(Owner) + assertEquals( + listOf( + "setAppStateForeground", "setAppStateForeground", + "appDidEnterBackground", + "setAppStateForeground", "setAppStateForeground", + ), + calls(), + ) + } + + @Test + fun eventsReachGoOnTheCallingThreadBeforeTheCallbackReturns() { + reporter.onStart(Owner) + assertEquals(listOf("setAppStateForeground"), calls()) + reporter.onResume(Owner) + assertEquals(listOf("setAppStateForeground", "setAppStateForeground"), calls()) + reporter.onStop(Owner) + assertEquals(listOf("setAppStateForeground", "setAppStateForeground", "appDidEnterBackground"), calls()) + assertEquals(setOf(Thread.currentThread()), bind.threads.toSet()) + } + + @Test + fun dialogOrPermissionPromptPauseNeverBackgrounds() { + launch() + reporter.onPause(Owner) + reporter.onResume(Owner) + reporter.onPause(Owner) + assertEquals(listOf("setAppStateForeground", "setAppStateForeground", "setAppStateForeground"), calls()) + } + + // A full-screen picker or camera stops the process like any other exit. + @Test + fun fullScreenPickerBackgroundsAndReturningForegrounds() { + launch() + stop() + reporter.onStart(Owner) + reporter.onResume(Owner) + assertEquals( + listOf( + "setAppStateForeground", "setAppStateForeground", + "appDidEnterBackground", + "setAppStateForeground", "setAppStateForeground", + ), + calls(), + ) + } + + @Test + fun onlyAFinishingActivityExits() { + launch() + reporter.onMainActivityDestroy(isFinishing = false, isChangingConfigurations = false) + reporter.onMainActivityDestroy(isFinishing = true, isChangingConfigurations = true) + assertEquals(listOf("setAppStateForeground", "setAppStateForeground"), calls()) + reporter.onMainActivityDestroy(isFinishing = true, isChangingConfigurations = false) + stop() + reporter.onStart(Owner) + reporter.onResume(Owner) + assertEquals( + listOf( + "setAppStateForeground", "setAppStateForeground", + "appWillExit", "appDidEnterBackground", + "setAppStateForeground", "setAppStateForeground", + ), + calls(), + ) + } +} + +class SendQuickReplyTest { + private val errors = mutableListOf>() + private var sent = false + + private fun send(currentUID: String = "uid", msgId: Long = 1, send: () -> Unit = { sent = true }) = + sendQuickReply(currentUID, msgId, { msg, e -> errors.add(msg to e) }, send) + + @Test + fun replySends() { + assertEquals(QUICK_REPLY_SENT, send()) + assertTrue(sent) + assertTrue(errors.isEmpty()) + } + + @Test + fun failedReplyIsNotReportedAsRepliedAndLogsTheException() { + val failure = IllegalStateException("outbox full") + assertEquals(QUICK_REPLY_FAILED, send { throw failure }) + assertEquals(listOf>("Failed to send quick reply" to failure), errors.toList()) + } + + // Go sends before it checks either, and swallows the send's error. + @Test + fun loggedOutReplyIsNotSent() { + assertEquals(QUICK_REPLY_FAILED, send(currentUID = "")) + assertFalse(sent) + assertEquals(1, errors.size) + } + + @Test + fun replyToAnInvalidMessageIsNotSent() { + assertEquals(QUICK_REPLY_FAILED, send(msgId = -1)) + assertFalse(sent) + assertEquals(1, errors.size) + } +} + +class RunReceiverWorkTest { + private val finishes = AtomicInteger() + private val finished = CountDownLatch(1) + private val warnings = Collections.synchronizedList(mutableListOf()) + private val errors = Collections.synchronizedList(mutableListOf()) + + private fun run(budgetMs: Long, work: () -> Unit) = runReceiverWork( + budgetMs, + { r -> Thread(r).start() }, + { warnings.add(it) }, + { _, e -> errors.add(e) }, + { + finishes.incrementAndGet() + finished.countDown() + }, + work, + ) + + @Test(timeout = 10_000) + fun finishesAfterTheWorkOffTheCallingThread() { + val ranOn = AtomicReference() + run(10_000) { ranOn.set(Thread.currentThread()) } + assertTrue(finished.await(5, TimeUnit.SECONDS)) + assertTrue(ranOn.get() != Thread.currentThread()) + Thread.sleep(50) + assertEquals(1, finishes.get()) + assertTrue(warnings.isEmpty()) + } + + @Test(timeout = 10_000) + fun finishesAndLogsWhenTheWorkThrows() { + val failure = IllegalStateException("boom") + run(10_000) { throw failure } + assertTrue(finished.await(5, TimeUnit.SECONDS)) + assertEquals(listOf(failure), errors.toList()) + assertEquals(1, finishes.get()) + } + + @Test(timeout = 10_000) + fun finishesAtTheBudgetWhileTheWorkIsStillRunning() { + val release = CountDownLatch(1) + val workDone = CountDownLatch(1) + run(100) { + release.await(5, TimeUnit.SECONDS) + workDone.countDown() + } + assertTrue(finished.await(5, TimeUnit.SECONDS)) + assertEquals(1, warnings.size) + release.countDown() + assertTrue(workDone.await(5, TimeUnit.SECONDS)) + Thread.sleep(50) + assertEquals("finishes once", 1, finishes.get()) + } +} diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt new file mode 100644 index 000000000000..0d82b561ddee --- /dev/null +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt @@ -0,0 +1,153 @@ +package io.keybase.ossifrage + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import keybase.ChatNotification +import keybase.PushNotifier +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test + +// Mirrors Go's MobileAppState for the calls the push window makes. +private class StateBind(var foreground: Boolean, var keepRunning: Boolean = false) : LifecycleBind { + val calls = mutableListOf() + + override fun setAppStateForeground() { + calls.add("setAppStateForeground") + foreground = true + } + + override fun setAppStateBackgroundActive() { + calls.add("setAppStateBackgroundActive") + foreground = false + } + + override fun isAppStateForeground() = foreground + + override fun appDidEnterBackground(): Boolean { + calls.add("appDidEnterBackground") + foreground = false + return keepRunning + } + + override fun appBeginBackgroundTaskNonblock() { + calls.add("appBeginBackgroundTaskNonblock") + } + + override fun appWillExit() { + calls.add("appWillExit") + } + + override fun emitAppLifecycle(state: String) {} +} + +// gomobile's ChatNotification loads the Go library when constructed, so the +// tests display null. +private class FakeNotifier : PushNotifier { + var displays = 0 + + override fun displayChatNotification(notification: ChatNotification?) { + displays++ + } + + override fun localNotification( + ident: String?, title: String?, msg: String?, badgeCount: Long, soundName: String?, + convID: String?, typ: String?, uid: String?, + ) {} +} + +private object ProcessOwner : LifecycleOwner { + override val lifecycle: Lifecycle + get() = throw UnsupportedOperationException() +} + +class WithBackgroundActiveTest { + private val notifier = FakeNotifier() + + // Stands in for handleBackgroundNotification: Go displays through the + // notifier it is handed. + private fun handlePush(bind: StateBind, during: () -> Unit = {}): PushNotifier? { + var handed: PushNotifier? = null + withBackgroundActive(bind, notifier, {}) { n -> + bind.calls.add("handleBackgroundNotification") + handed = n + during() + n?.displayChatNotification(null) + } + return handed + } + + @Test + fun foregroundPushIsHandledByGoWithDisplaySuppressed() { + val bind = StateBind(foreground = true) + assertNotNull(handlePush(bind)) + assertEquals(listOf("handleBackgroundNotification"), bind.calls) + assertEquals(0, notifier.displays) + } + + @Test + fun backgroundPushHoldsGoActiveThenHandsOverToABackgroundTask() { + val bind = StateBind(foreground = false, keepRunning = true) + handlePush(bind) + assertEquals( + listOf( + "setAppStateBackgroundActive", "handleBackgroundNotification", + "appDidEnterBackground", "appBeginBackgroundTaskNonblock", + ), + bind.calls, + ) + assertEquals(1, notifier.displays) + } + + // appDidEnterBackground reports BACKGROUND itself when nothing keeps the + // app running; each of those calls is a full leveldb flush. + @Test + fun backgroundPushWithNothingRunningGoesBackOnce() { + val bind = StateBind(foreground = false) + handlePush(bind) + assertEquals( + listOf("setAppStateBackgroundActive", "handleBackgroundNotification", "appDidEnterBackground"), + bind.calls, + ) + assertEquals(1, notifier.displays) + } + + @Test + fun pushAfterTheProcessWentToBackgroundDoesNotReportBackgroundAgain() { + val bind = StateBind(foreground = false) + val reporter = AppLifecycleReporter(bind) {} + reporter.onStart(ProcessOwner) + reporter.onResume(ProcessOwner) + reporter.onStop(ProcessOwner) + handlePush(bind) + assertEquals( + listOf( + "setAppStateForeground", "setAppStateForeground", "appDidEnterBackground", + "setAppStateBackgroundActive", "handleBackgroundNotification", "appDidEnterBackground", + ), + bind.calls, + ) + } + + @Test + fun appOpenedDuringThePushStaysForeground() { + val bind = StateBind(foreground = false) + handlePush(bind) { bind.setAppStateForeground() } + assertEquals( + listOf("setAppStateBackgroundActive", "handleBackgroundNotification", "setAppStateForeground"), + bind.calls, + ) + assertEquals(0, notifier.displays) + } + + @Test + fun silentPushGetsNoNotifier() { + val bind = StateBind(foreground = true) + var handed: PushNotifier? = notifier + withBackgroundActive(bind, null, {}) { handed = it } + assertNull(handed) + assertFalse(bind.calls.contains("setAppStateBackgroundActive")) + } +} diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/modules/BackgroundSyncScheduleTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/modules/BackgroundSyncScheduleTest.kt new file mode 100644 index 000000000000..42e7f560956f --- /dev/null +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/modules/BackgroundSyncScheduleTest.kt @@ -0,0 +1,64 @@ +package io.keybase.ossifrage.modules + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test + +class BackgroundSyncScheduleTest { + private val calls = mutableListOf() + private var enqueueFails = false + private var done = false + + private val jobs = object : BackgroundSyncJobs { + override fun cancelAll() { + calls.add("cancelAll") + } + + override fun enqueueUnique() { + if (enqueueFails) throw IllegalStateException("enqueue failed") + calls.add("enqueueUnique") + } + } + + private val flag = object : LegacyJobsCleanupFlag { + override fun isDone() = done + + override fun markDone() { + done = true + } + } + + @Test + fun firstRunCancelsLegacyJobsThenEnqueues() { + scheduleBackgroundSync(jobs, flag) + assertEquals(listOf("cancelAll", "enqueueUnique"), calls) + assertTrue(done) + } + + @Test + fun laterRunsOnlyEnqueue() { + scheduleBackgroundSync(jobs, flag) + calls.clear() + scheduleBackgroundSync(jobs, flag) + scheduleBackgroundSync(jobs, flag) + assertEquals(listOf("enqueueUnique", "enqueueUnique"), calls) + } + + @Test + fun failedEnqueueRetriesTheCleanupNextRun() { + enqueueFails = true + try { + scheduleBackgroundSync(jobs, flag) + fail("the failure propagates") + } catch (e: IllegalStateException) { + } + assertFalse(done) + enqueueFails = false + calls.clear() + scheduleBackgroundSync(jobs, flag) + assertEquals(listOf("cancelAll", "enqueueUnique"), calls) + assertTrue(done) + } +} From ca0c192a49e518382f024bd78be8adf48422e39d Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 13:27:51 -0400 Subject: [PATCH 2/4] fix(android): skip Go for a silent push in the foreground, fail a reply whose uid can't be read A silent push gets no notifier, and Go only acks a push it can display, so in the foreground Go unboxed it for nothing; master skipped it there. The quick reply keeps running in the foreground. Reading the current uid can throw; the reply now reports failure instead of leaving the RemoteInput stuck sending. Flush audit: no change to any flushing call. A foreground silent push makes no lifecycle call (as on master); a background one is unchanged from the previous commit. --- .../keybase/ossifrage/AppLifecycleReporter.kt | 10 +++- .../ossifrage/ChatBroadcastReceiver.kt | 2 +- .../KeybasePushNotificationListenerService.kt | 23 ++++++++- .../ossifrage/AppLifecycleReporterTest.kt | 30 +++++------- .../ossifrage/WithBackgroundActiveTest.kt | 48 +++++++++++++++++-- 5 files changed, 87 insertions(+), 26 deletions(-) diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt index e2759ad4145a..9f405cca1236 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt @@ -71,13 +71,19 @@ internal class AppLifecycleReporter( // Sends a notification quick reply. Returns the text for the replied // notification. internal fun sendQuickReply( - currentUID: String, + currentUID: () -> String, msgId: Long, error: (String, Throwable?) -> Unit, send: () -> Unit, ): String { + val uid = try { + currentUID() + } catch (e: Exception) { + error("Quick reply couldn't read the current uid", e) + return QUICK_REPLY_FAILED + } // Go sends before it checks either, and swallows the send's error. - if (currentUID.isEmpty()) { + if (uid.isEmpty()) { error("Quick reply while logged out", null) return QUICK_REPLY_FAILED } diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt index 50090cc358f0..c2e5f87b0f6b 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt @@ -30,7 +30,7 @@ class ChatBroadcastReceiver : BroadcastReceiver() { "Couldn't send reply - Failed to read input." } else { setupKBRuntime(context, false) - sendQuickReply(Keybase.currentUID(), convData.lastMsgId, { msg, e -> NativeLogger.error(msg, e) }) { + sendQuickReply({ Keybase.currentUID() }, convData.lastMsgId, { msg, e -> NativeLogger.error(msg, e) }) { withBackgroundActive(KeybaseLifecycleBind(context), null, { NativeLogger.info(it) }) { Keybase.handlePostTextReply(convData.convID, convData.tlfName, convData.lastMsgId, messageBody) } diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt index db96e4523295..5d4f7cf5cf93 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt @@ -133,7 +133,7 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { } notifier.setMsgCache(msgCache[n.convID]) try { - withBackgroundActive(KeybaseLifecycleBind(applicationContext), if (dontNotify) null else notifier, + handleChatPush(KeybaseLifecycleBind(applicationContext), notifier, dontNotify, { NativeLogger.info(it) }) { pusher -> Keybase.handleBackgroundNotification(n.convID, payload, n.serverMessageBody, n.sender, n.membersType.toLong(), n.displayPlaintext, n.messageId.toLong(), n.pushId, @@ -391,6 +391,27 @@ internal class NotificationData(type: String, bundle: Bundle) { } } +// A silent push gets no notifier, and Go only acks a push it can display, so in +// the foreground Go would unbox it for nothing: the loud push that follows is +// the one Go handles. +internal fun handleChatPush( + bind: LifecycleBind, + notifier: PushNotifier, + silent: Boolean, + log: (String) -> Unit, + work: (PushNotifier?) -> Unit, +) { + if (!silent) { + withBackgroundActive(bind, notifier, log, work) + return + } + if (bind.isAppStateForeground()) { + log("handleChatPush: silent push in the foreground, skipping Go") + return + } + withBackgroundActive(bind, null, log, work) +} + // Hands Go the work a push or a quick reply started. In the foreground Go // handles it as is, so the push is still unboxed and acked, but nothing is // displayed. Otherwise Go is held in BACKGROUNDACTIVE while the work runs and diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt index d905af7825a2..7d60a19fa172 100644 --- a/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt @@ -61,6 +61,7 @@ class AppLifecycleReporterTest { private fun calls(): List = bind.calls.toList() + // A full-screen picker or camera stops the process like any other exit. @Test fun processStartAndStopReportEventsInOrder() { launch() @@ -97,23 +98,6 @@ class AppLifecycleReporterTest { assertEquals(listOf("setAppStateForeground", "setAppStateForeground", "setAppStateForeground"), calls()) } - // A full-screen picker or camera stops the process like any other exit. - @Test - fun fullScreenPickerBackgroundsAndReturningForegrounds() { - launch() - stop() - reporter.onStart(Owner) - reporter.onResume(Owner) - assertEquals( - listOf( - "setAppStateForeground", "setAppStateForeground", - "appDidEnterBackground", - "setAppStateForeground", "setAppStateForeground", - ), - calls(), - ) - } - @Test fun onlyAFinishingActivityExits() { launch() @@ -139,7 +123,7 @@ class SendQuickReplyTest { private val errors = mutableListOf>() private var sent = false - private fun send(currentUID: String = "uid", msgId: Long = 1, send: () -> Unit = { sent = true }) = + private fun send(currentUID: () -> String = { "uid" }, msgId: Long = 1, send: () -> Unit = { sent = true }) = sendQuickReply(currentUID, msgId, { msg, e -> errors.add(msg to e) }, send) @Test @@ -159,11 +143,19 @@ class SendQuickReplyTest { // Go sends before it checks either, and swallows the send's error. @Test fun loggedOutReplyIsNotSent() { - assertEquals(QUICK_REPLY_FAILED, send(currentUID = "")) + assertEquals(QUICK_REPLY_FAILED, send(currentUID = { "" })) assertFalse(sent) assertEquals(1, errors.size) } + @Test + fun unreadableUidFailsTheReply() { + val failure = IllegalStateException("go not ready") + assertEquals(QUICK_REPLY_FAILED, send(currentUID = { throw failure })) + assertFalse(sent) + assertEquals(listOf(failure), errors.map { it.second }) + } + @Test fun replyToAnInvalidMessageIsNotSent() { assertEquals(QUICK_REPLY_FAILED, send(msgId = -1)) diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt index 0d82b561ddee..f983fe822fe6 100644 --- a/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt @@ -8,6 +8,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test // Mirrors Go's MobileAppState for the calls the push window makes. @@ -142,12 +143,53 @@ class WithBackgroundActiveTest { assertEquals(0, notifier.displays) } + // A quick reply has no notifier and must still send in the foreground. @Test - fun silentPushGetsNoNotifier() { + fun foregroundWorkWithoutANotifierStillRuns() { val bind = StateBind(foreground = true) var handed: PushNotifier? = notifier - withBackgroundActive(bind, null, {}) { handed = it } + withBackgroundActive(bind, null, {}) { + bind.calls.add("handlePostTextReply") + handed = it + } assertNull(handed) - assertFalse(bind.calls.contains("setAppStateBackgroundActive")) + assertEquals(listOf("handlePostTextReply"), bind.calls) + } + + private fun handleSilentPush(bind: StateBind): Boolean { + var ran = false + handleChatPush(bind, notifier, silent = true, {}) { n -> + bind.calls.add("handleBackgroundNotification") + ran = true + assertNull(n) + } + return ran + } + + // Go only acks a push it is handed a notifier for, so a silent push in the + // foreground would be unboxed for nothing. + @Test + fun foregroundSilentPushSkipsGo() { + val bind = StateBind(foreground = true) + assertFalse(handleSilentPush(bind)) + assertEquals(listOf(), bind.calls) + } + + @Test + fun backgroundSilentPushIsHandledWithoutANotifier() { + val bind = StateBind(foreground = false) + assertTrue(handleSilentPush(bind)) + assertEquals( + listOf("setAppStateBackgroundActive", "handleBackgroundNotification", "appDidEnterBackground"), + bind.calls, + ) + } + + @Test + fun loudPushIsHandedTheNotifier() { + val bind = StateBind(foreground = true) + var handed: PushNotifier? = null + handleChatPush(bind, notifier, silent = false, {}) { handed = it } + assertNotNull(handed) } } From 8717aa20d19609e06231de4e935ecd0b5df1689c Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:34:11 -0400 Subject: [PATCH 3/4] fix(android): hand Go back to the background when push work throws --- .../KeybasePushNotificationListenerService.kt | 17 +++++++++------- .../ossifrage/WithBackgroundActiveTest.kt | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt index 5d4f7cf5cf93..b3c36eb38942 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt @@ -431,13 +431,16 @@ internal fun withBackgroundActive( return } bind.setAppStateBackgroundActive() - work(pusher) - if (bind.isAppStateForeground()) { - log("withBackgroundActive: foregrounded during the work") - return - } - if (bind.appDidEnterBackground()) { - bind.appBeginBackgroundTaskNonblock() + // Work that throws still hands Go back to the background, or it would stay + // in BACKGROUNDACTIVE until the next lifecycle event. + try { + work(pusher) + } finally { + if (bind.isAppStateForeground()) { + log("withBackgroundActive: foregrounded during the work") + } else if (bind.appDidEnterBackground()) { + bind.appBeginBackgroundTaskNonblock() + } } } diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt index f983fe822fe6..6ae7d7cd25df 100644 --- a/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/WithBackgroundActiveTest.kt @@ -8,6 +8,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test @@ -143,6 +144,25 @@ class WithBackgroundActiveTest { assertEquals(0, notifier.displays) } + @Test + fun throwingWorkStillGoesBackToTheBackground() { + val bind = StateBind(foreground = false, keepRunning = true) + val thrown = assertThrows(IllegalStateException::class.java) { + withBackgroundActive(bind, notifier, {}) { + bind.calls.add("handleBackgroundNotification") + throw IllegalStateException("go failed") + } + } + assertEquals("go failed", thrown.message) + assertEquals( + listOf( + "setAppStateBackgroundActive", "handleBackgroundNotification", + "appDidEnterBackground", "appBeginBackgroundTaskNonblock", + ), + bind.calls, + ) + } + // A quick reply has no notifier and must still send in the foreground. @Test fun foregroundWorkWithoutANotifierStillRuns() { From 7288cbd08699ec86e7e88b066fe10a1558206b5b Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:34:12 -0400 Subject: [PATCH 4/4] fix(android): refuse a quick reply from another account's notification The notification carries the account it was shown for; Go posts as the current account, so a mismatch reports that the reply couldn't be sent. --- .../keybase/ossifrage/AppLifecycleReporter.kt | 9 +++++++- .../ossifrage/ChatBroadcastReceiver.kt | 10 ++++++--- .../io/keybase/ossifrage/KBPushNotifier.kt | 2 +- .../ossifrage/AppLifecycleReporterTest.kt | 22 +++++++++++++++++-- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt index 9f405cca1236..1a9625418a02 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt @@ -69,9 +69,12 @@ internal class AppLifecycleReporter( } // Sends a notification quick reply. Returns the text for the replied -// notification. +// notification. notificationUID is the account the notification was shown for; +// Go posts as whichever account is current, so a reply from another account's +// notification is refused. internal fun sendQuickReply( currentUID: () -> String, + notificationUID: String, msgId: Long, error: (String, Throwable?) -> Unit, send: () -> Unit, @@ -87,6 +90,10 @@ internal fun sendQuickReply( error("Quick reply while logged out", null) return QUICK_REPLY_FAILED } + if (uid != notificationUID) { + error("Quick reply from another account's notification", null) + return QUICK_REPLY_FAILED + } if (msgId < 0) { error("Quick reply to invalid message id $msgId", null) return QUICK_REPLY_FAILED diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt index c2e5f87b0f6b..7c61dbd81827 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/ChatBroadcastReceiver.kt @@ -30,7 +30,7 @@ class ChatBroadcastReceiver : BroadcastReceiver() { "Couldn't send reply - Failed to read input." } else { setupKBRuntime(context, false) - sendQuickReply({ Keybase.currentUID() }, convData.lastMsgId, { msg, e -> NativeLogger.error(msg, e) }) { + sendQuickReply({ Keybase.currentUID() }, convData.uid, convData.lastMsgId, { msg, e -> NativeLogger.error(msg, e) }) { withBackgroundActive(KeybaseLifecycleBind(context), null, { NativeLogger.info(it) }) { Keybase.handlePostTextReply(convData.convID, convData.tlfName, convData.lastMsgId, messageBody) } @@ -56,13 +56,16 @@ class ChatBroadcastReceiver : BroadcastReceiver() { internal data class ConvData( @JvmField val convID: String?, val tlfName: String?, - val lastMsgId: Long + val lastMsgId: Long, + // The account the notification belongs to. + val uid: String, ) { fun intoIntent(context: Context?): Intent { val data = Bundle() data.putString("convID", convID) data.putString("tlfName", tlfName) data.putLong("lastMsgId", lastMsgId) + data.putString("uid", uid) val intent = Intent(context, ChatBroadcastReceiver::class.java) intent.putExtra("ConvData", data) return intent @@ -74,7 +77,8 @@ internal data class ConvData( return ConvData( convID = data.getString("convID"), tlfName = data.getString("tlfName"), - lastMsgId = data.getLong("lastMsgId") + lastMsgId = data.getLong("lastMsgId"), + uid = data.getString("uid") ?: "", ) } } diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt index 1eed346fdc0b..e67c1927ef74 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt @@ -112,7 +112,7 @@ class KBPushNotifier internal constructor(private val context: Context, private bundle.putString("uid", chatNotification.uid) } val pending_intent = buildPendingIntent(bundle) - val convData = ConvData(chatNotification.convID, chatNotification.tlfName ?: "", chatNotification.message.id) + val convData = ConvData(chatNotification.convID, chatNotification.tlfName ?: "", chatNotification.message.id, chatNotification.uid ?: "") val builder = NotificationCompat.Builder(context, KeybasePushNotificationListenerService.CHAT_CHANNEL_ID) .setSmallIcon(R.drawable.ic_notif) .setContentTitle(chatNotification.title ?: "") diff --git a/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt index 7d60a19fa172..73bf70ec331e 100644 --- a/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt @@ -123,8 +123,12 @@ class SendQuickReplyTest { private val errors = mutableListOf>() private var sent = false - private fun send(currentUID: () -> String = { "uid" }, msgId: Long = 1, send: () -> Unit = { sent = true }) = - sendQuickReply(currentUID, msgId, { msg, e -> errors.add(msg to e) }, send) + private fun send( + currentUID: () -> String = { "uid" }, + notificationUID: String = "uid", + msgId: Long = 1, + send: () -> Unit = { sent = true }, + ) = sendQuickReply(currentUID, notificationUID, msgId, { msg, e -> errors.add(msg to e) }, send) @Test fun replySends() { @@ -156,6 +160,20 @@ class SendQuickReplyTest { assertEquals(listOf(failure), errors.map { it.second }) } + // Go would post it as the current account. + @Test + fun replyFromAnotherAccountsNotificationIsNotSent() { + assertEquals(QUICK_REPLY_FAILED, send(currentUID = { "uid" }, notificationUID = "other-uid")) + assertFalse(sent) + assertEquals(1, errors.size) + } + + @Test + fun replyFromANotificationWithoutAnAccountIsNotSent() { + assertEquals(QUICK_REPLY_FAILED, send(notificationUID = "")) + assertFalse(sent) + } + @Test fun replyToAnInvalidMessageIsNotSent() { assertEquals(QUICK_REPLY_FAILED, send(msgId = -1))