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

[Messaging] Add null check to acquire WakeLock on Android #2092

Merged
merged 2 commits into from
May 3, 2019

Conversation

timwangdev
Copy link
Contributor

@timwangdev timwangdev commented Apr 30, 2019

Summary

The Message module and Notification module will accidentally acquire a PARTIAL_WAKE_LOCK by calling HeadlessJsTaskService.acquireWakeLockNow(), and this lock will always hold if no js tasks to consume it. This will drain off user phones' battery.

This happens when you init the Message module without the optional step of adding RNFirebaseBackgroundMessagingService to the AndroidManifest.xml.

The context.startService() will not throw on unregistered service, it just return null. Adding a null check will handle the cases.

On the Notification module, the service is undocumented, should have fewer people using it.

Checklist

  • Supports Android
  • Supports iOS
  • e2e tests added or updated in /tests/e2e/*
  • Updated the documentation in the docs repo
    • LINK TO DOCS PR HERE
  • Flow types updated
  • Typescript types updated

Test Plan

Tested on a private production app.

Release Plan

[ANDROID][BUGFIX] [Messaging] - Add null checks to avoid accidentally acquiring wakelocks on Android.


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@timwangdev
Copy link
Contributor Author

timwangdev commented Apr 30, 2019

Maybe related to #1174

@timwangdev timwangdev changed the title Null check to aquire [Messaging] Add null check to acquire WakeLock on Android Apr 30, 2019
@timwangdev
Copy link
Contributor Author

Update a PR on facebook/react-native#24671 to handle the case if no tasks registered in js.

@Salakar Salakar added this to the v6.0.0 milestone May 1, 2019
@Salakar Salakar added plugin: messaging FCM only - ( messaging() ) - do not use for Notifications platform: android v6 labels May 3, 2019
@Salakar
Copy link
Member

Salakar commented May 3, 2019

@timwangdev thanks for this and also for sending up the React Native PR 👏

For the RN PR, I've made a request to get it cherry-picked into the RN 0.60.x release if you'd like to track it: react-native-community/releases#116 (comment)

I'll go ahead and merge this PR - I'll do a v5 patch release end of next week.

@Salakar Salakar merged commit fc4da37 into v5.x.x May 3, 2019
@Salakar Salakar deleted the null-check-for-wakelock branch May 3, 2019 11:51
robertbarclay added a commit to robertbarclay/react-native-firebase that referenced this pull request May 28, 2019
* v5.x.x:
  5.4.0
  [tests] disable admob (has a new runtime check that causes a crash as we don't have a valid admob identifier)
  [android] update firebase sdk versions - forced to upgrade due to invertase#2122
  [android][internals] rework internals utils to better support JSONObject/Array values
  [GENERAL][BUGFIX][build] - patch v5 build toolchains for XCode10.2 & RN0.59 compatibility (invertase#2166)
  [tests] update podfile lock
  [tests] sync v5.x.x local changes
  [tests] sync v5.x.x local changes
  [docs] update link
  [STORAGE][BUGFIX][ANDROID] - Preserve `file://` prefix
  [JS][BUGFIX] [package.json] - Fix build on Windows (BABEL_ENV error)
  [Messaging] Add null check to acquire WakeLock on Android (invertase#2092)
  [TYPES][BUGFIX][AUTHENTICATION] - Change type PhoneAuthSnapshot.Error to NativeError
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: android plugin: messaging FCM only - ( messaging() ) - do not use for Notifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants