ShapeDetection: add support for all CanvasImageSource input types

third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp

Index: third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp
diff --git a/third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp b/third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp
index 6a76c7c29cf372c4ebfc6f4f7705516963833933..4da8d7c02e90cd38f14dc4171cd7228afb774127 100644
--- a/third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp
+++ b/third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp
@@ -64,11 +64,14 @@ ScriptPromise ShapeDetector::detect(ScriptState* scriptState,
     canvasImageSource = imageSource.getAsImageBitmap();
   } else if (imageSource.isHTMLVideoElement()) {
     canvasImageSource = imageSource.getAsHTMLVideoElement();
+  } else if (imageSource.isHTMLCanvasElement()) {
+    canvasImageSource = imageSource.getAsHTMLCanvasElement();
+  } else if (imageSource.isOffscreenCanvas()) {
+    canvasImageSource = imageSource.getAsOffscreenCanvas();
   } else {
-    // TODO(mcasas): Implement more CanvasImageSources, https://crbug.com/659138
-    NOTIMPLEMENTED() << "Unsupported CanvasImageSource";
+    NOTREACHED() << "Unsupported CanvasImageSource";
     resolver->reject(
-        DOMException::create(NotFoundError, "Unsupported source."));
+        DOMException::create(NotSupportedError, "Unsupported source."));
     return promise;
   }
 
@@ -82,15 +85,58 @@ ScriptPromise ShapeDetector::detect(ScriptState* scriptState,
   if (imageSource.isHTMLImageElement()) {
     return detectShapesOnImageElement(resolver,
                                       imageSource.getAsHTMLImageElement());
+  }
+
+  // TODO(mcasas): Check if |video| is actually playing a MediaStream by using
+  // HTMLMediaElement::isMediaStreamURL(video->currentSrc().getString()); if
+  // there is a local WebCam associated, there might be sophisticated ways to
+  // detect faces on it. Until then, treat as a normal <video> element.
+
+  const FloatSize size(canvasImageSource->sourceWidth(),
+                       canvasImageSource->sourceHeight());
+
+  SourceImageStatus sourceImageStatus = InvalidSourceImageStatus;
+  RefPtr<Image> image = canvasImageSource->getSourceImageForCanvas(
+      &sourceImageStatus, PreferNoAcceleration, SnapshotReasonDrawImage, size);
+  if (!image || sourceImageStatus != NormalSourceImageStatus) {
+    resolver->reject(
+        DOMException::create(InvalidStateError, "Invalid element or state."));
+    return promise;
+  }
+
+  SkPixmap pixmap;
+  RefPtr<Uint8Array> pixelData;
+  uint8_t* pixelDataPtr = nullptr;
+  WTF::CheckedNumeric<int> allocationSize = 0;
+
+  sk_sp<SkImage> skImage = image->imageForCurrentFrame();
+  // Use |skImage|'s pixels if it has direct access to them.
+  if (skImage->peekPixels(&pixmap)) {
+    pixelDataPtr = static_cast<uint8_t*>(pixmap.writable_addr());

[Reilly Grant (use Gerrit), 2016/12/07 21:37:06]

Use pixmap.writable_addr8() and avoid the static_cast.

[mcasas, 2016/12/07 22:02:37]

Looks like that would be the one, but, misleadingly enough,
addr8() family of functions only work for 8 bit pixel formats,
see the check in [1].

[1]
https://cs.chromium.org/chromium/src/third_party/skia/include/core/SkPixmap.h...

+    allocationSize = pixmap.getSafeSize();
   } else if (imageSource.isImageBitmap()) {
-    return detectShapesOnImageBitmap(resolver, imageSource.getAsImageBitmap());
-  } else if (imageSource.isHTMLVideoElement()) {
-    return detectShapesOnVideoElement(resolver,
-                                      imageSource.getAsHTMLVideoElement());
+    ImageBitmap* imageBitmap = imageSource.getAsImageBitmap();
+    pixelData = imageBitmap->copyBitmapData(imageBitmap->isPremultiplied()
+                                                ? PremultiplyAlpha
+                                                : DontPremultiplyAlpha,

[Reilly Grant (use Gerrit), 2016/12/07 21:37:06]

Please double check that this isn't backwards. If the image is already
premultiplied then shouldn't we not premultiply again on copy?

[mcasas, 2016/12/07 22:02:37]

This is copied from [1]. |PremultiplyAlpha| boils down to
kPremul_SkAlphaType in [2]. What's your concern 
about using the wrong premultiplied type?

[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...
[2]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Ima...

[Reilly Grant (use Gerrit), 2016/12/07 22:09:35]

Then it is probably right. My concern was that passing PremultiplyAlpha when the
bitmap data is premultiplied would apply the alpha twice. I'm not familiar with
the semantics of these arguments though so I will defer to you and the example
you cite.

+                                            N32ColorType);
+    pixelDataPtr = pixelData->data();
+    allocationSize = imageBitmap->size().area() * 4 /* bytes per pixel */;
+  } else {
+    // TODO(mcasas): retrieve the pixels from elsewhere.
+    NOTREACHED();
+    resolver->reject(DOMException::create(
+        InvalidStateError, "Failed to get pixels for current frame."));
+    return promise;
   }
 
-  NOTREACHED();
-  return promise;
+  mojo::ScopedSharedBufferHandle sharedBufferHandle = getSharedBufferOnData(
+      resolver, pixelDataPtr, allocationSize.ValueOrDefault(0));
+  if (!sharedBufferHandle->is_valid())
+    return promise;
+
+  return doDetect(resolver, std::move(sharedBufferHandle), image->width(),
+                  image->height());
 }
 
 ScriptPromise ShapeDetector::detectShapesOnImageElement(
@@ -160,98 +206,4 @@ ScriptPromise ShapeDetector::detectShapesOnImageElement(
                   img->naturalHeight());
 }
 
-ScriptPromise ShapeDetector::detectShapesOnImageBitmap(
-    ScriptPromiseResolver* resolver,
-    ImageBitmap* imageBitmap) {
-  ScriptPromise promise = resolver->promise();
-
-  if (imageBitmap->isNeutered()) {
-    resolver->reject(
-        DOMException::create(InvalidStateError, "Neutered ImageBitmap."));
-    return promise;
-  }
-
-  if (imageBitmap->size().area() == 0) {
-    resolver->resolve(HeapVector<Member<DOMRect>>());
-    return promise;
-  }
-
-  SkPixmap pixmap;
-  RefPtr<Uint8Array> pixelData;
-  uint8_t* pixelDataPtr = nullptr;
-  WTF::CheckedNumeric<int> allocationSize = 0;
-  // Use |skImage|'s pixels if it has direct access to them, otherwise retrieve
-  // them from elsewhere via copyBitmapData().
-  sk_sp<SkImage> skImage = imageBitmap->bitmapImage()->imageForCurrentFrame();
-  if (skImage->peekPixels(&pixmap)) {
-    pixelDataPtr = static_cast<uint8_t*>(pixmap.writable_addr());
-    allocationSize = pixmap.getSafeSize();
-  } else {
-    pixelData = imageBitmap->copyBitmapData(imageBitmap->isPremultiplied()
-                                                ? PremultiplyAlpha
-                                                : DontPremultiplyAlpha,
-                                            N32ColorType);
-    pixelDataPtr = pixelData->data();
-    allocationSize = imageBitmap->size().area() * 4 /* bytes per pixel */;
-  }
-
-  mojo::ScopedSharedBufferHandle sharedBufferHandle = getSharedBufferOnData(
-      resolver, pixelDataPtr, allocationSize.ValueOrDefault(0));
-  if (!sharedBufferHandle->is_valid())
-    return promise;
-
-  return doDetect(resolver, std::move(sharedBufferHandle), imageBitmap->width(),
-                  imageBitmap->height());
-}
-
-ScriptPromise ShapeDetector::detectShapesOnVideoElement(
-    ScriptPromiseResolver* resolver,
-    const HTMLVideoElement* video) {
-  ScriptPromise promise = resolver->promise();
-  // TODO(mcasas): Check if |video| is actually playing a MediaStream by using
-  // HTMLMediaElement::isMediaStreamURL(video->currentSrc().getString()); if
-  // there is a local WebCam associated, there might be sophisticated ways to
-  // detect faces on it. Until then, treat as a normal <video> element.
-
-  // !hasAvailableVideoFrame() is a bundle of invalid states.
-  if (!video->hasAvailableVideoFrame()) {
-    resolver->reject(DOMException::create(
-        InvalidStateError, "Invalid HTMLVideoElement or state."));
-    return promise;
-  }
-
-  const FloatSize videoSize(video->videoWidth(), video->videoHeight());
-  SourceImageStatus sourceImageStatus = InvalidSourceImageStatus;
-  RefPtr<Image> image =
-      video->getSourceImageForCanvas(&sourceImageStatus, PreferNoAcceleration,
-                                     SnapshotReasonDrawImage, videoSize);
-
-  DCHECK_EQ(NormalSourceImageStatus, sourceImageStatus);
-
-  SkPixmap pixmap;
-  RefPtr<Uint8Array> pixelData;
-  uint8_t* pixelDataPtr = nullptr;
-  WTF::CheckedNumeric<int> allocationSize = 0;
-  // Use |skImage|'s pixels if it has direct access to them.
-  sk_sp<SkImage> skImage = image->imageForCurrentFrame();
-  if (skImage->peekPixels(&pixmap)) {
-    pixelDataPtr = static_cast<uint8_t*>(pixmap.writable_addr());
-    allocationSize = pixmap.getSafeSize();
-  } else {
-    // TODO(mcasas): retrieve the pixels from elsewhere.
-    NOTREACHED();
-    resolver->reject(DOMException::create(
-        InvalidStateError, "Failed to get pixels for current frame."));
-    return promise;
-  }
-
-  mojo::ScopedSharedBufferHandle sharedBufferHandle = getSharedBufferOnData(
-      resolver, pixelDataPtr, allocationSize.ValueOrDefault(0));
-  if (!sharedBufferHandle->is_valid())
-    return promise;
-
-  return doDetect(resolver, std::move(sharedBufferHandle), image->width(),
-                  image->height());
-}
-
 }  // namespace blink