ShapeDetection: use mojom::Bitmap for mojo interface.

This CL uses mojo::Bitmap for mojo ShapeDetection interfaces definition, so that the Detect API is completely flexible and friendly. BUG=665488 TEST(Layout)= third_party/WebKit/LayoutTests/fast/shapedetection/shapedetection-creation.html third_party/WebKit/LayoutTests/fast/shapedetection/shapedetection-empty-input.html third_party/WebKit/LayoutTests/shapedetection/detection-HTMLCanvasElement.html third_party/WebKit/LayoutTests/shapedetection/detection-HTMLImageElement.html third_party/WebKit/LayoutTests/shapedetection/detection-HTMLVideoElement.html third_party/WebKit/LayoutTests/shapedetection/detection-ImageBitmap.html third_party/WebKit/LayoutTests/shapedetection/detection-ImageData.html Review-Url: https://codereview.chromium.org/2629433003 Cr-Commit-Position: refs/heads/master@{#446142}
cbchan · Jan 25, 2017 · 33adf4c · 33adf4c
1 parent 965cfcc
commit 33adf4c
Show file tree

Hide file tree

Showing 23 changed files with 121 additions and 131 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -221,6 +221,7 @@ Frankie Dintino <fdintino@theatlantic.com>
 Franklin Ta <fta2012@gmail.com>
 Frédéric Jacob <frederic.jacob.78@gmail.com>
 Frédéric Wang <fred.wang@free.fr>
+Fu Junwei <junwei.fu@intel.com>
 Gaetano Mendola <mendola@gmail.com>
 Gajendra N <gajendra.n@samsung.com>
 Gajendra Singh <wxjg68@motorola.com>

diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn
@@ -209,6 +209,7 @@ android_library("chrome_java") {
     "//services/service_manager/public/interfaces:interfaces_java",
     "//services/service_manager/public/java:service_manager_java",
     "//services/shape_detection/public/interfaces:interfaces_java",
+    "//skia/public/interfaces:interfaces_java",
     "//third_party/WebKit/public:android_mojo_bindings_java",
     "//third_party/WebKit/public:blink_headers_java",
     "//third_party/WebKit/public:mojo_bindings_java",

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/shapedetection/BarcodeDetectionImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/shapedetection/BarcodeDetectionImpl.java
@@ -20,11 +20,10 @@
 import org.chromium.gfx.mojom.PointF;
 import org.chromium.gfx.mojom.RectF;
 import org.chromium.mojo.system.MojoException;
-import org.chromium.mojo.system.SharedBufferHandle;
-import org.chromium.mojo.system.SharedBufferHandle.MapFlags;
 import org.chromium.services.service_manager.InterfaceFactory;
 import org.chromium.shape_detection.mojom.BarcodeDetection;
 import org.chromium.shape_detection.mojom.BarcodeDetectionResult;
+import org.chromium.skia.mojom.ColorType;
 
 import java.nio.ByteBuffer;
 
@@ -43,8 +42,7 @@ public BarcodeDetectionImpl(Context context) {
     }
 
     @Override
-    public void detect(
-            SharedBufferHandle frameData, int width, int height, DetectResponse callback) {
+    public void detect(org.chromium.skia.mojom.Bitmap bitmapData, DetectResponse callback) {
         if (!ExternalAuthUtils.getInstance().canUseGooglePlayServices(
                     mContext, new UserRecoverableErrorHandler.Silent())) {
             Log.e(TAG, "Google Play Services not available");
@@ -61,17 +59,29 @@ public void detect(
             return;
         }
 
+        // TODO(junwei.fu): Consider supporting other bitmap pixel formats,
+        // https://crbug.com/684921.
+        if (bitmapData.colorType != ColorType.RGBA_8888
+                && bitmapData.colorType != ColorType.BGRA_8888) {
+            Log.e(TAG, "Unsupported bitmap pixel format");
+            callback.call(new BarcodeDetectionResult[0]);
+            return;
+        }
+
+        int width = bitmapData.width;
+        int height = bitmapData.height;
         final long numPixels = (long) width * height;
         // TODO(mcasas): https://crbug.com/670028 homogeneize overflow checking.
-        if (!frameData.isValid() || width <= 0 || height <= 0 || numPixels > (Long.MAX_VALUE / 4)) {
+        if (bitmapData.pixelData == null || width <= 0 || height <= 0
+                || numPixels > (Long.MAX_VALUE / 4)) {
             callback.call(new BarcodeDetectionResult[0]);
             return;
         }
 
         // Mapping |frameData| will fail if the intended mapped size is larger
         // than its actual capacity, which is limited by the appropriate
         // mojo::edk::Configuration entry.
-        ByteBuffer imageBuffer = frameData.map(0, numPixels * 4, MapFlags.none());
+        ByteBuffer imageBuffer = ByteBuffer.wrap(bitmapData.pixelData);
         if (imageBuffer.capacity() <= 0) {
             callback.call(new BarcodeDetectionResult[0]);
             return;

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/shapedetection/TextDetectionImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/shapedetection/TextDetectionImpl.java
@@ -18,11 +18,10 @@
 import org.chromium.chrome.browser.externalauth.UserRecoverableErrorHandler;
 import org.chromium.gfx.mojom.RectF;
 import org.chromium.mojo.system.MojoException;
-import org.chromium.mojo.system.SharedBufferHandle;
-import org.chromium.mojo.system.SharedBufferHandle.MapFlags;
 import org.chromium.services.service_manager.InterfaceFactory;
 import org.chromium.shape_detection.mojom.TextDetection;
 import org.chromium.shape_detection.mojom.TextDetectionResult;
+import org.chromium.skia.mojom.ColorType;
 
 import java.nio.ByteBuffer;
 
@@ -41,8 +40,7 @@ public TextDetectionImpl(Context context) {
     }
 
     @Override
-    public void detect(
-            SharedBufferHandle frameData, int width, int height, DetectResponse callback) {
+    public void detect(org.chromium.skia.mojom.Bitmap bitmapData, DetectResponse callback) {
         if (!ExternalAuthUtils.getInstance().canUseGooglePlayServices(
                     mContext, new UserRecoverableErrorHandler.Silent())) {
             Log.e(TAG, "Google Play Services not available");
@@ -59,17 +57,29 @@ public void detect(
             return;
         }
 
+        // TODO(junwei.fu): Consider supporting other bitmap pixel formats,
+        // https://crbug.com/684921.
+        if (bitmapData.colorType != ColorType.RGBA_8888
+                && bitmapData.colorType != ColorType.BGRA_8888) {
+            Log.e(TAG, "Unsupported bitmap pixel format");
+            callback.call(new TextDetectionResult[0]);
+            return;
+        }
+
+        int width = bitmapData.width;
+        int height = bitmapData.height;
         final long numPixels = (long) width * height;
         // TODO(xianglu): https://crbug.com/670028 homogeneize overflow checking.
-        if (!frameData.isValid() || width <= 0 || height <= 0 || numPixels > (Long.MAX_VALUE / 4)) {
+        if (bitmapData.pixelData == null || width <= 0 || height <= 0
+                || numPixels > (Long.MAX_VALUE / 4)) {
             callback.call(new TextDetectionResult[0]);
             return;
         }
 
         // Mapping |frameData| will fail if the intended mapped size is larger
         // than its actual capacity, which is limited by the appropriate
         // mojo::edk::Configuration entry.
-        ByteBuffer imageBuffer = frameData.map(0, numPixels * 4, MapFlags.none());
+        ByteBuffer imageBuffer = ByteBuffer.wrap(bitmapData.pixelData);
         if (imageBuffer.capacity() <= 0) {
             callback.call(new TextDetectionResult[0]);
             return;

diff --git a/content/public/android/BUILD.gn b/content/public/android/BUILD.gn
@@ -58,6 +58,7 @@ android_library("content_java") {
     "//services/service_manager/public/interfaces:interfaces_java",
     "//services/service_manager/public/java:service_manager_java",
     "//services/shape_detection/public/interfaces:interfaces_java",
+    "//skia/public/interfaces:interfaces_java",
     "//third_party/WebKit/public:blink_headers_java",
     "//third_party/WebKit/public:mojo_bindings_java",
     "//third_party/android_tools:android_support_annotations_java",

diff --git a/...ublic/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java b/...ublic/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java
@@ -12,11 +12,10 @@
 import org.chromium.base.Log;
 import org.chromium.gfx.mojom.RectF;
 import org.chromium.mojo.system.MojoException;
-import org.chromium.mojo.system.SharedBufferHandle;
-import org.chromium.mojo.system.SharedBufferHandle.MapFlags;
 import org.chromium.shape_detection.mojom.FaceDetection;
 import org.chromium.shape_detection.mojom.FaceDetectionResult;
 import org.chromium.shape_detection.mojom.FaceDetectorOptions;
+import org.chromium.skia.mojom.ColorType;
 
 import java.nio.ByteBuffer;
 
@@ -36,29 +35,42 @@ public class FaceDetectionImpl implements FaceDetection {
     }
 
     @Override
-    public void detect(
-            SharedBufferHandle frameData, int width, int height, DetectResponse callback) {
+    public void detect(org.chromium.skia.mojom.Bitmap bitmapData, DetectResponse callback) {
+        int width = bitmapData.width;
+        int height = bitmapData.height;
         final long numPixels = (long) width * height;
         // TODO(xianglu): https://crbug.com/670028 homogeneize overflow checking.
-        if (!frameData.isValid() || width <= 0 || height <= 0 || numPixels > (Long.MAX_VALUE / 4)) {
+        if (bitmapData.pixelData == null || width <= 0 || height <= 0
+                || numPixels > (Long.MAX_VALUE / 4)) {
             Log.d(TAG, "Invalid argument(s).");
             callback.call(new FaceDetectionResult());
             return;
         }
 
-        ByteBuffer imageBuffer = frameData.map(0, numPixels * 4, MapFlags.none());
+        // TODO(junwei.fu): Consider supporting other bitmap pixel formats,
+        // https://crbug.com/684921.
+        if (bitmapData.colorType != ColorType.RGBA_8888
+                && bitmapData.colorType != ColorType.BGRA_8888) {
+            Log.e(TAG, "Unsupported bitmap pixel format");
+            callback.call(new FaceDetectionResult());
+            return;
+        }
+
+        ByteBuffer imageBuffer = ByteBuffer.wrap(bitmapData.pixelData);
         if (imageBuffer.capacity() <= 0) {
-            Log.d(TAG, "Failed to map from SharedBufferHandle.");
+            Log.d(TAG, "Failed to wrap from Bitmap.");
             callback.call(new FaceDetectionResult());
             return;
         }
 
+        // TODO(junwei.fu): Use |bitmapData| directly for |unPremultipliedBitmap| to spare a copy
+        // if the bitmap pixel format is RGB_565, the ARGB_8888 Bitmap doesn't need to be created
+        // in this case, https://crbug.com/684930.
         Bitmap bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888);
 
         // An int array is needed to construct a Bitmap. However the Bytebuffer
-        // we get from |sharedBufferHandle| is directly allocated and does not
-        // have a supporting array. Therefore we need to copy from |imageBuffer|
-        // to create this intermediate Bitmap.
+        // we get from |bitmapData| is directly allocated and does not have a supporting array.
+        // Therefore we need to copy from |imageBuffer| to create this intermediate Bitmap.
         // TODO(xianglu): Consider worker pool as appropriate threads.
         // http://crbug.com/655814
         bitmap.copyPixelsFromBuffer(imageBuffer);

diff --git a/services/shape_detection/public/interfaces/BUILD.gn b/services/shape_detection/public/interfaces/BUILD.gn
@@ -14,6 +14,7 @@ mojom("interfaces") {
   ]
 
   public_deps = [
+    "//skia/public/interfaces",
     "//ui/gfx/geometry/mojo",
   ]
 }
diff --git a/services/shape_detection/public/interfaces/barcodedetection.mojom b/services/shape_detection/public/interfaces/barcodedetection.mojom
@@ -6,6 +6,7 @@
 
 module shape_detection.mojom;
 
+import "skia/public/interfaces/bitmap.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
 
 struct BarcodeDetectionResult {
@@ -17,9 +18,7 @@ struct BarcodeDetectionResult {
 };
 
 interface BarcodeDetection {
-  // |frame_data| contains tightly packed image pixels in ARGB32 format,
-  // row-major order.
-  // TODO(mcasas): Consider using mojo::Bitmap here, https://crbug.com/665488.
-  Detect(handle<shared_buffer> frame_data, uint32 width, uint32 height)
+  // |bitmap_data| contains tightly packed image pixels in row-major order.
+  Detect(skia.mojom.Bitmap bitmap_data)
     => (array<BarcodeDetectionResult> results);
 };
diff --git a/services/shape_detection/public/interfaces/facedetection.mojom b/services/shape_detection/public/interfaces/facedetection.mojom
@@ -6,6 +6,7 @@
 
 module shape_detection.mojom;
 
+import "skia/public/interfaces/bitmap.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
 
 // Since "//ui/gfx/geometry/mojo" is not exposed to blink, we need to declare
@@ -21,9 +22,7 @@ struct FaceDetectorOptions {
 };
 
 interface FaceDetection {
-  // |frame_data| contains tightly packed image pixels in ARGB32 format,
-  // row-major order.
-  // TODO(mcasas): Consider using mojo::Bitmap here, https://crbug.com/665488.
-  Detect(handle<shared_buffer> frame_data, uint32 width, uint32 height)
+  // |bitmap_data| contains tightly packed image pixels in row-major order.
+  Detect(skia.mojom.Bitmap bitmap_data)
     => (FaceDetectionResult result);
 };
diff --git a/services/shape_detection/public/interfaces/textdetection.mojom b/services/shape_detection/public/interfaces/textdetection.mojom
@@ -4,6 +4,7 @@
 
 module shape_detection.mojom;
 
+import "skia/public/interfaces/bitmap.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
 
 struct TextDetectionResult {
@@ -12,8 +13,7 @@ struct TextDetectionResult {
 };
 
 interface TextDetection {
-  // |frame_data| contains tightly packed image pixels in ARGB32 format,
-  // row-major order.
-  Detect(handle<shared_buffer> frame_data, uint32 width, uint32 height)
+  // |bitmap_data| contains tightly packed image pixels in row-major order.
+  Detect(skia.mojom.Bitmap bitmap_data)
     => (array<TextDetectionResult> results);
 };
diff --git a/third_party/WebKit/LayoutTests/shapedetection/resources/mock-barcodedetection.js b/third_party/WebKit/LayoutTests/shapedetection/resources/mock-barcodedetection.js
@@ -18,8 +18,8 @@ let mockBarcodeDetectionReady = define(
           handle => this.bindingSet_.addBinding(this, handle));
     }
 
-    detect(frame_data, width, height) {
-      let receivedStruct = mojo.mapBuffer(frame_data, 0, width*height*4, 0);
+    detect(bitmap_data) {
+      let receivedStruct = new Uint8Array(bitmap_data.pixel_data);
       this.buffer_data_ = new Uint32Array(receivedStruct.buffer);
       return Promise.resolve({
         results: [
@@ -45,7 +45,6 @@ let mockBarcodeDetectionReady = define(
           },
         ],
       });
-      mojo.unmapBuffer(receivedStruct.buffer);
     }
 
     getFrameData() {

diff --git a/third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js b/third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js
@@ -44,8 +44,8 @@ let mockFaceDetectionProviderReady = define(
                                            request);
     }
 
-    detect(frame_data, width, height) {
-      let receivedStruct = mojo.mapBuffer(frame_data, 0, width*height*4, 0);
+    detect(bitmap_data) {
+      let receivedStruct = new Uint8Array(bitmap_data.pixel_data);
       this.buffer_data_ = new Uint32Array(receivedStruct.buffer);
       return Promise.resolve({
         result: {
@@ -56,7 +56,6 @@ let mockFaceDetectionProviderReady = define(
           ]
         }
       });
-      mojo.unmapBuffer(receivedStruct.buffer);
     }
   }
   return new MockFaceDetectionProvider();

diff --git a/third_party/WebKit/LayoutTests/shapedetection/resources/mock-textdetection.js b/third_party/WebKit/LayoutTests/shapedetection/resources/mock-textdetection.js
@@ -17,8 +17,8 @@ let mockTextDetectionReady = define(
           handle => this.bindingSet_.addBinding(this, handle));
     }
 
-    detect(frame_data, width, height) {
-      let receivedStruct = mojo.mapBuffer(frame_data, 0, width*height*4, 0);
+    detect(bitmap_data) {
+      let receivedStruct = new Uint8Array(bitmap_data.pixel_data);
       this.buffer_data_ = new Uint32Array(receivedStruct.buffer);
       return Promise.resolve({
         results: [
@@ -32,7 +32,6 @@ let mockTextDetectionReady = define(
           },
         ],
       });
-      mojo.unmapBuffer(receivedStruct.buffer);
     }
 
     getFrameData() {

diff --git a/third_party/WebKit/Source/modules/shapedetection/BUILD.gn b/third_party/WebKit/Source/modules/shapedetection/BUILD.gn
@@ -24,5 +24,6 @@ blink_modules_sources("shapedetection") {
 
   public_deps = [
     "//services/shape_detection/public/interfaces:interfaces_blink",
+    "//skia/public/interfaces:interfaces_blink",
   ]
 }
diff --git a/third_party/WebKit/Source/modules/shapedetection/BarcodeDetector.cpp b/third_party/WebKit/Source/modules/shapedetection/BarcodeDetector.cpp
@@ -26,11 +26,8 @@ BarcodeDetector::BarcodeDetector() : ShapeDetector() {
                 wrapWeakPersistent(this))));
 }
 
-ScriptPromise BarcodeDetector::doDetect(
-    ScriptPromiseResolver* resolver,
-    mojo::ScopedSharedBufferHandle sharedBufferHandle,
-    int imageWidth,
-    int imageHeight) {
+ScriptPromise BarcodeDetector::doDetect(ScriptPromiseResolver* resolver,
+                                        skia::mojom::blink::BitmapPtr bitmap) {
   ScriptPromise promise = resolver->promise();
   if (!m_barcodeService) {
     resolver->reject(DOMException::create(
@@ -39,10 +36,9 @@ ScriptPromise BarcodeDetector::doDetect(
   }
   m_barcodeServiceRequests.add(resolver);
   m_barcodeService->Detect(
-      std::move(sharedBufferHandle), imageWidth, imageHeight,
-      convertToBaseCallback(WTF::bind(&BarcodeDetector::onDetectBarcodes,
-                                      wrapPersistent(this),
-                                      wrapPersistent(resolver))));
+      std::move(bitmap), convertToBaseCallback(WTF::bind(
+                             &BarcodeDetector::onDetectBarcodes,
+                             wrapPersistent(this), wrapPersistent(resolver))));
   return promise;
 }
 

diff --git a/third_party/WebKit/Source/modules/shapedetection/BarcodeDetector.h b/third_party/WebKit/Source/modules/shapedetection/BarcodeDetector.h
@@ -29,9 +29,7 @@ class MODULES_EXPORT BarcodeDetector final : public ShapeDetector,
   ~BarcodeDetector() override = default;
 
   ScriptPromise doDetect(ScriptPromiseResolver*,
-                         mojo::ScopedSharedBufferHandle,
-                         int imageWidth,
-                         int imageHeight) override;
+                         skia::mojom::blink::BitmapPtr) override;
   void onDetectBarcodes(
       ScriptPromiseResolver*,
       Vector<shape_detection::mojom::blink::BarcodeDetectionResultPtr>);

diff --git a/third_party/WebKit/Source/modules/shapedetection/DEPS b/third_party/WebKit/Source/modules/shapedetection/DEPS
@@ -10,6 +10,7 @@ include_rules = [
     "+platform",
     "+public/platform",
     "+services/shape_detection",
+    "+skia/public/interfaces/bitmap.mojom-blink.h",
     "+third_party/skia/include/core/SkImage.h",
     "+third_party/skia/include/core/SkImageInfo.h",
     "+ui/gfx/geometry",

diff --git a/third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp b/third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp
@@ -35,19 +35,16 @@ FaceDetector::FaceDetector(const FaceDetectorOptions& options)
       &FaceDetector::onFaceServiceConnectionError, wrapWeakPersistent(this))));
 }
 
-ScriptPromise FaceDetector::doDetect(
-    ScriptPromiseResolver* resolver,
-    mojo::ScopedSharedBufferHandle sharedBufferHandle,
-    int imageWidth,
-    int imageHeight) {
+ScriptPromise FaceDetector::doDetect(ScriptPromiseResolver* resolver,
+                                     skia::mojom::blink::BitmapPtr bitmap) {
   ScriptPromise promise = resolver->promise();
   if (!m_faceService) {
     resolver->reject(DOMException::create(
         NotSupportedError, "Face detection service unavailable."));
     return promise;
   }
   m_faceServiceRequests.add(resolver);
-  m_faceService->Detect(std::move(sharedBufferHandle), imageWidth, imageHeight,
+  m_faceService->Detect(std::move(bitmap),
                         convertToBaseCallback(WTF::bind(
                             &FaceDetector::onDetectFaces, wrapPersistent(this),
                             wrapPersistent(resolver))));