Use skia.mojom.BitmapN32 in Detection mojoms

These bitmaps are safer for transport from untrustworthy sources since
all bitmaps should be in N32 format and the browser can make bad
assumptions as a result.

R=dcheng@chromium.org, reillyg@chromium.org

Bug: 1144462
Change-Id: I7d02cb187c62be37259f445a182f8c971aa03661
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580483
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835685}

services/shape_detection/android/java/src/org/chromium/shape_detection/BarcodeDetectionImpl.java

@@ -75,8 +75,7 @@ public class BarcodeDetectionImpl implements BarcodeDetection {
     }
 
     @Override
-    public void detect(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmapData, DetectResponse callback) {
+    public void detect(org.chromium.skia.mojom.BitmapN32 bitmapData, DetectResponse callback) {
         // The vision library will be downloaded the first time the API is used
         // on the device; this happens "fast", but it might have not completed,
         // bail in this case. Also, the API was disabled between and v.9.0 and

services/shape_detection/android/java/src/org/chromium/shape_detection/BitmapUtils.java

@@ -9,7 +9,6 @@ import android.graphics.Bitmap;
 import com.google.android.gms.vision.Frame;
 
 import org.chromium.mojo_base.BigBufferUtil;
-import org.chromium.skia.mojom.ColorType;
 
 import java.nio.ByteBuffer;
 
@@ -17,20 +16,19 @@ import java.nio.ByteBuffer;
  * Utility class to convert a Bitmap to a GMS core YUV Frame.
  */
 public class BitmapUtils {
-    public static Bitmap convertToBitmap(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmapData) {
-        if (bitmapData.imageInfo == null) return null;
+    public static Bitmap convertToBitmap(org.chromium.skia.mojom.BitmapN32 bitmapData) {
+        // A null BitmapN32 has null pixelData. Otherwise, BitmapN32 always has
+        // a valid N32 (aka RGBA_8888 or BGRA_8888 depending on the build
+        // config) colorType.
+        if (bitmapData.pixelData == null) {
+            return null;
+        }
+
         int width = bitmapData.imageInfo.width;
         int height = bitmapData.imageInfo.height;
         final long numPixels = (long) width * height;
         // TODO(mcasas): https://crbug.com/670028 homogeneize overflow checking.
-        if (bitmapData.pixelData == null || width <= 0 || height <= 0
-                || numPixels > (Long.MAX_VALUE / 4)) {
-            return null;
-        }
-
-        if (bitmapData.imageInfo.colorType != ColorType.RGBA_8888
-                && bitmapData.imageInfo.colorType != ColorType.BGRA_8888) {
+        if (width <= 0 || height <= 0 || numPixels > (Long.MAX_VALUE / 4)) {
             return null;
         }
 
@@ -46,7 +44,7 @@ public class BitmapUtils {
         return bitmap;
     }
 
-    public static Frame convertToFrame(org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmapData) {
+    public static Frame convertToFrame(org.chromium.skia.mojom.BitmapN32 bitmapData) {
         Bitmap bitmap = convertToBitmap(bitmapData);
         if (bitmap == null) {
             return null;

services/shape_detection/android/java/src/org/chromium/shape_detection/FaceDetectionImpl.java

@@ -36,8 +36,8 @@ public class FaceDetectionImpl implements FaceDetection {
     }
 
     @Override
-    public void detect(org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmapData,
-            final DetectResponse callback) {
+    public void detect(
+            org.chromium.skia.mojom.BitmapN32 bitmapData, final DetectResponse callback) {
         Bitmap bitmap = BitmapUtils.convertToBitmap(bitmapData);
         if (bitmap == null) {
             Log.e(TAG, "Error converting Mojom Bitmap to Android Bitmap");

services/shape_detection/android/java/src/org/chromium/shape_detection/FaceDetectionImplGmsCore.java

@@ -59,8 +59,7 @@ public class FaceDetectionImplGmsCore implements FaceDetection {
     }
 
     @Override
-    public void detect(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmapData, DetectResponse callback) {
+    public void detect(org.chromium.skia.mojom.BitmapN32 bitmapData, DetectResponse callback) {
         // The vision library will be downloaded the first time the API is used
         // on the device; this happens "fast", but it might have not completed,
         // bail in this case.

services/shape_detection/android/java/src/org/chromium/shape_detection/TextDetectionImpl.java

@@ -34,8 +34,7 @@ public class TextDetectionImpl implements TextDetection {
     }
 
     @Override
-    public void detect(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmapData, DetectResponse callback) {
+    public void detect(org.chromium.skia.mojom.BitmapN32 bitmapData, DetectResponse callback) {
         // The vision library will be downloaded the first time the API is used
         // on the device; this happens "fast", but it might have not completed,
         // bail in this case. Also, the API was disabled between and v.9.0 and

services/shape_detection/android/javatests/src/org/chromium/shape_detection/BarcodeDetectionImplTest.java

@@ -40,7 +40,7 @@ import java.util.concurrent.TimeUnit;
 @UseRunnerDelegate(BaseJUnit4RunnerDelegate.class)
 @DisableIf.Build(sdk_is_greater_than = Build.VERSION_CODES.N_MR1, message = "crbug.com/1153716")
 public class BarcodeDetectionImplTest {
-    private static final org.chromium.skia.mojom.BitmapWithArbitraryBpp QR_CODE_BITMAP =
+    private static final org.chromium.skia.mojom.BitmapN32 QR_CODE_BITMAP =
             TestUtils.mojoBitmapFromFile("qr_code.png");
 
     private static final int[] SUPPORTED_FORMATS = {BarcodeFormat.AZTEC, BarcodeFormat.CODE_128,
@@ -69,14 +69,13 @@ public class BarcodeDetectionImplTest {
         return toReturn;
     }
 
-    private static BarcodeDetectionResult[] detect(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap) {
+    private static BarcodeDetectionResult[] detect(org.chromium.skia.mojom.BitmapN32 mojoBitmap) {
         BarcodeDetectorOptions options = new BarcodeDetectorOptions();
         return detectWithOptions(mojoBitmap, options);
     }
 
     private static BarcodeDetectionResult[] detectWithHint(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap, int format) {
+            org.chromium.skia.mojom.BitmapN32 mojoBitmap, int format) {
         Assert.assertTrue(BarcodeFormat.isKnownValue(format));
         BarcodeDetectorOptions options = new BarcodeDetectorOptions();
         options.formats = new int[] {format};
@@ -84,8 +83,7 @@ public class BarcodeDetectionImplTest {
     }
 
     private static BarcodeDetectionResult[] detectWithOptions(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap,
-            BarcodeDetectorOptions options) {
+            org.chromium.skia.mojom.BitmapN32 mojoBitmap, BarcodeDetectorOptions options) {
         BarcodeDetection detector = new BarcodeDetectionImpl(options);
 
         final ArrayBlockingQueue<BarcodeDetectionResult[]> queue = new ArrayBlockingQueue<>(1);
@@ -175,8 +173,7 @@ public class BarcodeDetectionImplTest {
         if (!TestUtils.IS_GMS_CORE_SUPPORTED) {
             return;
         }
-        org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmap =
-                TestUtils.mojoBitmapFromFile(inputFile);
+        org.chromium.skia.mojom.BitmapN32 bitmap = TestUtils.mojoBitmapFromFile(inputFile);
         BarcodeDetectionResult[] results = detectWithHint(bitmap, format);
         Assert.assertEquals(1, results.length);
         Assert.assertEquals(value, results[0].rawValue);
@@ -196,8 +193,7 @@ public class BarcodeDetectionImplTest {
         if (!TestUtils.IS_GMS_CORE_SUPPORTED) {
             return;
         }
-        org.chromium.skia.mojom.BitmapWithArbitraryBpp bitmap =
-                TestUtils.mojoBitmapFromFile(inputFile);
+        org.chromium.skia.mojom.BitmapN32 bitmap = TestUtils.mojoBitmapFromFile(inputFile);
         BarcodeDetectionResult[] results = detect(bitmap);
         Assert.assertEquals(1, results.length);
         Assert.assertEquals(value, results[0].rawValue);

services/shape_detection/android/javatests/src/org/chromium/shape_detection/FaceDetectionImplTest.java

@@ -34,9 +34,9 @@ import java.util.concurrent.TimeUnit;
 @Batch(Batch.UNIT_TESTS)
 @DisableIf.Build(sdk_is_greater_than = Build.VERSION_CODES.N_MR1, message = "crbug.com/1153716")
 public class FaceDetectionImplTest {
-    private static final org.chromium.skia.mojom.BitmapWithArbitraryBpp MONA_LISA_BITMAP =
+    private static final org.chromium.skia.mojom.BitmapN32 MONA_LISA_BITMAP =
             TestUtils.mojoBitmapFromFile("mona_lisa.jpg");
-    private static final org.chromium.skia.mojom.BitmapWithArbitraryBpp FACE_POSE_BITMAP =
+    private static final org.chromium.skia.mojom.BitmapN32 FACE_POSE_BITMAP =
             TestUtils.mojoBitmapFromFile("face_pose.png");
     // Different versions of Android have different implementations of FaceDetector.findFaces(), so
     // we have to use a large error threshold.
@@ -47,9 +47,8 @@ public class FaceDetectionImplTest {
 
     public FaceDetectionImplTest() {}
 
-    private static FaceDetectionResult[] detect(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap, boolean fastMode,
-            DetectionProviderType api) {
+    private static FaceDetectionResult[] detect(org.chromium.skia.mojom.BitmapN32 mojoBitmap,
+            boolean fastMode, DetectionProviderType api) {
         FaceDetectorOptions options = new FaceDetectorOptions();
         options.fastMode = fastMode;
         options.maxDetectedFaces = 32;
@@ -116,8 +115,7 @@ public class FaceDetectionImplTest {
                 MONA_LISA_BITMAP.imageInfo.height, Bitmap.Config.ARGB_8888);
         Canvas canvas = new Canvas(paddedBitmap);
         canvas.drawBitmap(BitmapUtils.convertToBitmap(MONA_LISA_BITMAP), 0, 0, null);
-        org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap =
-                TestUtils.mojoBitmapFromBitmap(paddedBitmap);
+        org.chromium.skia.mojom.BitmapN32 mojoBitmap = TestUtils.mojoBitmapFromBitmap(paddedBitmap);
         Assert.assertEquals(1, mojoBitmap.imageInfo.width % 2);
 
         FaceDetectionResult[] results = detect(mojoBitmap, true, DetectionProviderType.ANDROID);

services/shape_detection/android/javatests/src/org/chromium/shape_detection/TestUtils.java

@@ -13,8 +13,7 @@ import android.graphics.Paint;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.UrlUtils;
 import org.chromium.gms.ChromiumPlayServicesAvailability;
-import org.chromium.skia.mojom.ColorType;
-import org.chromium.skia.mojom.ImageInfo;
+import org.chromium.skia.mojom.BitmapN32ImageInfo;
 
 import java.nio.ByteBuffer;
 
@@ -31,31 +30,26 @@ public class TestUtils {
                 ContextUtils.getApplicationContext());
     }
 
-    public static org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmapFromBitmap(
-            Bitmap bitmap) {
+    public static org.chromium.skia.mojom.BitmapN32 mojoBitmapFromBitmap(Bitmap bitmap) {
         ByteBuffer buffer = ByteBuffer.allocate(bitmap.getByteCount());
         bitmap.copyPixelsToBuffer(buffer);
 
-        org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap =
-                new org.chromium.skia.mojom.BitmapWithArbitraryBpp();
-        mojoBitmap.imageInfo = new ImageInfo();
+        org.chromium.skia.mojom.BitmapN32 mojoBitmap = new org.chromium.skia.mojom.BitmapN32();
+        mojoBitmap.imageInfo = new BitmapN32ImageInfo();
         mojoBitmap.imageInfo.width = bitmap.getWidth();
         mojoBitmap.imageInfo.height = bitmap.getHeight();
-        mojoBitmap.imageInfo.colorType = ColorType.RGBA_8888;
         mojoBitmap.pixelData = new org.chromium.mojo_base.mojom.BigBuffer();
         mojoBitmap.pixelData.setBytes(buffer.array());
         return mojoBitmap;
     }
 
-    public static org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmapFromFile(
-            String relPath) {
+    public static org.chromium.skia.mojom.BitmapN32 mojoBitmapFromFile(String relPath) {
         String path = UrlUtils.getIsolatedTestFilePath("services/test/data/" + relPath);
         Bitmap bitmap = BitmapFactory.decodeFile(path);
         return mojoBitmapFromBitmap(bitmap);
     }
 
-    public static org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmapFromText(
-            String[] texts) {
+    public static org.chromium.skia.mojom.BitmapN32 mojoBitmapFromText(String[] texts) {
         final int x = 10;
         final int baseline = 100;
 

services/shape_detection/android/javatests/src/org/chromium/shape_detection/TextDetectionImplTest.java

@@ -34,11 +34,10 @@ public class TextDetectionImplTest {
             "The quick brown fox jumped over the lazy dog.", "Helvetica Neue 36."};
     private static final float[][] TEXT_BOUNDING_BOX = {
             {0.0f, 71.0f, 753.0f, 36.0f}, {4.0f, 173.0f, 307.0f, 28.0f}};
-    private static final org.chromium.skia.mojom.BitmapWithArbitraryBpp TEXT_DETECTION_BITMAP =
+    private static final org.chromium.skia.mojom.BitmapN32 TEXT_DETECTION_BITMAP =
             TestUtils.mojoBitmapFromText(DETECTION_EXPECTED_TEXT);
 
-    private static TextDetectionResult[] detect(
-            org.chromium.skia.mojom.BitmapWithArbitraryBpp mojoBitmap) {
+    private static TextDetectionResult[] detect(org.chromium.skia.mojom.BitmapN32 mojoBitmap) {
         TextDetection detector = new TextDetectionImpl();
 
         final ArrayBlockingQueue<TextDetectionResult[]> queue = new ArrayBlockingQueue<>(1);

services/shape_detection/android/junit/src/org/chromium/shape_detection/BitmapUtilsTest.java

@@ -16,9 +16,8 @@ import org.robolectric.shadows.ShadowLog;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.Feature;
 import org.chromium.mojo_base.BigBufferUtil;
-import org.chromium.skia.mojom.BitmapWithArbitraryBpp;
-import org.chromium.skia.mojom.ColorType;
-import org.chromium.skia.mojom.ImageInfo;
+import org.chromium.skia.mojom.BitmapN32;
+import org.chromium.skia.mojom.BitmapN32ImageInfo;
 
 /**
  * Test suite for conversion-to-Frame utils.
@@ -46,9 +45,9 @@ public class BitmapUtilsTest {
     @Test
     @Feature({"ShapeDetection"})
     public void testConversionFailsWithInvalidBitmap() {
-        BitmapWithArbitraryBpp bitmap = new BitmapWithArbitraryBpp();
+        BitmapN32 bitmap = new BitmapN32();
         bitmap.pixelData = null;
-        bitmap.imageInfo = new ImageInfo();
+        bitmap.imageInfo = new BitmapN32ImageInfo();
 
         assertNull(BitmapUtils.convertToFrame(bitmap));
     }
@@ -59,8 +58,8 @@ public class BitmapUtilsTest {
     @Test
     @Feature({"ShapeDetection"})
     public void testConversionFailsWithInvalidDimensions() {
-        BitmapWithArbitraryBpp bitmap = new BitmapWithArbitraryBpp();
-        bitmap.imageInfo = new ImageInfo();
+        BitmapN32 bitmap = new BitmapN32();
+        bitmap.imageInfo = new BitmapN32ImageInfo();
         bitmap.pixelData = BigBufferUtil.createBigBufferFromBytes(EMPTY_DATA);
         bitmap.imageInfo.width = INVALID_WIDTH;
         bitmap.imageInfo.height = VALID_HEIGHT;
@@ -74,12 +73,11 @@ public class BitmapUtilsTest {
     @Test
     @Feature({"ShapeDetection"})
     public void testConversionFailsWithWronglyWrappedData() {
-        BitmapWithArbitraryBpp bitmap = new BitmapWithArbitraryBpp();
-        bitmap.imageInfo = new ImageInfo();
+        BitmapN32 bitmap = new BitmapN32();
+        bitmap.imageInfo = new BitmapN32ImageInfo();
         bitmap.pixelData = BigBufferUtil.createBigBufferFromBytes(EMPTY_DATA);
         bitmap.imageInfo.width = VALID_WIDTH;
         bitmap.imageInfo.height = VALID_HEIGHT;
-        bitmap.imageInfo.colorType = ColorType.RGBA_8888;
 
         assertNull(BitmapUtils.convertToFrame(bitmap));
     }

services/shape_detection/detection_utils_win.cc

@@ -20,17 +20,11 @@ ComPtr<ISoftwareBitmap> CreateWinBitmapFromSkBitmap(
     const SkBitmap& bitmap,
     ISoftwareBitmapStatics* bitmap_factory) {
   DCHECK(bitmap_factory);
+  DCHECK_EQ(bitmap.colorType(), kN32_SkColorType);
   if (!base::CheckedNumeric<uint32_t>(bitmap.computeByteSize()).IsValid()) {
     DLOG(ERROR) << "Data overflow.";
     return nullptr;
   }
-  // CreateCopyFromBuffer() assumes the pixels we pass in are 32bits each and
-  // are tightly packed. Receiving a bitmap of a different bits-per-pixel would
-  // create a buffer overflow. The `pixel_format` we use below assumes the
-  // format of the bitmap is N32.
-  CHECK_EQ(bitmap.colorType(), kN32_SkColorType);
-  CHECK_EQ(4, bitmap.info().bytesPerPixel());
-  CHECK_EQ(bitmap.rowBytes(), bitmap.width() * static_cast<size_t>(4));
 
   // Create IBuffer from bitmap data.
   ComPtr<ABI::Windows::Storage::Streams::IBuffer> buffer;

services/shape_detection/public/mojom/barcodedetection.mojom

@@ -37,7 +37,6 @@ struct BarcodeDetectionResult {
 };
 
 interface BarcodeDetection {
-  // |bitmap_data| contains tightly packed image pixels in row-major order.
-  Detect(skia.mojom.BitmapWithArbitraryBpp bitmap_data)
+  Detect(skia.mojom.BitmapN32 bitmap_data)
     => (array<BarcodeDetectionResult> results);
 };

services/shape_detection/public/mojom/facedetection.mojom

@@ -32,7 +32,6 @@ struct FaceDetectorOptions {
 };
 
 interface FaceDetection {
-  // |bitmap_data| contains tightly packed image pixels in row-major order.
-  Detect(skia.mojom.BitmapWithArbitraryBpp bitmap_data)
+  Detect(skia.mojom.BitmapN32 bitmap_data)
     => (array<FaceDetectionResult> results);
 };

services/shape_detection/public/mojom/textdetection.mojom

@@ -16,7 +16,6 @@ struct TextDetectionResult {
 };
 
 interface TextDetection {
-  // |bitmap_data| contains tightly packed image pixels in row-major order.
-  Detect(skia.mojom.BitmapWithArbitraryBpp bitmap_data)
+  Detect(skia.mojom.BitmapN32 bitmap_data)
     => (array<TextDetectionResult> results);
 };

third_party/blink/renderer/modules/shapedetection/shape_detector.cc

@@ -121,7 +121,8 @@ ScriptPromise ShapeDetector::DetectShapesOnImageData(
 
   SkPixmap image_data_pixmap = image_data->GetSkPixmap();
   SkBitmap sk_bitmap;
-  if (!sk_bitmap.tryAllocPixels(image_data_pixmap.info(),
+  if (!sk_bitmap.tryAllocPixels(
+          image_data_pixmap.info().makeColorType(kN32_SkColorType),
           image_data_pixmap.rowBytes())) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError,