Move MediaControlsWindowEventListener to modules/media_controls/.

Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,haraken@chromium.org

Review-Url: https://codereview.chromium.org/2802133002
Cr-Commit-Position: refs/heads/master@{#463246}
Committed: https://chromium.googlesource.com/chromium/src/+/a0364a2865e00032fc00d2619371e4975ebdbcf5

[mlamouri (slow - plz ping), 2017-04-06 22:20:26]

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

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

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-06 22:21:52]

rbyers@, please rubberstamp core/HTML/BUILD.gn

avayvod@, please review the CL :)

[commit-bot: I haz the power, 2017-04-07 00:36:30]

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

[commit-bot: I haz the power, 2017-04-07 00:36:31]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-04-07 01:57:56]

gn LGTM

[Rick Byers, 2017-04-07 17:35:42]

Description was changed from

==========
Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,rbyers@chromium.org
==========

to

==========
Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,rbyers@chromium.org
==========

[Rick Byers, 2017-04-07 17:35:42]

rbyers@chromium.org changed reviewers:
+ haraken@chromium.org
- rbyers@chromium.org

[Rick Byers, 2017-04-07 17:35:51]

Description was changed from

==========
Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,rbyers@chromium.org
==========

to

==========
Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,haraken@chromium.org
==========

[Rick Byers, 2017-04-07 17:35:58]

On 2017/04/07 01:57:56, haraken wrote:
> gn LGTM

Thanks haraken@

[Rick Byers, 2017-04-07 17:36:00]

On 2017/04/07 01:57:56, haraken wrote:
> gn LGTM

Thanks haraken@

[whywhat, 2017-04-07 20:13:59]

lgtm

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
(right):

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h:126: // For
composition.
nit: what does composition mean here? As in "composition vs inheritance"?
Maybe rather explain what these classes need access to that can't be exposed in
other ways?

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.cpp
(right):

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.cpp:69:
m_mediaControls->m_volumeSlider->addEventListener(EventTypeNames::click, this,
nit: not sure why accessing buttons directly is better. we could perhaps
refactor this in a follow up so that the buttons add the needed event handlers
themselves?

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

The CQ bit was checked by mlamouri@chromium.org

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

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, avayvod@chromium.org
Link to the patchset: https://codereview.chromium.org/2802133002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-09 15:33:10]

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-09 15:36:41]

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
(right):

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h:126: // For
composition.
On 2017/04/07 at 20:13:59, whywhat wrote:
> nit: what does composition mean here? As in "composition vs inheritance"?
> Maybe rather explain what these classes need access to that can't be exposed
in other ways?

I gave more details.

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.cpp
(right):

https://codereview.chromium.org/2802133002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.cpp:69:
m_mediaControls->m_volumeSlider->addEventListener(EventTypeNames::click, this,
On 2017/04/07 at 20:13:59, whywhat wrote:
> nit: not sure why accessing buttons directly is better. we could perhaps
refactor this in a follow up so that the buttons add the needed event handlers
themselves?

The idea of MediaControlsWindowEventListener being a separate object is that
MediaControlsImpl doesn't need to worry about it and let it live its own life.
This said, this solution doesn't scale very well indeed. I will add a TODO.

[commit-bot: I haz the power, 2017-04-09 16:47:34]

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

[commit-bot: I haz the power, 2017-04-09 16:47:36]

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-10 11:04:05]

The CQ bit was checked by mlamouri@chromium.org

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

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, avayvod@chromium.org
Link to the patchset: https://codereview.chromium.org/2802133002/#ps40001
(title: "rebase after blink rename")

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

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-10 13:42:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491822245879040,
"parent_rev": "62fd3802519b8f941b453c8011dcc78b178bd2ec", "commit_rev":
"a0364a2865e00032fc00d2619371e4975ebdbcf5"}

[commit-bot: I haz the power, 2017-04-10 13:43:44]

Description was changed from

==========
Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,haraken@chromium.org
==========

to

==========
Move MediaControlsWindowEventListener to modules/media_controls/.

BUG=662761
R=avayvod@chromium.org,haraken@chromium.org

Review-Url: https://codereview.chromium.org/2802133002
Cr-Commit-Position: refs/heads/master@{#463246}
Committed:
https://chromium.googlesource.com/chromium/src/+/a0364a2865e00032fc00d2619371...
==========

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

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a0364a2865e00032fc00d2619371...