diff --git a/go/bind/keybase.go b/go/bind/keybase.go index f8beb533fac4..3c8aece0fa7c 100644 --- a/go/bind/keybase.go +++ b/go/bind/keybase.go @@ -1005,42 +1005,43 @@ func AppUIBackground(pusher PushNotifier) int64 { return kbCtx.MobileLifecycle.UIBackground(backgroundTaskDeps(pusher)) } -// AppWaitBackgroundTask returns once the background task whose token -// AppUIBackground returned no longer needs any time in the background. -func AppWaitBackgroundTask(token int64) { +// inPushWindow runs work, which handles a push or a notification action. On +// Android it holds a backgrounded app up while work runs, and work learns +// whether the UI is active, in which case nothing is held. pusher warns about +// messages that won't send if the window hands over to a background task. +func inPushWindow(pusher PushNotifier, work func(uiActive bool) error) error { if !isInited() { - return + return work(false) } - defer kbCtx.Trace("AppWaitBackgroundTask", nil)() - kbCtx.MobileLifecycle.WaitBackgroundTask(token) + return runPushWindow(kbCtx.MobileLifecycle, runtime.GOOS, backgroundTaskDeps(pusher), work) } -// AppPushWindowBegin holds a backgrounded app up while native handles a push or -// a notification action. It returns the window's token, or 0 when the app is -// active and nothing needs holding. -// -// Transitional: it exists only because Android still opens the push window from -// Kotlin. It goes away once the bind layer wraps push handling in the window -// itself, which is where the decision belongs -- iOS needs no window at all, -// since it suspends the app at the push's completion handler. -func AppPushWindowBegin() int64 { - if !isInited() { - return 0 +func runPushWindow(lc *lifecycle.Controller, goos string, deps lifecycle.BackgroundTaskDeps, + work func(uiActive bool) error, +) error { + if goos != "android" { + // iOS handles a push within the time it grants for it and suspends the + // app at the completion handler, so nothing needs holding up; a window + // would only take the app through BACKGROUNDACTIVE and back. work only + // uses uiActive on Android. + return work(false) + } + token := lc.PushWindowBegin() + if token == 0 { + return work(true) } - defer kbCtx.Trace("AppPushWindowBegin", nil)() - return kbCtx.MobileLifecycle.PushWindowBegin() + defer lc.PushWindowEnd(token, deps) + return work(false) } -// AppPushWindowEnd ends the window AppPushWindowBegin opened. If the UI is -// still in the background it first hands over to a background task, which keeps -// the app up while work must keep going; pusher warns about messages that -// won't send. Transitional, for the same reason as AppPushWindowBegin. -func AppPushWindowEnd(token int64, pusher PushNotifier) { +// AppWaitBackgroundTask returns once the background task whose token +// AppUIBackground returned no longer needs any time in the background. +func AppWaitBackgroundTask(token int64) { if !isInited() { return } - defer kbCtx.Trace("AppPushWindowEnd", nil)() - kbCtx.MobileLifecycle.PushWindowEnd(token, backgroundTaskDeps(pusher)) + defer kbCtx.Trace("AppWaitBackgroundTask", nil)() + kbCtx.MobileLifecycle.WaitBackgroundTask(token) } func backgroundTaskDeps(pusher PushNotifier) lifecycle.BackgroundTaskDeps { diff --git a/go/bind/notifications.go b/go/bind/notifications.go index f3ca13136805..93eef9f7e9ec 100644 --- a/go/bind/notifications.go +++ b/go/bind/notifications.go @@ -114,47 +114,63 @@ type ChatNotification struct { Uid string } -func HandlePostTextReply(strConvID, tlfName string, intMessageID int, body string) (err error) { +// HandlePostTextReply sends a notification quick reply, in the foreground too. +// pusher warns about the reply if it won't send. +func HandlePostTextReply(strConvID, tlfName string, intMessageID int, body string, pusher PushNotifier) (err error) { ctx := context.Background() defer kbCtx.CTrace(ctx, "HandlePostTextReply", &err)() defer func() { err = flattenError(err) }() - outboxID, err := storage.NewOutboxID() + return inPushWindow(pusher, func(bool) error { + return postTextReply(ctx, globals.NewContext(kbCtx, kbChatCtx), strConvID, tlfName, intMessageID, body) + }) +} + +// postTextReply sends a notification quick reply and marks the conversation +// read. The send is nonblocking: an error means the message couldn't be +// queued, not that delivery failed. +func postTextReply(ctx context.Context, gc *globals.Context, strConvID, tlfName string, intMessageID int, + body string, +) error { + convID, err := chat1.MakeConvID(strConvID) if err != nil { return err } - convID, err := chat1.MakeConvID(strConvID) + if intMessageID < 0 { + return fmt.Errorf("invalid message ID: %d", intMessageID) + } + uid, err := utils.AssertLoggedInUID(ctx, gc) if err != nil { return err } - _, err = kbCtx.ChatHelper.SendTextByIDNonblock(context.Background(), convID, tlfName, body, &outboxID, nil) - - kbCtx.Log.CDebugf(ctx, "Marking as read from QuickReply: convID: %s", strConvID) - gc := globals.NewContext(kbCtx, kbChatCtx) - uid, err := utils.AssertLoggedInUID(ctx, gc) + outboxID, err := storage.NewOutboxID() if err != nil { return err } - - if intMessageID < 0 { - return fmt.Errorf("invalid message ID: %d", intMessageID) + if _, err := gc.ChatHelper.SendTextByIDNonblock(ctx, convID, tlfName, body, &outboxID, nil); err != nil { + return err } + gc.Log.CDebugf(ctx, "Marking as read from QuickReply: convID: %s", strConvID) msgID := chat1.MessageID(intMessageID) - if err = kbChatCtx.InboxSource.MarkAsRead(context.Background(), convID, uid, &msgID, false /* forceUnread */); err != nil { - kbCtx.Log.CDebugf(ctx, "Failed to mark as read from QuickReply: convID: %s. Err: %s", strConvID, err) - // We don't want to fail this method call just because we couldn't mark it as aread - err = nil + if err := gc.InboxSource.MarkAsRead(ctx, convID, uid, &msgID, false /* forceUnread */); err != nil { + // The reply went out; failing to mark it read doesn't fail the reply. + gc.Log.CDebugf(ctx, "Failed to mark as read from QuickReply: convID: %s. Err: %s", strConvID, err) } - return nil } var spoileRegexp = regexp.MustCompile(`!>(.*?) 0 || len(chatNotification.Message.ServerMessage) > 0) { - // Lock and check if we've already processed this notification. - seenNotificationsMtx.Lock() - defer seenNotificationsMtx.Unlock() - if _, ok := getSeenNotificationsCache().Get(dupKey); ok { - // Cancel any duplicate visible notifications + ackPush := func() { if ack != nil { ack.Ack(ctx, []string{pushID}) } - kbCtx.Log.CDebugf(ctx, "HandleBackgroundNotification: duplicate notification convID=%s msgID=%d", strConvID, intMessageID) - // Return nil (not an error) so Android does not treat this as failure and show a fallback notification. - return nil } - // Add to cache before displaying so that any concurrent goroutine that - // reaches the second check while DisplayChatNotification is running will - // see the entry and bail out rather than displaying a duplicate. - getSeenNotificationsCache().Add(dupKey, struct{}{}) - pusher.DisplayChatNotification(&chatNotification) - if ack != nil { - ack.Ack(ctx, []string{pushID}) + if displayOnce(dupKey, &chatNotification, pusher, runtime.GOOS, uiActive, ackPush) { + kbCtx.Log.CDebugf(ctx, "HandleBackgroundNotification: duplicate notification convID=%s msgID=%d", strConvID, intMessageID) } } return nil } + +// displayOnce displays n unless its push was already handled, then acks the +// push. On Android, while the UI is active it only acks: the app already shows +// the message. iOS always displays, because its display also removes the +// server's generic notification for this message, which can land while the +// push is being handled; a local notification never shows while active. +func displayOnce(dupKey string, n *ChatNotification, pusher PushNotifier, goos string, uiActive bool, + ack func(), +) (dup bool) { + seenNotificationsMtx.Lock() + defer seenNotificationsMtx.Unlock() + if _, ok := getSeenNotificationsCache().Get(dupKey); ok { + // Cancel any duplicate visible notifications + ack() + return true + } + // Add to cache before displaying so that any concurrent goroutine that + // reaches the check while DisplayChatNotification is running sees the + // entry and bails out rather than displaying a duplicate. + getSeenNotificationsCache().Add(dupKey, struct{}{}) + if !uiActive || goos != "android" { + pusher.DisplayChatNotification(n) + } + ack() + return false +} diff --git a/go/bind/notifications_test.go b/go/bind/notifications_test.go new file mode 100644 index 000000000000..470fde9223d4 --- /dev/null +++ b/go/bind/notifications_test.go @@ -0,0 +1,215 @@ +package keybase + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/keybase/client/go/chat/globals" + "github.com/keybase/client/go/chat/types" + "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/libkb/lifecycle" + "github.com/keybase/client/go/libkb/lifecycle/lifecycletest" + "github.com/keybase/client/go/protocol/chat1" + "github.com/keybase/client/go/protocol/gregor1" + "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/require" +) + +type replyChatHelper struct { + libkb.ChatHelper + sendErr error + sent []string +} + +func (h *replyChatHelper) SendTextByIDNonblock(_ context.Context, _ chat1.ConversationID, _ string, text string, + _ *chat1.OutboxID, _ *chat1.MessageID, +) (chat1.OutboxID, error) { + if h.sendErr != nil { + return nil, h.sendErr + } + h.sent = append(h.sent, text) + return nil, nil +} + +type replyInboxSource struct { + types.InboxSource + markErr error + marked []chat1.MessageID +} + +func (s *replyInboxSource) MarkAsRead(_ context.Context, _ chat1.ConversationID, _ gregor1.UID, + msgID *chat1.MessageID, _ bool, +) error { + s.marked = append(s.marked, *msgID) + return s.markErr +} + +func TestPostTextReply(t *testing.T) { + const convID = "0000bbbbccccddddeeeeffff0000aaaabbbbccccddddeeeeffff0000aaaabbbb" + setup := func(t *testing.T, loggedIn bool) (*globals.Context, *replyChatHelper, *replyInboxSource) { + tc := libkb.SetupTest(t, "PostTextReply", 0) + t.Cleanup(tc.Cleanup) + helper := &replyChatHelper{} + inbox := &replyInboxSource{} + tc.G.ChatHelper = helper + if loggedIn { + uid := keybase1.MakeTestUID(1) + deviceID := keybase1.DeviceID("00000000000000000000000000000018") + sigKey, err := libkb.GenerateNaclSigningKeyPair() + require.NoError(t, err) + encKey, err := libkb.GenerateNaclDHKeyPair() + require.NoError(t, err) + require.NoError(t, tc.G.ActiveDevice.Set(libkb.NewMetaContextForTest(tc), + keybase1.UserVersion{Uid: uid, EldestSeqno: 1}, deviceID, + sigKey, encKey, "testuser-device", 0, libkb.KeychainModeNone)) + require.NoError(t, tc.G.Env.GetConfigWriter().SetUserConfig( + libkb.NewUserConfig(uid, "testuser", nil, deviceID), true)) + require.NoError(t, tc.G.Env.GetConfigWriter().SwitchUser("testuser")) + } + return globals.NewContext(tc.G, &globals.ChatContext{InboxSource: inbox}), helper, inbox + } + ctx := context.Background() + + t.Run("sends and marks read", func(t *testing.T) { + gc, helper, inbox := setup(t, true) + require.NoError(t, postTextReply(ctx, gc, convID, "testuser", 5, "hi")) + require.Equal(t, []string{"hi"}, helper.sent) + require.Equal(t, []chat1.MessageID{5}, inbox.marked) + }) + t.Run("send error is returned", func(t *testing.T) { + gc, helper, inbox := setup(t, true) + helper.sendErr = errors.New("outbox full") + require.EqualError(t, postTextReply(ctx, gc, convID, "testuser", 5, "hi"), "outbox full") + require.Empty(t, inbox.marked) + }) + t.Run("mark read failure doesn't fail a sent reply", func(t *testing.T) { + gc, helper, inbox := setup(t, true) + inbox.markErr = errors.New("offline") + require.NoError(t, postTextReply(ctx, gc, convID, "testuser", 5, "hi")) + require.Equal(t, []string{"hi"}, helper.sent) + }) + t.Run("logged out doesn't send", func(t *testing.T) { + gc, helper, _ := setup(t, false) + require.ErrorAs(t, postTextReply(ctx, gc, convID, "testuser", 5, "hi"), &libkb.LoginRequiredError{}) + require.Empty(t, helper.sent) + }) + t.Run("invalid message ID doesn't send", func(t *testing.T) { + gc, helper, _ := setup(t, true) + require.Error(t, postTextReply(ctx, gc, convID, "testuser", -1, "hi")) + require.Empty(t, helper.sent) + }) +} + +// pendingDeliveryDeps reports a message still sending, so a push window that +// may hand over to a background task does. +func pendingDeliveryDeps() lifecycle.BackgroundTaskDeps { + return lifecycle.BackgroundTaskDeps{ + Stay: func() bool { return true }, + ActiveDeliveries: func(context.Context) ([]chat1.OutboxRecord, error) { + return make([]chat1.OutboxRecord, 1), nil + }, + NextFailure: func() (chan []chat1.OutboxRecord, func()) { return make(chan []chat1.OutboxRecord), func() {} }, + NotifyFailure: func([]chat1.OutboxRecord) {}, + } +} + +func TestBackgroundNotificationOpensAndClosesPushWindow(t *testing.T) { + const ( + fg = keybase1.MobileAppState_FOREGROUND + bg = keybase1.MobileAppState_BACKGROUND + bga = keybase1.MobileAppState_BACKGROUNDACTIVE + ) + for _, platform := range []lifecycletest.Platform{lifecycletest.IOS, lifecycletest.Android} { + t.Run(platform.String(), func(t *testing.T) { + tc := libkb.SetupTest(t, "PushWindow", 0) + defer tc.Cleanup() + h := lifecycletest.NewHarness(t, libkb.NewMobileAppState(tc.G), platform) + defer h.Close() + h.Controller.UIInactive() + lifecycletest.ToBackground(h.Controller) + require.Equal(t, bg, h.AppState.State()) + // The recorder observes the setup transitions from its own + // goroutine; without this the baseline can be short by one and the + // iOS assertion below reads a late setup state as a push. + h.Recorder.Sync(t) + seen := len(h.Recorder.States()) + + unboxFailed := errors.New("unbox failed") + var during keybase1.MobileAppState + err := runPushWindow(h.Controller, platform.String(), pendingDeliveryDeps(), func(uiActive bool) error { + require.False(t, uiActive) + during = h.AppState.State() + return unboxFailed + }) + require.ErrorIs(t, err, unboxFailed) + if platform == lifecycletest.IOS { + require.Equal(t, bg, during, "iOS handles the push without holding the app up") + require.Equal(t, bg, h.AppState.State()) + h.Recorder.Sync(t) + require.Len(t, h.Recorder.States(), seen, "the push never reached the controller") + } else { + require.Equal(t, bga, during, "the push is handled in BACKGROUNDACTIVE") + require.Equal(t, bga, h.AppState.State(), "a background task keeps sending") + h.Controller.BackgroundTaskExpired(func() {}) + require.Equal(t, bg, h.AppState.State(), "the background task held the app, not the push window") + } + + h.Controller.UIActive() + ran := false + require.NoError(t, runPushWindow(h.Controller, platform.String(), pendingDeliveryDeps(), func(uiActive bool) error { + require.Equal(t, platform == lifecycletest.Android, uiActive, "only Android's work asks") + ran = true + return nil + })) + require.True(t, ran, "the work runs while the UI is active") + require.Equal(t, fg, h.AppState.State()) + }) + } +} + +type recordingPusher struct { + PushNotifier + displayed []string +} + +func (p *recordingPusher) DisplayChatNotification(n *ChatNotification) { + p.displayed = append(p.displayed, n.ConvID) +} + +func TestBackgroundNotificationActiveSkipsDisplayButAcks(t *testing.T) { + for _, goos := range []string{"android", "ios"} { + t.Run(goos, func(t *testing.T) { + pusher := &recordingPusher{} + acks := 0 + ack := func() { acks++ } + show := func(convID string, uiActive bool) bool { + return displayOnce(convID+"||1", &ChatNotification{ConvID: convID}, pusher, goos, uiActive, ack) + } + // The seen cache is global, so each run needs its own push ids. + run := fmt.Sprintf("%s/%d/", t.Name(), time.Now().UnixNano()) + active := run + "active" + require.False(t, show(active, true)) + if goos == "android" { + require.Empty(t, pusher.displayed, "the app already shows the message") + } else { + require.Equal(t, []string{active}, pusher.displayed, + "iOS displays to remove the server's generic notification; the local one never shows while active") + } + require.Equal(t, 1, acks, "the push is acked so the server's fallback doesn't show it") + displayed := len(pusher.displayed) + + require.True(t, show(active, false), "a push handled while active isn't shown later") + require.Len(t, pusher.displayed, displayed) + require.Equal(t, 2, acks) + + background := run + "background" + require.False(t, show(background, false)) + require.Equal(t, background, pusher.displayed[len(pusher.displayed)-1]) + require.Len(t, pusher.displayed, displayed+1) + require.Equal(t, 3, acks) + }) + } +} 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/AppLifecycleReporter.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt new file mode 100644 index 000000000000..f644bf3e399e --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/AppLifecycleReporter.kt @@ -0,0 +1,109 @@ +package io.keybase.ossifrage + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +// The Go lifecycle entry points. Kept free of Android and gomobile types so +// the event mapping runs in JVM tests. +internal interface LifecycleBind { + fun uiActive() + fun uiInactive() + fun uiBackground() + fun willExit() +} + +// Reports the app's process lifecycle to Go as events; Go decides the state. +// +// Events reach Go on the calling thread, before the callback returns: every Go +// lifecycle call returns at once, except willExit, whose warning about +// messages still sending reads the outbox. +// +// Only the process lifecycle counts. Activity pauses (dialogs, permission +// prompts, choosers, the photo picker sheet) report nothing, not even +// UIInactive: only the process lifecycle decides what Go sees. A full-screen +// picker or camera stops the process like any other exit. +// +// A process started without UI (a push, a quick reply) reports nothing: Go +// starts in the background. +internal class AppLifecycleReporter( + private val bind: LifecycleBind, + private val log: (String) -> Unit, +) : DefaultLifecycleObserver { + override fun onStart(owner: LifecycleOwner) { + report("uiInactive") { bind.uiInactive() } + } + + override fun onResume(owner: LifecycleOwner) { + report("uiActive") { bind.uiActive() } + } + + override fun onStop(owner: LifecycleOwner) { + report("uiBackground") { bind.uiBackground() } + } + + // Activity recreation and a task moved to the back are not an exit. + fun onMainActivityDestroy(isFinishing: Boolean, isChangingConfigurations: Boolean) { + if (!isFinishing || isChangingConfigurations) { + return + } + report("willExit") { bind.willExit() } + } + + 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(error: (String, Throwable) -> Unit, send: () -> Unit): String = + 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..8a2bd9bdc198 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,36 @@ 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() { - Keybase.handlePostTextReply(convData.convID, convData.tlfName, convData.lastMsgId, messageBody) - } + 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({ msg, e -> NativeLogger.error(msg, e) }) { + Keybase.handlePostTextReply(convData.convID, convData.tlfName, convData.lastMsgId, messageBody, + KBPushNotifier(context, Bundle())) } - 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..34f0a34e8ca1 --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybaseLifecycleBind.kt @@ -0,0 +1,17 @@ +package io.keybase.ossifrage + +import android.content.Context +import android.os.Bundle +import keybase.Keybase + +internal class KeybaseLifecycleBind(private val context: Context) : LifecycleBind { + override fun uiActive() = Keybase.appUIActive() + + override fun uiInactive() = Keybase.appUIInactive() + + override fun uiBackground() { + Keybase.appUIBackground(KBPushNotifier(context, Bundle())) + } + + override fun willExit() = Keybase.appWillExit(KBPushNotifier(context, Bundle())) +} 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 6bdce186b21d..15d05055970a 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 @@ -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 - } - } + // Go holds the app up while it handles the push, and in the + // foreground acks it without displaying it. + 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, KBPushNotifier(applicationContext, Bundle())) + 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,42 +391,3 @@ internal class NotificationData(type: String, bundle: Bundle) { } } -// Runs some task inside a push window, which keeps a backgrounded app running -// while the task does. If already foreground, ignore. -// -// Transitional: it exists only because Android still opens the push window from -// here. It goes away, with Keybase.appPushWindow*, once the bind layer wraps -// push handling in the window itself. -internal interface WithBackgroundActive { - @Throws(Exception::class) - fun task() - - @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 - } - // 0 when the app is active and nothing needs holding up. - val token = Keybase.appPushWindowBegin() - NativeLogger.info("WithBackgroundActive.whileActive push window $token, calling task") - try { - task() - NativeLogger.info("WithBackgroundActive.whileActive task completed") - } finally { - if (token > 0) { - // Hands over to a background task if the UI is still in the - // background and work must keep going. - Keybase.appPushWindowEnd(token, KBPushNotifier(context, Bundle())) - } - } - } catch (ex: Exception) { - NativeLogger.error("WithBackgroundActive.whileActive exception: " + ex.message) - throw ex - } - } -} 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 c93a7982e1e0..f1aaabaf42b4 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 @@ -149,55 +138,50 @@ class MainActivity : ReactActivity() { return filePath } - // Native reports only what the UI is doing; Go derives the app state from - // these reports and the holds background work opens (go/libkb/lifecycle). override fun onResume() { NativeLogger.info("Activity onResume") super.onResume() - Keybase.appUIActive() handleIntent() } override fun onStart() { NativeLogger.info("Activity onStart") super.onStart() - Keybase.appUIInactive() - } - - override fun onStop() { - NativeLogger.info("Activity onStop") - super.onStop() - // The token is for iOS, which has to wait out Go's background task - // before the OS suspends it; Android keeps the process running. - Keybase.appUIBackground(KBPushNotifier(this, Bundle())) } override fun onDestroy() { NativeLogger.info("Activity onDestroy") super.onDestroy() - Keybase.appWillExit(KBPushNotifier(this, Bundle())) + (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) { @@ -209,9 +193,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 @@ -247,37 +233,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. @@ -327,10 +285,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 } @@ -348,7 +307,6 @@ class MainActivity : ReactActivity() { } cachedIntent = null - return true } private fun emitShareText(text: String) { @@ -444,12 +402,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 65de3e828c1c..f5398a9449e8 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?) : @@ -52,9 +60,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 or service starts, so no process event is missed. + ProcessLifecycleOwner.get().lifecycle.addObserver(lifecycleReporter) try { DefaultNewArchitectureEntryPoint.releaseLevel = ReleaseLevel.valueOf(BuildConfig.REACT_NATIVE_RELEASE_LEVEL.uppercase()) } catch (e: IllegalArgumentException) { @@ -73,15 +87,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?) { @@ -100,3 +112,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..0bc3013d0490 --- /dev/null +++ b/shared/android/app/src/test/java/io/keybase/ossifrage/AppLifecycleReporterTest.kt @@ -0,0 +1,202 @@ +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.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 uiActive() = record("uiActive") + + override fun uiInactive() = record("uiInactive") + + override fun uiBackground() = record("uiBackground") + + override fun willExit() = record("willExit") +} + +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( + "uiInactive", "uiActive", + "uiBackground", + "uiInactive", "uiActive", + ), + calls(), + ) + } + + @Test + fun eventsReachGoOnTheCallingThreadBeforeTheCallbackReturns() { + reporter.onStart(Owner) + assertEquals(listOf("uiInactive"), calls()) + reporter.onResume(Owner) + assertEquals(listOf("uiInactive", "uiActive"), calls()) + reporter.onStop(Owner) + assertEquals(listOf("uiInactive", "uiActive", "uiBackground"), calls()) + assertEquals(setOf(Thread.currentThread()), bind.threads.toSet()) + } + + @Test + fun dialogOrPermissionPromptPauseNeverBackgrounds() { + launch() + reporter.onPause(Owner) + reporter.onResume(Owner) + reporter.onPause(Owner) + assertEquals(listOf("uiInactive", "uiActive", "uiActive"), 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( + "uiInactive", "uiActive", + "uiBackground", + "uiInactive", "uiActive", + ), + calls(), + ) + } + + @Test + fun onlyAFinishingActivityExits() { + launch() + reporter.onMainActivityDestroy(isFinishing = false, isChangingConfigurations = false) + reporter.onMainActivityDestroy(isFinishing = true, isChangingConfigurations = true) + assertEquals(listOf("uiInactive", "uiActive"), calls()) + reporter.onMainActivityDestroy(isFinishing = true, isChangingConfigurations = false) + stop() + reporter.onStart(Owner) + reporter.onResume(Owner) + assertEquals( + listOf( + "uiInactive", "uiActive", + "willExit", "uiBackground", + "uiInactive", "uiActive", + ), + calls(), + ) + } +} + +class SendQuickReplyTest { + private val errors = mutableListOf>() + + private fun send(send: () -> Unit) = sendQuickReply({ msg, e -> errors.add(msg to e) }, send) + + @Test + fun replySends() { + var sent = false + assertEquals(QUICK_REPLY_SENT, send { sent = true }) + 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()) + } +} + +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/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) + } +} diff --git a/shared/ios/Keybase/AppDelegate.swift b/shared/ios/Keybase/AppDelegate.swift index 2dd6f45c3b72..42cac6f2eeae 100644 --- a/shared/ios/Keybase/AppDelegate.swift +++ b/shared/ios/Keybase/AppDelegate.swift @@ -337,7 +337,7 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi var err: NSError? Keybasego.KeybaseHandleBackgroundNotification( convID, body, "", sender, membersType, displayPlaintext, messageID, pushID, badgeCount, - unixTime, soundName, pusher, false, targetUID, &err) + unixTime, soundName, pusher, false, targetUID, pusher, &err) if let err { log.error("Failed to handle in engine: \(err.localizedDescription, privacy: .public)") } completionHandler(.newData) log.info("Remote notification handle finished...")