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, "HTMLImageElement is broken."));

[mcasas, 2016/10/21 04:41:46]

I suggest you line up the comments in l.30, 38 and 49 to the
DLOG(ERROR)s nearby. 
Also, in this case, both l. 28 and l.30 could say something like
"Failed to acquire Resource for Image".

[xianglu, 2016/10/21 21:08:30]

Done.

     return mojo::ScopedSharedBufferHandle();
   }
 
   Image* const blinkImage = imageResource->getImage();
   if (!blinkImage) {
     DLOG(ERROR) << "Failed to convert ImageSource to blink::Image.";

[mcasas, 2016/10/21 04:41:46]

s/ImageSource/ImageResource/

[xianglu, 2016/10/21 21:08:30]

Done.

+    resolver->reject(
+        DOMException::create(InvalidStateError, "HTMLImageElement is broken."));
     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, "HTMLImageElement is broken."));
     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";

[mcasas, 2016/10/21 04:41:46]

This number is [1], right? Consider pulling it here if 
so, don't write a hardcoded number.

[1]
https://cs.chromium.org/chromium/src/mojo/edk/system/broker_host.cc?q=mojo+sh...

[xianglu, 2016/10/21 21:08:30]

Done.

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

Well, I meant using |kMaxSharedBufferSize|,
but now I see that is not static and is in another
compilation unit, what about something like:

    DLOG(ERROR) << "Requested allocation : " << allocationSize
                << "B, larger than |mojo::edk::kMaxSharedBufferSize| == 16MB ";

But then again, it might be better to give this info
in the rejection text -- think about a Web developer having 
a  gigantic HTMLImageElement, they might be able to react
to it on their side upon seeing such rejection message.

[mcasas, 2016/10/24 17:38:56]

This has not been addressed.

+    // 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."));
     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, "HTMLImageElement is broken."));
     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