Skip to content

notif: Handle when app in background, too (on Android) #352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 1, 2023
Merged

notif: Handle when app in background, too (on Android) #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd88a84
binding: Add firebaseMessagingOnBackgroundMessage
gnprice Oct 19, 2023
783b6a8
notif [nfc]: Have debugReset reset more thoroughly
gnprice Nov 1, 2023
9be01bd
notif: Handle when app in background, too (on Android)
gnprice Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/model/binding.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ abstract class ZulipBinding {
/// Wraps [firebase_messaging.FirebaseMessaging.onMessage].
Stream<firebase_messaging.RemoteMessage> get firebaseMessagingOnMessage;

/// Wraps [firebase_messaging.FirebaseMessaging.onBackgroundMessage].
void firebaseMessagingOnBackgroundMessage(firebase_messaging.BackgroundMessageHandler handler);

/// Wraps the [FlutterLocalNotificationsPlugin] singleton constructor.
FlutterLocalNotificationsPlugin get notifications;
}
Expand Down Expand Up @@ -199,6 +202,11 @@ class LiveZulipBinding extends ZulipBinding {
return firebase_messaging.FirebaseMessaging.onMessage;
}

@override
void firebaseMessagingOnBackgroundMessage(firebase_messaging.BackgroundMessageHandler handler) {
firebase_messaging.FirebaseMessaging.onBackgroundMessage(handler);
}

@override
FlutterLocalNotificationsPlugin get notifications => FlutterLocalNotificationsPlugin();
}
54 changes: 51 additions & 3 deletions lib/notifications.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,23 @@ class NotificationService {
@visibleForTesting
static void debugReset() {
instance.token.dispose();
instance.token = ValueNotifier(null);
_instance = null;
assert(debugBackgroundIsolateIsLive = true);
}

/// Whether a background isolate should initialize [LiveZulipBinding].
///
/// Ordinarily a [ZulipBinding.firebaseMessagingOnBackgroundMessage] callback
/// will be invoked in a background isolate where it must set up its
/// [ZulipBinding], just as the `main` function does for most of the app.
/// Consequently, by default we have that callback initialize
/// [LiveZulipBinding], just like `main` does.
///
/// In a test that behavior is undesirable. Tests that will cause
/// [ZulipBinding.firebaseMessagingOnBackgroundMessage] callbacks
/// to get invoked should therefore set this to false.
static bool debugBackgroundIsolateIsLive = true;

/// The FCM registration token for this install of the app.
///
/// This is unique to the (app, device) pair, but not permanent.
Expand All @@ -41,7 +55,8 @@ class NotificationService {
// (in order to avoid calling for permissions)

await NotificationDisplayManager._init();
ZulipBinding.instance.firebaseMessagingOnMessage.listen(_onRemoteMessage);
ZulipBinding.instance.firebaseMessagingOnMessage.listen(_onForegroundMessage);
ZulipBinding.instance.firebaseMessagingOnBackgroundMessage(_onBackgroundMessage);

// Get the FCM registration token, now and upon changes. See FCM API docs:
// https://firebase.google.com/docs/cloud-messaging/android/client#sample-register
Expand Down Expand Up @@ -71,8 +86,41 @@ class NotificationService {
token.value = value;
}

static void _onRemoteMessage(FirebaseRemoteMessage message) {
static void _onForegroundMessage(FirebaseRemoteMessage message) {
assert(debugLog("notif message: ${message.data}"));
_onRemoteMessage(message);
}

static Future<void> _onBackgroundMessage(FirebaseRemoteMessage message) async {
// This callback will run in a separate isolate from the rest of the app.
// See docs:
// https://firebase.flutter.dev/docs/messaging/usage/#background-messages
_initBackgroundIsolate();

assert(debugLog("notif message in background: ${message.data}"));
_onRemoteMessage(message);
}

static void _initBackgroundIsolate() {
bool isolateIsLive = true;
assert(() {
isolateIsLive = debugBackgroundIsolateIsLive;
return true;
}());
if (!isolateIsLive) {
return;
}

// Compare these setup steps to the ones in `main` in lib/main.dart .
assert(() {
debugLogEnabled = true;
return true;
}());
Comment on lines +115 to +118
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for setting debugLogEnabled to true here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as in main (the one in lib/main.dart) — it causes the debugLog calls to be active when running a debug build of the app, so that you get them in flutter run output.

Or perhaps the question is: why does this code need to do that, when the whole rest of the app is covered by main doing so? That's because it's in a separate isolate, as mentioned in _onBackgroundMessage; same reason it needs the LiveZulipBinding.ensureInitialized and NotificationDisplayManager._init calls.

I'll add a comment pointing out the comparison to main.

LiveZulipBinding.ensureInitialized();
NotificationDisplayManager._init(); // TODO call this just once per isolate
}

static void _onRemoteMessage(FirebaseRemoteMessage message) {
final data = FcmMessage.fromJson(message.data);
switch (data) {
case MessageFcmMessage(): NotificationDisplayManager._onMessageFcmMessage(data, message.data);
Expand Down
11 changes: 11 additions & 0 deletions test/model/binding.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ class TestZulipBinding extends ZulipBinding {
@override
Stream<RemoteMessage> get firebaseMessagingOnMessage => firebaseMessaging.onMessage.stream;

@override
void firebaseMessagingOnBackgroundMessage(BackgroundMessageHandler handler) {
firebaseMessaging.onBackgroundMessage.stream.listen(handler);
}

void _resetNotifications() {
_notificationsPlugin = null;
}
Expand Down Expand Up @@ -241,6 +246,12 @@ class FakeFirebaseMessaging extends Fake implements FirebaseMessaging {
Stream<String> get onTokenRefresh => _tokenController.stream;

StreamController<RemoteMessage> onMessage = StreamController.broadcast();

/// Controls [TestZulipBinding.firebaseMessagingOnBackgroundMessage].
///
/// Calling [StreamController.add] on this will cause a call
/// to any handler registered through that method.
StreamController<RemoteMessage> onBackgroundMessage = StreamController.broadcast();
}

class FakeFlutterLocalNotificationsPlugin extends Fake implements FlutterLocalNotificationsPlugin {
Expand Down
35 changes: 25 additions & 10 deletions test/notifications_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ void main() {
addTearDown(testBinding.reset);
testBinding.firebaseMessagingInitialToken = '012abc';
addTearDown(NotificationService.debugReset);
NotificationService.debugBackgroundIsolateIsLive = false;
await NotificationService.instance.start();
}

Expand All @@ -93,13 +94,10 @@ void main() {
});

group('NotificationDisplayManager', () {
Future<void> checkNotification(MessageFcmMessage data, {
void checkNotification(MessageFcmMessage data, {
required String expectedTitle,
required String expectedTagComponent,
}) async {
testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: data.toJson()));
await null;
}) {
check(testBinding.notifications.takeShowCalls()).single
..id.equals(NotificationDisplayManager.kNotificationId)
..title.equals(expectedTitle)
Expand All @@ -112,11 +110,28 @@ void main() {
);
}

Future<void> checkNotifications(MessageFcmMessage data, {
required String expectedTitle,
required String expectedTagComponent,
}) async {
testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: data.toJson()));
await null;
checkNotification(data, expectedTitle: expectedTitle,
expectedTagComponent: expectedTagComponent);

testBinding.firebaseMessaging.onBackgroundMessage.add(
RemoteMessage(data: data.toJson()));
await null;
checkNotification(data, expectedTitle: expectedTitle,
expectedTagComponent: expectedTagComponent);
}

test('stream message', () async {
await init();
final stream = eg.stream();
final message = eg.streamMessage(stream: stream);
await checkNotification(messageFcmMessage(message, streamName: stream.name),
await checkNotifications(messageFcmMessage(message, streamName: stream.name),
expectedTitle: '${stream.name} > ${message.subject}',
expectedTagComponent: 'stream:${message.streamId}:${message.subject}');
});
Expand All @@ -125,31 +140,31 @@ void main() {
await init();
final stream = eg.stream();
final message = eg.streamMessage(stream: stream);
await checkNotification(messageFcmMessage(message, streamName: null),
await checkNotifications(messageFcmMessage(message, streamName: null),
expectedTitle: '(unknown stream) > ${message.subject}',
expectedTagComponent: 'stream:${message.streamId}:${message.subject}');
});

test('group DM', () async {
await init();
final message = eg.dmMessage(from: eg.thirdUser, to: [eg.otherUser, eg.selfUser]);
await checkNotification(messageFcmMessage(message),
await checkNotifications(messageFcmMessage(message),
expectedTitle: "${eg.thirdUser.fullName} to you and 1 others",
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
});

test('1:1 DM', () async {
await init();
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
await checkNotification(messageFcmMessage(message),
await checkNotifications(messageFcmMessage(message),
expectedTitle: eg.otherUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
});

test('self-DM', () async {
await init();
final message = eg.dmMessage(from: eg.selfUser, to: []);
await checkNotification(messageFcmMessage(message),
await checkNotifications(messageFcmMessage(message),
expectedTitle: eg.selfUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
});
Expand Down