Media Controls: move all MediaControlDivElement sub-classes to modules/.

Media Controls: move all MediaControlDivElement sub-classes to modules/.

This isn't moving MediaControlDivElement as all the classes in
MediaControlElementTypes will be handled later. However, there is no
usage of this class outside of modules/ or MediaControlElementTypes
itself now.

BUG=662761
R=avayvod@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[mlamouri (slow - plz ping), 2017-04-12 10:56:38]

Description was changed from

==========
Media Controls: move all MediaControlDivElement sub-classes to modules/.

This isn't moving MediaControlDivElement as all the classes in
MediaControlElementTypes will be handled later. However, there is no
usage of this class outside of modules/ or MediaControlElementTypes
itself now.

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

to

==========
Media Controls: move all MediaControlDivElement sub-classes to modules/.

This isn't moving MediaControlDivElement as all the classes in
MediaControlElementTypes will be handled later. However, there is no
usage of this class outside of modules/ or MediaControlElementTypes
itself now.

BUG=662761
R=avayvod@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mlamouri (slow - plz ping), 2017-04-12 11:00:20]

Anton, your daily media controls refactoring joy :)

[mlamouri (slow - plz ping), 2017-04-12 11:00:34]

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

[commit-bot: I haz the power, 2017-04-12 11:00:49]

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 13:28:53]

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

[commit-bot: I haz the power, 2017-04-12 13:28:53]

Dry run: This issue passed the CQ dry run.

[whywhat, 2017-04-12 15:36:41]

lgtm

https://codereview.chromium.org/2810173003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/media/MediaControls.h (right):

https://codereview.chromium.org/2810173003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/media/MediaControls.h:72: // TODO: the
following are required by other parts of the media controls
nit: do you need to update the comment (and add your username)?

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

https://codereview.chromium.org/2810173003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementsHelper.cpp:16:
event->IsKeyboardEvent() || event->IsTouchEvent();
q: would isTouchEvent() return true if I fling over the media element when
scrolling the page and happen to touch one of the controls? Some touch events
are not considered to be user gesture anymore, right?

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

https://codereview.chromium.org/2810173003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementsHelper.h:14:
class MediaControlElementsHelper final {
Class comment? It's not clear why it's needed at all with one static method :)

[mlamouri (slow - plz ping), 2017-04-13 21:59:36]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-04-13 21:59:36]

The patchset sent to the CQ was uploaded after l-g-t-m from avayvod@chromium.org
Link to the patchset: https://codereview.chromium.org/2810173003/#ps20001
(title: "add class comments")

[mlamouri (slow - plz ping), 2017-04-13 21:59:54]

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

https://codereview.chromium.org/2810173003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementsHelper.cpp:16:
event->IsKeyboardEvent() || event->IsTouchEvent();
On 2017/04/12 at 15:36:41, whywhat wrote:
> q: would isTouchEvent() return true if I fling over the media element when
scrolling the page and happen to touch one of the controls? Some touch events
are not considered to be user gesture anymore, right?

It's not meant to check for user gesture but user interaction. This code was
added by aberent@ IIRC. The purpose is to prevent some events from being seen
outside the media controls shadow DOM IIRC. It should be re-written to use
pointer events IMO.

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

https://codereview.chromium.org/2810173003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementsHelper.h:14:
class MediaControlElementsHelper final {
On 2017/04/12 at 15:36:41, whywhat wrote:
> Class comment?

Got it.

> It's not clear why it's needed at all with one static method :)

There should be more I believe. Also, it's a method that is used by a few
elements.

[commit-bot: I haz the power, 2017-04-13 22:00:18]

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-13 22:09:15]

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

[commit-bot: I haz the power, 2017-04-13 22:09:16]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

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

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

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

+chrishtr@ for core/paint/

[chrishtr, 2017-04-13 22:52:04]

lgtm

https://codereview.chromium.org/2810173003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/MediaControlsPainter.cpp (right):

https://codereview.chromium.org/2810173003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/MediaControlsPainter.cpp:184: //
TODO(mlamouri): it might be possible to use the PanelLayoutOjbect() call
Nit: spelling

[mlamouri (slow - plz ping), 2017-04-13 23:40:32]

thanks for the quick review

[mlamouri (slow - plz ping), 2017-04-13 23:40:33]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-04-13 23:40:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, avayvod@chromium.org
Link to the patchset: https://codereview.chromium.org/2810173003/#ps40001
(title: "fix typo")

[commit-bot: I haz the power, 2017-04-13 23:41:03]

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 01:39:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492126833030660,
"parent_rev": "4dcdb6bbf30724ed511754410da319c35c3546ca", "commit_rev":
"a77b04a2af76e6eb19db28716651912d2621d040"}

[commit-bot: I haz the power, 2017-04-14 01:40:24]

Description was changed from

==========
Media Controls: move all MediaControlDivElement sub-classes to modules/.

This isn't moving MediaControlDivElement as all the classes in
MediaControlElementTypes will be handled later. However, there is no
usage of this class outside of modules/ or MediaControlElementTypes
itself now.

BUG=662761
R=avayvod@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Media Controls: move all MediaControlDivElement sub-classes to modules/.

This isn't moving MediaControlDivElement as all the classes in
MediaControlElementTypes will be handled later. However, there is no
usage of this class outside of modules/ or MediaControlElementTypes
itself now.

BUG=662761
R=avayvod@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2017-04-14 01:40:25]

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