MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and grabFrame()

MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks,
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807
TEST= run demo html in
https://cdn.rawgit.com/Miguelao/demos/master/imagecapture.html
with flag --enable-blink-features=ImageCapture
(click on "Open Camera 320x240", then on
"Create Image Capturer" and finally, repeatedly,
on "grabFrame").

Committed: https://crrev.com/d13e5bd95c4a6799f68d53c3e923ae2cd972da4f
Cr-Commit-Position: refs/heads/master@{#390321}

[mcasas, 2016-04-21 18:08:30]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-04-21 18:08:41]

Patchset #2 (id:40001) has been deleted

[mcasas, 2016-04-21 18:08:48]

Patchset #1 (id:20001) has been deleted

[mcasas, 2016-04-22 21:33:31]

Patchset #1 (id:60001) has been deleted

[mcasas, 2016-04-22 21:33:38]

Patchset #1 (id:80001) has been deleted

[mcasas, 2016-04-22 21:39:03]

Patchset #1 (id:100001) has been deleted

[mcasas, 2016-04-22 22:10:09]

Description was changed from

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

WIP!!! Only for Backup.

BUG=518807
==========

to

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.

For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

It also splits the current LayoutTest
 ImageCapture-creationAndGrabFrame.html
into better -creation.html and -grabFrame.html.


BUG=518807
==========

[mcasas, 2016-04-22 22:10:15]

Patchset #1 (id:120001) has been deleted

[mcasas, 2016-04-22 22:12:43]

Description was changed from

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.

For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

It also splits the current LayoutTest
 ImageCapture-creationAndGrabFrame.html
into better -creation.html and -grabFrame.html.


BUG=518807
==========

to

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTest
 ImageCapture-creationAndGrabFrame.html
into better -creation.html and -grabFrame.html.


BUG=518807
==========

[mcasas, 2016-04-22 22:13:50]

Patchset #1 (id:140001) has been deleted

[mcasas, 2016-04-22 22:15:49]

Description was changed from

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTest
 ImageCapture-creationAndGrabFrame.html
into better -creation.html and -grabFrame.html.


BUG=518807
==========

to

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807
==========

[mcasas, 2016-04-23 00:48:12]

mcasas@chromium.org changed reviewers:
+ emircan@chromium.org, reillyg@chromium.org

[mcasas, 2016-04-23 00:48:12]

reillyg@ PTAL at WebKit parts and in particular to the usage of
the CallbackPromiseAdapter and ScopedWebCallbacks (the 
latter in ImageCaptureFrameGrabber).

emircan@ PTAL at ImageCaptureFrameGrabber.

[mcasas, 2016-04-23 00:48:40]

Description was changed from

==========
[WIP] MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807
==========

to

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807
==========

[mcasas, 2016-04-23 01:55:57]

Patchset #1 (id:160001) has been deleted

[Reilly Grant (use Gerrit), 2016-04-25 22:37:33]

https://codereview.chromium.org/1899403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1899403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:409:
adoptRef(SkImage::NewFromBitmap(*bitmap.leakPtr())));
I think you just want SkImage::NewFromBitmap(*bitmap). It seems like the pixels
will be shared with the new SkImage if necessary but I'm not an expert in the
Skia API.

https://codereview.chromium.org/1899403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:411: return
ImageBitmap::create(image.release());
It shouldn't be necessary to call release here. The RefPtr<StaticBitmapImage>
will implicitly cast to a PassRefPtr<StaticBitmapImage>.

[mcasas, 2016-04-26 18:18:52]

Patchset #2 (id:200001) has been deleted

[emircan, 2016-04-26 19:19:32]

lgtm % nits.

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html
(right):

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:22:
assert_equals(document.getElementById('canvas0').height, bitmap.height);
You can additionally draw a certain color on canvas and check for it.

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp (right):

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:14: #include
"platform/graphics/StaticBitmapImage.h"
Extra?

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:72: if
(!m_frameGrabber) {
Can you move this inside the if on l.69? I dont think we need to double check
each time?

[Reilly Grant (use Gerrit), 2016-04-26 19:28:43]

CallbackPromiseAdapter usage lgtm.

[mcasas, 2016-04-27 00:50:28]

Patchset #3 (id:240001) has been deleted

[mcasas, 2016-04-27 00:51:29]

https://codereview.chromium.org/1899403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1899403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:409:
adoptRef(SkImage::NewFromBitmap(*bitmap.leakPtr())));
On 2016/04/25 22:37:33, Reilly Grant wrote:
> I think you just want SkImage::NewFromBitmap(*bitmap). It seems like the
pixels
> will be shared with the new SkImage if necessary but I'm not an expert in the
> Skia API.

Done.

https://codereview.chromium.org/1899403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:411: return
ImageBitmap::create(image.release());
On 2016/04/25 22:37:33, Reilly Grant wrote:
> It shouldn't be necessary to call release here. The RefPtr<StaticBitmapImage>
> will implicitly cast to a PassRefPtr<StaticBitmapImage>.

Done.

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html
(right):

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:22:
assert_equals(document.getElementById('canvas0').height, bitmap.height);
On 2016/04/26 19:19:31, emircan wrote:
> You can additionally draw a certain color on canvas and check for it.

Yes and no. ImageBitmap per se does not give
access to the pixels, it needs to be rendered onto
_another_ canvas and I felt meh about it... (there
will be content_browsertests doing that).

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp (right):

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:14: #include
"platform/graphics/StaticBitmapImage.h"
On 2016/04/26 19:19:31, emircan wrote:
> Extra?

Done.

https://codereview.chromium.org/1899403002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:72: if
(!m_frameGrabber) {
On 2016/04/26 19:19:31, emircan wrote:
> Can you move this inside the if on l.69? I dont think we need to double check
> each time?

This a get-or-create snippet, right? The first 
time around we create |m_frameGrabber|, but
still we need to check that it has been created
properly right? I guess I could move it back 
inside the first if(!) then create, but it'll need
to be there. Alternatives?

[mcasas, 2016-04-27 00:53:24]

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

[mcasas, 2016-04-27 00:53:25]

junov@ PTAL at image_capture_frame_grabber.* where 
a VideoFrame is converted to a sk_sp<SkImage> that is
subsequently sent to Blink via WebCallbacks. 
ImageBitmap is modified to take() such sk_sp<SkImage>.

[Justin Novosad, 2016-04-27 15:03:18]

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html
(right):

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:13:
// We need to paint something on the canvas to force it to create a context.
This comment smells like there is an underlying bug. The implementation of
canvas does defer the allocation of it's image buffer, but that behavior is an
optimization that should not have side effects that are observable in
JavaScript.  Is there a bug about this?

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:22:
assert_equals(document.getElementById('canvas0').height, bitmap.height);
Test is incomplete. You should verify that the pixel values were correctly
copied all the way through.  To do that, you should draw 'bitmap' into a fresh
canvas and call getImageData on that canvas to inspect the values.

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:409: return
ImageBitmap::create(StaticBitmapImage::create(fromSkSp(image)));
You must determine whether 'image' is tainted with cross-origin data and call
setOriginClean() with the right value on the new StaticBitmapImage object. You
also need a layout test that creates a stream from a cross origin video source
and does a grab from it. To test that the tainting mechanism worked, you should
draw the grabbed ImageBitmap into a canvas and verify that calling
getImageData() on that canvas throws a SecurityError exception.

[mcasas, 2016-04-27 17:48:18]

junov@ PTAL

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html
(right):

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:13:
// We need to paint something on the canvas to force it to create a context.
On 2016/04/27 15:03:17, Justin Novosad wrote:
> This comment smells like there is an underlying bug. The implementation of
> canvas does defer the allocation of it's image buffer, but that behavior is an
> optimization that should not have side effects that are observable in
> JavaScript.  Is there a bug about this?

What I meant is that canvas.captureStream() in 
l.16 needs something painted on the canvas to work 
correctly (emircan@ dixit). No bug AFAIK, I was told 
this is a WAI. Rewritten for clarity.

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:22:
assert_equals(document.getElementById('canvas0').height, bitmap.height);
On 2016/04/27 15:03:17, Justin Novosad wrote:
> Test is incomplete. You should verify that the pixel values were correctly
> copied all the way through.  To do that, you should draw 'bitmap' into a fresh
> canvas and call getImageData on that canvas to inspect the values.

Done.

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/ImageBitmap.cpp:409: return
ImageBitmap::create(StaticBitmapImage::create(fromSkSp(image)));
On 2016/04/27 15:03:17, Justin Novosad wrote:
> You must determine whether 'image' is tainted with cross-origin data and call
> setOriginClean() with the right value on the new StaticBitmapImage object. You
> also need a layout test that creates a stream from a cross origin video source
> and does a grab from it. To test that the tainting mechanism worked, you
should
> draw the grabbed ImageBitmap into a canvas and verify that calling
> getImageData() on that canvas throws a SecurityError exception.

|image| comes from a MediaStream so it's origin is
clean already. Moreover, the MS Video Track pixels
are already accessible on ToT by plugging it into a
<video> element, then reading it back via a <canvas>,
and the CORS test for the <video> tag says always
OK [1].

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

[mcasas, 2016-04-27 17:49:47]

Description was changed from

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807
==========

to

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807

TEST= run demo html in 
https://cdn.rawgit.com/Miguelao/demos/master/imagecapture.html
with flag --enable-blink-features=ImageCapture
(click on "Open Camera 320x240", then on
"Create Image Capturer" and finally, repeatedly,
on "grabFrame").
==========

[Justin Novosad, 2016-04-27 20:58:02]

On 2016/04/27 17:48:18, mcasas wrote:
> junov@ PTAL
> 
>
https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
> File
> third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html
> (right):
> 
>
https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:13:
> // We need to paint something on the canvas to force it to create a context.
> On 2016/04/27 15:03:17, Justin Novosad wrote:
> > This comment smells like there is an underlying bug. The implementation of
> > canvas does defer the allocation of it's image buffer, but that behavior is
an
> > optimization that should not have side effects that are observable in
> > JavaScript.  Is there a bug about this?
> 
> What I meant is that canvas.captureStream() in 
> l.16 needs something painted on the canvas to work 
> correctly (emircan@ dixit). No bug AFAIK, I was told 
> this is a WAI. Rewritten for clarity.

This still seems fishy to me. The canvas should be considered a rectangle of
transparent black pixels prior to the first draw.  The capture should work in
that case, unless the spec explicitly says otherwise.  Please follow-up on this.
But I don't want to hold up this CL for that.

lgtm

[mcasas, 2016-04-27 23:09:44]

On 2016/04/27 20:58:02, Justin Novosad wrote:
> On 2016/04/27 17:48:18, mcasas wrote:
> > junov@ PTAL
> > 
> >
>
https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
> > File
> > third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/1899403002/diff/260001/third_party/WebKit/Lay...
> >
>
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-grabFrame.html:13:
> > // We need to paint something on the canvas to force it to create a context.
> > On 2016/04/27 15:03:17, Justin Novosad wrote:
> > > This comment smells like there is an underlying bug. The implementation of
> > > canvas does defer the allocation of it's image buffer, but that behavior
is
> an
> > > optimization that should not have side effects that are observable in
> > > JavaScript.  Is there a bug about this?
> > 
> > What I meant is that canvas.captureStream() in 
> > l.16 needs something painted on the canvas to work 
> > correctly (emircan@ dixit). No bug AFAIK, I was told 
> > this is a WAI. Rewritten for clarity.
> 
> This still seems fishy to me. The canvas should be considered a rectangle of
> transparent black pixels prior to the first draw.  The capture should work in
> that case, unless the spec explicitly says otherwise.  Please follow-up on
this.
> But I don't want to hold up this CL for that.
> 
> lgtm

Emircan@ explained that the comment is wrong: I need the
context to draw something meaningful to the context in 
order for the comparison below to not be black-vs-black.
Ergo, removed the comment.

[mcasas, 2016-04-27 23:13:25]

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

[mcasas, 2016-04-27 23:13:26]

esprehn@: plz RS
content/renderer/render_blink_platform_impl.*
third_party/WebKit/public/platform/

haraken@ plz RS 
third_party/WebKit/Source/modules/imagecapture/*

[esprehn, 2016-04-28 01:57:36]

lgtm, it's a bit sad this is adding more surface area to platform, I wish we
could most of the logic here into blink's platform/ or the module/.

[haraken, 2016-04-28 06:28:33]

LGTM

[mcasas, 2016-04-28 06:34:21]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2016-04-28 06:34:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
emircan@chromium.org, reillyg@chromium.org, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/1899403002/#ps300001
(title: "Rebase. LayoutTests: Replace assert_array_equals with an for-each:
assert_aprox_equals")

[commit-bot: I haz the power, 2016-04-28 06:34:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899403002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899403002/300001

[commit-bot: I haz the power, 2016-04-28 07:42:06]

Description was changed from

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807

TEST= run demo html in 
https://cdn.rawgit.com/Miguelao/demos/master/imagecapture.html
with flag --enable-blink-features=ImageCapture
(click on "Open Camera 320x240", then on
"Create Image Capturer" and finally, repeatedly,
on "grabFrame").
==========

to

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807

TEST= run demo html in 
https://cdn.rawgit.com/Miguelao/demos/master/imagecapture.html
with flag --enable-blink-features=ImageCapture
(click on "Open Camera 320x240", then on
"Create Image Capturer" and finally, repeatedly,
on "grabFrame").
==========

[commit-bot: I haz the power, 2016-04-28 07:42:08]

Committed patchset #5 (id:300001)

[Marijn Kruisselbrink, 2016-04-28 19:05:20]

mek@chromium.org changed reviewers:
+ mek@chromium.org

[Marijn Kruisselbrink, 2016-04-28 19:05:21]

https://codereview.chromium.org/1899403002/diff/300001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-creation.html
(right):

https://codereview.chromium.org/1899403002/diff/300001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-creation.html:10:
var testVideo = promise_test(function() {
Both these tests make no sense. For some reason you're changing the tests to
promise_test, but you're not actually returning a promise from your test
function. Instead you're still calling step_func and done as if this is a
regular async_test, which it isn't.

Maybe if you're lucky this still happens to actually do something that somehow
makes sense, but I wouldn't be surprised if this ends up not actually testing
anyway since both promise_tests return nothing, so immediately resolve and will
be marked as passing without giving any chance for any async code later to
actually test anything.

[commit-bot: I haz the power, 2016-04-30 17:16:49]

Patchset 5 (id:??) landed as
https://crrev.com/d13e5bd95c4a6799f68d53c3e923ae2cd972da4f
Cr-Commit-Position: refs/heads/master@{#390321}

[commit-bot: I haz the power, 2016-04-30 17:54:34]

Description was changed from

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks, 
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807

TEST= run demo html in 
https://cdn.rawgit.com/Miguelao/demos/master/imagecapture.html
with flag --enable-blink-features=ImageCapture
(click on "Open Camera 320x240", then on
"Create Image Capturer" and finally, repeatedly,
on "grabFrame").
==========

to

==========
MediaStream Image Capture (2): Platform::ImageCaptureFrameGrabber and
grabFrame()

This CL adds support in Blink and content/ for capturing
VideoFrames via ImageCapture.grabFrame() method.
For that, it adds a web/WebImageCaptureFrameGrabber
interface implemented in a new content class
ImageCaptureFrameGrabber.

In order to pass ImageBitmaps in WebCallbacks,
ImageBitmap gets a method take().

It also splits the current LayoutTests into extended
-creation.html and -grabFrame.html.

BUG=518807

TEST= run demo html in
https://cdn.rawgit.com/Miguelao/demos/master/imagecapture.html
with flag --enable-blink-features=ImageCapture
(click on "Open Camera 320x240", then on
"Create Image Capturer" and finally, repeatedly,
on "grabFrame").

Committed: https://crrev.com/d13e5bd95c4a6799f68d53c3e923ae2cd972da4f
Cr-Commit-Position: refs/heads/master@{#390321}
==========