[Android] Photo Picker: Enforce UI thread for DecoderServiceHost.

Chromium uses the DecoderServiceHost on the UI thread, but the test
was using it from the test thread, which causes flakiness due to
multi-threading issues (decoder replies are bounced to the UI thread).
Flakiness was discovered locally while testing another thing (no
special bug linked).

Also marked a couple of functions private (that don't need to be
public).

Bug: 656015
Change-Id: Iaae56f6ce10b9747c3deba6f2318ecc421910050
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2074758Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744704}

chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java

@@ -24,6 +24,7 @@ import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.task.AsyncTask;
 import org.chromium.base.task.PostTask;
@@ -248,6 +249,8 @@ public class DecoderServiceHost
      */
     public void decodeImage(Uri uri, @PickerBitmap.TileTypes int fileType, int width,
             boolean fullWidth, ImagesDecodedCallback callback) {
+        ThreadUtils.assertOnUiThread();
+
         DecoderServiceParams params = new DecoderServiceParams(
                 uri, width, fullWidth, fileType, /*firstFrame=*/true, callback);
         mPendingRequests.add(params);
@@ -409,7 +412,7 @@ public class DecoderServiceHost
         });
     }
 
-    public void closeRequestWithError(String filePath) {
+    private void closeRequestWithError(String filePath) {
         closeRequest(filePath, false, false, null, null, -1, 1.0f);
     }
 
@@ -424,7 +427,7 @@ public class DecoderServiceHost
      * @param decodeTime The length of time it took to decode the bitmaps.
      * @param ratio The ratio of the images (>1.0=portrait, <1.0=landscape).
      */
-    public void closeRequest(String filePath, boolean isVideo, boolean fullWidth,
+    private void closeRequest(String filePath, boolean isVideo, boolean fullWidth,
             @Nullable List<Bitmap> bitmaps, String videoDuration, long decodeTime, float ratio) {
         // If this assert triggers, it means that simultaneous requests have been sent for
         // decoding, which should not happen.
@@ -541,6 +544,8 @@ public class DecoderServiceHost
      * @param filePath The path to the image to cancel decoding.
      */
     public void cancelDecodeImage(String filePath) {
+        ThreadUtils.assertOnUiThread();
+
         // It is important not to null out only pending requests and not mProcessingRequest, because
         // it is used as a signal to see if the decoder is busy.
         Iterator it = mPendingRequests.iterator();

chrome/android/javatests/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHostTest.java

@@ -25,6 +25,7 @@ import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.io.File;
 import java.util.List;
@@ -101,6 +102,16 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         onDecodedCallback.waitForCallback(callCount, 1, WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
     }
 
+    private void decodeImage(DecoderServiceHost host, Uri uri, @PickerBitmap.TileTypes int fileType,
+            int width, boolean fullWidth, DecoderServiceHost.ImagesDecodedCallback callback) {
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> host.decodeImage(uri, fileType, width, fullWidth, callback));
+    }
+
+    private void cancelDecodeImage(DecoderServiceHost host, String filePath) {
+        TestThreadUtils.runOnUiThreadBlocking(() -> host.cancelDecodeImage(filePath));
+    }
+
     @Test
     @LargeTest
     public void testDecodingOrder() throws Throwable {
@@ -116,12 +127,12 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         File file2 = new File(UrlUtils.getIsolatedTestFilePath(filePath + video2));
         File file3 = new File(UrlUtils.getIsolatedTestFilePath(filePath + jpg1));
 
-        host.decodeImage(
-                Uri.fromFile(file1), PickerBitmap.TileTypes.VIDEO, 10, /*fullWidth=*/false, this);
-        host.decodeImage(
-                Uri.fromFile(file2), PickerBitmap.TileTypes.VIDEO, 10, /*fullWidth=*/false, this);
-        host.decodeImage(
-                Uri.fromFile(file3), PickerBitmap.TileTypes.PICTURE, 10, /*fullWidth=*/false, this);
+        decodeImage(host, Uri.fromFile(file1), PickerBitmap.TileTypes.VIDEO, 10,
+                /*fullWidth=*/false, this);
+        decodeImage(host, Uri.fromFile(file2), PickerBitmap.TileTypes.VIDEO, 10,
+                /*fullWidth=*/false, this);
+        decodeImage(host, Uri.fromFile(file3), PickerBitmap.TileTypes.PICTURE, 10,
+                /*fullWidth=*/false, this);
 
         // First decoding result should be first frame of video 1. Even though still images take
         // priority over video decoding, video 1 will be the only item in the queue when the first
@@ -180,8 +191,8 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         File file2 = new File(UrlUtils.getIsolatedTestFilePath(filePath + jpg1));
 
         // Thumbnail photo. 100 x 100 -> 10 x 10.
-        host.decodeImage(
-                Uri.fromFile(file2), PickerBitmap.TileTypes.PICTURE, 10, /*fullWidth=*/false, this);
+        decodeImage(host, Uri.fromFile(file2), PickerBitmap.TileTypes.PICTURE, 10,
+                /*fullWidth=*/false, this);
         waitForThumbnailDecode();
         Assert.assertTrue(mLastDecodedPath.contains(jpg1));
         Assert.assertEquals(false, mLastIsVideo);
@@ -192,8 +203,8 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         Assert.assertEquals(10, mLastInitialFrame.getHeight());
 
         // Full-width photo. 100 x 100 -> 200 x 200.
-        host.decodeImage(
-                Uri.fromFile(file2), PickerBitmap.TileTypes.PICTURE, 200, /*fullWidth=*/true, this);
+        decodeImage(host, Uri.fromFile(file2), PickerBitmap.TileTypes.PICTURE, 200,
+                /*fullWidth=*/true, this);
         waitForThumbnailDecode();
         Assert.assertTrue(mLastDecodedPath.contains(jpg1));
         Assert.assertEquals(false, mLastIsVideo);
@@ -204,8 +215,8 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         Assert.assertEquals(200, mLastInitialFrame.getHeight());
 
         // Thumbnail video. 1920 x 1080 -> 10 x 10.
-        host.decodeImage(
-                Uri.fromFile(file1), PickerBitmap.TileTypes.VIDEO, 10, /*fullWidth=*/false, this);
+        decodeImage(host, Uri.fromFile(file1), PickerBitmap.TileTypes.VIDEO, 10,
+                /*fullWidth=*/false, this);
         waitForThumbnailDecode(); // Initial frame.
         Assert.assertTrue(mLastDecodedPath.contains(video1));
         Assert.assertEquals(true, mLastIsVideo);
@@ -224,8 +235,8 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         Assert.assertEquals(10, mLastInitialFrame.getHeight());
 
         // Full-width video. 1920 x 1080 -> 2000 x 1125.
-        host.decodeImage(
-                Uri.fromFile(file1), PickerBitmap.TileTypes.VIDEO, 2000, /*fullWidth=*/true, this);
+        decodeImage(host, Uri.fromFile(file1), PickerBitmap.TileTypes.VIDEO, 2000,
+                /*fullWidth=*/true, this);
         waitForThumbnailDecode(); // Initial frame.
         Assert.assertTrue(mLastDecodedPath.contains(video1));
         Assert.assertEquals(true, mLastIsVideo);
@@ -262,15 +273,15 @@ public class DecoderServiceHostTest implements DecoderServiceHost.ServiceReadyCa
         String yellowPath = UrlUtils.getIsolatedTestFilePath(filePath + yellow);
         String redPath = UrlUtils.getIsolatedTestFilePath(filePath + red);
 
-        host.decodeImage(Uri.fromFile(new File(greenPath)), PickerBitmap.TileTypes.PICTURE, 10,
+        decodeImage(host, Uri.fromFile(new File(greenPath)), PickerBitmap.TileTypes.PICTURE, 10,
                 /*fullWidth=*/false, this);
-        host.decodeImage(Uri.fromFile(new File(yellowPath)), PickerBitmap.TileTypes.PICTURE, 10,
+        decodeImage(host, Uri.fromFile(new File(yellowPath)), PickerBitmap.TileTypes.PICTURE, 10,
                 /*fullWidth=*/false, this);
 
         // Now add and subsequently remove the request.
-        host.decodeImage(Uri.fromFile(new File(redPath)), PickerBitmap.TileTypes.PICTURE, 10,
+        decodeImage(host, Uri.fromFile(new File(redPath)), PickerBitmap.TileTypes.PICTURE, 10,
                 /*fullWidth=*/false, this);
-        host.cancelDecodeImage(redPath);
+        cancelDecodeImage(host, redPath);
 
         // First decoding result should be the green image.
         waitForThumbnailDecode();