Let MediaControls use the parent HTMLMediaElement

Let MediaControls use the parent HTMLMediaElement

Likewise, let MediaControlElements use the parent MediaControls.

The goal is that all instances of mediaControllerInterface() should be
replaced by either the media element or its controller. For example,
text tracks and fullscreen will just use the media element, while e.g.
seeking must be done via the MediaController if it is present.

No observable changes are intended in this commit.

BUG=341813
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169014

[philipj_slow, 2014-03-11 17:36:32]

Depends on https://codereview.chromium.org/192453005/

The time difference makes this slow, so I'm throwing on another patch before the
previous one is landed. I can't say this patch makes everything pretty, but
getting rid of setMediaController was pretty nice and now I can more easily
switch things over to using HTMLMediaElement in small steps.

I looked into using toParentMediaElement but decided against it because either
one would need null checks everywhere, or write a getter that cannot fail, but
that would make the setup of the controls more convoluted as you'd have to
insert them before any setup code runs, and then remove them if setup fails. Not
worth it, I think.

Next steps will be to make text tracks and fullscreen use HTMLMediaElement, and
probably to make scrubbing a controls-internal concept. Then we'll be close to
removing m_pausedInternal, which complicated HTMLMediaElement.

[philipj_slow, 2014-03-11 17:38:09]

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElements.cpp (left):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.cpp:446: , m_controls(controls)
This can be removed now, but I've done it in a separate patch which I'll submit
if this one goes well.

[acolwell GONE FROM CHROMIUM, 2014-03-12 00:22:31]

lgtm % nits

The comment about handing in MediaControls is totally optional. I just wanted to
bring the issue to your attention.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElementTypes.cpp (right):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.cpp:70:
MediaControlElement::MediaControlElement(HTMLMediaElement& mediaElement,
MediaControlElementType displayType, HTMLElement* element)
WDYT about handing in MediaControls into these base classes instead? Part of my
reasoning is to make it easier for subcontrols to keep their visibility state in
sync with the parent control object along the lines of the discussion I've been
having in https://codereview.chromium.org/182613006/ .

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElementTypes.h (right):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.h:84: explicit
MediaControlElement(HTMLMediaElement&, MediaControlElementType, HTMLElement*);
Drop explicit since this isn't a 1 arg constructor.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.h:91: HTMLMediaElement&
m_mediaElement;
Why does this need to be protected instead of private?

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.h:103: explicit
MediaControlDivElement(HTMLMediaElement&, MediaControlElementType);
nit: drop explicit here and other cases of non-single argument constructors.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElements.h (right):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.h:177: explicit
MediaControlTimelineElement(HTMLMediaElement&, MediaControls*);
nit: drop explicit

[philipj_slow, 2014-03-12 04:30:11]

Aaron, I've made all the changes you suggested. Since you are OOO and it was
mostly search replace I'll be moderately bold and commit this. Still, please
take another look after the fact when you're back, in case I overlooked
something.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElementTypes.cpp (right):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.cpp:70:
MediaControlElement::MediaControlElement(HTMLMediaElement& mediaElement,
MediaControlElementType displayType, HTMLElement* element)
On 2014/03/12 00:22:32, acolwell_OOO_til_3-15 wrote:
> WDYT about handing in MediaControls into these base classes instead? Part of
my
> reasoning is to make it easier for subcontrols to keep their visibility state
in
> sync with the parent control object along the lines of the discussion I've
been
> having in https://codereview.chromium.org/182613006/ .

I started writing this patch before merging MediaControlsAndroid and at the time
I tried doing this but hit some silly bump in the road and went with
HTMLMediaElement. I tried again now and like the outcome, so I'll go with that.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElementTypes.h (right):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.h:84: explicit
MediaControlElement(HTMLMediaElement&, MediaControlElementType, HTMLElement*);
On 2014/03/12 00:22:32, acolwell_OOO_til_3-15 wrote:
> Drop explicit since this isn't a 1 arg constructor.

Done.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.h:91: HTMLMediaElement&
m_mediaElement;
On 2014/03/12 00:22:32, acolwell_OOO_til_3-15 wrote:
> Why does this need to be protected instead of private?

Oops, that was needed for a following patch, but I'll add a public getter
instead.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElementTypes.h:103: explicit
MediaControlDivElement(HTMLMediaElement&, MediaControlElementType);
On 2014/03/12 00:22:32, acolwell_OOO_til_3-15 wrote:
> nit: drop explicit here and other cases of non-single argument constructors.

Done.

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControlElements.h (right):

https://codereview.chromium.org/195473002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.h:177: explicit
MediaControlTimelineElement(HTMLMediaElement&, MediaControls*);
On 2014/03/12 00:22:32, acolwell_OOO_til_3-15 wrote:
> nit: drop explicit

Done.

[philipj_slow, 2014-03-12 04:30:51]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-12 04:31:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/195473002/40001

[commit-bot: I haz the power, 2014-03-12 07:46:09]

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

[commit-bot: I haz the power, 2014-03-12 07:46:10]

Try jobs failed on following builders: linux_blink

[philipj_slow, 2014-03-12 08:12:55]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-12 08:12:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/195473002/40001

[commit-bot: I haz the power, 2014-03-12 08:39:28]

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

[commit-bot: I haz the power, 2014-03-12 08:39:29]

Try jobs failed on following builders: linux_blink

[philipj_slow, 2014-03-12 08:59:26]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-12 08:59:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/195473002/40001

[commit-bot: I haz the power, 2014-03-12 09:55:29]

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

[commit-bot: I haz the power, 2014-03-12 09:55:30]

Try jobs failed on following builders: linux_blink

[philipj_slow, 2014-03-12 10:33:23]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-12 10:33:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/195473002/40001

[commit-bot: I haz the power, 2014-03-12 11:34:32]

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

[commit-bot: I haz the power, 2014-03-12 11:34:32]

Try jobs failed on following builders: linux_blink

[philipj_slow, 2014-03-12 12:40:53]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-12 12:41:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/195473002/40001

[commit-bot: I haz the power, 2014-03-12 13:56:21]

Change committed as 169014