Implement heuristic for showing media controls during playback w/o a mouse

Source/core/html/shadow/MediaControls.cpp

Index: Source/core/html/shadow/MediaControls.cpp
diff --git a/Source/core/html/shadow/MediaControls.cpp b/Source/core/html/shadow/MediaControls.cpp
index 05c56e8d88923dcd665c1f2f32c593d620b9cc69..017229b9bdb51295ceb052ec597c5ec7836a1d36 100644
--- a/Source/core/html/shadow/MediaControls.cpp
+++ b/Source/core/html/shadow/MediaControls.cpp
@@ -191,6 +191,12 @@ void MediaControls::show()
     m_panel->show();
 }
 
+void MediaControls::mediaElementFocused()
+{
+    show();
+    stopHideMediaControlsTimer();
+}
+
 void MediaControls::hide()
 {
     m_panel->setIsDisplayed(false);
@@ -207,9 +213,20 @@ void MediaControls::makeTransparent()
     m_panel->makeTransparent();
 }
 
-bool MediaControls::shouldHideMediaControls()
+bool MediaControls::shouldHideMediaControls(HideBehavior behavior) const
 {
-    return !m_panel->hovered();
+    // Never hide for <audio>.

[philipj_slow, 2014/05/23 13:22:43]

The comment doesn't quite match the code. isHTMLAudioElement(mediaElement())
would match the comment, but I suppose that treating video-less <video> like
<audio> is OK too.

[fs, 2014/05/23 14:39:30]

Will change to something more long-winded about "visual representation" or
somesuch.

+    if (!mediaElement().hasVideo())
+        return false;
+    // Don't hide if hovered or the mouse is over the controls.
+    if (m_panel->hovered() || m_isMouseOverControls)

[philipj_slow, 2014/05/23 13:22:43]

Almost completely off topic: When I've seen m_isMouseOverControls before I
haven't understood why using hovered() instead wouldn't work. Until now I
assumed I didn't understand how hovered work with nested elements, but this demo
makes it seem like it should work:
http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3036

[fs, 2014/05/23 14:39:30]

Yes... Personally I'm more worried that this is too broad for some cases. (Since
the predicate used in playbackProgressed() was wildly different before.)

+        return false;
+    // Don't hide if focus is on the HTMLMediaElement or within the
+    // controls/shadow tree. (Perform the checks separately to avoid going

[philipj_slow, 2014/05/23 13:22:43]

I feel like this comment should make me understand, but it doesn't. Using just
contains(document().focusedElement()) looks like it should be just as cheap
looking at how Document::focusedElement() and Node::contains() is implemented.
What are these ancestor hosts the comment refers to?

[fs, 2014/05/23 14:39:30]

contains(...) will not cross into the shadow tree. There's a
containsIncludingShadowDOM(...) which does the same as contains(...) but does
traverse the ancestor (shadow) host chain.

[philipj_slow, 2014/05/23 20:08:34]

Ah, that's what I overlooked, thanks! The comment made me think it had something
to do with avoiding useless traversal, which I couldn't find :)

+    // through all the potential ancestor hosts for the focused element.)
+    if (behavior != IgnoreFocus && (mediaElement().focused() || contains(document().focusedElement())))
+        return false;
+    return true;
 }
 
 void MediaControls::playbackStarted()
@@ -229,7 +246,7 @@ void MediaControls::playbackProgressed()
     m_timeline->setPosition(mediaElement().currentTime());
     updateCurrentTimeDisplay();
 
-    if (!m_isMouseOverControls && mediaElement().hasVideo())
+    if (shouldHideMediaControls())
         makeTransparent();
 }
 
@@ -369,7 +386,7 @@ void MediaControls::hideMediaControlsTimerFired(Timer<MediaControls>*)
     if (mediaElement().togglePlayStateWillPlay())
         return;
 
-    if (!shouldHideMediaControls())
+    if (!shouldHideMediaControls(IgnoreFocus))

[acolwell GONE FROM CHROMIUM, 2014/05/22 18:44:09]

What is the situation where we would want the controls to hide yet we still have
focus? What would the user have to do to make them appear again?

[fs, 2014/05/23 07:05:40]

The case where I found this a useful behavior was when navigating to the <video>
by keyboard (<tab>) and then selecting the play button and activating it
(starting playback). Without this tweak, the controls would then stay visible.
It felt like a reasonable behavior to me (as opposed to having to blur to get
the controls to disappear), but that's of course just an opinion. I'd be happy
to remove this tweak if it's felt that non-tweaked behavior is better (or just
leave that undecided for later).
To make them re-appear again in that situation, you'd need to re-focus the
<video>.

         return;
 
     makeTransparent();