[Video] MediaSession API event handlers can resume background video.

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 dbbcdb2a5f44f36eca2d3e73a0eb1f310c6f99f7..af3ceb2fdeb442cb62530d80efed4ae66f91dddc 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -26,6 +26,7 @@
 
 #include "core/html/HTMLMediaElement.h"
 
+#include <limits>
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/Microtask.h"
 #include "bindings/core/v8/ScriptController.h"
@@ -35,6 +36,7 @@
 #include "core/css/MediaList.h"
 #include "core/dom/Attribute.h"
 #include "core/dom/DOMException.h"
+#include "core/dom/DocumentUserGestureToken.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/ElementVisibilityObserver.h"
 #include "core/dom/Fullscreen.h"
@@ -97,7 +99,6 @@
 #include "wtf/MathExtras.h"
 #include "wtf/PtrUtil.h"
 #include "wtf/text/CString.h"
-#include <limits>
 
 #ifndef BLINK_MEDIA_LOG
 #define BLINK_MEDIA_LOG DVLOG(3)
@@ -424,6 +425,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName,
       m_pendingActionFlags(0),
       m_lockedPendingUserGesture(false),
       m_lockedPendingUserGestureIfCrossOriginExperimentEnabled(true),
+      m_wasLockedWhenBackgrounded(false),
       m_playing(false),
       m_shouldDelayLoadEvent(false),
       m_haveFiredLoadedData(false),
@@ -2315,9 +2317,10 @@ void HTMLMediaElement::pause() {
 
   // Only buffer aggressively on a user-initiated pause. Other types of pauses
   // (which go directly to pauseInternal()) should not cause this behavior.
-  if (webMediaPlayer() && UserGestureIndicator::utilizeUserGesture())
+  if (webMediaPlayer() && UserGestureIndicator::utilizeUserGesture()) {
     webMediaPlayer()->setBufferingStrategy(
         WebMediaPlayer::BufferingStrategy::Aggressive);
+  }
 
   if (m_autoplayVisibilityObserver) {
     m_autoplayVisibilityObserver->stop();
@@ -3120,10 +3123,13 @@ void HTMLMediaElement::playbackStateChanged() {
   if (!webMediaPlayer())
     return;
 
+  // playbackStateChanged() is always a result of a user gesture.
+  UserGestureIndicator gestureIndicator(
+      DocumentUserGestureToken::create(&document()));
   if (webMediaPlayer()->paused())
-    pauseInternal();
+    pause();
   else
-    playInternal();
+    play();
 }

[mlamouri (slow - plz ping), 2017/02/20 12:00:17]

I know you already replied to this comment but this change will have a side
effect on other callers (such as WebMediaPlayerCast). Would it make sense to use
WebScopedUserGesture in WebMediaPlayerImpl?

[whywhat, 2017/02/20 17:16:04]

If I understood Dan correctly - in general, no :)
How will this affect other callers exactly?
I'm not sure the Cast notification call DelegateOnPlay/Pause. It calls
playbackStateChanged when switching back from the remote to local player to
continue local playback. I don't think it's broken.
I think ideally, instead of choosing whether to use MediaSession handler or the
renderer webmediaplayer delegate path, play/pause command should always come
through HTMLMediaElement. Doing it in this change will make it too big though :/

 
 void HTMLMediaElement::requestSeek(double time) {
@@ -3185,6 +3191,19 @@ void HTMLMediaElement::remotePlaybackStarted() {
     remotePlaybackClient()->stateChanged(WebRemotePlaybackState::Connected);
 }
 
+void HTMLMediaElement::wasPausedWhenBackgrounded() {
+  // Prevent websites from resuming elements paused when backgrounded if
+  // user gesture is required for playback.
+  m_wasLockedWhenBackgrounded = m_lockedPendingUserGesture;
+  m_lockedPendingUserGesture = computeLockedPendingUserGesture(document());
+}
+
+void HTMLMediaElement::wasResumedWhenForegrounded() {
+  // If the element was resumed when foregrounded, restore its unlocked state.
+  if (m_lockedPendingUserGesture && !m_wasLockedWhenBackgrounded)
+    m_lockedPendingUserGesture = false;
+}

[mlamouri (slow - plz ping), 2017/02/20 12:00:17]

Instead of this logic, would it make sense if the autoplay checks (I guess
isLockedPendingUserGesture()) would check whether the video is visible and
require a user gesture if it is not. Does that make sense? Obviously, we would
need to reproduce some of the checks currently in WebMediaPlayerImpl in HTMLME
but that's the direction we want to take anyway, right?

[whywhat, 2017/02/20 17:16:04]

It should only require a user gesture until it's unlocked in the background and
before it's foregrounded again. So it would require a boolean bit anyhow.
It would also need to know whether the player has to be paused when hidden -
we'd have to expose ShouldPauseWhenHidden from WMPI (as we check for audio/video
only, keyframe distance, remote, etc).

+
 bool HTMLMediaElement::hasSelectedVideoTrack() {
   DCHECK(RuntimeEnabledFeatures::backgroundVideoTrackOptimizationEnabled());
 
@@ -3209,6 +3228,10 @@ bool HTMLMediaElement::isAutoplayingMuted() {
   return !paused() && muted() && isLockedPendingUserGesture();
 }
 
+bool HTMLMediaElement::isBackgroundVideoPlaybackUnlocked() {
+  return !isLockedPendingUserGesture();
+}
+
 void HTMLMediaElement::requestReload(const WebURL& newUrl) {
   DCHECK(webMediaPlayer());
   DCHECK(!m_srcObject);
@@ -3853,7 +3876,7 @@ void HTMLMediaElement::unlockUserGesture() {
 }
 
 bool HTMLMediaElement::isGestureNeededForPlayback() const {
-  if (!m_lockedPendingUserGesture)
+  if (!isLockedPendingUserGesture())
     return false;
 
   return isGestureNeededForPlaybackIfPendingUserGestureIsLocked();