New media playback UI.

Source/core/html/shadow/MediaControls.cpp

 /*
  * Copyright (C) 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 29 matching lines @@
 
 // If you change this value, then also update the corresponding value in
 // LayoutTests/media/media-controls.js.
 static const double timeWithoutMouseMovementBeforeHidingMediaControls = 3;
 
 static bool fullscreenIsSupported(const Document& document)
 {
     return !document.settings() || document.settings()->fullscreenSupported();
 }
 
+static bool preferHiddenAudioElements(const Document& document)
+{
+    return !document.settings() || document.settings()->preferHiddenAudioElements();
+}
+
 MediaControls::MediaControls(HTMLMediaElement& mediaElement)
     : HTMLDivElement(mediaElement.document())
     , m_mediaElement(&mediaElement)
     , m_overlayEnclosure(nullptr)
     , m_overlayPlayButton(nullptr)
     , m_overlayCastButton(nullptr)
     , m_enclosure(nullptr)
     , m_panel(nullptr)
     , m_playButton(nullptr)
     , m_timeline(nullptr)
     , m_currentTimeDisplay(nullptr)
     , m_durationDisplay(nullptr)
     , m_muteButton(nullptr)
     , m_volumeSlider(nullptr)
     , m_toggleClosedCaptionsButton(nullptr)
     , m_castButton(nullptr)
     , m_fullScreenButton(nullptr)
     , m_hideMediaControlsTimer(this, &MediaControls::hideMediaControlsTimerFired)
     , m_hideTimerBehaviorFlags(IgnoreNone)
     , m_isMouseOverControls(false)
     , m_isPausedForScrubbing(false)
+    , m_panelWidthChangedTimer(this, &MediaControls::panelWidthChangedTimerFired)
+    , m_panelWidth(0)
+    , m_allowHiddenAudioElements(RuntimeEnabledFeatures::newMediaPlaybackUiEnabled())
+    , m_keepMuteButton(false)
 {
 }
 
 PassRefPtrWillBeRawPtr<MediaControls> MediaControls::create(HTMLMediaElement& mediaElement)
 {
     RefPtrWillBeRawPtr<MediaControls> controls = adoptRefWillBeNoop(new MediaControls(mediaElement));
     controls->setShadowPseudoId(AtomicString("-webkit-media-controls", AtomicString::ConstructFromLiteral));
     controls->initializeControls();
     return controls.release();
 }
 
 // The media controls DOM structure looks like:
 //
 // MediaControls                                       (-webkit-media-controls)
 // +-MediaControlOverlayEnclosureElement               (-webkit-media-controls-overlay-enclosure)
 // | +-MediaControlOverlayPlayButtonElement            (-webkit-media-controls-overlay-play-button)
 // | | {if mediaControlsOverlayPlayButtonEnabled}
 // | \-MediaControlCastButtonElement                   (-internal-media-controls-overlay-cast-button)
 // \-MediaControlPanelEnclosureElement                 (-webkit-media-controls-enclosure)
 //   \-MediaControlPanelElement                        (-webkit-media-controls-panel)
 //     +-MediaControlPlayButtonElement                 (-webkit-media-controls-play-button)
+//     | {if !RTE::newMediaPlaybackUi()}
 //     +-MediaControlTimelineElement                   (-webkit-media-controls-timeline)
 //     +-MediaControlCurrentTimeDisplayElement         (-webkit-media-controls-current-time-display)
 //     +-MediaControlTimeRemainingDisplayElement       (-webkit-media-controls-time-remaining-display)
+//     | {if RTE::newMediaPlaybackUi()}
+//     +-MediaControlTimelineElement                   (-webkit-media-controls-timeline)
 //     +-MediaControlMuteButtonElement                 (-webkit-media-controls-mute-button)
 //     +-MediaControlVolumeSliderElement               (-webkit-media-controls-volume-slider)
 //     +-MediaControlToggleClosedCaptionsButtonElement (-webkit-media-controls-toggle-closed-captions-button)
 //     +-MediaControlCastButtonElement                 (-internal-media-controls-cast-button)
 //     \-MediaControlFullscreenButtonElement           (-webkit-media-controls-fullscreen-button)
 void MediaControls::initializeControls()
 {
+    const bool useNewUi = RuntimeEnabledFeatures::newMediaPlaybackUiEnabled();
     RefPtrWillBeRawPtr<MediaControlOverlayEnclosureElement> overlayEnclosure = MediaControlOverlayEnclosureElement::create(*this);
 
     if (document().settings() && document().settings()->mediaControlsOverlayPlayButtonEnabled()) {
         RefPtrWillBeRawPtr<MediaControlOverlayPlayButtonElement> overlayPlayButton = MediaControlOverlayPlayButtonElement::create(*this);
         m_overlayPlayButton = overlayPlayButton.get();
         overlayEnclosure->appendChild(overlayPlayButton.release());
     }
 
     RefPtrWillBeRawPtr<MediaControlCastButtonElement> overlayCastButton = MediaControlCastButtonElement::create(*this, true);
     m_overlayCastButton = overlayCastButton.get();
     overlayEnclosure->appendChild(overlayCastButton.release());
 
     m_overlayEnclosure = overlayEnclosure.get();
     appendChild(overlayEnclosure.release());
 
     // Create an enclosing element for the panel so we can visually offset the controls correctly.
     RefPtrWillBeRawPtr<MediaControlPanelEnclosureElement> enclosure = MediaControlPanelEnclosureElement::create(*this);
 
     RefPtrWillBeRawPtr<MediaControlPanelElement> panel = MediaControlPanelElement::create(*this);
 
     RefPtrWillBeRawPtr<MediaControlPlayButtonElement> playButton = MediaControlPlayButtonElement::create(*this);
     m_playButton = playButton.get();
     panel->appendChild(playButton.release());
 
     RefPtrWillBeRawPtr<MediaControlTimelineElement> timeline = MediaControlTimelineElement::create(*this);
     m_timeline = timeline.get();
-    panel->appendChild(timeline.release());
+    // In old UX, timeline is before the time / duration text.
+    if (!useNewUi)
+        panel->appendChild(timeline.release());
+    // else we will attach it later.
 
     RefPtrWillBeRawPtr<MediaControlCurrentTimeDisplayElement> currentTimeDisplay = MediaControlCurrentTimeDisplayElement::create(*this);
     m_currentTimeDisplay = currentTimeDisplay.get();
-    m_currentTimeDisplay->hide();
+    m_currentTimeDisplay->setIsWanted(useNewUi);
+

[philipj_slow, 2015/07/21 12:02:21]

Revert extra blank line

[liberato (no reviews please), 2015/07/27 20:26:08]

Done.

     panel->appendChild(currentTimeDisplay.release());
 
     RefPtrWillBeRawPtr<MediaControlTimeRemainingDisplayElement> durationDisplay = MediaControlTimeRemainingDisplayElement::create(*this);
     m_durationDisplay = durationDisplay.get();
     panel->appendChild(durationDisplay.release());
 
+    // Timeline is after the time / duration text if newMediaPlaybackUiEnabled.
+    if (useNewUi)
+        panel->appendChild(timeline.release());
+
     RefPtrWillBeRawPtr<MediaControlMuteButtonElement> muteButton = MediaControlMuteButtonElement::create(*this);
     m_muteButton = muteButton.get();
     panel->appendChild(muteButton.release());
+    if (m_allowHiddenAudioElements && preferHiddenAudioElements(document()))
+        m_muteButton->setIsWanted(false);
 
     RefPtrWillBeRawPtr<MediaControlVolumeSliderElement> slider = MediaControlVolumeSliderElement::create(*this);
     m_volumeSlider = slider.get();
     panel->appendChild(slider.release());
+    if (m_allowHiddenAudioElements && preferHiddenAudioElements(document()))
+        m_volumeSlider->setIsWanted(false);
 
     RefPtrWillBeRawPtr<MediaControlToggleClosedCaptionsButtonElement> toggleClosedCaptionsButton = MediaControlToggleClosedCaptionsButtonElement::create(*this);
     m_toggleClosedCaptionsButton = toggleClosedCaptionsButton.get();
     panel->appendChild(toggleClosedCaptionsButton.release());
 
     RefPtrWillBeRawPtr<MediaControlCastButtonElement> castButton = MediaControlCastButtonElement::create(*this, false);
     m_castButton = castButton.get();
     panel->appendChild(castButton.release());
 
     RefPtrWillBeRawPtr<MediaControlFullscreenButtonElement> fullscreenButton = MediaControlFullscreenButtonElement::create(*this);
     m_fullScreenButton = fullscreenButton.get();
     panel->appendChild(fullscreenButton.release());
 
     m_panel = panel.get();
     enclosure->appendChild(panel.release());
 
     m_enclosure = enclosure.get();
     appendChild(enclosure.release());
 }
 
 void MediaControls::reset()
 {
-    double duration = mediaElement().duration();
+    const bool useNewUi = RuntimeEnabledFeatures::newMediaPlaybackUiEnabled();
+    const double duration = mediaElement().duration();
     m_durationDisplay->setInnerText(LayoutTheme::theme().formatMediaControlsTime(duration), ASSERT_NO_EXCEPTION);
     m_durationDisplay->setCurrentValue(duration);
 
+    if (useNewUi) {
+        // Show everything that we might hide.
+        // If we don't have a duration, then mark it to be hidden.  For the
+        // old UI case, want / don't want is the same as show / hide since
+        // it is never marked as not fitting.
+        if (std::isfinite(duration))
+            m_durationDisplay->setIsWanted(true);

[philipj_slow, 2015/07/21 12:02:21]

Can fold this into a single setIsWanted call.

[liberato (no reviews please), 2015/07/27 20:26:08]

done.  don't know why this wasn't one line to begin with -- i kept many existing
cases separate originally to keep the diff more readable.  easier to find
changes in old ui behavior that way.

+        else
+            m_durationDisplay->setIsWanted(false);
+        m_currentTimeDisplay->setIsWanted(true);
+        m_timeline->setIsWanted(true);
+    }
+
     updatePlayState();
 
     updateCurrentTimeDisplay();
 
     m_timeline->setDuration(duration);
     m_timeline->setPosition(mediaElement().currentTime());
 
-    if (!mediaElement().hasAudio())
-        m_volumeSlider->hide();
-    else
-        m_volumeSlider->show();
     updateVolume();
 
     refreshClosedCaptionsButtonVisibility();
 
     // Unconditionally allow the user to exit fullscreen if we are in it
     // now.  Especially on android, when we might not yet know if
     // fullscreen is supported, we sometimes guess incorrectly and show
     // the button earlier, and we don't want to remove it here if the
     // user chose to enter fullscreen.  crbug.com/500732 .
-    if ((mediaElement().hasVideo() && fullscreenIsSupported(document()))
-        || mediaElement().isFullscreen())
-        m_fullScreenButton->show();
-    else
-        m_fullScreenButton->hide();
+    m_fullScreenButton->setIsWanted((mediaElement().hasVideo()
+        && fullscreenIsSupported(document()))
+        || mediaElement().isFullscreen());
 
-    refreshCastButtonVisibility();
+    refreshCastButtonVisibilityWithoutUpdate();
     makeOpaque();
+    changedControlSelections();
 }
 
 LayoutObject* MediaControls::layoutObjectForTextTrackLayout()
 {
     return m_panel->layoutObject();
 }
 
 void MediaControls::show()
 {
     makeOpaque();
-    m_panel->show();
+    m_panel->setIsWanted(true);
     m_panel->setIsDisplayed(true);
     if (m_overlayPlayButton)
         m_overlayPlayButton->updateDisplayType();
 }
 
 void MediaControls::mediaElementFocused()
 {
     if (mediaElement().shouldShowControls()) {
         show();
         resetHideMediaControlsTimer();
     }
 }
 
 void MediaControls::hide()
 {
-    m_panel->hide();
+    m_panel->setIsWanted(false);
     m_panel->setIsDisplayed(false);
     if (m_overlayPlayButton)
-        m_overlayPlayButton->hide();
+        m_overlayPlayButton->setIsWanted(false);
 }
 
 void MediaControls::makeOpaque()
 {
     m_panel->makeOpaque();
 }
 
 void MediaControls::makeTransparent()
 {
     m_panel->makeTransparent();
+    m_overlayCastButton->setIsWanted(false);
 }
 
 bool MediaControls::shouldHideMediaControls(unsigned behaviorFlags) const
 {
     // Never hide for a media element without visual representation.
     if (!mediaElement().hasVideo() || mediaElement().isPlayingRemotely())
         return false;
     // Don't hide if the mouse is over the controls.
     const bool ignoreControlsHover = behaviorFlags & IgnoreControlsHover;
     if (!ignoreControlsHover && m_panel->hovered())
         return false;
     // Don't hide if the mouse is over the video area.
     const bool ignoreVideoHover = behaviorFlags & IgnoreVideoHover;
     if (!ignoreVideoHover && m_isMouseOverControls)
         return false;
     // Don't hide if focus is on the HTMLMediaElement or within the
     // controls/shadow tree. (Perform the checks separately to avoid going
     // through all the potential ancestor hosts for the focused element.)
     const bool ignoreFocus = behaviorFlags & IgnoreFocus;
     if (!ignoreFocus && (mediaElement().focused() || contains(document().focusedElement())))
         return false;
     return true;
 }
 
 void MediaControls::playbackStarted()
 {
-    m_currentTimeDisplay->show();
-    m_durationDisplay->hide();
+    if (!RuntimeEnabledFeatures::newMediaPlaybackUiEnabled()) {
+        m_currentTimeDisplay->setIsWanted(true);
+        m_durationDisplay->setIsWanted(false);
+    }
 
     updatePlayState();
     m_timeline->setPosition(mediaElement().currentTime());
     updateCurrentTimeDisplay();
 
     startHideMediaControlsTimer();
 }
 
 void MediaControls::playbackProgressed()
 {
@@ skipping 40 matching lines @@
             mediaElement().togglePlayState();
     }
 }
 
 void MediaControls::updateCurrentTimeDisplay()
 {
     double now = mediaElement().currentTime();
     double duration = mediaElement().duration();
 
     // After seek, hide duration display and show current time.
-    if (now > 0) {
-        m_currentTimeDisplay->show();
-        m_durationDisplay->hide();
+    if (!RuntimeEnabledFeatures::newMediaPlaybackUiEnabled() && now > 0) {
+        m_currentTimeDisplay->setIsWanted(true);
+        m_durationDisplay->setIsWanted(false);
     }
 
     // Allow the theme to format the time.
     m_currentTimeDisplay->setInnerText(LayoutTheme::theme().formatMediaControlsCurrentTime(now, duration), IGNORE_EXCEPTION);
     m_currentTimeDisplay->setCurrentValue(now);
 }
 
 void MediaControls::updateVolume()
 {
     m_muteButton->updateDisplayType();
     // Invalidate the mute button because it paints differently according to volume.
     if (LayoutObject* layoutObject = m_muteButton->layoutObject())
         layoutObject->setShouldDoFullPaintInvalidation();
 
-    if (mediaElement().muted())
+    if (mediaElement().muted()) {

[philipj_slow, 2015/07/21 12:02:21]

Drop the extra {}, presumably left as part of a now-reverted change.

[liberato (no reviews please), 2015/07/27 20:26:08]

Done.

         m_volumeSlider->setVolume(0);
-    else
+    } else {
         m_volumeSlider->setVolume(mediaElement().volume());
+    }
+
+    // Update the visibility of our audio elements.
+    // We never want the volume slider if there's no audio.
+    // If there is audio, then we want it unless hiding audio is enabled and
+    // we prefer to hide it.
+    m_volumeSlider->setIsWanted(mediaElement().hasAudio()
+        && !(m_allowHiddenAudioElements && preferHiddenAudioElements(document())));
+
+    // The mute button is a little more complicated.  If enableNewMediaPlaybackUi
+    // is true, then we choose to hide or show the mute button to save space.
+    // If enableNew* is not set, then we never touch the mute button, and
+    // instead let it up to the CSS.

[philipj_slow, 2015/07/21 12:02:21]

s/let/leave/

[liberato (no reviews please), 2015/07/27 20:26:08]

Done.

+    // Note that this is why m_allowHiddenAudioElements isn't rolled into prefer...().
+    if (m_allowHiddenAudioElements) {
+        // If there is no audio track, then hide the mute button.  If there
+        // is an audio track, then we always show the mute button unless
+        // we prefer to hide it and the media isn't muted.  If it's muted,
+        // then we show it to let the user unmute it.  In this case, we don't
+        // want to re-hide the mute button later.
+        m_keepMuteButton |= mediaElement().muted();
+        m_muteButton->setIsWanted(mediaElement().hasAudio()
+            && (!preferHiddenAudioElements(document()) || m_keepMuteButton));
+    }
+
+
     // Invalidate the volume slider because it paints differently according to volume.
     if (LayoutObject* layoutObject = m_volumeSlider->layoutObject())
         layoutObject->setShouldDoFullPaintInvalidation();
 }
 
 void MediaControls::changedClosedCaptionsVisibility()
 {
     m_toggleClosedCaptionsButton->updateDisplayType();
 }
 
 void MediaControls::refreshClosedCaptionsButtonVisibility()
 {
-    if (mediaElement().hasClosedCaptions())
-        m_toggleClosedCaptionsButton->show();
-    else
-        m_toggleClosedCaptionsButton->hide();
+    m_toggleClosedCaptionsButton->setIsWanted(mediaElement().hasClosedCaptions());
 }
 
 static Element* elementFromCenter(Element& element)
 {
     ClientRect* clientRect = element.getBoundingClientRect();
     int centerX = static_cast<int>((clientRect->left() + clientRect->right()) / 2);
     int centerY = static_cast<int>((clientRect->top() + clientRect->bottom()) / 2);
 
     return element.document().elementFromPoint(centerX , centerY);
 }
 
 void MediaControls::tryShowOverlayCastButton()
 {
     // The element needs to be shown to have its dimensions and position.
-    m_overlayCastButton->show();
-
+    m_overlayCastButton->setIsWanted(true);
     if (elementFromCenter(*m_overlayCastButton) != &mediaElement())
-        m_overlayCastButton->hide();
+        m_overlayCastButton->setIsWanted(false);
 }
 
 void MediaControls::refreshCastButtonVisibility()
 {
+    refreshCastButtonVisibilityWithoutUpdate();
+    changedControlSelections();
+}
+
+void MediaControls::refreshCastButtonVisibilityWithoutUpdate()
+{
     if (mediaElement().hasRemoteRoutes()) {
         // The reason for the autoplay test is that some pages (e.g. vimeo.com) have an autoplay background video, which
         // doesn't autoplay on Chrome for Android (we prevent it) so starts paused. In such cases we don't want to automatically
         // show the cast button, since it looks strange and is unlikely to correspond with anything the user wants to do.
         // If a user does want to cast a paused autoplay video then they can still do so by touching or clicking on the
         // video, which will cause the cast button to appear.
         if (!mediaElement().shouldShowControls() && !mediaElement().autoplay() && mediaElement().paused()) {
-            showOverlayCastButton();
+            // Note that this is a case where we add the overlay cast button
+            // without wanting the panel cast button.  We depend on the fact
+            // that computeWhichControlsFit() won't change overlay cast button
+            // visibility in the case where the cast button isn't wanted.
+            // We don't call compute...() here, but it will be called as
+            // non-cast changes (e.g., resize) occur.  If the panel button
+            // is shown, however, compute...() will take control of the
+            // overlay cast button if it needs to hide it from the panel.
+            tryShowOverlayCastButton();
+            m_castButton->setIsWanted(false);
         } else if (mediaElement().shouldShowControls()) {
-            m_overlayCastButton->hide();
-            m_castButton->show();
-            // Check that the cast button actually fits on the bar.
-            if (m_fullScreenButton->getBoundingClientRect()->right() > m_panel->getBoundingClientRect()->right()) {
-                m_castButton->hide();
+            m_overlayCastButton->setIsWanted(false);
+            m_castButton->setIsWanted(true);
+            // Check that the cast button actually fits on the bar.  For the
+            // newMediaPlaybackUiEnabled case, we let changedControlSelections()
+            // handle this.
+            if ( !RuntimeEnabledFeatures::newMediaPlaybackUiEnabled()
+                && m_fullScreenButton->getBoundingClientRect()->right() > m_panel->getBoundingClientRect()->right()) {
+                m_castButton->setIsWanted(false);
                 tryShowOverlayCastButton();
             }
         }
     } else {
-        m_castButton->hide();
-        m_overlayCastButton->hide();
+        m_castButton->setIsWanted(false);
+        m_overlayCastButton->setIsWanted(false);
     }
 }
 
 void MediaControls::showOverlayCastButton()
 {
     tryShowOverlayCastButton();
     resetHideMediaControlsTimer();
 }
 
 void MediaControls::enteredFullscreen()
@@ skipping 70 matching lines @@
     unsigned behaviorFlags = m_hideTimerBehaviorFlags | IgnoreFocus | IgnoreVideoHover;
     m_hideTimerBehaviorFlags = IgnoreNone;
 
     if (mediaElement().togglePlayStateWillPlay())
         return;
 
     if (!shouldHideMediaControls(behaviorFlags))
         return;
 
     makeTransparent();
-    m_overlayCastButton->hide();
 }
 
 void MediaControls::startHideMediaControlsTimer()
 {
     m_hideMediaControlsTimer.startOneShot(timeWithoutMouseMovementBeforeHidingMediaControls, FROM_HERE);
 }
 
 void MediaControls::stopHideMediaControlsTimer()
 {
     m_hideMediaControlsTimer.stop();
@@ skipping 10 matching lines @@
 bool MediaControls::containsRelatedTarget(Event* event)
 {
     if (!event->isMouseEvent())
         return false;
     EventTarget* relatedTarget = toMouseEvent(event)->relatedTarget();
     if (!relatedTarget)
         return false;
     return contains(relatedTarget->toNode());
 }
 
+void MediaControls::notifyPanelWidthChanged(int panelWidth)
+{
+    // Don't bother to do any work if this matches the most recent panel
+    // width, since we're called after layout.
+    // Note that this code permits a bad frame on resize, since it is
+    // run after the relayout / paint happens.  It would be great to improve
+    // this, but it would be even greater to move this code entirely to
+    // JS and fix it there.
+    if (panelWidth == 0)

[philipj_slow, 2015/07/21 12:02:21]

Repeat of earlier issue, why not update m_panelWidth first?

[liberato (no reviews please), 2015/07/27 20:26:08]

this isn't the same.  this is the callback that notfies about new width.  the
old issues was with computeWhichControlsFit(), which no longer takes panelWidth
as an argument.

[philipj_slow, 2015/07/28 15:23:49]

Oh, OK. In this case, would it break anything if m_panelWidth were first set and
then the check was done? It's nice if the state is always in sync, if it isn't
one might start to look for a reason.

[liberato (no reviews please), 2015/07/30 05:54:23]

sure, i'll set it here unconditionally and let changedControlSelections() worry
about it, since it checks too.  i don't think that the case ever happens anyway.

[philipj_slow, 2015/08/03 13:10:45]

Acknowledged.

+        return;
+
+    if (!RuntimeEnabledFeatures::newMediaPlaybackUiEnabled())
+        return;
+
+    m_panelWidth = panelWidth;
+
+    // Adjust for effective zoom.
+    if (!m_panel->layoutObject() || !m_panel->layoutObject()->style())
+        return;
+    m_panelWidth = ceil(m_panelWidth / m_panel->layoutObject()->style()->effectiveZoom());
+
+    m_panelWidthChangedTimer.startOneShot(0, FROM_HERE);
+}
+
+void MediaControls::changedControlSelections()
+{
+    int panelWidth = m_panel->clientWidth();

[philipj_slow, 2015/07/21 12:02:21]

When is this needed, is it not redundant with notifyPanelWidthChanged?

It seems like what ought to happen is that any time a control changes its
isWanted state, computeWhichControlsFit() should be called, right? This is
currently called in reset(), but there ought to be many cases where a control is
removed without recomputing. An easy case to test with is the CC button, where
removing all tracks should cause it to go away without a reset() call.

[liberato (no reviews please), 2015/07/27 20:26:08]

when visibility changes, one should call changedControlSelections().  It grabs
the clientWidth to update m_panelWidth.  especially during initial construction,
this is helpful.

letting the panel width show up via notifyPanelWidthChanged() caused an extra
bad frame, unfortunately.

i'll verify why it no longer calls this more often -- it did before, and i
removed it.

... not sure.  Added calls more often, and included a simple batching mechanism
so that only the outermost call actually calls changedControlSelections(). 
there were a few routines that were called both as top-level entry points and as
part of some other top-level entry.  factoring into top-level and internal calls
wouldn't have helped, since some setIsWanted() calls are conditional.

[philipj_slow, 2015/07/28 15:23:49]

Hmm, so it seems that if this helps avoid a bad frame, it's because
clientWidth() is forcing a layout instead of waiting for the callback from
layout. The new documentation of changedControlSelections() is helpful, but can
calling setIsWanted() ever actually change the size of the controls (I think no)
or does this forced layout help us learn the size a frame earlier in some
scenario?

Put differently, when would an ASSERT(m_panelWidth == m_panel->clientWidth()) at
the top of this function fail?

An explicit document().updateLayoutIgnorePendingStylesheets() and clearing the
timeout might help the responsibilities more clear, although so might more
documentation.

[liberato (no reviews please), 2015/07/30 05:54:23]

i removed this function.  the only case where the clientWidth() check helped
avoid a bad frame was in reset().  it just sets m_panelWidth there.  everybody
that called changedControlSelections() before now calls
computeWhichControlsFit().  it's all much clearer.

as for the assert, it would crash if called at the wrong time.  now that we only
get panelWidth from notify*() or reset, though, this isn't an issue.  i'd still
like to understand it better:
STDOUT: #CRASHED - renderer (pid 41184)
STDERR: ASSERTION FAILED: !node.needsStyleRecalc()
STDERR: ../../third_party/WebKit/Source/core/dom/Document.cpp(1692) : void
blink::assertLayoutTreeUpdated(blink::Node &)
STDERR: 1   0x10d39b0ef
blink::Document::updateLayoutTree(blink::StyleRecalcChange)
STDERR: 2   0x10d39c1ac blink::Document::updateLayout()
STDERR: 3   0x10d39c5b6
blink::Document::updateLayoutIgnorePendingStylesheets(blink::Document::RunPostLayoutTasks)
STDERR: 4   0x10d3c9f45 blink::Element::clientWidth()
STDERR: 5   0x10d65a276
blink::MediaControls::refreshClosedCaptionsButtonVisibility()
STDERR: 6   0x10d586e69 blink::HTMLMediaElement::addTextTrack(blink::TextTrack*)
STDERR: 7   0x10d5871f7
blink::HTMLMediaElement::didAddTrackElement(blink::HTMLTrackElement*)
STDERR: 8   0x10d5b9f1d
blink::HTMLTrackElement::insertedInto(blink::ContainerNode*)
STDERR: 9   0x10d37af80
blink::ContainerNode::notifyNodeInsertedInternal(blink::Node&,
WTF::Vector<WTF::RefPtr<blink::Node>, 11ul, WTF::DefaultAllocator>&)
STDERR: 10  0x10d378d9a blink::ContainerNode::notifyNodeInserted(blink::Node&,
blink::ContainerNode::ChildrenChangeSource)
STDERR: 11  0x10d3783a9
blink::ContainerNode::updateTreeAfterInsertion(blink::Node&)
STDERR: 12  0x10d377bdb
blink::ContainerNode::appendChild(WTF::PassRefPtr<blink::Node>,
blink::ExceptionState&)
STDERR: 13  0x10d3f974c blink::Node::appendChild(WTF::PassRefPtr<blink::Node>,
blink::ExceptionState&)
STDERR: 14  0x10dfd2c2a
blink::NodeV8Internal::appendChildMethodCallbackForMainWorld(v8::FunctionCallbackInfo<v8::Value>
const&)
STDERR: 15  0x10c8019a8 v8::internal::FunctionCallbackArguments::Call(void
(*)(v8::FunctionCallbackInfo<v8::Value> const&))
STDERR: 16  0x10c8321db v8::internal::MaybeHandle<v8::internal::Object>
v8::internal::HandleApiCallHelper<false>(v8::internal::Isolate*,
v8::internal::(anonymous
namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>&)
STDERR: 17  0x10c8358c4 v8::internal::Builtin_HandleApiCall(int,
v8::internal::Object**, v8::internal::Isolate*)

[philipj_slow, 2015/08/03 13:10:45]

What was the source of the appendChild() call here? The assert doesn't look
familiar, but I guess it isn't allowed to attempt to update the layout during a
tree modification, presumably that will be done after the whole modification is
done.

+    // Don't short-circuit update if the panel hasn't changed.  The fit state
+    // is a function of the controls we want, so recompute if we can.
+    if (panelWidth == 0)
+        return;
+
+    m_panelWidth = panelWidth;
+    computeWhichControlsFit();
+}
+
+void MediaControls::panelWidthChangedTimerFired(Timer<MediaControls>*)
+{
+    computeWhichControlsFit();
+}
+
+void MediaControls::computeWhichControlsFit()
+{
+    // Hide all controls that don't fit, and show the ones that do.
+    // This might be better suited for a layout, but since JS media controls
+    // won't benefit from that anwyay, we just do it here like JS will.
+    // The order, in order of decreasing droppiness:
+    // Volume, time, seek bar, cast.
+
+    if (!RuntimeEnabledFeatures::newMediaPlaybackUiEnabled())
+        return;
+
+    if (!m_panelWidth)
+        return;
+
+    // Controls that we'll hide / show, in order of decreasing priority.
+    MediaControlElement* elements[] = {
+        m_playButton.get(),
+        m_toggleClosedCaptionsButton.get(),
+        m_fullScreenButton.get(),
+        m_timeline.get(),
+        m_currentTimeDisplay.get(),
+        m_volumeSlider.get(),
+        m_castButton.get(),
+        m_muteButton.get(),
+        m_durationDisplay.get(),
+    };
+
+    int usedWidth = 0;
+    bool droppedCastButton = false;
+    for (MediaControlElement* element : elements) {
+        if (!element)
+            continue;
+
+        const int minimumWidth = element->minimumWidth();

[philipj_slow, 2015/07/21 12:02:21]

Move this inside the branch where it is used.

[liberato (no reviews please), 2015/07/27 20:26:08]

Done.

+        if (element->isWanted()) {
+            if (usedWidth + minimumWidth <= m_panelWidth) {
+                element->setDoesFit(true);
+                usedWidth += minimumWidth;
+            } else {
+                element->setDoesFit(false);
+                if (element == m_castButton.get())
+                    droppedCastButton = true;
+            }
+        }
+    }
+
+    // Special case for cast: if we want a cast button but dropped it, then
+    // show the overlay cast button instead.
+    if (m_castButton->isWanted())
+        m_overlayCastButton->setIsWanted(droppedCastButton);
+}
+
+void MediaControls::setAllowHiddenAudioElements(bool allow)
+{
+    m_allowHiddenAudioElements = allow;
+    // Clear the 'keep muted flag', for tests.
+    m_keepMuteButton = false;

[philipj_slow, 2015/07/21 12:02:21]

Do you also want it to be reset when loading a new media resource? MediaControls
so far does not have any persistent state, so that calling reset() will always
(modulo bugs) have the same effect.

[liberato (no reviews please), 2015/07/27 20:26:08]

good point, will reset in reset().

[philipj_slow, 2015/08/03 13:10:45]

Acknowledged.

+    // Update the controls visibility.
+    updateVolume();
+}
+
 DEFINE_TRACE(MediaControls)
 {
     visitor->trace(m_mediaElement);
     visitor->trace(m_panel);
     visitor->trace(m_overlayPlayButton);
     visitor->trace(m_overlayEnclosure);
     visitor->trace(m_playButton);
     visitor->trace(m_currentTimeDisplay);
     visitor->trace(m_timeline);
     visitor->trace(m_muteButton);
     visitor->trace(m_volumeSlider);
     visitor->trace(m_toggleClosedCaptionsButton);
     visitor->trace(m_fullScreenButton);
     visitor->trace(m_durationDisplay);
     visitor->trace(m_enclosure);
     visitor->trace(m_castButton);
     visitor->trace(m_overlayCastButton);
     HTMLDivElement::trace(visitor);
 }
 
 }