Skip to content
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.

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

[firebase_messaging] Add support for handling messages in background #38

Merged
merged 8 commits into from
Sep 5, 2019

Conversation

collinjackson
Copy link
Contributor

@collinjackson collinjackson commented Aug 26, 2019

Description

When Flutter app is in the background or terminated allow it to handle incoming FCM messages. This update uses a similar strategy to the one used by the android_alarm_manager plugin. When the application starts an additional background channel is started to handle incoming messages when the app is not in the foreground.

Migration of flutter/plugins#1900

Related Issues

flutter/flutter#32372

Remaining work

TODOs before this can be merged

  • Migrate away from Hungarian notation in the Java file
  • Move fcmSetupBackgroundChannel to a static void method
  • Update CHANGELOG
  • Update pubspec.yaml

import java.util.List;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;

public class FlutterFirebaseMessagingService extends FirebaseMessagingService {
Copy link
Contributor

Choose a reason for hiding this comment

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

We, as clients of Firebase SDK, do not control the create and stop of this service. So I would recommend to not execute asynchronous/long term operations within this class.
I would recommend to start a new service where you, as plugin developer, can control the calls to Service.stop(). Just make sure to send the data you need to that service.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is a good point. Instead of creating a new service, could we use a JobScheduler to perform the necessary work required for process the message?

Copy link
Contributor

Choose a reason for hiding this comment

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

The JobScheduler will require you to write a service.. So.. I think it is not needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually meant to use WorkManager which I believe does not require a separate service.

Copy link
Contributor

Choose a reason for hiding this comment

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

WorkManager workers can be triggered at most 15 min interval. Service is a better abstraction as it does not have that limitation. A client might receive the push token and right way a push notification.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In situations where the app is in the background Android does not allow the creation of background services.

With Android 8.0, there is a complication; the system doesn't allow a background app to create a background service.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh god...
Options:

  1. Leave as it is and let's see if it breaks.
  2. Start a foreground service on Android +8.0.

WorkManager and JobScheduler will delay the execution, so I don't think they are a good option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's go with option 1, I will add a note to the README indicating that background handling should be quick since background tasks are limited.

// If another thread is waiting, then wake that thread when the callback returns a result.
MethodChannel.Result result = null;
if (latch != null) {
result =
Copy link
Contributor

Choose a reason for hiding this comment

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

How about to create a new class LatchResult to wrap this inlined logic? Just passing an instance of LatchResult or null would have the same result.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done


private MethodChannel.Result result;

public LatchResult(final CountDownLatch latch) {
Copy link
Contributor

Choose a reason for hiding this comment

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

My initial suggestion, I thought of something like

public class LatchResult  extends MethodChannel.Result {
  final CountDownLatch latch;
  public LatchResult(final int latchCount){
   this.latch = new CountDownLatch(latchCount);
  }
}

@genert
Copy link

genert commented Aug 28, 2019

Same feature has been developed for iOS. See #53

@kroikie
Copy link
Collaborator

kroikie commented Aug 28, 2019

@genert Much appreciated, I'll take a look, however I will likely land this PR first. Then we can rebase #53 and then land it.

@genert
Copy link

genert commented Aug 28, 2019

@kroikie That was the plan. Let's get this PR done first ;)

@XExistence
Copy link

How do I try the latest changes on this branch

Ive been using this the whole time before migration of the plugin to flutterfire
firebase_messaging: git: url: https://github.com/kroikie/plugins.git ref: fcm_background path: packages/firebase_messaging

Anyone know what I can use now?

@MTRNord
Copy link

MTRNord commented Aug 29, 2019

How do I try the latest changes on this branch

Ive been using this the whole time before migration of the plugin to flutterfire
firebase_messaging: git: url: https://github.com/kroikie/plugins.git ref: fcm_background path: packages/firebase_messaging

Anyone know what I can use now?

@XSiyabonga
You could do this: https://github.com/genome/docs/wiki/How-do-I-%60git-checkout%60-a-pull-request%3F which would get you the actual PR directly

@kroikie kroikie force-pushed the flutter/plugins#1900 branch from 266ef40 to 37d4156 Compare September 4, 2019 23:26
@kdy1
Copy link

kdy1 commented Sep 5, 2019

I tried this, but got a strange error

E/flutter (18808): [ERROR:flutter/shell/common/shell.cc(178)] Dart Error: Unhandled exception:
E/flutter (18808): NoSuchMethodError: No top-level getter 'fcmSetupBackgroundChannel' declared.
E/flutter (18808): Receiver: top-level
E/flutter (18808): Tried calling: fcmSetupBackgroundChannel
E/flutter (18808): #0      NoSuchMethodError._throwNew (dart:core-patch/errors_patch.dart:200:5)
E/flutter (18808): [ERROR:flutter/runtime/dart_isolate.cc(459)] Could not resolve main entrypoint function.
E/flutter (18808): [ERROR:flutter/shell/common/engine.cc(202)] Could not run the isolate.
E/flutter (18808): [ERROR:flutter/shell/common/engine.cc(127)] Engine not prepare and launch isolate.
E/flutter (18808): [ERROR:flutter/shell/common/shell.cc(407)] Could not launch engine with configuration.

@kroikie
Copy link
Collaborator

kroikie commented Sep 5, 2019

@kdy1 that is strange, let me take a look.

@kroikie
Copy link
Collaborator

kroikie commented Sep 5, 2019

@kdy1 Thanks for flagging that, it should be resolved now.

@kroikie kroikie merged commit bfc666d into firebase:master Sep 5, 2019
@kdy1
Copy link

kdy1 commented Sep 6, 2019

@kroikie How can I test this?
I configured firebase messaging like onBackgroundMessage: onBackgroundMessage,, but service is not started according to developer mode - running services, and handler is not called.

(onBackgroundMessage is a top level function)

@kroikie
Copy link
Collaborator

kroikie commented Sep 6, 2019

@kdy1 The service that receives the messages while the app is in the background is managed by Google Play Services, so it may not show up with other developer managed services.

Note: onBackgroundMessage will only be triggered when a data-message is received while the app is not in the foreground.

@kdy1
Copy link

kdy1 commented Sep 7, 2019

Note: onBackgroundMessage will only be triggered when a data-message is received while the app is not in the foreground.

@kroikie Thank you for detail. I was trying to catch notification message which also has data.

@addy419
Copy link

addy419 commented Sep 16, 2019

Will this work only when the app is in the background, or even if it's killed or not in memory?

@sidhijakpat
Copy link

Hi @kroikie , I'm kind of confuse,

does this merge resolve data message on app terminated problem,
or it only resolve notification with title and content on app terminated?

as on https://pub.dev/packages/firebase_messaging, it still showing unsupported for data message

Screen Shot 2020-06-24 at 11 54 59

Thank you for your answer and attentions

@firebase firebase locked and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.