Reland "Android: Switch to thread pool executor"

Original CL: http://crrev.com/2562643004

Fixes BackgroundSyncLauncherTest#testNewLauncherDisablesNextOnline.

TBR=mariakhomenko@chromium.org
BUG=601053,674293

Review-Url: https://codereview.chromium.org/2580523002
Cr-Commit-Position: refs/heads/master@{#439511}

chrome/android/java/src/org/chromium/chrome/browser/BackgroundSyncLauncher.java

@@ -24,7 +24,7 @@ import org.chromium.chrome.browser.externalauth.UserRecoverableErrorHandler;
 /**
  * The {@link BackgroundSyncLauncher} singleton is created and owned by the C++ browser. It
  * registers interest in waking up the browser the next time the device goes online after the
- * browser closes via the {@link #setLaunchWhenNextOnline} method.
+ * browser closes via the {@link #launchBrowserIfStopped} method.
  *
  * Thread model: This class is to be run on the UI thread only.
  */
@@ -50,6 +50,9 @@ public class BackgroundSyncLauncher {
      */
     private static boolean sGCMEnabled = true;
 
+    @VisibleForTesting
+    protected AsyncTask<Void, Void, Void> mLaunchBrowserIfStoppedTask;
+
     /**
      * Create a BackgroundSyncLauncher object, which is owned by C++.
      * @param context The app context.
@@ -79,18 +82,21 @@ public class BackgroundSyncLauncher {
      * Callback for {@link #shouldLaunchBrowserIfStopped}. The run method is invoked on the UI
      * thread.
      */
-    public static interface ShouldLaunchCallback { public void run(Boolean shouldLaunch); }
+    public interface ShouldLaunchCallback { void run(Boolean shouldLaunch); }
 
     /**
      * Returns whether the browser should be launched when the device next goes online.
      * This is set by C++ and reset to false each time {@link BackgroundSyncLauncher}'s singleton is
      * created (the native browser is started). This call is asynchronous and will run the callback
      * on the UI thread when complete.
-     * @param context The application context.
-     * @param sharedPreferences The shared preferences.
+     *
+     * {@link AsyncTask} is necessary as the browser process will not have warmed up the
+     * {@link SharedPreferences} before it is used here. This is likely the first usage of
+     * {@link ContextUtils#getAppSharedPreferences}.
+     *
+     * @param callback The callback after fetching prefs.
      */
-    protected static void shouldLaunchBrowserIfStopped(
-            final Context context, final ShouldLaunchCallback callback) {
+    protected static void shouldLaunchBrowserIfStopped(final ShouldLaunchCallback callback) {
         new AsyncTask<Void, Void, Boolean>() {
             @Override
             protected Boolean doInBackground(Void... params) {
@@ -101,7 +107,7 @@ public class BackgroundSyncLauncher {
             protected void onPostExecute(Boolean shouldLaunch) {
                 callback.run(shouldLaunch);
             }
-        }.execute();
+        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
     }
 
     /**
@@ -110,15 +116,16 @@ public class BackgroundSyncLauncher {
      * This method is called by C++ as background sync registrations are added and removed. When the
      * {@link BackgroundSyncLauncher} singleton is created (on browser start), this is called to
      * remove any pre-existing scheduled tasks.
-     * @param context The application context.
+     *
+     * See {@link #shouldLaunchBrowserIfStopped} for {@link AsyncTask}.
+     *
      * @param shouldLaunch Whether or not to launch the browser in the background.
      * @param minDelayMs The minimum time to wait before checking on the browser process.
      */
     @VisibleForTesting
     @CalledByNative
-    protected void launchBrowserIfStopped(
-            final Context context, final boolean shouldLaunch, final long minDelayMs) {
-        new AsyncTask<Void, Void, Void>() {
+    protected void launchBrowserIfStopped(final boolean shouldLaunch, final long minDelayMs) {
+        mLaunchBrowserIfStoppedTask = new AsyncTask<Void, Void, Void>() {
             @Override
             protected Void doInBackground(Void... params) {
                 SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
@@ -133,7 +140,7 @@ public class BackgroundSyncLauncher {
                     if (shouldLaunch) {
                         RecordHistogram.recordBooleanHistogram(
                                 "BackgroundSync.LaunchTask.ScheduleSuccess",
-                                scheduleLaunchTask(context, mScheduler, minDelayMs));
+                                scheduleLaunchTask(mScheduler, minDelayMs));
                     } else {
                         RecordHistogram.recordBooleanHistogram(
                                 "BackgroundSync.LaunchTask.CancelSuccess",
@@ -141,7 +148,7 @@ public class BackgroundSyncLauncher {
                     }
                 }
             }
-        }.execute();
+        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
     }
 
     /**
@@ -154,7 +161,7 @@ public class BackgroundSyncLauncher {
 
     protected BackgroundSyncLauncher(Context context) {
         mScheduler = GcmNetworkManager.getInstance(context);
-        launchBrowserIfStopped(context, false, 0);
+        launchBrowserIfStopped(false, 0);
     }
 
     private static boolean canUseGooglePlayServices(Context context) {
@@ -188,8 +195,7 @@ public class BackgroundSyncLauncher {
         return !sGCMEnabled;
     }
 
-    private static boolean scheduleLaunchTask(
-            Context context, GcmNetworkManager scheduler, long minDelayMs) {
+    private static boolean scheduleLaunchTask(GcmNetworkManager scheduler, long minDelayMs) {
         // Google Play Services may not be up to date, if the application was not installed through
         // the Play Store. In this case, scheduling the task will fail silently.
         final long minDelaySecs = minDelayMs / 1000;
@@ -252,11 +258,11 @@ public class BackgroundSyncLauncher {
                             // without delay and let the browser reschedule if necessary.
                             // TODO(iclelland): If this fails, report the failure via UMA (not now,
                             // since the browser is not running, but on next startup.)
-                            scheduleLaunchTask(context, scheduler, 0);
+                            scheduleLaunchTask(scheduler, 0);
                         }
                     }
                 };
-        BackgroundSyncLauncher.shouldLaunchBrowserIfStopped(context, callback);
+        BackgroundSyncLauncher.shouldLaunchBrowserIfStopped(callback);
     }
 
     @VisibleForTesting

chrome/android/javatests/src/org/chromium/chrome/browser/BackgroundSyncLauncherTest.java

@@ -13,6 +13,7 @@ import org.chromium.base.test.util.AdvancedMockContext;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
 
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Semaphore;
 
 /**
@@ -29,6 +30,8 @@ public class BackgroundSyncLauncherTest extends InstrumentationTestCase {
         BackgroundSyncLauncher.setGCMEnabled(false);
         RecordHistogram.disableForTests();
         mLauncher = BackgroundSyncLauncher.create(mContext);
+        // Ensure that the initial task is given enough time to complete.
+        waitForLaunchBrowserTask();
     }
 
     private void deleteLauncherInstance() {
@@ -51,7 +54,7 @@ public class BackgroundSyncLauncherTest extends InstrumentationTestCase {
                     }
                 };
 
-        BackgroundSyncLauncher.shouldLaunchBrowserIfStopped(mContext, callback);
+        BackgroundSyncLauncher.shouldLaunchBrowserIfStopped(callback);
         try {
             // Wait on the callback to be called.
             semaphore.acquire();
@@ -61,6 +64,16 @@ public class BackgroundSyncLauncherTest extends InstrumentationTestCase {
         return mShouldLaunchResult;
     }
 
+    private void waitForLaunchBrowserTask() {
+        try {
+            mLauncher.mLaunchBrowserIfStoppedTask.get();
+        } catch (InterruptedException e) {
+            fail("Launch task was interrupted");
+        } catch (ExecutionException e) {
+            fail("Launch task had execution exception");
+        }
+    }
+
     @SmallTest
     @Feature({"BackgroundSync"})
     @RetryOnFailure
@@ -81,9 +94,11 @@ public class BackgroundSyncLauncherTest extends InstrumentationTestCase {
     @RetryOnFailure
     public void testSetLaunchWhenNextOnline() {
         assertFalse(shouldLaunchBrowserIfStoppedSync());
-        mLauncher.launchBrowserIfStopped(mContext, true, 0);
+        mLauncher.launchBrowserIfStopped(true, 0);
+        waitForLaunchBrowserTask();
         assertTrue(shouldLaunchBrowserIfStoppedSync());
-        mLauncher.launchBrowserIfStopped(mContext, false, 0);
+        mLauncher.launchBrowserIfStopped(false, 0);
+        waitForLaunchBrowserTask();
         assertFalse(shouldLaunchBrowserIfStoppedSync());
     }
 
@@ -91,12 +106,14 @@ public class BackgroundSyncLauncherTest extends InstrumentationTestCase {
     @Feature({"BackgroundSync"})
     @RetryOnFailure
     public void testNewLauncherDisablesNextOnline() {
-        mLauncher.launchBrowserIfStopped(mContext, true, 0);
+        mLauncher.launchBrowserIfStopped(true, 0);
+        waitForLaunchBrowserTask();
         assertTrue(shouldLaunchBrowserIfStoppedSync());
 
         // Simulate restarting the browser by deleting the launcher and creating a new one.
         deleteLauncherInstance();
         mLauncher = BackgroundSyncLauncher.create(mContext);
+        waitForLaunchBrowserTask();
         assertFalse(shouldLaunchBrowserIfStoppedSync());
     }
 }

chrome/browser/android/background_sync_launcher_android.cc

@@ -44,8 +44,7 @@ void BackgroundSyncLauncherAndroid::LaunchBrowserIfStoppedImpl(
 
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_BackgroundSyncLauncher_launchBrowserIfStopped(
-      env, java_launcher_, base::android::GetApplicationContext(),
-      launch_when_next_online, min_delay_ms);
+      env, java_launcher_, launch_when_next_online, min_delay_ms);
 }
 
 // static