Closed
Bug 1257492
Opened 9 years ago
Closed 4 years ago
DLC Sync: Better scheduling of sync action
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: sebastian, Assigned: k.krish, Mentored)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Details |
Currently the sync action is launched after app start after receiving the DelayedStartup event.
We can do better. Especially if we can use more advanced APIs like JobScheduler or GcmNetworkManager - see bug 1248901.
Reporter | ||
Updated•9 years ago
|
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8779485 [details]
Bug 1257492 - DLC Sync: Better scheduling of sync action
https://reviewboard.mozilla.org/r/70464/#review68282
::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1945
(Diff revision 1)
> }, oneSecondInMillis);
> }
>
> if (AppConstants.MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE) {
> - // TODO: Better scheduling of sync action (Bug 1257492)
> - DownloadContentService.startSync(this);
> + //To check whether the SyncAction (Periodic Job) is already scheduled to run.
> + if (JobManager.instance().getAllJobRequestsForTag(SyncAction.JOBTAG).size() < 1) {
.isEmpty() ?
::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:4124
(Diff revision 1)
> +
> + /**
> + * Scheduling the SyncAction as a periodic job. Repeats once in 5 days.
> + */
> + private void scheduleSyncAction() {
> + long time = 60000 * 60 * 24 * 5;
Any specific reason why you picked 5 days? Currently we run sync on every (cold) app start. Maybe start with 1 day? Can we add additional requirements here (like network conncetion)?
::: mobile/android/base/java/org/mozilla/gecko/dlc/SyncAction.java:292
(Diff revision 1)
> + perform(context, catalog);
> +
> + return null;
> + }
> +
> + protected HttpURLConnection buildHttpURLConnection(String url)
I assume this is copied from the base class? Can we move this to a helper class to avoid duplicating this code?
Attachment #8779485 -
Flags: review?(s.kaspari)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8779485 [details]
Bug 1257492 - DLC Sync: Better scheduling of sync action
https://reviewboard.mozilla.org/r/70464/#review70592
::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:4123
(Diff revision 2)
> +
> + /**
> + * Scheduling the SyncAction as a periodic job. Repeats once in a day.
> + */
> + private void scheduleSyncAction() {
> + int jobId = new JobRequest.Builder(SyncAction.JOBTAG)
nit: The job id is not used. No need to assign it here, I guess?
::: mobile/android/base/java/org/mozilla/gecko/dlc/DownloadContentHelper.java:21
(Diff revision 2)
> +import java.net.URISyntaxException;
> +
> +/**
> + * DownloadContentHelper Class contains all the utility methods for the the Downloadable Contents
> + */
> +public class DownloadContentHelper {
This new class needs to be added to mobile/android/base/moz.build
::: mobile/android/base/java/org/mozilla/gecko/dlc/DownloadContentService.java:14
(Diff revision 2)
> +import org.mozilla.gecko.AppConstants;
> +import org.mozilla.gecko.GeckoAppShell;
> +import org.mozilla.gecko.dlc.catalog.DownloadContent;
> +import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
> +import org.mozilla.gecko.util.HardwareUtils;
Did Android Studio move all those lines?
::: mobile/android/base/java/org/mozilla/gecko/dlc/SyncAction.java:294
(Diff revision 2)
> +
> + @NonNull
> + @Override
> + protected Result onRunJob(Params params) {
> +
> + perform(context, catalog);
The DownloadContentService was responsible for persiting the catalog after an action was executed. It seems like this is not happening anymore and the synchronized state might get lost as soon as the app is killed.
::: mobile/android/base/java/org/mozilla/gecko/dlc/SyncAction.java:296
(Diff revision 2)
> + @Override
> + protected Result onRunJob(Params params) {
> +
> + perform(context, catalog);
> +
> + return null;
Doesn't this need to return Result.SUCCESS (or other results depending on whether the sync was successful).
Attachment #8779485 -
Flags: review?(s.kaspari)
Comment 5•4 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•