[M73 merge] Sync WMPI's play/pause during RemotePlayback

Currently, if a device starts a RemotePlayback session and an external
device plays/pauses the video (via the Android cast notification), the
WMPI that started the casting session never gets updated. This means
that the user will see the video element on their phone as "playing",
with no media time update, or they will see the local video as paused
when the cast video is clearly playing.

This CL addresses the issue by requesting Blink to play/pause the
HTMLMediaElement to match the state of the remotely playing video.
This is done by bubbling up play state changes from the
FlingingRenderer to WMPI via the OnRemotePlayStateChange interface.

To detect an external play state change, we save whether the last
command from WMPI was a play or a pause, and propagate a change whenever
we get a MediaStatus update that is of the opposite play state. The
cast device does not care if it receives a "PLAY" command while already
playing (or "PAUSE" when paused). Therefore, after calling
OnRemotePlayStateChange, FlingingRenderer does not need to treat the
commands it receives any differently than a user initiated command.

NOTE: This approach works reasonably well, but is not an ironclad
guarantee of consistency. We are fine with missing a few changes,
because all the user has to do is tap the video element once to return
to a consistent state. Being too aggressive in trying to detect play
state changes sometimes causes the phone to go in a feedback loop,
where it constantly issues commands while the UI wildly switches
between playing and pausing.

Bug: 790766, 925576
Change-Id: If230c412a6a020c88970e1c9475d8fdc97976c3b
Reviewed-on: https://chromium-review.googlesource.com/c/1413398
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#626188}(cherry picked from commit 9f575d2)
Reviewed-on: https://chromium-review.googlesource.com/c/1440205
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#25}
Cr-Branched-From: e510299-refs/heads/master@{#625896}

content/browser/media/flinging_renderer.cc

@@ -98,10 +98,13 @@ void FlingingRenderer::StartPlayingFrom(base::TimeDelta time) {
 
 void FlingingRenderer::SetPlaybackRate(double playback_rate) {
   DVLOG(2) << __func__;
-  if (playback_rate == 0)
+  if (playback_rate == 0) {
+    SetTargetPlayState(PlayState::PAUSED);
     controller_->GetMediaController()->Pause();
-  else
+  } else {
+    SetTargetPlayState(PlayState::PLAYING);
     controller_->GetMediaController()->Play();
+  }
 }
 
 void FlingingRenderer::SetVolume(float volume) {
@@ -113,8 +116,49 @@ base::TimeDelta FlingingRenderer::GetMediaTime() {
   return controller_->GetApproximateCurrentTime();
 }
 
+void FlingingRenderer::SetTargetPlayState(PlayState state) {
+  DVLOG(3) << __func__ << " : state " << static_cast<int>(state);
+  DCHECK(state == PlayState::PLAYING || state == PlayState::PAUSED);
+  reached_target_play_state_ = false;
+  target_play_state_ = state;
+}
+
 void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) {
-  // TODO(tguilbert): propagate important changes to RendererClient.
+  const auto& current_state = status.state;
+
+  if (current_state == target_play_state_)
+    reached_target_play_state_ = true;
+
+  // Because we can get a MediaStatus update at any time from the device, only
+  // handle state updates after we have reached the target state.
+  // If we do not, we can encounter the following scenario:
+  // - A user pauses the video.
+  // - While the PAUSE command is in flight, an unrelated MediaStatus with a
+  //   PLAYING state is sent from the cast device.
+  // - We call OnRemotePlaybackStateChange(PLAYING).
+  // - As the PAUSE command completes and we receive a PlayState::PAUSE, we
+  //   queue a new PLAYING.
+  // - The local device enters a tick/tock feedback loop of constantly
+  //   requesting the wrong state of PLAYING/PAUSED.
+  if (!reached_target_play_state_)
+    return;
+
+  // Ignore all non PLAYING/PAUSED states.
+  // UNKNOWN and BUFFERING states are uninteresting and can be safely ignored.
+  // STOPPED normally causes the session to teardown, and |this| is destroyed
+  // shortly after.
+  if (current_state != PlayState::PLAYING &&
+      current_state != PlayState::PAUSED) {
+    DVLOG(3) << __func__ << " : external state ignored: "
+             << static_cast<int>(current_state);
+    return;
+  }
+
+  // We previously reached a stable target PlayState, and the cast device has
+  // reached a new stable PlayState without WMPI having asked for it.
+  // Let WMPI know it should update itself.
+  if (current_state != target_play_state_)
+    client_->OnRemotePlayStateChange(current_state);
 }
 
 }  // namespace content

content/browser/media/flinging_renderer.h

@@ -54,10 +54,19 @@ class CONTENT_EXPORT FlingingRenderer : public media::Renderer,
 
  private:
   friend class FlingingRendererTest;
+  using PlayState = media::MediaStatus::State;
 
   explicit FlingingRenderer(
       std::unique_ptr<media::FlingingController> controller);
 
+  void SetTargetPlayState(PlayState state);
+
+  // The state that we expect the remotely playing video to transition into.
+  // This is used to differentiate between state changes that originated from
+  // this device versus external devices.
+  PlayState target_play_state_ = PlayState::UNKNOWN;
+  bool reached_target_play_state_ = false;
+
   media::RendererClient* client_;
 
   std::unique_ptr<media::FlingingController> controller_;

media/blink/webmediaplayer_impl.cc

@@ -2224,7 +2224,15 @@ void WebMediaPlayerImpl::OnVideoDecoderChange(const std::string& name) {
 }
 
 void WebMediaPlayerImpl::OnRemotePlayStateChange(MediaStatus::State state) {
-  // TODO(tguilbert): request play/pause appropriately.
+  DCHECK(is_flinging_);
+
+  if (state == MediaStatus::State::PLAYING && Paused()) {
+    DVLOG(1) << __func__ << " requesting PLAY.";
+    client_->RequestPlay();
+  } else if (state == MediaStatus::State::PAUSED && !Paused()) {
+    DVLOG(1) << __func__ << " requesting PAUSE.";
+    client_->RequestPause();
+  }
 }
 
 void WebMediaPlayerImpl::OnFrameHidden() {

media/mojo/clients/mojo_renderer.cc

@@ -283,7 +283,7 @@ void MojoRenderer::OnDurationChange(base::TimeDelta duration) {
 }
 
 void MojoRenderer::OnRemotePlayStateChange(media::MediaStatus::State state) {
-  DVLOG(2) << __func__ << ": state" << (int)state;
+  DVLOG(2) << __func__ << ": state [" << static_cast<int>(state) << "]";
   client_->OnRemotePlayStateChange(state);
 }