Move a few media controls elements into modules/media_controls/elements/.

Move a few media controls elements into modules/media_controls/elements/.

This serve two purpose: move the elements implementations to modules/
but also split MediaControlElements.cpp into multiple files as it as
become very messy.

BUG=662761
R=avayvod@chromium.org

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

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

mlamouri@chromium.org changed reviewers:
- haraken@chromium.org

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

avayvod@ PTAL :)

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

Description was changed from

==========
Move a few media controls elements into modules/media_controls/elements/.

This serve two purpose: move the elements implementations to modules/
but also split MediaControlElements.cpp into multiple files as it as
become very messy.

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

to

==========
Move a few media controls elements into modules/media_controls/elements/.

This serve two purpose: move the elements implementations to modules/
but also split MediaControlElements.cpp into multiple files as it as
become very messy.

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

[mlamouri (slow - plz ping), 2017-04-10 12:51:36]

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

[commit-bot: I haz the power, 2017-04-10 12:51:56]

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-10 12:55:36]

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

[commit-bot: I haz the power, 2017-04-10 12:55:36]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[whywhat, 2017-04-10 17:50:48]

lgtm % directory structure comment

https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):

https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
"elements/MediaControlMuteButtonElement.cpp",
nit: why do you think having a separate subdir for elements is justified?
I'd err on keeping the directory depth as low as possible

[mlamouri (slow - plz ping), 2017-04-10 19:07:23]

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

[mlamouri (slow - plz ping), 2017-04-10 19:07:23]

haraken@, see comment below. avayvod@ is concerned about the sub-dir. I tend to
love having sub-directories so I thought you could help us decide :)

https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):

https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
"elements/MediaControlMuteButtonElement.cpp",
On 2017/04/10 at 17:50:47, whywhat wrote:
> nit: why do you think having a separate subdir for elements is justified?
> I'd err on keeping the directory depth as low as possible

There will likely be around a dozen elements in there and that number will grow.
I'm worried that it would pollute the top directory. +haraken@ to see what he
thinks.

[haraken, 2017-04-11 01:34:44]

On 2017/04/10 19:07:23, mlamouri wrote:
> haraken@, see comment below. avayvod@ is concerned about the sub-dir. I tend
to
> love having sub-directories so I thought you could help us decide :)
> 
>
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
> "elements/MediaControlMuteButtonElement.cpp",
> On 2017/04/10 at 17:50:47, whywhat wrote:
> > nit: why do you think having a separate subdir for elements is justified?
> > I'd err on keeping the directory depth as low as possible
> 
> There will likely be around a dozen elements in there and that number will
grow.
> I'm worried that it would pollute the top directory. +haraken@ to see what he
> thinks.

We don't have an explicit rule about creating sub-directories.

We're planning to add README.md to all directories. If you think that
media_controls/elements/ is big enough to have README.md, it would be a worth :)

(Honestly speaking, I don't have any strong opinion.)

[dglazkov, 2017-04-11 02:55:51]

On 2017/04/11 at 01:34:44, haraken wrote:
> On 2017/04/10 19:07:23, mlamouri wrote:
> > haraken@, see comment below. avayvod@ is concerned about the sub-dir. I tend
to
> > love having sub-directories so I thought you could help us decide :)
> > 
> >
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> > File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):
> > 
> >
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> > third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
> > "elements/MediaControlMuteButtonElement.cpp",
> > On 2017/04/10 at 17:50:47, whywhat wrote:
> > > nit: why do you think having a separate subdir for elements is justified?
> > > I'd err on keeping the directory depth as low as possible
> > 
> > There will likely be around a dozen elements in there and that number will
grow.
> > I'm worried that it would pollute the top directory. +haraken@ to see what
he
> > thinks.
> 
> We don't have an explicit rule about creating sub-directories.
> 
> We're planning to add README.md to all directories. If you think that
media_controls/elements/ is big enough to have README.md, it would be a worth :)
> 
> (Honestly speaking, I don't have any strong opinion.)

LGTM.

[mlamouri (slow - plz ping), 2017-04-11 08:42:57]

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

[mlamouri (slow - plz ping), 2017-04-11 08:42:57]

Thanks for the feedback :)

[mlamouri (slow - plz ping), 2017-04-11 08:42:59]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2017-04-11 08:43:32]

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-11 09:14:38]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491900179911750, "parent_rev":
"dd288990130665b1a0e4b5d3de33f68fdb9df9fd", "commit_rev":
"acec8c887e478089826d2e52a93c1a173d2d1fbc"}

[commit-bot: I haz the power, 2017-04-11 09:15:27]

Description was changed from

==========
Move a few media controls elements into modules/media_controls/elements/.

This serve two purpose: move the elements implementations to modules/
but also split MediaControlElements.cpp into multiple files as it as
become very messy.

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

to

==========
Move a few media controls elements into modules/media_controls/elements/.

This serve two purpose: move the elements implementations to modules/
but also split MediaControlElements.cpp into multiple files as it as
become very messy.

BUG=662761
R=avayvod@chromium.org

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

[commit-bot: I haz the power, 2017-04-11 09:15:29]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/acec8c887e478089826d2e52a93c...

[whywhat, 2017-04-11 15:04:10]

On 2017/04/10 at 19:07:23, mlamouri wrote:
> haraken@, see comment below. avayvod@ is concerned about the sub-dir. I tend
to love having sub-directories so I thought you could help us decide :)
> 
>
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
"elements/MediaControlMuteButtonElement.cpp",
> On 2017/04/10 at 17:50:47, whywhat wrote:
> > nit: why do you think having a separate subdir for elements is justified?
> > I'd err on keeping the directory depth as low as possible
> 
> There will likely be around a dozen elements in there and that number will
grow. I'm worried that it would pollute the top directory. +haraken@ to see what
he thinks.

Polluting a directory with a dozen files seems like a small cost unless you
often search in there by looking at the directory (even then, binary search is
your friend as the contents is most often sorted by alphabet).
Subdirectories have a cost of longer paths in includes/build files/command
line/etc. Not to mention a need for a wider code tree window in your editor and
codesearch :)

[whywhat, 2017-04-11 15:05:22]

On 2017/04/11 at 15:04:10, whywhat wrote:
> On 2017/04/10 at 19:07:23, mlamouri wrote:
> > haraken@, see comment below. avayvod@ is concerned about the sub-dir. I tend
to love having sub-directories so I thought you could help us decide :)
> > 
> >
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> > File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):
> > 
> >
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> > third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
"elements/MediaControlMuteButtonElement.cpp",
> > On 2017/04/10 at 17:50:47, whywhat wrote:
> > > nit: why do you think having a separate subdir for elements is justified?
> > > I'd err on keeping the directory depth as low as possible
> > 
> > There will likely be around a dozen elements in there and that number will
grow. I'm worried that it would pollute the top directory. +haraken@ to see what
he thinks.
> 
> Polluting a directory with a dozen files seems like a small cost unless you
often search in there by looking at the directory (even then, binary search is
your friend as the contents is most often sorted by alphabet).
> Subdirectories have a cost of longer paths in includes/build files/command
line/etc. Not to mention a need for a wider code tree window in your editor and
codesearch :)

Are you planning to drop MediaControl*Element from the names at least and
putting everything into media_controls namespace?

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

On 2017/04/11 at 15:05:22, avayvod wrote:
> On 2017/04/11 at 15:04:10, whywhat wrote:
> > On 2017/04/10 at 19:07:23, mlamouri wrote:
> > > haraken@, see comment below. avayvod@ is concerned about the sub-dir. I
tend to love having sub-directories so I thought you could help us decide :)
> > > 
> > >
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> > > File third_party/WebKit/Source/modules/media_controls/BUILD.gn (right):
> > > 
> > >
https://codereview.chromium.org/2811703002/diff/1/third_party/WebKit/Source/m...
> > > third_party/WebKit/Source/modules/media_controls/BUILD.gn:17:
"elements/MediaControlMuteButtonElement.cpp",
> > > On 2017/04/10 at 17:50:47, whywhat wrote:
> > > > nit: why do you think having a separate subdir for elements is
justified?
> > > > I'd err on keeping the directory depth as low as possible
> > > 
> > > There will likely be around a dozen elements in there and that number will
grow. I'm worried that it would pollute the top directory. +haraken@ to see what
he thinks.
> > 
> > Polluting a directory with a dozen files seems like a small cost unless you
often search in there by looking at the directory (even then, binary search is
your friend as the contents is most often sorted by alphabet).
> > Subdirectories have a cost of longer paths in includes/build files/command
line/etc. Not to mention a need for a wider code tree window in your editor and
codesearch :)
> 
> Are you planning to drop MediaControl*Element from the names at least and
putting everything into media_controls namespace?

I was thinking about it. Happy to do that! We would need to check what's the
policy for namespace in Blink though.