Disable background video track behind a feature flag

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

Index: third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
index 8da456fa48e3c9d72688c3ec15b23a0fa74c0947..abc9014d601eea6f90f66a70cfd48d0c9a4c9d72 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -2553,7 +2553,6 @@ void HTMLMediaElement::togglePlayState() {
 }
 
 AudioTrackList& HTMLMediaElement::audioTracks() {
-  DCHECK(RuntimeEnabledFeatures::audioVideoTracksEnabled());

[mlamouri (slow - plz ping), 2016/11/12 23:35:48]

Here and below, did you have to remove the checks? When is it called? At least,
in the case of audio, it sounds odd that it would be called.

[whywhat, 2016/11/14 21:50:12]

When the Media Tracks API is enabled, HTMLMediaElement creates default
placeholder tracks for the media so it's consistent and I want to create a
default video track to be able to unselect it. I'm not sure if creating just the
video track and ignoring the audio one would work and it would probably make the
existing code that creates the default tracks more complex.

See createPlaceholderTracksIfNecessary below.

   return *m_audioTracks;
 }
 
@@ -2595,9 +2594,6 @@ WebMediaPlayer::TrackId HTMLMediaElement::addAudioTrack(
                   << (String)label << "', '" << (String)language << "', "
                   << boolString(enabled) << ")";
 
-  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled())
-    return blink::WebMediaPlayer::TrackId();
-
   AudioTrack* audioTrack =
       AudioTrack::create(id, kindString, label, language, enabled);
   audioTracks().add(audioTrack);
@@ -2608,14 +2604,10 @@ WebMediaPlayer::TrackId HTMLMediaElement::addAudioTrack(
 void HTMLMediaElement::removeAudioTrack(WebMediaPlayer::TrackId trackId) {
   BLINK_MEDIA_LOG << "removeAudioTrack(" << (void*)this << ")";
 
-  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled())
-    return;
-
   audioTracks().remove(trackId);
 }
 
 VideoTrackList& HTMLMediaElement::videoTracks() {
-  DCHECK(RuntimeEnabledFeatures::audioVideoTracksEnabled());
   return *m_videoTracks;
 }
 
@@ -2650,9 +2642,6 @@ WebMediaPlayer::TrackId HTMLMediaElement::addVideoTrack(
                   << (String)label << "', '" << (String)language << "', "
                   << boolString(selected) << ")";
 
-  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled())
-    return blink::WebMediaPlayer::TrackId();
-
   // If another track was selected (potentially by the user), leave it selected.
   if (selected && videoTracks().selectedIndex() != -1)
     selected = false;
@@ -2667,9 +2656,6 @@ WebMediaPlayer::TrackId HTMLMediaElement::addVideoTrack(
 void HTMLMediaElement::removeVideoTrack(WebMediaPlayer::TrackId trackId) {
   BLINK_MEDIA_LOG << "removeVideoTrack(" << (void*)this << ")";
 
-  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled())
-    return;
-
   videoTracks().remove(trackId);
 }
 
@@ -3195,6 +3181,26 @@ void HTMLMediaElement::remotePlaybackStarted() {
     remotePlaybackClient()->stateChanged(WebRemotePlaybackState::Connected);
 }
 
+void HTMLMediaElement::hidden() {
+  if (!RuntimeEnabledFeatures::backgroundVideoTrackOptimizationEnabled())
+    return;
+
+  webMediaPlayer()->selectedVideoTrackChanged(nullptr);

[mlamouri (slow - plz ping), 2016/11/12 23:35:48]

Here and below, you go from WMPI to HTMLMediaElement to then go back to WMPI.
Any reason why not implementing this directly in WMPI?

[whywhat, 2016/11/14 21:50:13]

1. I can restore the previously selected video track easily as HTMLMediaElement
has access to those.
2. It works with both WMPI and WMPA (I assume Sergey's implementation works with
both).
3. In WMPI I'd have to change updatePlayState_computePlayState to react to the
backgrounded state change while also playing and so on... It doesn't really deal
with selected tracks at all right now.

[sandersd (OOO until July 31), 2016/11/15 00:09:11]

This also looks strange from the WMPI side (which independently checks
IsHidden() to decide what to do). At the very least more comments are necessary
to explain those choices.

[whywhat, 2016/11/15 03:08:56]

More thoughts:

- WMPA doesn't implement selectedVideoTrackChanged and probably won't so my
argument 2 doesn't apply.

- I check IsHidden() in WMPI::selectedVideoTrackChanged because I didn't want to
add isHidden() to WMP interface or keep a m_isHidden boolean in
HTMLMediaElement. Perhaps the former would keep the code in HTMLMediaElement.

- If I were to implement the whole change in WMPI, I'd have to keep track of the
unselected video track id there which would not keep WMPI stateless. Maybe I
could add a method to WMPClient to get the current video track id?

WDYT?

+}
+
+void HTMLMediaElement::shown() {
+  if (!RuntimeEnabledFeatures::backgroundVideoTrackOptimizationEnabled())
+    return;
+
+  int selectedTrackIndex = m_videoTracks->selectedIndex();
+  if (selectedTrackIndex == -1)
+    return;
+
+  VideoTrack* track = m_videoTracks->anonymousIndexedGetter(selectedTrackIndex);
+  WebMediaPlayer::TrackId id = track->id();
+  webMediaPlayer()->selectedVideoTrackChanged(&id);
+}
+
 bool HTMLMediaElement::isAutoplayingMuted() {
   if (!isHTMLVideoElement() ||
       !RuntimeEnabledFeatures::autoplayMutedVideosEnabled()) {
@@ -3852,8 +3858,10 @@ DEFINE_TRACE_WRAPPERS(HTMLMediaElement) {
 }
 
 void HTMLMediaElement::createPlaceholderTracksIfNecessary() {
-  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled())
+  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled() &&
+      !RuntimeEnabledFeatures::backgroundVideoTrackOptimizationEnabled()) {
     return;
+  }
 
   // Create a placeholder audio track if the player says it has audio but it
   // didn't explicitly announce the tracks.
@@ -3869,8 +3877,10 @@ void HTMLMediaElement::createPlaceholderTracksIfNecessary() {
 }
 
 void HTMLMediaElement::selectInitialTracksIfNecessary() {
-  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled())
+  if (!RuntimeEnabledFeatures::audioVideoTracksEnabled() &&
+      !RuntimeEnabledFeatures::backgroundVideoTrackOptimizationEnabled()) {
     return;
+  }
 
   // Enable the first audio track if an audio track hasn't been enabled yet.
   if (audioTracks().length() > 0 && !audioTracks().hasEnabledTrack())