Shape Detection: Add two layout tests for face detection

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/shapedetection/FaceDetector.h"
 
 #include "core/dom/DOMException.h"
 #include "core/dom/DOMRect.h"
 #include "core/dom/Document.h"
 #include "core/fetch/ImageResource.h"
 #include "core/frame/LocalDOMWindow.h"
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLImageElement.h"
 #include "platform/graphics/Image.h"
 #include "public/platform/InterfaceProvider.h"
 #include "third_party/skia/include/core/SkImage.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 
 namespace blink {
 
 namespace {
 
 mojo::ScopedSharedBufferHandle getSharedBufferHandle(
-    const HTMLImageElement* img) {
+    const HTMLImageElement* img,
+    ScriptPromiseResolver* resolver) {
   ImageResource* const imageResource = img->cachedImage();
   if (!imageResource) {
-    DLOG(ERROR) << "Failed to convert HTMLImageElement to ImageSource.";
+    resolver->reject(DOMException::create(
+        InvalidStateError,
+        "Failed to acquire ImageResource from HTMLImageElement."));
     return mojo::ScopedSharedBufferHandle();
   }
 
   Image* const blinkImage = imageResource->getImage();

[mcasas, 2016/10/21 23:43:16]

I'm not 100% sure, but I think before doing this
we could check imageResource->errorOccurred() [1]
which will tell us if the image is broken. If so, that'd
be the appropriate place to reject with the 'broken
image' error message, and not in l.80. WDYT?

(If you do this, don't say 'Image is broken' in l.80,
but keep rejecting).

[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fetch/Res...

[xianglu, 2016/10/22 02:19:09]

I agree. However, the testing image ("imported/wpt/images/broken.png") is
actually one with decompression error, so it will pass this check. It seems in
this context "broken" is a little ambiguous now. I will rephrase the error
message.

For the check in l.27. I would add another test case with an appropriate testing
image in a future CL. 

   if (!blinkImage) {
-    DLOG(ERROR) << "Failed to convert ImageSource to blink::Image.";
+    resolver->reject(DOMException::create(
+        InvalidStateError, "Failed to get blink::Image from ImageResource."));
     return mojo::ScopedSharedBufferHandle();
   }
 
   const sk_sp<SkImage> image = blinkImage->imageForCurrentFrame();
   DCHECK_EQ(img->naturalWidth(), image->width());
   DCHECK_EQ(img->naturalHeight(), image->height());
 
   if (!image) {
-    DLOG(ERROR) << "Failed to convert blink::Image to sk_sp<SkImage>.";
+    resolver->reject(DOMException::create(
+        InvalidStateError,
+        "Failed to convert blink::Image to sk_sp<SkImage>."));
     return mojo::ScopedSharedBufferHandle();
   }
 
   const SkImageInfo skiaInfo =
       SkImageInfo::MakeN32(image->width(), image->height(), image->alphaType());
 
   const uint32_t allocationSize = skiaInfo.getSafeSize(skiaInfo.minRowBytes());
 
   mojo::ScopedSharedBufferHandle sharedBufferHandle =
       mojo::SharedBufferHandle::Create(allocationSize);
   if (!sharedBufferHandle.is_valid()) {
-    // TODO(xianglu): Do something when the image is too large.
-    DLOG(ERROR) << "Failed to create a sharedBufferHandle. allocationSize = "
-                << allocationSize << "bytes. limit = 16777216";
+    DLOG(ERROR) << "AllocationSize = " << allocationSize
+                << "bytes. limit = 16777216, referred from "
+                   "https://cs.chromium.org/chromium/src/"
+                   "mojo/edk/system/broker_host.cc?l=24";
+    // TODO(xianglu): For now we reject the promise if the image is too large.
+    // But consider resizing the image to remove restriction on the user side.
+    // Also, add layouttest for this case later.
+    resolver->reject(DOMException::create(
+        InvalidStateError,
+        "HTMLImageElement is too large to fit in mojo shared buffer."));
     return mojo::ScopedSharedBufferHandle();
   }
 
   const mojo::ScopedSharedBufferMapping mappedBuffer =
       sharedBufferHandle->Map(allocationSize);
 
   const SkPixmap pixmap(skiaInfo, mappedBuffer.get(), skiaInfo.minRowBytes());
   if (!image->readPixels(pixmap, 0, 0)) {
-    DLOG(ERROR) << "Failed to read pixels from sk_sp<SkImage>.";
+    resolver->reject(DOMException::create(
+        InvalidStateError,
+        "Image is broken. Failed to read pixels from sk_sp<SkImage>."));
     return mojo::ScopedSharedBufferHandle();
   }
 
   return sharedBufferHandle;
 }
 
 }  // anonymous namespace
 
 FaceDetector* FaceDetector::create(ScriptState* scriptState) {
   return new FaceDetector(*scriptState->domWindow()->frame());
 }
 
 FaceDetector::FaceDetector(LocalFrame& frame) {
   DCHECK(!m_service.is_bound());
   DCHECK(frame.interfaceProvider());
   frame.interfaceProvider()->getInterface(mojo::GetProxy(&m_service));
 }
 
 ScriptPromise FaceDetector::detect(ScriptState* scriptState,
                                    const HTMLImageElement* img) {
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
 
-  if (!m_service) {
+  // TODO(xianglu): Add test cases for cross-origin-images.
+  if (img->wouldTaintOrigin(
+          scriptState->getExecutionContext()->getSecurityOrigin())) {
     resolver->reject(DOMException::create(
-        NotFoundError, "Face detection service unavailable."));
+        SecurityError, "Image source from a different origin."));
     return promise;
   }
 
-  if (!img) {
-    resolver->reject(DOMException::create(
-        SyntaxError, "The provided HTMLImageElement is empty."));
+  if (img->bitmapSourceSize().isZero()) {
+    resolver->reject(
+        DOMException::create(InvalidStateError, "HTMLImageElement is empty."));
     return promise;
   }
 
-  // TODO(xianglu): Add security check when the spec is ready.
-  // https://crbug.com/646083
   mojo::ScopedSharedBufferHandle sharedBufferHandle =
-      getSharedBufferHandle(img);
-  if (!sharedBufferHandle->is_valid()) {
+      getSharedBufferHandle(img, resolver);
+  if (!sharedBufferHandle->is_valid())
+    return promise;
+
+  if (!m_service) {
     resolver->reject(DOMException::create(
-        SyntaxError, "Request for sharedBufferHandle failed."));
+        NotSupportedError, "Face detection service unavailable."));
     return promise;
   }
 
   m_serviceRequests.add(resolver);
   DCHECK(m_service.is_bound());
   m_service->DetectFace(std::move(sharedBufferHandle), img->naturalWidth(),
                         img->naturalHeight(),
                         convertToBaseCallback(WTF::bind(
                             &FaceDetector::onDetectFace, wrapPersistent(this),
                             wrapPersistent(resolver))));
@@ skipping 16 matching lines @@
 
   resolver->resolve(detectedFaces);
   m_serviceRequests.remove(resolver);
 }
 
 DEFINE_TRACE(FaceDetector) {
   visitor->trace(m_serviceRequests);
 }
 
 }  // namespace blink