fix(android): report the process lifecycle, quick replies and share intents

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 {

go/bind/notifications.go

@@ -114,60 +114,87 @@ 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(`!>(.*?)<!`)
 
+// HandleBackgroundNotification unboxes a chat push, displays it through
+// pusher and acks it. A nil pusher displays nothing. On Android, while the UI
+// is active it acks without displaying, since the app already shows the
+// message.
+// taskPusher warns about messages that won't send if the push window hands
+// over to a background task.
 func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender string, intMembersType int,
 	displayPlaintext bool, intMessageID int, pushID string, badgeCount, unixTime int, soundName string,
-	pusher PushNotifier, showIfStale bool, targetUID string,
-) (err error) {
+	pusher PushNotifier, showIfStale bool, targetUID string, taskPusher PushNotifier,
+) error {
 	// iOS gives roughly 30 seconds of background time for a remote
 	// notification; leave enough of that budget for unboxing and acking.
 	start := time.Now()
 	if err := waitForInit(15 * time.Second); err != nil {
 		return err
 	}
 	initDuration := time.Since(start)
+	return inPushWindow(taskPusher, func(uiActive bool) error {
+		return handleBackgroundNotification(strConvID, body, serverMessageBody, sender, intMembersType,
+			displayPlaintext, intMessageID, pushID, badgeCount, unixTime, soundName, pusher, showIfStale,
+			targetUID, uiActive, initDuration)
+	})
+}
+
+func handleBackgroundNotification(strConvID, body, serverMessageBody, sender string, intMembersType int,
+	displayPlaintext bool, intMessageID int, pushID string, badgeCount, unixTime int, soundName string,
+	pusher PushNotifier, showIfStale bool, targetUID string, uiActive bool, initDuration time.Duration,
+) (err error) {
 	gc := globals.NewContext(kbCtx, kbChatCtx)
 	ctx := globals.ChatCtx(context.Background(), gc,
 		keybase1.TLFIdentifyBehavior_CHAT_GUI, nil, chat.NewCachingIdentifyNotifier(gc))
 
-	defer kbCtx.CTrace(ctx, fmt.Sprintf("HandleBackgroundNotification(%s,%s,%v,%d,%d,%s,%d,%d)",
-		strConvID, sender, displayPlaintext, intMembersType, intMessageID, pushID, badgeCount, unixTime), &err)()
+	defer kbCtx.CTrace(ctx, fmt.Sprintf("HandleBackgroundNotification(%s,%s,%v,%d,%d,%s,%d,%d,%v)",
+		strConvID, sender, displayPlaintext, intMembersType, intMessageID, pushID, badgeCount, unixTime, uiActive), &err)()
 	defer func() { err = flattenError(err) }()
 	kbCtx.Log.CDebugf(ctx, "HandleBackgroundNotification: waitForInit took %v", initDuration)
 
@@ -267,7 +294,7 @@ func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender str
 
 		if displayPlaintext && !msgUnboxed.Valid().IsEphemeral() {
 			// We show avatars on Android
-			if runtime.GOOS == "android" {
+			if runtime.GOOS == "android" && !uiActive {
 				avatar, err := kbSvc.GetUserAvatar(username)
 
 				if err != nil {
@@ -318,26 +345,40 @@ func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender str
 
 	// only display and ack this notification if we actually have something to display
 	if pusher != nil && (len(chatNotification.Message.Plaintext) > 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
+}