Expand suspension of idle media players to all platforms.

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 5583574608bfc2487fd82a91244f081cb07fb746..fdd840ecdb4e95dfa9ad6b75d4f2b53001b0a80a 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -106,6 +106,17 @@ void SetSinkIdOnMediaThread(scoped_refptr<WebAudioSourceProviderImpl> sink,
   }
 }
 
+bool IsBackgroundSuspendResumeEnabled() {
+#if !defined(OS_ANDROID)
+  // Suspend/Resume is only enabled by default on Android.
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableMediaSuspend);
+#else
+  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDisableMediaSuspend);
+#endif
+}
+
 }  // namespace
 
 class BufferedDataSourceHostImpl;
@@ -188,6 +199,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       volume_(1.0),
       volume_multiplier_(1.0),
       renderer_factory_(std::move(renderer_factory)),
+      background_playback_state_(
+          BackgroundPlaybackState::PLAY_FORBIDDEN_NEVER_CALLED_IN_FOREGROUND),
       surface_manager_(params.surface_manager()),
       suppress_destruction_errors_(false) {
   DCHECK(!adjust_allocated_memory_cb_.is_null());
@@ -340,6 +353,7 @@ void WebMediaPlayerImpl::play() {
   }
 #endif
 
+  const bool was_paused = paused_;
   paused_ = false;
   pipeline_.SetPlaybackRate(playback_rate_);
 
@@ -348,13 +362,17 @@ void WebMediaPlayerImpl::play() {
 
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PLAY));
 
-  if (playback_rate_ > 0) {
-    NotifyPlaybackStarted();
+  if (background_playback_state_ ==
+          BackgroundPlaybackState::PLAY_FORBIDDEN_NEVER_CALLED_IN_FOREGROUND &&

[sandersd (OOO until July 31), 2016/03/07 23:27:51]

The existence of this check isn't immediately necessary from the names, which
probably means that the enum names need work. (It's still true in this case that
play() was never called in the foreground, so why would it change?)

I'm no better at naming than you are, though.

[DaleCurtis, 2016/03/09 02:56:14]

Removed.

+      delegate_ && !delegate_->IsHidden()) {
+    background_playback_state_ =
+        hasVideo() ? BackgroundPlaybackState::PLAY_FORBIDDEN_WHEN_SUSPENDED
+                   : BackgroundPlaybackState::PLAY_ALLOWED_WHEN_SUSPENDED;

[mlamouri (slow - plz ping), 2016/03/05 22:37:36]

Should this include IsBackgroundSuspendResumeEnabled()?

[DaleCurtis, 2016/03/09 02:56:13]

No, as indicated below. I've also renamed it to IsSuspendUponHiddenEnabled() for
clarity.

+  }
 
-    // Resume the player if playback was initiated in the foreground. Resume()
-    // will do nothing if the pipeline is not suspended state, but will clear
-    // some internal pending state, so it should always be called.
-    if (delegate_ && !delegate_->IsHidden())
+  if (playback_rate_ > 0 && was_paused) {
+    NotifyPlaybackStarted();

[sandersd (OOO until July 31), 2016/03/07 23:27:51]

Add a comment here that this method won't do anything when suspended. I get the
feeling it's not obvious due to the name, but I'm not going to ask for a rename
in this CL.

+    if (IsResumeAllowed())
       pipeline_controller_.Resume();
   }
 }
@@ -443,10 +461,7 @@ void WebMediaPlayerImpl::DoSeek(base::TimeDelta time, bool time_updated) {
     paused_time_ = time;
   pipeline_controller_.Seek(time, time_updated);
 
-  // Resume the pipeline if the seek is initiated in the foreground so that
-  // the correct frame is displayed. If the pipeline is not suspended, Resume()
-  // will do nothing but clear some pending state.
-  if (delegate_ && !delegate_->IsHidden())
+  if (IsResumeAllowed())
     pipeline_controller_.Resume();
 }
 
@@ -857,8 +872,6 @@ void WebMediaPlayerImpl::OnPipelineSuspended() {
   }
 #endif
 
-  if (delegate_)
-    delegate_->PlayerGone(delegate_id_);
   memory_usage_reporting_timer_.Stop();
   ReportMemoryUsage();
 
@@ -935,7 +948,7 @@ void WebMediaPlayerImpl::OnPipelineMetadata(
     // If there is video and the frame is hidden, then it may be time to suspend
     // playback.
     if (delegate_ && delegate_->IsHidden())
-      OnHidden(false);
+      OnHidden();
   }
 }
 
@@ -996,49 +1009,37 @@ void WebMediaPlayerImpl::OnAddTextTrack(
   done_cb.Run(std::move(text_track));
 }
 
-void WebMediaPlayerImpl::OnHidden(bool must_suspend) {
+void WebMediaPlayerImpl::OnHidden() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
-
-#if !defined(OS_ANDROID)
-  // Suspend/Resume is enabled by default on Android.
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableMediaSuspend)) {
+  if (!IsBackgroundSuspendResumeEnabled())
     return;
-  }
-#endif  // !defined(OS_ANDROID)
-
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableMediaSuspend)) {
-    return;
-  }
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID)  // WMPI_CAST
   // If we're remote, the pipeline should already be suspended.
   if (isRemote())
     return;
 #endif
 
-  if (must_suspend || (paused_ && ended_) || hasVideo())
-    pipeline_controller_.Suspend();
+  // Don't suspend players which started playback in the foreground, only have
+  // audio, and have not completed playback.  The user can still control these
+  // players via the MediaSession UI.
+  if (background_playback_state_ >
+          BackgroundPlaybackState::PLAY_FORBIDDEN_NEVER_CALLED_IN_FOREGROUND &&
+      !hasVideo() && !ended_) {
+    return;
+  }
+
+  pipeline_controller_.Suspend();
+  if (delegate_)
+    delegate_->PlayerGone(delegate_id_);
 }
 
 void WebMediaPlayerImpl::OnShown() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
-
-#if !defined(OS_ANDROID)
-  // Suspend/Resume is enabled by default on Android.
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableMediaSuspend)) {
+  if (!IsBackgroundSuspendResumeEnabled())
     return;
-  }
-#endif  // !defined(OS_ANDROID)
 
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableMediaSuspend)) {
-    return;
-  }
-
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID)  // WMPI_CAST
   // If we're remote, the pipeline should stay suspended.
   if (isRemote())
     return;
@@ -1048,6 +1049,29 @@ void WebMediaPlayerImpl::OnShown() {
     pipeline_controller_.Resume();
 }
 
+void WebMediaPlayerImpl::OnSuspend(bool must_suspend) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+#if defined(OS_ANDROID)  // WMPI_CAST
+  // If we're remote, the pipeline should already be suspended.
+  if (isRemote())
+    return;
+#endif
+
+  // Suspend should never be sent unless always required or we're already in a
+  // paused or ended state.
+  DCHECK(must_suspend || paused_ || ended_);
+
+  NotifyPlaybackPaused();
+
+  // Always suspend, but only notify the delegate if we must; this allows any
+  // exposed UI for player controls to continue to function even though the
+  // player has now been suspended.
+  pipeline_controller_.Suspend();
+  if (must_suspend && delegate_)
+    delegate_->PlayerGone(delegate_id_);
+}
+
 void WebMediaPlayerImpl::OnPlay() {
   play();
   client_->playbackStateChanged();
@@ -1424,4 +1448,15 @@ void WebMediaPlayerImpl::FinishMemoryUsageReport(int64_t demuxer_memory_usage) {
   adjust_allocated_memory_cb_.Run(delta);
 }
 
+bool WebMediaPlayerImpl::IsResumeAllowed() {
+#if defined(OS_ANDROID)
+  return background_playback_state_ ==
+             BackgroundPlaybackState::PLAY_ALLOWED_WHEN_SUSPENDED ||
+         (delegate_ && !delegate_->IsHidden());

[mlamouri (slow - plz ping), 2016/03/05 22:37:36]

Should this include IsBackgroundSuspendResumeEnabled()?

[DaleCurtis, 2016/03/07 19:08:06]

No, I tried to make it clear that IsBackgroundSuspendResumeEnabled() is _only_
for enabling automatic suspension (with clearing of MediaSession) on
backgrounding. I.e., it's not for suspending idle players as the opportunity
arises. Let me know if there's any way I can make this clearer. Thanks!

+#else
+  // On non-Android platforms Resume() is always allowed.
+  return true;
+#endif
+}
+
 }  // namespace media