Remove MediaControls::showVolumeSlider()

Remove MediaControls::showVolumeSlider()

This is a remnant from WebKit when there was a volume slider container
that could be shown and hidden. What remains is essentially dead code,
because at most it shows the slider when m_mediaController->hasAudio(),
but MediaControls::reset() already does that.

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

[acolwell GONE FROM CHROMIUM, 2014-03-04 02:06:57]

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

https://codereview.chromium.org/176883020/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.cpp:287:
MediaControlPanelMuteButtonElement::MediaControlPanelMuteButtonElement(Document&
document, MediaControls* controls)
Do we even need this class anymore? It appears that this is the only subclass of
MediaControlMuteButtonElement. Any reason we shouldn't just move the remaining
code into that class?

[philipj_slow, 2014-03-04 08:10:15]

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

https://codereview.chromium.org/176883020/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.cpp:287:
MediaControlPanelMuteButtonElement::MediaControlPanelMuteButtonElement(Document&
document, MediaControls* controls)
On 2014/03/04 02:06:58, acolwell wrote:
> Do we even need this class anymore? It appears that this is the only subclass
of
> MediaControlMuteButtonElement. Any reason we shouldn't just move the remaining
> code into that class?

Sorry, I noticed the same thing and should have pointed it out in the commit
message. MediaControlVolumeSliderElement can also be merged, so I've merged both
as a separate CL that depends on this and the other recent controls changes:
https://codereview.chromium.org/185363011/

[acolwell GONE FROM CHROMIUM, 2014-03-04 23:23:17]

lgtm

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

https://codereview.chromium.org/176883020/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.cpp:305: if (event->type() ==
EventTypeNames::mouseover)
We don't need this now because the slider is always already visible when this
button is visible?

[philipj_slow, 2014-03-05 07:22:15]

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

https://codereview.chromium.org/176883020/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControlElements.cpp:305: if (event->type() ==
EventTypeNames::mouseover)
On 2014/03/04 23:23:17, acolwell wrote:
> We don't need this now because the slider is always already visible when this
> button is visible?

showVolumeSlider() simply shows the slider if (m_mediaController->hasAudio()),
but MediaControls::reset() already does exactly that. I haven't dug deep in the
WebKit history, but this looks like it would show a vertical slider which would
then be hidden on some mouseout event, but that code doesn't exist anymore, the
slider is part of the main panel...

Note that because of MediaController this wasn't 100% dead code, you could have
a case where there's initially no audio but adding another media element to the
controller causes there to be. In that case hovering the mute button ought to
cause the volume slider to become visible. However, I doubt anyone had thought
of that, because the sane thing to do would be do call reset() when the
MediaController changes in some way. (I'm still trying to figure out everything
that is broken about MediaController in combination with MediaControls, this is
far from the only problem.)

[philipj_slow, 2014-03-05 07:23:26]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-05 07:24:22]

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

[commit-bot: I haz the power, 2014-03-05 08:52:32]

Change committed as 168452