[Mojo Video Capture] Adapt video_capture service to refactored video capture stack

[Mojo Video Capture] Adapt video_capture service to refactored video capture stack

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL22b.

Note: The video capture service implementation is currently in an incomplete and
outdated state and its tests are disabled. With the refactoring of the legacy
video capture stack now being complete, the next goal is to update the service
implementation and fit it into the refactored stack.

Changes in this CL:
* Update Mojo interfaces to more closely resemble their native counterparts.
  - video_capture.mojom.Device offers functionality similar to
    VideoCaptureDeviceLauncher + LaunchedVideoCaptureDevice.
  - video_capture.mojom.DeviceFactory offers functionality similar to
    media::VideoCaptureProvider.
  - video_capture.mojom.Receiver offers functionality similar to
    media::VideoFrameReceiver.
* In the service implementation, use a VideoCaptureSystem instead of a
  VideoCaptureDeviceFactory directly.
* Add new adapter classes for plumbing between Mojo service and its usage in
  the native code.
* Re-enable existing video_capture_unittests and add a few more.

BUG=584797
TEST=
  service_unittests --gtest_filter="*Video*"
  content_unittests --gtest_filter="*Video*"
  content_browsertests --gtest_filter="VideoCaptureBrowserTest.*"

[1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing

Review-Url: https://codereview.chromium.org/2818513003
Cr-Commit-Position: refs/heads/master@{#467398}
Committed: https://chromium.googlesource.com/chromium/src/+/308b44e5bfb17c42ca3ea5b4fa5ed66c2698107b

[chfremer, 2017-04-12 23:25:26]

Patchset #1 (id:1) has been deleted

[chfremer, 2017-04-12 23:30:49]

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

[commit-bot: I haz the power, 2017-04-12 23:31:43]

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, 2017-04-12 23:39:26]

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

[commit-bot: I haz the power, 2017-04-12 23:39:27]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chfremer, 2017-04-14 21:10:20]

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

[commit-bot: I haz the power, 2017-04-14 21:10:52]

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, 2017-04-14 22:16:34]

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

[commit-bot: I haz the power, 2017-04-14 22:16:35]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[chfremer, 2017-04-14 22:25:50]

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

[commit-bot: I haz the power, 2017-04-14 22:26:21]

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, 2017-04-15 00:48:57]

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

[commit-bot: I haz the power, 2017-04-15 00:48:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chfremer, 2017-04-17 22:00:36]

Patchset #1 (id:20001) has been deleted

[chfremer, 2017-04-17 22:00:49]

Patchset #1 (id:40001) has been deleted

[chfremer, 2017-04-17 22:01:50]

chfremer@chromium.org changed reviewers:
+ emircan@chromium.org, mcasas@chromium.org, miu@chromium.org

[chfremer, 2017-04-17 22:01:50]

mcasas@: PTAL
emircan@: optional review
miu@: FYI

[emircan, 2017-04-19 18:16:48]

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
File media/capture/video/shared_memory_buffer_handle.cc (right):

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.cc:24: return
static_cast<uint8_t*>(shared_memory_->memory());
We should add checks to make sure |share_memory_| is not null at any stage,
since we don't have a mechanism to determine its lifetime. Also, please add
comments about lifetime.

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
File media/capture/video/shared_memory_buffer_handle.h (right):

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.h:21: uint8_t* data() override;
Can we also mark this method as const in the base class?

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.h:25: base::SharedMemory*
shared_memory_;
base::SharedMemory* const shared_memory_;

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.h:26: size_t mapped_size_;
const

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
File services/video_capture/device_media_to_mojo_adapter.cc (right):

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/device_media_to_mojo_adapter.cc:15: static const int
kMaxBufferCount = 3;
Add a comment.

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
File services/video_capture/public/cpp/receiver_media_to_mojo_adapter.cc
(right):

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/public/cpp/receiver_media_to_mojo_adapter.cc:38: return; 
// already done before
nit: You can also use base::Optional for this and keep the memory on stack.

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
File services/video_capture/test/fake_device_unittest.cc (right):

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/test/fake_device_unittest.cc:130: if (num_frames_arrived
>= kNumFramesToWaitFor) {
++num_frames_arrived

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/test/fake_device_unittest.cc:138:
ASSERT_LT(num_buffers_created, num_frames_arrived);
ASSERT_LE(num_buffers_created, kMaxBufferPoolBuffers);

[chfremer, 2017-04-19 22:59:47]

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

[commit-bot: I haz the power, 2017-04-19 23:00:16]

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

[chfremer, 2017-04-19 23:03:36]

chfremer@chromium.org changed reviewers:
+ ochang@chromium.org

[chfremer, 2017-04-19 23:03:38]

emircan@: PTAL
ochang@: Please RS *.mojom

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
File media/capture/video/shared_memory_buffer_handle.cc (right):

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.cc:24: return
static_cast<uint8_t*>(shared_memory_->memory());
On 2017/04/19 18:16:48, emircan wrote:
> We should add checks to make sure |share_memory_| is not null at any stage,
> since we don't have a mechanism to determine its lifetime. Also, please add
> comments about lifetime.

I don't think checks would work here. 
Added comment to the class declaration.

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
File media/capture/video/shared_memory_buffer_handle.h (right):

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.h:21: uint8_t* data() override;
On 2017/04/19 18:16:48, emircan wrote:
> Can we also mark this method as const in the base class? 

Done.

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.h:25: base::SharedMemory*
shared_memory_;
On 2017/04/19 18:16:48, emircan wrote:
> base::SharedMemory* const shared_memory_;

Done.

https://codereview.chromium.org/2818513003/diff/60001/media/capture/video/sha...
media/capture/video/shared_memory_buffer_handle.h:26: size_t mapped_size_;
On 2017/04/19 18:16:48, emircan wrote:
> const

Done.

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
File services/video_capture/device_media_to_mojo_adapter.cc (right):

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/device_media_to_mojo_adapter.cc:15: static const int
kMaxBufferCount = 3;
On 2017/04/19 18:16:48, emircan wrote:
> Add a comment.

Done.

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
File services/video_capture/public/cpp/receiver_media_to_mojo_adapter.cc
(right):

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/public/cpp/receiver_media_to_mojo_adapter.cc:38: return; 
// already done before
On 2017/04/19 18:16:48, emircan wrote:
> nit: You can also use base::Optional for this and keep the memory on stack.

Done.

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
File services/video_capture/test/fake_device_unittest.cc (right):

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/test/fake_device_unittest.cc:130: if (num_frames_arrived
>= kNumFramesToWaitFor) {
On 2017/04/19 18:16:48, emircan wrote:
> ++num_frames_arrived

Done.

https://codereview.chromium.org/2818513003/diff/60001/services/video_capture/...
services/video_capture/test/fake_device_unittest.cc:138:
ASSERT_LT(num_buffers_created, num_frames_arrived);
On 2017/04/19 18:16:48, emircan wrote:
> ASSERT_LE(num_buffers_created, kMaxBufferPoolBuffers);

Done.

[commit-bot: I haz the power, 2017-04-19 23:11:03]

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

[commit-bot: I haz the power, 2017-04-19 23:11:04]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[emircan, 2017-04-19 23:11:29]

lgtm

[chfremer, 2017-04-19 23:16:08]

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

[commit-bot: I haz the power, 2017-04-19 23:16:35]

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, 2017-04-20 01:29:47]

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

[commit-bot: I haz the power, 2017-04-20 01:29:48]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Oliver Chang, 2017-04-25 20:51:46]

RS LGTM

[chfremer, 2017-04-26 16:48:29]

The CQ bit was checked by chfremer@chromium.org

[chfremer, 2017-04-26 16:48:30]

The patchset sent to the CQ was uploaded after l-g-t-m from emircan@chromium.org
Link to the patchset: https://codereview.chromium.org/2818513003/#ps100001
(title: "Fix compile errors")

[commit-bot: I haz the power, 2017-04-26 16:49:13]

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, 2017-04-26 18:52:41]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493225309430990,
"parent_rev": "4f0ba69f0ae32dc9e48fc021852585abb0df35b5", "commit_rev":
"308b44e5bfb17c42ca3ea5b4fa5ed66c2698107b"}

[commit-bot: I haz the power, 2017-04-26 18:52:52]

Description was changed from

==========
[Mojo Video Capture] Adapt video_capture service to refactored video capture
stack

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL22b.

Note: The video capture service implementation is currently in an incomplete and
outdated state and its tests are disabled. With the refactoring of the legacy
video capture stack now being complete, the next goal is to update the service
implementation and fit it into the refactored stack.

Changes in this CL:
* Update Mojo interfaces to more closely resemble their native counterparts.
  - video_capture.mojom.Device offers functionality similar to
    VideoCaptureDeviceLauncher + LaunchedVideoCaptureDevice.
  - video_capture.mojom.DeviceFactory offers functionality similar to
    media::VideoCaptureProvider.
  - video_capture.mojom.Receiver offers functionality similar to
    media::VideoFrameReceiver.
* In the service implementation, use a VideoCaptureSystem instead of a
  VideoCaptureDeviceFactory directly.
* Add new adapter classes for plumbing between Mojo service and its usage in
  the native code.
* Re-enable existing video_capture_unittests and add a few more.

BUG=584797
TEST=
  service_unittests --gtest_filter="*Video*"
  content_unittests --gtest_filter="*Video*"
  content_browsertests --gtest_filter="VideoCaptureBrowserTest.*"

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

to

==========
[Mojo Video Capture] Adapt video_capture service to refactored video capture
stack

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL22b.

Note: The video capture service implementation is currently in an incomplete and
outdated state and its tests are disabled. With the refactoring of the legacy
video capture stack now being complete, the next goal is to update the service
implementation and fit it into the refactored stack.

Changes in this CL:
* Update Mojo interfaces to more closely resemble their native counterparts.
  - video_capture.mojom.Device offers functionality similar to
    VideoCaptureDeviceLauncher + LaunchedVideoCaptureDevice.
  - video_capture.mojom.DeviceFactory offers functionality similar to
    media::VideoCaptureProvider.
  - video_capture.mojom.Receiver offers functionality similar to
    media::VideoFrameReceiver.
* In the service implementation, use a VideoCaptureSystem instead of a
  VideoCaptureDeviceFactory directly.
* Add new adapter classes for plumbing between Mojo service and its usage in
  the native code.
* Re-enable existing video_capture_unittests and add a few more.

BUG=584797
TEST=
  service_unittests --gtest_filter="*Video*"
  content_unittests --gtest_filter="*Video*"
  content_browsertests --gtest_filter="VideoCaptureBrowserTest.*"

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...

Review-Url: https://codereview.chromium.org/2818513003
Cr-Commit-Position: refs/heads/master@{#467398}
Committed:
https://chromium.googlesource.com/chromium/src/+/308b44e5bfb17c42ca3ea5b4fa5e...
==========

[commit-bot: I haz the power, 2017-04-26 18:52:54]

Committed patchset #3 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/308b44e5bfb17c42ca3ea5b4fa5e...

[alph, 2017-04-26 20:47:09]

A revert of this CL (patchset #3 id:100001) has been created in
https://codereview.chromium.org/2844813002/ by alph@chromium.org.

The reason for reverting is:
FakeVideoCaptureDeviceDescriptorTest.AccessIsRevokedOnSecondAccess test is
flaky.
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType....

[zmin, 2017-04-26 20:50:48]

A revert of this CL (patchset #3 id:100001) has been created in
https://codereview.chromium.org/2844833002/ by zmin@chromium.org.

The reason for reverting is: service_unittests service_unittests
Run on OS: 'Ubuntu-14.04'
failures:
FakeVideoCaptureDeviceDescriptorTest.AccessIsRevokedOnSecondAccess

https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20C....