1467840 - Use JobIntentService for other background work

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Description

[Petru-Mugurel Lingurar [:petru]]

GeckoService is a simple Android Service [1] which can be started while in background.

After starting to target Oreo a service can be started in background only if [2]:
- is started as a foreground service (with a notification visible to the user)
- is a JobIntentService
Otherwise the app would crash.

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/GeckoService.java#26
[2] https://developer.android.com/about/versions/oreo/background

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1467840 - Migrate GeckoService to JobIntentService;

This simple Service needed to be migrated to JobIntentService because it could
be started from background and we don't want it as a foreground service
(with a notification).
(For example: when the app is updated org.mozilla.gecko.PackageReplacedReceiver
would try and start this service. If in background, the app would crash)

Had to break the initial Service into separate JobIntentServices because in the
event that there are concurrent calls (even with different Intent actions)
JobScheduler would assume they are for the same already running service.

INTENT_ACTION_UPDATE_ADDONS was removed as it was being unused.

Review commit: https://reviewboard.mozilla.org/r/250388/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250388/

Comment 2

[Jan Henning [:JanH]]

Comment on attachment 8984521 [details]
Bug 1467840 - Migrate GeckoService to JobIntentService;

https://reviewboard.mozilla.org/r/250388/#review256918

Looks reasonable, just the jobId issues need resolving like in all the other JobIntentService conversion patches.

::: mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java:11
(Diff revision 1)
> + * To try and mitigate this situations we will use IDs with negative values in our code.
> + *
> + * @see <a href="https://developer.android.com/reference/android/app/job/JobInfo.Builder#JobInfo.Builder(int, android.content.ComponentName)">
> + *     JobId importance</a>
> + */
> +public class JobIdsConstants {

This needs rebasing onto whatever other patches will land first, but I guess you already know that.
Mentioning it just so this doesn't get accidentally landed like this.

::: mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java:36
(Diff revision 1)
> +    // Our package is unavailable to org.mozilla.gecko classes.
> +    // Currently used in org.mozilla.gecko classes.CrashReporterService
> +    // The id is kept here to ensure unicity between all job ids.
> +    private static final int JOB_ID_CRASH_REPORTER = -14;
> +
> +    public static final int JOB_ID_GECKO_STARTER = -15;

Also as mentioned elsewhere, the ID offset configurable through the build config needs to be done first.

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1467840 - Use a JobIntentService for GCM token refresh;

Whenever the GCM token expires it need to be refreshed.
For this, after targeting Android 8.0 (API level 26) or higher
Google recommends using a JobIntentService
https://developers.google.com/cloud-messaging/android/client

Review commit: https://reviewboard.mozilla.org/r/253138/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/253138/

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 8984521 [details]
Bug 1467840 - Migrate GeckoService to JobIntentService;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250388/diff/1-2/

Comment 5

[Petru-Mugurel Lingurar [:petru]]

- rebased the initial patch with the latest version of `JobIdsConstants` to be ready for landing.
- added a new commit for a JobIntentService used for background refresh of the GCM token as suggested in bug 1465323 comment 15.
- renamed to ticket accordingly.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8987863 [details]
Bug 1467840 - Use a JobIntentService for GCM token refresh;

https://reviewboard.mozilla.org/r/253138/#review260034

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 8984521 [details]
Bug 1467840 - Migrate GeckoService to JobIntentService;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250388/diff/2-3/

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1467840 - Use a JobIntentService for GCM token refresh;

Whenever the GCM token expires it need to be refreshed.
For this, after targeting Android 8.0 (API level 26) or higher
Google recommends using a JobIntentService
https://developers.google.com/cloud-messaging/android/client

Review commit: https://reviewboard.mozilla.org/r/256926/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/256926/

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1467840 - Migrate GeckoService to JobIntentService;

This simple Service needed to be migrated to JobIntentService because it could
be started from background and we don't want it as a foreground service
(with a notification).
(For example: when the app is updated org.mozilla.gecko.PackageReplacedReceiver
would try and start this service. If in background, the app would crash)

Had to break the initial Service into separate JobIntentServices because in the
event that there are concurrent calls (even with different Intent actions)
JobScheduler would assume they are for the same already running service.

INTENT_ACTION_UPDATE_ADDONS was removed as it was being unused.

Review commit: https://reviewboard.mozilla.org/r/257184/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257184/

Comment 10

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 8992028 [details]
Bug 1467840 - Use a JobIntentService for GCM token refresh;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256926/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/90f3740655ea
Migrate GeckoService to JobIntentService; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/e16e23725134
Use a JobIntentService for GCM token refresh; r=JanH

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/90f3740655ea
https://hg.mozilla.org/mozilla-central/rev/e16e23725134