[Background Sync] Refactor scheduling of one off tasks.

Refactor the scheduling and cancellation of one off tasks that
wake up the browser to process (Periodic) Background Sync registrations
into two methods.

This gets rid of the method that calls the appropriate method and
removes special casing base::TimeDelta::Max() as the delay that causes
cancellation of one-off task.

Unit tests have also been updated.

Bug: 996166
Change-Id: Ic35f4833c627220d464641db82feee8bbfc12ea7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906213
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715284}

chrome/android/java/src/org/chromium/chrome/browser/background_sync/BackgroundSyncBackgroundTaskScheduler.java

@@ -135,6 +135,7 @@ public class BackgroundSyncBackgroundTaskScheduler {
      * @param taskType The Background Sync task to cancel.
      */
     @VisibleForTesting
+    @CalledByNative
     protected void cancelOneOffTask(@BackgroundSyncTask int taskType) {
         BackgroundTaskSchedulerFactory.getScheduler().cancel(
                 ContextUtils.getApplicationContext(), getAppropriateTaskId(taskType));
@@ -146,13 +147,15 @@ public class BackgroundSyncBackgroundTaskScheduler {
 
     /**
      * Schedules a one-off background task to wake the browser up on network
-     * connectivity and call into native code to fire ready periodic Background Sync
+     * connectivity and call into native code to fire ready (periodic) Background Sync
      * events.
      *
      * @param minDelayMs The minimum time to wait before waking the browser.
      * @param taskType   The Background Sync task to schedule.
      */
-    protected boolean scheduleOneOffTask(long minDelayMs, @BackgroundSyncTask int taskType) {
+    @VisibleForTesting
+    @CalledByNative
+    protected boolean scheduleOneOffTask(@BackgroundSyncTask int taskType, long minDelayMs) {
         // Pack SOONEST_EXPECTED_WAKETIME in extras.
         Bundle taskExtras = new Bundle();
         taskExtras.putLong(SOONEST_EXPECTED_WAKETIME, System.currentTimeMillis() + minDelayMs);
@@ -182,26 +185,6 @@ public class BackgroundSyncBackgroundTaskScheduler {
         return didSchedule;
     }
 
-    /**
-     * Based on shouldLaunch, either creates or cancels a one-off background
-     * task to wake up Chrome upon network connectivity.
-     *
-     * @param taskType The one-off background task to create.
-     * @param shouldLaunch Whether to launch the browser in the background.
-     * @param minDelayMs   The minimum time to wait before waking the browser.
-     */
-    @VisibleForTesting
-    @CalledByNative
-    protected void launchBrowserIfStopped(
-            @BackgroundSyncTask int taskType, boolean shouldLaunch, long minDelayMs) {
-        if (!shouldLaunch) {
-            cancelOneOffTask(taskType);
-            return;
-        }
-
-        scheduleOneOffTask(minDelayMs, taskType);
-    }
-
     /**
      * Method for rescheduling a background task to wake up Chrome for processing
      * Background Sync events in the event of an OS upgrade or Google Play Services
@@ -210,7 +193,7 @@ public class BackgroundSyncBackgroundTaskScheduler {
      * @param taskType The Background Sync task to reschedule.
      */
     public void reschedule(@BackgroundSyncTask int taskType) {
-        scheduleOneOffTask(MIN_SYNC_RECOVERY_TIME, taskType);
+        scheduleOneOffTask(taskType, MIN_SYNC_RECOVERY_TIME);
     }
 
     @NativeMethods

chrome/android/junit/src/org/chromium/chrome/browser/background_sync/BackgroundSyncBackgroundTaskSchedulerTest.java

@@ -51,8 +51,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
     @Captor
     ArgumentCaptor<TaskInfo> mTaskInfo;
 
-    private static final long ONE_DAY_IN_MILLISECONDS = TimeUnit.DAYS.toMillis(1);
-    private static final long ONE_WEEK_IN_MILLISECONDS = TimeUnit.DAYS.toMillis(7);
+    private static final int ONE_DAY_IN_MILLISECONDS = (int) TimeUnit.DAYS.toMillis(1);
+    private static final int ONE_WEEK_IN_MILLISECONDS = (int) TimeUnit.DAYS.toMillis(7);
 
     @Before
     public void setUp() {
@@ -87,10 +87,9 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
 
     @Test
     @Feature({"BackgroundSync"})
-    public void testLaunchBrowserIfStopped() {
-        BackgroundSyncBackgroundTaskScheduler.getInstance().launchBrowserIfStopped(
+    public void testScheduleOneOffTask() {
+        BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
                 BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP,
-                /* shouldLaunch= */ true,
                 /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
@@ -107,8 +106,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
     @Feature({"BackgroundSync"})
     public void testSchedulePeriodicSyncWakeUpTask() {
         BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
-                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS,
-                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP);
+                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP,
+                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
 
@@ -123,9 +122,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
     @Test
     @Feature({"BackgroundSync"})
     public void testCancelOneShotTask() {
-        BackgroundSyncBackgroundTaskScheduler.getInstance().launchBrowserIfStopped(
+        BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
                 BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP,
-                /* shouldLaunch= */ true,
                 /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
@@ -146,8 +144,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
     @Feature({"BackgroundSync"})
     public void testCancelPeriodicSyncWakeUpTask() {
         BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
-                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS,
-                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP);
+                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP,
+                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
 
@@ -165,10 +163,9 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
 
     @Test
     @Feature({"BackgroundSync"})
-    public void testLaunchBrowserCalledTwice() {
-        BackgroundSyncBackgroundTaskScheduler.getInstance().launchBrowserIfStopped(
+    public void testScheduleOneOffTaskCalledTwice() {
+        BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
                 BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP,
-                /* shouldLaunch= */ true,
                 /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
@@ -176,9 +173,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
         TaskInfo taskInfo = mTaskInfo.getValue();
         assertEquals(ONE_DAY_IN_MILLISECONDS, taskInfo.getOneOffInfo().getWindowStartTimeMs());
 
-        BackgroundSyncBackgroundTaskScheduler.getInstance().launchBrowserIfStopped(
+        BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
                 BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP,
-                /* shouldLaunch= */ true,
                 /* minDelayMs= */ ONE_WEEK_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
@@ -191,8 +187,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
     @Feature({"BackgroundSync"})
     public void testSchedulePeriodicSyncTaskTwice() {
         BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
-                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS,
-                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP);
+                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP,
+                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
 
@@ -200,8 +196,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
         assertEquals(ONE_DAY_IN_MILLISECONDS, taskInfo.getOneOffInfo().getWindowStartTimeMs());
 
         BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
-                /* minDelayMs= */ ONE_WEEK_IN_MILLISECONDS,
-                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP);
+                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP,
+                /* minDelayMs= */ ONE_WEEK_IN_MILLISECONDS);
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
 
@@ -211,15 +207,12 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
 
     @Test
     @Feature({"BackgroundSync"})
-    public void testLaunchBrowserThenCancel() {
-        BackgroundSyncBackgroundTaskScheduler.getInstance().launchBrowserIfStopped(
-                BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP,
-                /* shouldLaunch= */ true,
-                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
-        BackgroundSyncBackgroundTaskScheduler.getInstance().launchBrowserIfStopped(
+    public void testScheduleOneShotSyncWakeUpThenCancel() {
+        BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
                 BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP,
-                /* shouldLaunch= */ false,
                 /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
+        BackgroundSyncBackgroundTaskScheduler.getInstance().cancelOneOffTask(
+                BackgroundSyncTask.ONE_SHOT_SYNC_CHROME_WAKE_UP);
 
         verify(mTaskScheduler, times(1))
                 .schedule(eq(ContextUtils.getApplicationContext()), eq(mTaskInfo.getValue()));
@@ -232,8 +225,8 @@ public class BackgroundSyncBackgroundTaskSchedulerTest {
     @Feature({"BackgroundSync"})
     public void schedulePeriodicSyncWakeUpTaskThenCancel() {
         BackgroundSyncBackgroundTaskScheduler.getInstance().scheduleOneOffTask(
-                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS,
-                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP);
+                BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP,
+                /* minDelayMs= */ ONE_DAY_IN_MILLISECONDS);
         BackgroundSyncBackgroundTaskScheduler.getInstance().cancelOneOffTask(
                 BackgroundSyncTask.PERIODIC_SYNC_CHROME_WAKE_UP);
         verify(mTaskScheduler, times(1))

chrome/browser/android/background_sync_launcher_android.cc

@@ -110,10 +110,9 @@ void BackgroundSyncLauncherAndroid::ScheduleBrowserWakeUpWithDelayImpl(
   JNIEnv* env = base::android::AttachCurrentThread();
   int64_t min_delay_ms = soonest_wakeup_delta.InMilliseconds();
 
-  Java_BackgroundSyncBackgroundTaskScheduler_launchBrowserIfStopped(
+  Java_BackgroundSyncBackgroundTaskScheduler_scheduleOneOffTask(
       env, java_background_sync_background_task_scheduler_launcher_,
-      GetBackgroundTaskType(sync_type), !soonest_wakeup_delta.is_max(),
-      min_delay_ms);
+      GetBackgroundTaskType(sync_type), min_delay_ms);
 }
 
 void BackgroundSyncLauncherAndroid::CancelBrowserWakeupImpl(
@@ -122,11 +121,9 @@ void BackgroundSyncLauncherAndroid::CancelBrowserWakeupImpl(
 
   JNIEnv* env = base::android::AttachCurrentThread();
 
-  // TODO(crbug.com/996166): Add a new method to cancel browser launch.
-  Java_BackgroundSyncBackgroundTaskScheduler_launchBrowserIfStopped(
+  Java_BackgroundSyncBackgroundTaskScheduler_cancelOneOffTask(
       env, java_background_sync_background_task_scheduler_launcher_,
-      GetBackgroundTaskType(sync_type), /* shouldLaunch= */ false,
-      /* minDelayMs= */ 0);
+      GetBackgroundTaskType(sync_type));
 }
 
 void BackgroundSyncLauncherAndroid::FireBackgroundSyncEvents(