FaceDetection: add support for <video> input

FaceDetection: add support for <video> input

This CL adds support for <video> as input to detect().
Following canvas approach, what is done is to snap the
current video frame and use for the detection process.

- Some code is refactored in FaceDetector.cpp since,
from a c++ perspective, an ImageBitmap and an
HTMLVideoElement behave similarly.

- some LayoutTests code is refactored since, from the
JS/DOM perspective, an HMTLImageElement and an
HTMLVideoElement behave similarly.

BUG=659138
TEST=LayoutTests.

Committed: https://crrev.com/18e8e2880dea2fa45b5edf85a418c12f53547f01
Cr-Commit-Position: refs/heads/master@{#429016}

[mcasas, 2016-10-29 00:03:59]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-10-29 00:07:39]

Description was changed from

==========
FaceDetection: add support for <video> input

wip

BUG=659138
==========

to

==========
FaceDetection: add support for <video> input

This CL adds support for <video> as input to detect(). 
Following canvas approach, what is done is to snap the
current video frame and use for the detection process. 

- Some code is refactored in FaceDetector.cpp since,
from a c++ perspective, an ImageBitmap and an 
HTMLVideoElement behave similarly.

- some LayoutTests code is refactored since, from the
JS/DOM perspective, an HMTLImageElement and an 
HTMLVideoElement behave similarly.

BUG=659138
TEST=LayoutTests.
==========

[mcasas, 2016-10-29 00:11:46]

mcasas@chromium.org changed reviewers:
+ xianglu@chromium.org

[mcasas, 2016-10-29 00:11:47]

xianglu@ PTAL

[xianglu, 2016-10-29 01:33:39]

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html
(right):

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html:62:
promise_test(function(t) {
Consider creating ImageBitmap here and testing in
detectFaceOnElementAndExpectError(element, url) as well.

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/shapedetection/detectface-HTMLVideoElement.html
(right):

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/shapedetection/detectface-HTMLVideoElement.html:15:
video.src = "../imported/wpt/media/white.webm";
Do you still need this if it's specified in the tag?

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp (right):

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp:259: int
width,
nit: const

[mcasas, 2016-10-31 16:36:51]

ptal

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html
(right):

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html:62:
promise_test(function(t) {
On 2016/10/29 01:33:39, xianglu wrote:
> Consider creating ImageBitmap here and testing in
> detectFaceOnElementAndExpectError(element, url) as well.

Trouble is that ImageBitmap does not behave like
an HTMLElement; in particular, it doesn't have an
onerror or onload event, nor an |src| attribute, so 
l.24-26 would not make sense. Or what do you propose?

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/shapedetection/detectface-HTMLVideoElement.html
(right):

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/shapedetection/detectface-HTMLVideoElement.html:15:
video.src = "../imported/wpt/media/white.webm";
On 2016/10/29 01:33:39, xianglu wrote:
> Do you still need this if it's specified in the tag?

Oh, apologies, I should remove l.7 altogether.

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp (right):

https://codereview.chromium.org/2455973007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp:259: int
width,
On 2016/10/29 01:33:39, xianglu wrote:
> nit: const

Not for POD types: those are passed by value [1].
However, it's not needed to pass |size| as object,
so: simplified.

[1] https://google.github.io/styleguide/cppguide.html#Reference_Arguments

[xianglu, 2016-10-31 23:07:52]

lgtm with two minor comments.

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html
(right):

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html:46:
image.onerror = () => {};  // Explicitly ignore expected error events.
For HTMLImageElement I remember only onerror() event is triggered. If
imageBitmap and imageElement behave differently, I might consider ignoring
onload() event for HTMLImageElement too. We can do that in the next CL though.

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp (right):

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp:34: return
nullptr;
I wonder if we can put them in the same if statement like this?

if (value.isImageBitmap() &&
!static_cast<ImageBitmap*>(value.getAsImageBitmap())->isNeutered())
  return value.getAsImageBitmap();

[mcasas, 2016-11-01 01:10:57]

mcasas@chromium.org changed reviewers:
+ haraken@chromium.org

[mcasas, 2016-11-01 01:10:58]

haraken@ RS plz modules/shapedetection

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html
(right):

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/shapedetection/shapedetection-cross-origin.html:46:
image.onerror = () => {};  // Explicitly ignore expected error events.
On 2016/10/31 23:07:52, xianglu wrote:
> For HTMLImageElement I remember only onerror() event is triggered. If
> imageBitmap and imageElement behave differently, I might consider ignoring
> onload() event for HTMLImageElement too. We can do that in the next CL though.

Acknowledged.

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp (right):

https://codereview.chromium.org/2455973007/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/FaceDetector.cpp:34: return
nullptr;
On 2016/10/31 23:07:52, xianglu wrote:
> I wonder if we can put them in the same if statement like this?
> 
> if (value.isImageBitmap() &&
> !static_cast<ImageBitmap*>(value.getAsImageBitmap())->isNeutered())
>   return value.getAsImageBitmap();

Done.

[mcasas, 2016-11-01 02:29:25]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 02:29:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 03:09:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-01 03:09:32]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-11-01 04:08:57]

Implementation-wise LGTM

(Elliott might have some thoughts but he is ooo.)

[mcasas, 2016-11-01 16:25:46]

On 2016/11/01 04:08:57, haraken wrote:
> Implementation-wise LGTM
> 
> (Elliott might have some thoughts but he is ooo.)

I'll send it and he'll def will take a look when
we send for review the canvas version.

[mcasas, 2016-11-01 16:25:51]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2016-11-01 16:25:52]

The patchset sent to the CQ was uploaded after l-g-t-m from xianglu@chromium.org
Link to the patchset: https://codereview.chromium.org/2455973007/#ps60001
(title: "xianglu@s comments")

[commit-bot: I haz the power, 2016-11-01 16:26:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 16:33:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-01 16:33:16]

Failed to apply the patch.
On branch working_branch
Your branch is up-to-date with 'origin/refs/pending/heads/master'.
nothing to commit, working tree clean

[commit-bot: I haz the power, 2016-11-01 17:22:15]

Description was changed from

==========
FaceDetection: add support for <video> input

This CL adds support for <video> as input to detect(). 
Following canvas approach, what is done is to snap the
current video frame and use for the detection process. 

- Some code is refactored in FaceDetector.cpp since,
from a c++ perspective, an ImageBitmap and an 
HTMLVideoElement behave similarly.

- some LayoutTests code is refactored since, from the
JS/DOM perspective, an HMTLImageElement and an 
HTMLVideoElement behave similarly.

BUG=659138
TEST=LayoutTests.
==========

to

==========
FaceDetection: add support for <video> input

This CL adds support for <video> as input to detect().
Following canvas approach, what is done is to snap the
current video frame and use for the detection process.

- Some code is refactored in FaceDetector.cpp since,
from a c++ perspective, an ImageBitmap and an
HTMLVideoElement behave similarly.

- some LayoutTests code is refactored since, from the
JS/DOM perspective, an HMTLImageElement and an
HTMLVideoElement behave similarly.

BUG=659138
TEST=LayoutTests.

Committed: https://crrev.com/18e8e2880dea2fa45b5edf85a418c12f53547f01
Cr-Commit-Position: refs/heads/master@{#429016}
==========

[commit-bot: I haz the power, 2016-11-01 17:22:16]

Patchset 3 (id:??) landed as
https://crrev.com/18e8e2880dea2fa45b5edf85a418c12f53547f01
Cr-Commit-Position: refs/heads/master@{#429016}