VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

This CL migrates video capture IPC messages Start and Resume
from IPC to mojom. It needs 2 EnumTraits (ResolutionChangePolicy
and PowerLineFrequency) in //services/video_capture, and a
StructTraits for VideoCaptureParams in //content.

The conversion of the IPC-related classes VideoCaptureHost and
VideoCaptureImpl is quite straightforward. The bulk of the CL
is unit tests adaptation (and cleanup!) and type renaming.

BUG=651897
TEST=content_unittests, content_browsertests, video_capture_unittests

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

[mcasas, 2016-10-06 14:57:52]

Description was changed from

==========
VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

BUG=651897
==========

to

==========
VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

This CL migrates video capture IPC messages Start and Resume
from IPC to mojom. It needs 2 EnumTraits (ResolutionChangePolicy 
and PowerLineFrequency) in //services/video_capture, and a 
StructTraits for VideoCaptureParams in //content. 

The conversion of the IPC-related classes VideoCaptureHost and
VideoCaptureImpl is quite straightforward. The bulk of the CL
is unit tests adaptation (and cleanup!) and type renaming.

BUG=651897
TEST=content_unittests, content_browsertests, video_capture_unittests
==========

[mcasas, 2016-10-06 15:08:54]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-10-06 15:09:03]

Patchset #1 (id:20001) has been deleted

[mcasas, 2016-10-06 15:13:46]

mcasas@chromium.org changed reviewers:
+ chfremer@chromium.org, rockot@chromium.org

[mcasas, 2016-10-06 15:13:47]

chfremer@ ptal at changes in //services/video_capture

rockot@ ptal at //content changes: more of the same.

[chfremer, 2016-10-06 17:12:17]

lgtm

[mcasas, 2016-10-06 20:53:18]

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

[mcasas, 2016-10-06 20:53:19]

rockot@ is working in // to land a small patch
fixing the failing unittests, in the meantime:

tsepez@ PTAL at the moving of stuff IPC --> mojom

[Tom Sepez, 2016-10-06 21:05:18]

On 2016/10/06 20:53:19, mcasas wrote:
> rockot@ is working in // to land a small patch
> fixing the failing unittests, in the meantime:
> 
> tsepez@ PTAL at the moving of stuff IPC --> mojom

Mojom itself looks good, can you confirm that we're safe against a untrusted
renderer sending stop messages for IDs it didn't create or no longer exist?

[Tom Sepez, 2016-10-06 22:23:18]

Doesn't change existing behaviour WRT IDs as keys, so LGTM.

[Ken Rockot(use gerrit already), 2016-10-06 23:09:05]

LGTM. FYI the fix for BrowserAssociatedInterface has just landed in
https://crrev.com/0e666505

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
File content/common/video_capture_traits.h (right):

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
content/common/video_capture_traits.h:1: // Copyright 2016 The Chromium Authors.
All rights reserved.
nit: I would prefer we name this file more clearly to
video_capture_struct_traits.h. There are many type traits around the sources,
mojo StructTraits are just one kind

[Ken Rockot(use gerrit already), 2016-10-06 23:12:30]

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
File content/common/video_capture_traits.h (right):

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
content/common/video_capture_traits.h:25: return
static_cast<video_capture::mojom::ResolutionChangePolicy>(
Oops, just noticed this - you shouldn't need to do any casting here if you have
a typemap for ResolutionChangePolicy. You can just return
params.resolution_change_policy and use  media::ResolutionChangePolicy for the
return type. Same for media::PowerLineFrequency below.

[mcasas, 2016-10-06 23:46:35]

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

[mcasas, 2016-10-06 23:46:36]

avi@ plz RS additions to content/common
(minus security stuff, already seen by tsepez@)
thanks!

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
File content/common/video_capture_traits.h (right):

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
content/common/video_capture_traits.h:1: // Copyright 2016 The Chromium Authors.
All rights reserved.
On 2016/10/06 23:09:05, Ken Rockot wrote:
> nit: I would prefer we name this file more clearly to
> video_capture_struct_traits.h. There are many type traits around the sources,
> mojo StructTraits are just one kind

Done.

https://codereview.chromium.org/2395163002/diff/80001/content/common/video_ca...
content/common/video_capture_traits.h:25: return
static_cast<video_capture::mojom::ResolutionChangePolicy>(
On 2016/10/06 23:12:30, Ken Rockot wrote:
> Oops, just noticed this - you shouldn't need to do any casting here if you
have
> a typemap for ResolutionChangePolicy. You can just return
> params.resolution_change_policy and use  media::ResolutionChangePolicy for the
> return type. Same for media::PowerLineFrequency below.

Done.

[mcasas, 2016-10-07 00:52:40]

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

[commit-bot: I haz the power, 2016-10-07 00:52:58]

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-10-07 02:47:10]

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

[commit-bot: I haz the power, 2016-10-07 02:47:11]

Dry run: Exceeded global retry quota

[Avi (use Gerrit), 2016-10-07 14:38:33]

lgtm

stampity stamp

[mcasas, 2016-10-07 14:49:49]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2016-10-07 14:49:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
chfremer@chromium.org, tsepez@chromium.org, rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2395163002/#ps100001
(title: "rockot@s comments on renaming and return types")

[commit-bot: I haz the power, 2016-10-07 14:50:02]

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-10-07 14:56:03]

Description was changed from

==========
VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

This CL migrates video capture IPC messages Start and Resume
from IPC to mojom. It needs 2 EnumTraits (ResolutionChangePolicy 
and PowerLineFrequency) in //services/video_capture, and a 
StructTraits for VideoCaptureParams in //content. 

The conversion of the IPC-related classes VideoCaptureHost and
VideoCaptureImpl is quite straightforward. The bulk of the CL
is unit tests adaptation (and cleanup!) and type renaming.

BUG=651897
TEST=content_unittests, content_browsertests, video_capture_unittests
==========

to

==========
VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

This CL migrates video capture IPC messages Start and Resume
from IPC to mojom. It needs 2 EnumTraits (ResolutionChangePolicy 
and PowerLineFrequency) in //services/video_capture, and a 
StructTraits for VideoCaptureParams in //content. 

The conversion of the IPC-related classes VideoCaptureHost and
VideoCaptureImpl is quite straightforward. The bulk of the CL
is unit tests adaptation (and cleanup!) and type renaming.

BUG=651897
TEST=content_unittests, content_browsertests, video_capture_unittests
==========

[commit-bot: I haz the power, 2016-10-07 14:56:05]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-10-07 14:58:01]

Description was changed from

==========
VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

This CL migrates video capture IPC messages Start and Resume
from IPC to mojom. It needs 2 EnumTraits (ResolutionChangePolicy 
and PowerLineFrequency) in //services/video_capture, and a 
StructTraits for VideoCaptureParams in //content. 

The conversion of the IPC-related classes VideoCaptureHost and
VideoCaptureImpl is quite straightforward. The bulk of the CL
is unit tests adaptation (and cleanup!) and type renaming.

BUG=651897
TEST=content_unittests, content_browsertests, video_capture_unittests
==========

to

==========
VideoCapture: migrate VideoCapture renderer-->host messages to mojo, part 2

This CL migrates video capture IPC messages Start and Resume
from IPC to mojom. It needs 2 EnumTraits (ResolutionChangePolicy
and PowerLineFrequency) in //services/video_capture, and a
StructTraits for VideoCaptureParams in //content.

The conversion of the IPC-related classes VideoCaptureHost and
VideoCaptureImpl is quite straightforward. The bulk of the CL
is unit tests adaptation (and cleanup!) and type renaming.

BUG=651897
TEST=content_unittests, content_browsertests, video_capture_unittests

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

[commit-bot: I haz the power, 2016-10-07 14:58:03]

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