Move core MediaControls implementation to modules/media_controls/.

Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=662761
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2795783004
Cr-Commit-Position: refs/heads/master@{#462464}
Committed: https://chromium.googlesource.com/chromium/src/+/56ee1a3113f01d783795890d0235387fdcc8d02e

[mlamouri (slow - plz ping), 2017-04-03 23:12:41]

Description was changed from

==========
Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=<TODO>
==========

to

==========
Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=<TODO>
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mlamouri (slow - plz ping), 2017-04-03 23:20:16]

mlamouri@chromium.org changed reviewers:
+ foolip@chromium.org

[mlamouri (slow - plz ping), 2017-04-03 23:20:17]

This might require a bit of cleanup but I think the gist of it is here.

[mlamouri (slow - plz ping), 2017-04-03 23:27:10]

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

[commit-bot: I haz the power, 2017-04-03 23:27:40]

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-04 00:08:27]

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

[commit-bot: I haz the power, 2017-04-04 00:08:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[haraken, 2017-04-04 05:35:41]

> This CL will allow other parts of MediaControls to move to modules/ as
> MediaControlsImpl has a few components with rare dependencies in core/.

I'm just curious but what IDL files are going to be moved to
modules/media_controls/ in the future?

[mlamouri (slow - plz ping), 2017-04-04 09:15:54]

On 2017/04/04 at 05:35:41, haraken wrote:
> > This CL will allow other parts of MediaControls to move to modules/ as
> > MediaControlsImpl has a few components with rare dependencies in core/.
> 
> I'm just curious but what IDL files are going to be moved to
modules/media_controls/ in the future?

I thought I replied but can't find my draft :-/

There should be no IDL in modules/. The goal is to have the media controls
pluggable so they can be more easily interchanged in the future. Having two
implementations isn't part of the project. The end goal is that core/ will only
know about a media control interface which will allow alternative
implementations. It will allow us to implement media controls per platform or at
least easily implement platform-specific modules for media controls. There is no
need to be in modules/ but as we want to make the controls modular, moving them
out of core/ is semantically correct and enforces the DEPS. We can't move them
in platform/ as they still need to access many Web Platform primitives. This was
discussed with esprehn@ a while ago and the conclusion was that this was place
and that there might be a possibility to move them somewhere else in the future,
based on other plans the architecture team has. I think it required IDL files to
have exported headers. I don't want to speak for him so I CC'd him :)

[foolip, 2017-04-04 09:23:35]

I thought this might conflict (not just textually) with
https://codereview.chromium.org/2793273002/ but it looks like there are plenty
of internal hooks left, so that should be OK.

Leaving MediaControlsPainter.*, MediaControlElementTypes.* and
MediaControlElements.* in core looks a bit odd to me, they are a large chunk of
the implementation of the media controls. What would it take to move them as
well?

[mlamouri (slow - plz ping), 2017-04-04 10:06:25]

On 2017/04/04 at 09:23:35, foolip wrote:
> I thought this might conflict (not just textually) with
https://codereview.chromium.org/2793273002/ but it looks like there are plenty
of internal hooks left, so that should be OK.

Yes, it shouldn't be dramatic but most of the hooks are temporary. Ideally,
there should be the factory and a couple of callbacks such as
remotePlaybackDisabled/controlsList attribute change because they are coming
from the content and thus the content isn't notify from them being changed.

> Leaving MediaControlsPainter.*, MediaControlElementTypes.* and
MediaControlElements.* in core looks a bit odd to me, they are a large chunk of
the implementation of the media controls. What would it take to move them as
well?

Some parts of the remaining bits will be trivial to move away and some would be
a bit more complex. I have a design doc somewhere. I did not want to move
everything in this one CL as it's already ridiculously huge :) The goal though
is to have the classes you mentioned out of core/ as an alternative
implementation of media controls will have to re-implement them.

[foolip, 2017-04-04 10:24:59]

On 2017/04/04 10:06:25, mlamouri wrote:
> On 2017/04/04 at 09:23:35, foolip wrote:
> > I thought this might conflict (not just textually) with
> https://codereview.chromium.org/2793273002/ but it looks like there are plenty
> of internal hooks left, so that should be OK.
> 
> Yes, it shouldn't be dramatic but most of the hooks are temporary. Ideally,
> there should be the factory and a couple of callbacks such as
> remotePlaybackDisabled/controlsList attribute change because they are coming
> from the content and thus the content isn't notify from them being changed.
> 
> > Leaving MediaControlsPainter.*, MediaControlElementTypes.* and
> MediaControlElements.* in core looks a bit odd to me, they are a large chunk
of
> the implementation of the media controls. What would it take to move them as
> well?
> 
> Some parts of the remaining bits will be trivial to move away and some would
be
> a bit more complex. I have a design doc somewhere. I did not want to move
> everything in this one CL as it's already ridiculously huge :) The goal though
> is to have the classes you mentioned out of core/ as an alternative
> implementation of media controls will have to re-implement them.

Since the plan is to move almost everything and there isn't anything currently
known to be unmovable, then LGTM.

[mlamouri (slow - plz ping), 2017-04-04 13:43:57]

Description was changed from

==========
Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=<TODO>
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=662761
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mlamouri (slow - plz ping), 2017-04-04 14:14:59]

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

[commit-bot: I haz the power, 2017-04-04 14:15:18]

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-04 15:26:14]

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

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

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[mlamouri (slow - plz ping), 2017-04-04 22:29:29]

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

[commit-bot: I haz the power, 2017-04-04 22:30:28]

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

[mlamouri (slow - plz ping), 2017-04-04 22:43:01]

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

[commit-bot: I haz the power, 2017-04-04 22:43:45]

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

[mlamouri (slow - plz ping), 2017-04-04 22:47:24]

Unless someone objects (including the CQ), I will land this tomorrow.

[commit-bot: I haz the power, 2017-04-05 00:23:50]

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

[commit-bot: I haz the power, 2017-04-05 00:23:51]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mlamouri (slow - plz ping), 2017-04-05 16:38:53]

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

[commit-bot: I haz the power, 2017-04-05 16:39:27]

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

[mlamouri (slow - plz ping), 2017-04-05 16:42:58]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-04-05 16:42:59]

The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2795783004/#ps100001
(title: "cleanup")

[commit-bot: I haz the power, 2017-04-05 16:43:19]

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-05 17:28:45]

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

[commit-bot: I haz the power, 2017-04-05 17:28:46]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[mlamouri (slow - plz ping), 2017-04-06 10:21:19]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2017-04-06 10:21:40]

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-06 10:43:25]

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

[commit-bot: I haz the power, 2017-04-06 10:43:27]

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_...)

[mlamouri (slow - plz ping), 2017-04-06 11:11:44]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-04-06 11:11:45]

The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2795783004/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-06 11:11:59]

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-06 14:30:14]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491477104527310,
"parent_rev": "2400df3e07cbda738c7089f1f8a71ba9e19357c4", "commit_rev":
"56ee1a3113f01d783795890d0235387fdcc8d02e"}

[commit-bot: I haz the power, 2017-04-06 14:31:12]

Description was changed from

==========
Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=662761
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Move core MediaControls implementation to modules/media_controls/.

This is also moving MediaControlsOrientationLockDelegate because the
tests required to use MediaControlsImpl and the class is only used by
MediaControlsImpl.

This CL will allow other parts of MediaControls to move to modules/ as
MediaControlsImpl has a few components with rare dependencies in core/.

BUG=662761
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2795783004
Cr-Commit-Position: refs/heads/master@{#462464}
Committed:
https://chromium.googlesource.com/chromium/src/+/56ee1a3113f01d783795890d0235...
==========

[commit-bot: I haz the power, 2017-04-06 14:31:13]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/56ee1a3113f01d783795890d0235...