RenderTheme relies on the mediaElement supportsFullscreen and supportMuting to know wether it should paint the fullscreen and mute buttons or not. The problem is that the MediaPlayer first loads a NullMediaPlayerPrivate which doesn't support those. So if you display a page with a video element and controls enabled (no autoplay), the theme will be rendered without those buttons until video playback starts and the controls are painted again. So I think we could have a mediaPlayerBackendChanged() callback in the mediaPlayerClient interface which in the case of the HTMLMediaElement trigger an update of the theme. Would that make sense, Eric? The new callback would be called by the MediaPlayer each time m_private variable is cleared/changed.
(In reply to comment #0) > RenderTheme relies on the mediaElement supportsFullscreen and supportMuting to know wether > it should paint the fullscreen and mute buttons or not. The problem is that the MediaPlayer first > loads a NullMediaPlayerPrivate which doesn't support those. So if you display a page with a video > element and controls enabled (no autoplay), the theme will be rendered without those buttons > until video playback starts and the controls are painted again. > > So I think we could have a mediaPlayerBackendChanged() callback in the mediaPlayerClient > interface which in the case of the HTMLMediaElement trigger an update of the theme. Would > that make sense, Eric? The new callback would be called by the MediaPlayer each time > m_private variable is cleared/changed. Sounds like a good idea.
Sorry this bug is invalid, the issue of controls buttons not displayed some times is specific to the GStreamer media-player. Will file a separate bug.
Turns out this is really needed, at least in the GStreamer player case where hasVideo and hasAudio are dependant on the player state. So when these values are updated the controls update needs to be triggered as well. See also bug 50382
Created attachment 75376 [details] proposed patch
Attachment 75376 [details] did not build on mac: Build output: http://queues.webkit.org/results/6839008
Created attachment 75385 [details] proposed patch
Comment on attachment 75385 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=75385&action=review > WebCore/html/HTMLMediaElement.cpp:1964 > +void HTMLMediaElement::mediaPlayerBackendUpdated(MediaPlayer*) I am not sure about this function name, the term "backend" isn't used anywhere else in this file. "Media engine" is used in both comments and "mediaEngineError", so maybe "mediaPlayerEngineUpdated" or "mediaPlayerEngineChanged" instead? > WebCore/html/HTMLMediaElement.cpp:1969 > + toRenderMedia(renderer())->updateFromElement(); Why is this cast necessary, updateFromElement() is called elsewhere in the file without it? r+, though I would prefer a different name for the callback.
Thanks, I went for mediaPlayerEngineUpdated. Landed in http://trac.webkit.org/changeset/73249