Media Remoting: Don't auto suspend the media pipeline.

Disable the optimization to auto suspend media pipeline for background video or
idle pipeline in media remoting to prevent the end of the session while still
in remoting.

BUG=691673

Review-Url: https://codereview.chromium.org/2696663002
Cr-Commit-Position: refs/heads/master@{#450805}

content/renderer/render_frame_impl.cc

@@ -2912,11 +2912,6 @@ blink::WebMediaPlayer* RenderFrameImpl::createMediaPlayer(
 #endif  // defined(OS_ANDROID)
 
 #if BUILDFLAG(ENABLE_MEDIA_REMOTING)
-  remoting_controller_ptr->SetSwitchRendererCallback(base::Bind(
-      &media::WebMediaPlayerImpl::ScheduleRestart, media_player->AsWeakPtr()));
-  remoting_controller_ptr->SetRemoteSinkAvailableChangedCallback(base::Bind(
-      &media::WebMediaPlayerImpl::ActivateViewportIntersectionMonitoring,
-      media_player->AsWeakPtr()));
   remoting_controller_ptr->SetDownloadPosterCallback(base::Bind(
       &SingleImageDownloader::DownloadImage, weak_factory_.GetWeakPtr()));
 #endif

media/base/media_observer.h

@@ -11,6 +11,19 @@
 
 namespace media {
 
+class MEDIA_EXPORT MediaObserverClient {
+ public:
+  virtual ~MediaObserverClient() {}
+
+  // Requests to restart the media pipeline and create a new renderer as soon as
+  // possible. |disable_pipeline_auto_suspend| indicates whether to disable
+  // any optimizations that might suspend the media pipeline.
+  virtual void SwitchRenderer(bool disable_pipeline_auto_suspend) = 0;
+
+  // Requests to activate monitoring changes on viewport intersection.
+  virtual void ActivateViewportIntersectionMonitoring(bool activate) = 0;
+};
+
 // This class is an observer of media player events.
 class MEDIA_EXPORT MediaObserver {
  public:
@@ -45,6 +58,9 @@ class MEDIA_EXPORT MediaObserver {
   // Called when a poster image URL is set, which happens when media is loaded
   // or the poster attribute is changed.
   virtual void OnSetPoster(const GURL& poster) = 0;
+
+  // Set the MediaObserverClient.
+  virtual void SetClient(MediaObserverClient* client) = 0;
 };
 
 }  // namespace media

media/blink/webmediaplayer_impl.cc

@@ -267,6 +267,9 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
   // e.g. GetCurrentFrameFromCompositor(). See http://crbug.com/434861
   audio_source_provider_ =
       new WebAudioSourceProviderImpl(params.audio_renderer_sink(), media_log_);
+
+  if (observer_)
+    observer_->SetClient(this);
 }
 
 WebMediaPlayerImpl::~WebMediaPlayerImpl() {
@@ -1839,16 +1842,16 @@ void WebMediaPlayerImpl::UpdatePlayState() {
 
 #if defined(OS_ANDROID)  // WMPI_CAST
   bool is_remote = isRemote();
-  bool is_streaming = false;
+  bool can_auto_suspend = true;
 #else
   bool is_remote = false;
-  bool is_streaming = IsStreaming();
+  bool can_auto_suspend = !disable_pipeline_auto_suspend_ && !IsStreaming();
 #endif
 
   bool is_suspended = pipeline_controller_.IsSuspended();
   bool is_backgrounded = IsBackgroundedSuspendEnabled() && IsHidden();
   PlayState state = UpdatePlayState_ComputePlayState(
-      is_remote, is_streaming, is_suspended, is_backgrounded);
+      is_remote, can_auto_suspend, is_suspended, is_backgrounded);
   SetDelegateState(state.delegate_state, state.is_idle);
   SetMemoryReportingState(state.is_memory_reporting_enabled);
   SetSuspendState(state.is_suspended || pending_suspend_resume_cycle_);
@@ -1939,7 +1942,7 @@ void WebMediaPlayerImpl::SetSuspendState(bool is_suspended) {
 
 WebMediaPlayerImpl::PlayState
 WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
-                                                     bool is_streaming,
+                                                     bool can_auto_suspend,
                                                      bool is_suspended,
                                                      bool is_backgrounded) {
   PlayState result;
@@ -1967,10 +1970,10 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
   bool can_play_backgrounded = is_backgrounded_video && !is_remote &&
                                hasAudio() && IsResumeBackgroundVideosEnabled();
   bool is_background_playing = delegate_->IsBackgroundVideoPlaybackUnlocked();
-  bool background_suspended = !is_streaming && is_backgrounded_video &&
+  bool background_suspended = can_auto_suspend && is_backgrounded_video &&
                               !(can_play_backgrounded && is_background_playing);
   bool background_pause_suspended =
-      !is_streaming && is_backgrounded && paused_ && have_future_data;
+      can_auto_suspend && is_backgrounded && paused_ && have_future_data;
 
   // Idle suspension is allowed prior to have future data since there exist
   // mechanisms to exit the idle state when the player is capable of reaching
@@ -1979,7 +1982,7 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
   // TODO(sandersd): Make the delegate suspend idle players immediately when
   // hidden.
   bool idle_suspended =
-      !is_streaming && is_stale && paused_ && !seeking_ && !overlay_enabled_;
+      can_auto_suspend && is_stale && paused_ && !seeking_ && !overlay_enabled_;
 
   // If we're already suspended, see if we can wait for user interaction. Prior
   // to HaveFutureData, we require |is_stale| to remain suspended. |is_stale|
@@ -2291,4 +2294,10 @@ void WebMediaPlayerImpl::ReportTimeFromForegroundToFirstFrame(
   }
 }
 
+void WebMediaPlayerImpl::SwitchRenderer(bool disable_pipeline_auto_suspend) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  disable_pipeline_auto_suspend_ = disable_pipeline_auto_suspend;
+  ScheduleRestart();
+}
+
 }  // namespace media

media/blink/webmediaplayer_impl.h

@@ -88,6 +88,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
     : public NON_EXPORTED_BASE(blink::WebMediaPlayer),
       public NON_EXPORTED_BASE(WebMediaPlayerDelegate::Observer),
       public NON_EXPORTED_BASE(Pipeline::Client),
+      public MediaObserverClient,
       public base::SupportsWeakPtr<WebMediaPlayerImpl> {
  public:
   // Constructs a WebMediaPlayer implementation using Chromium's media stack.
@@ -214,19 +215,14 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   void SetUseFallbackPath(bool use_fallback_path);
 #endif
 
+  // MediaObserverClient implementation.
+  void SwitchRenderer(bool disable_pipeline_auto_suspend) override;
+  void ActivateViewportIntersectionMonitoring(bool activate) override;
+
   // Called from WebMediaPlayerCast.
   // TODO(hubbe): WMPI_CAST make private.
   void OnPipelineSeeked(bool time_updated);
 
-  // Restart the player/pipeline as soon as possible. This will destroy the
-  // current renderer, if any, and create a new one via the RendererFactory; and
-  // then seek to resume playback at the current position.
-  void ScheduleRestart();
-
-  // Called when requests to activate monitoring changes on viewport
-  // intersection.
-  void ActivateViewportIntersectionMonitoring(bool activate);
-
   // Distinct states that |delegate_| can be in. (Public for testing.)
   enum class DelegateState {
     GONE,
@@ -297,6 +293,11 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // Finishes starting the pipeline due to a call to load().
   void StartPipeline();
 
+  // Restart the player/pipeline as soon as possible. This will destroy the
+  // current renderer, if any, and create a new one via the RendererFactory; and
+  // then seek to resume playback at the current position.
+  void ScheduleRestart();
+
   // Helpers that set the network/ready state and notifies the client if
   // they've changed.
   void SetNetworkState(blink::WebMediaPlayer::NetworkState state);
@@ -340,7 +341,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
 
   // Methods internal to UpdatePlayState().
   PlayState UpdatePlayState_ComputePlayState(bool is_remote,
-                                             bool is_streaming,
+                                             bool can_auto_suspend,
                                              bool is_suspended,
                                              bool is_backgrounded);
   void SetDelegateState(DelegateState new_state, bool is_idle);
@@ -690,6 +691,9 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // Whether the pipeline is being resumed at the moment.
   bool is_pipeline_resuming_ = false;
 
+  // When this is true, pipeline will not be auto suspended.
+  bool disable_pipeline_auto_suspend_ = false;
+
   // Pipeline statistics overridden by tests.
   base::Optional<PipelineStatistics> pipeline_statistics_for_test_;
 

media/blink/webmediaplayer_impl_unittest.cc

@@ -252,23 +252,23 @@ class WebMediaPlayerImplTest : public testing::Test {
   }
 
   WebMediaPlayerImpl::PlayState ComputePlayState() {
-    return wmpi_->UpdatePlayState_ComputePlayState(false, false, false, false);
+    return wmpi_->UpdatePlayState_ComputePlayState(false, true, false, false);
   }
 
   WebMediaPlayerImpl::PlayState ComputePlayState_FrameHidden() {
-    return wmpi_->UpdatePlayState_ComputePlayState(false, false, false, true);
+    return wmpi_->UpdatePlayState_ComputePlayState(false, true, false, true);
   }
 
   WebMediaPlayerImpl::PlayState ComputePlayState_Suspended() {
-    return wmpi_->UpdatePlayState_ComputePlayState(false, false, true, false);
+    return wmpi_->UpdatePlayState_ComputePlayState(false, true, true, false);
   }
 
   WebMediaPlayerImpl::PlayState ComputePlayState_Remote() {
-    return wmpi_->UpdatePlayState_ComputePlayState(true, false, false, false);
+    return wmpi_->UpdatePlayState_ComputePlayState(true, true, false, false);
   }
 
   WebMediaPlayerImpl::PlayState ComputePlayState_BackgroundedStreaming() {
-    return wmpi_->UpdatePlayState_ComputePlayState(false, true, false, true);
+    return wmpi_->UpdatePlayState_ComputePlayState(false, false, false, true);
   }
 
   bool IsSuspended() { return wmpi_->pipeline_controller_.IsSuspended(); }

media/remoting/renderer_controller.cc

@@ -32,8 +32,8 @@ void RendererController::OnStarted(bool success) {
     VLOG(1) << "Remoting started successively.";
     if (remote_rendering_started_) {
       metrics_recorder_.DidStartSession();
-      DCHECK(!switch_renderer_cb_.is_null());
-      switch_renderer_cb_.Run();
+      DCHECK(client_);
+      client_->SwitchRenderer(true);
     } else {
       session_->StopRemoting(this);
     }
@@ -52,8 +52,8 @@ void RendererController::OnSessionStateChanged() {
 void RendererController::UpdateFromSessionState(StartTrigger start_trigger,
                                                 StopTrigger stop_trigger) {
   VLOG(1) << "UpdateFromSessionState: " << session_->state();
-  if (!sink_available_changed_cb_.is_null())
-    sink_available_changed_cb_.Run(IsRemoteSinkAvailable());
+  if (client_)
+    client_->ActivateViewportIntersectionMonitoring(IsRemoteSinkAvailable());
 
   UpdateInterstitial(base::nullopt);
   UpdateAndMaybeSwitch(start_trigger, stop_trigger);
@@ -151,26 +151,6 @@ void RendererController::OnSetPoster(const GURL& poster_url) {
   }
 }
 
-void RendererController::SetSwitchRendererCallback(const base::Closure& cb) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!cb.is_null());
-
-  switch_renderer_cb_ = cb;
-  // Note: Not calling UpdateAndMaybeSwitch() here since this method should be
-  // called during the initialization phase of this RendererController;
-  // and definitely before a whole lot of other things that are needed to
-  // trigger a switch.
-}
-
-void RendererController::SetRemoteSinkAvailableChangedCallback(
-    const base::Callback<void(bool)>& cb) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-
-  sink_available_changed_cb_ = cb;
-  if (!sink_available_changed_cb_.is_null())
-    sink_available_changed_cb_.Run(IsRemoteSinkAvailable());
-}
-
 base::WeakPtr<RpcBroker> RendererController::GetRpcBroker() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
@@ -286,7 +266,7 @@ void RendererController::OnPaused() {
 bool RendererController::ShouldBeRemoting() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (switch_renderer_cb_.is_null()) {
+  if (!client_) {
     DCHECK(!remote_rendering_started_);
     return false;  // No way to switch to the remoting renderer.
   }
@@ -365,16 +345,16 @@ void RendererController::UpdateAndMaybeSwitch(StartTrigger start_trigger,
   // Switch between local renderer and remoting renderer.
   remote_rendering_started_ = should_be_remoting;
 
+  DCHECK(client_);
   if (remote_rendering_started_) {
-    DCHECK(!switch_renderer_cb_.is_null());
     if (session_->state() == SharedSession::SESSION_PERMANENTLY_STOPPED) {
-      switch_renderer_cb_.Run();
+      client_->SwitchRenderer(true);
       return;
     }
     DCHECK_NE(start_trigger, UNKNOWN_START_TRIGGER);
     metrics_recorder_.WillStartSession(start_trigger);
-    // |switch_renderer_cb_.Run()| will be called after remoting is started
-    // successfully.
+    // |MediaObserverClient::SwitchRenderer()| will be called after remoting is
+    // started successfully.
     session_->StartRemoting(this);
   } else {
     // For encrypted content, it's only valid to switch to remoting renderer,
@@ -387,7 +367,7 @@ void RendererController::UpdateAndMaybeSwitch(StartTrigger start_trigger,
     // Update the interstitial one last time before switching back to the local
     // Renderer.
     UpdateInterstitial(base::nullopt);
-    switch_renderer_cb_.Run();
+    client_->SwitchRenderer(false);
     session_->StopRemoting(this);
   }
 }
@@ -489,5 +469,14 @@ void RendererController::OnRendererFatalError(StopTrigger stop_trigger) {
   UpdateAndMaybeSwitch(UNKNOWN_START_TRIGGER, stop_trigger);
 }
 
+void RendererController::SetClient(MediaObserverClient* client) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(client);
+  DCHECK(!client_);
+
+  client_ = client;
+  client_->ActivateViewportIntersectionMonitoring(IsRemoteSinkAvailable());
+}
+
 }  // namespace remoting
 }  // namespace media

media/remoting/renderer_controller.h

@@ -44,10 +44,7 @@ class RendererController final : public SharedSession::Client,
   void OnPlaying() override;
   void OnPaused() override;
   void OnSetPoster(const GURL& poster) override;
-
-  void SetSwitchRendererCallback(const base::Closure& cb);
-  void SetRemoteSinkAvailableChangedCallback(
-      const base::Callback<void(bool)>& cb);
+  void SetClient(MediaObserverClient* client) override;
 
   using ShowInterstitialCallback = base::Callback<
       void(const SkBitmap&, const gfx::Size&, InterstitialType type)>;
@@ -172,12 +169,6 @@ class RendererController final : public SharedSession::Client,
   // and never start again for the lifetime of this controller.
   bool encountered_renderer_fatal_error_ = false;
 
-  // The callback to switch the media renderer.
-  base::Closure switch_renderer_cb_;
-
-  // Called when remoting sink availability is changed.
-  base::Callback<void(bool)> sink_available_changed_cb_;
-
   // This is initially the SharedSession passed to the ctor, and might be
   // replaced with a different instance later if OnSetCdm() is called.
   scoped_refptr<SharedSession> session_;
@@ -213,6 +204,9 @@ class RendererController final : public SharedSession::Client,
   // Records session events of interest.
   SessionMetricsRecorder metrics_recorder_;
 
+  // Not own by this class. Can only be set once by calling SetClient().
+  MediaObserverClient* client_ = nullptr;
+
   base::WeakPtrFactory<RendererController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RendererController);