Shape Detection: Add two layout tests for face detection

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

Index: third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp
diff --git a/third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp b/third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp
index bb969d71c63d44b04e7969fcfa5c55999071a146..9a375449705306e086eeb82199ab54356720383c 100644
--- a/third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp
+++ b/third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp
@@ -21,16 +21,21 @@ 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();
   }
 
@@ -40,6 +45,8 @@ mojo::ScopedSharedBufferHandle getSharedBufferHandle(
 
   if (!image) {
     DLOG(ERROR) << "Failed to convert blink::Image to sk_sp<SkImage>.";
+    resolver->reject(
+        DOMException::create(InvalidStateError, "HTMLImageElement is broken."));
     return mojo::ScopedSharedBufferHandle();
   }
 
@@ -51,9 +58,13 @@ mojo::ScopedSharedBufferHandle getSharedBufferHandle(
   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();
   }
 
@@ -63,6 +74,8 @@ mojo::ScopedSharedBufferHandle getSharedBufferHandle(
   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();
   }
 
@@ -86,25 +99,28 @@ ScriptPromise FaceDetector::detect(ScriptState* scriptState,
   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;
   }