Refactor WebMediaPlayerDelegate interface.

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 131 matching lines @@
   return natural_size;
 }
 
 base::TimeDelta GetCurrentTimeInternal(WebMediaPlayerImpl* p_this) {
   // We wrap currentTime() instead of using pipeline_.GetMediaTime() since there
   // are a variety of cases in which that time is not accurate; e.g., while
   // remoting and during a pause or seek.
   return base::TimeDelta::FromSecondsD(p_this->currentTime());
 }
 
-// How much time must have elapsed since loading last progressed before the
-// player is eligible for idle suspension.
-constexpr base::TimeDelta kLoadingToIdleTimeout =
+// How much time must have elapsed since loading last progressed before we
+// assume that the decoder will have had time to complete preroll.
+constexpr base::TimeDelta kPrerollAttemptTimeout =
     base::TimeDelta::FromSeconds(3);
 
 }  // namespace
 
 class BufferedDataSourceHostImpl;
 
 STATIC_ASSERT_ENUM(WebMediaPlayer::CORSModeUnspecified,
                    UrlData::CORS_UNSPECIFIED);
 STATIC_ASSERT_ENUM(WebMediaPlayer::CORSModeAnonymous, UrlData::CORS_ANONYMOUS);
 STATIC_ASSERT_ENUM(WebMediaPlayer::CORSModeUseCredentials,
                    UrlData::CORS_USE_CREDENTIALS);
 
 #define BIND_TO_RENDER_LOOP(function) \
   (DCHECK(main_task_runner_->BelongsToCurrentThread()), \
   BindToCurrentLoop(base::Bind(function, AsWeakPtr())))
 
 #define BIND_TO_RENDER_LOOP1(function, arg1) \
   (DCHECK(main_task_runner_->BelongsToCurrentThread()), \
   BindToCurrentLoop(base::Bind(function, AsWeakPtr(), arg1)))
 
 WebMediaPlayerImpl::WebMediaPlayerImpl(
     blink::WebLocalFrame* frame,
     blink::WebMediaPlayerClient* client,
     blink::WebMediaPlayerEncryptedMediaClient* encrypted_client,
     base::WeakPtr<WebMediaPlayerDelegate> delegate,
     std::unique_ptr<RendererFactory> renderer_factory,
     linked_ptr<UrlIndex> url_index,
     const WebMediaPlayerParams& params)
     : frame_(frame),
-      delegate_state_(DelegateState::GONE),
-      is_idle_(false),
-      must_suspend_(false),
       network_state_(WebMediaPlayer::NetworkStateEmpty),
       ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       highest_ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       preload_(MultibufferDataSource::AUTO),
       buffering_strategy_(MultibufferDataSource::BUFFERING_STRATEGY_NORMAL),
       main_task_runner_(frame->loadingTaskRunner()),
       media_task_runner_(params.media_task_runner()),
       worker_task_runner_(params.worker_task_runner()),
       media_log_(params.media_log()),
       pipeline_(media_task_runner_, media_log_.get()),
@@ skipping 36 matching lines @@
       volume_(1.0),
       volume_multiplier_(1.0),
       renderer_factory_(std::move(renderer_factory)),
       surface_manager_(params.surface_manager()),
       overlay_surface_id_(SurfaceManager::kNoSurfaceID),
       suppress_destruction_errors_(false),
       can_suspend_state_(CanSuspendState::UNKNOWN),
       use_fallback_path_(false),
       is_encrypted_(false),
       underflow_count_(0),
+      preroll_attempt_pending_(false),
       observer_(params.media_observer()) {
   DCHECK(!adjust_allocated_memory_cb_.is_null());
   DCHECK(renderer_factory_);
   DCHECK(client_);
 
   tick_clock_.reset(new base::DefaultTickClock());
 
   force_video_overlays_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kForceVideoOverlays);
 
-  if (delegate_)
+  if (delegate_) {
     delegate_id_ = delegate_->AddObserver(this);
+    delegate_->SetIdle(delegate_id_, true);
+  }
 
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_CREATED));
 
   if (params.initial_cdm())
     SetCdm(params.initial_cdm());
 
   // TODO(xhwang): When we use an external Renderer, many methods won't work,
   // e.g. GetCurrentFrameFromCompositor(). See http://crbug.com/434861
   audio_source_provider_ =
@@ skipping 150 matching lines @@
 void WebMediaPlayerImpl::play() {
   DVLOG(1) << __func__;
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
 #if defined(OS_ANDROID)  // WMPI_CAST
   if (isRemote()) {
     cast_impl_.play();
     return;
   }
 #endif
+  // TODO(sandersd): Do we want to reset the idle timer here?
+  if (delegate_)
+    delegate_->SetIdle(delegate_id_, false);
   paused_ = false;
-  is_idle_ = false;
   pipeline_.SetPlaybackRate(playback_rate_);
   background_pause_timer_.Stop();
 
   if (data_source_)
     data_source_->MediaIsPlaying();
 
   if (observer_)
     observer_->OnPlaying();
 
   DCHECK(watch_time_reporter_);
@@ skipping 80 matching lines @@
                                 AsWeakPtr(), BUFFERING_HAVE_ENOUGH));
     }
     return;
   }
 
   // Call this before setting |seeking_| so that the current media time can be
   // recorded by the reporter.
   if (watch_time_reporter_)
     watch_time_reporter_->OnSeeking();
 
-  // TODO(sandersd): Ideally we would not clear the idle state if
-  // |pipeline_controller_| can elide the seek.
-  is_idle_ = false;
+  // TODO(sandersd): Move |seeking_| to PipelineController.
+  // TODO(sandersd): Do we want to reset the idle timer here?
+  if (delegate_)
+    delegate_->SetIdle(delegate_id_, false);
   ended_ = false;
-
   seeking_ = true;
   seek_time_ = time;
   if (paused_)
     paused_time_ = time;
   pipeline_controller_.Seek(time, time_updated);
 
   // This needs to be called after Seek() so that if a resume is triggered, it
   // is to the correct time.
   UpdatePlayState();
 }
@@ skipping 246 matching lines @@
                                   std::isfinite(seekable_end);
 
   // TODO(dalecurtis): Technically this allows seeking on media which return an
   // infinite duration so long as DataSource::IsStreaming() is false.  While not
   // expected, disabling this breaks semi-live players, http://crbug.com/427412.
   const blink::WebTimeRange seekable_range(
       0.0, allow_seek_to_zero ? 0.0 : seekable_end);
   return blink::WebTimeRanges(&seekable_range, 1);
 }
 
+bool WebMediaPlayerImpl::IsPrerollAttemptNeeded() {
+  // TODO(sandersd): Replace with |highest_ready_state_since_seek_| if we need
+  // to ensure that preroll always gets a chance to complete.
+  // See http://crbug.com/671525.
+  if (highest_ready_state_ >= ReadyState::ReadyStateHaveFutureData)
+    return false;
+
+  if (preroll_attempt_pending_)
+    return true;
+
+  // Freshly initialized; there has never been any loading progress. (Otherwise
+  // |preroll_attempt_pending_| would be true when the start time is null.)
+  if (preroll_attempt_start_time_.is_null())
+    return false;
+
+  base::TimeDelta preroll_attempt_duration =
+      tick_clock_->NowTicks() - preroll_attempt_start_time_;
+  return preroll_attempt_duration < kPrerollAttemptTimeout;
+}
+
 bool WebMediaPlayerImpl::didLoadingProgress() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   // Note: Separate variables used to ensure both methods are called every time.
   const bool pipeline_progress = pipeline_.DidLoadingProgress();
   const bool data_progress = buffered_data_source_host_.DidLoadingProgress();
   const bool did_loading_progress = pipeline_progress || data_progress;
 
-  // If we've idle suspended before reaching kHaveFutureData and loading has
-  // progressed we need to spin up the renderer and figure out if we have enough
-  // data yet; |client_| may be waiting on this signal to trigger playback. The
-  // idle timeout is long enough that this is a low-cost operation.
-  if (highest_ready_state_ < ReadyState::ReadyStateHaveFutureData &&
-      pipeline_controller_.IsSuspended() && did_loading_progress && is_idle_) {
-    is_idle_ = false;
+  if (did_loading_progress &&
+      highest_ready_state_ < ReadyState::ReadyStateHaveFutureData) {
+    // Reset the preroll attempt clock.
+    preroll_attempt_pending_ = true;
+    preroll_attempt_start_time_ = base::TimeTicks();
+
+    // Clear any 'stale' flag and give the pipeline a chance to resume. If we
+    // are already resumed, this will cause |preroll_attempt_start_time_| to be
+    // set.
+    // TODO(sandersd): Should this be on the same stack? It might be surprising
+    // that didLoadingProgress() can synchronously change state.
+    if (delegate_)
+      delegate_->ClearStaleFlag(delegate_id_);
     UpdatePlayState();
   }
 
-  if (did_loading_progress)
-    last_time_loading_progressed_ = tick_clock_->NowTicks();
-
   return did_loading_progress;
 }
 
 void WebMediaPlayerImpl::paint(blink::WebCanvas* canvas,
                                const blink::WebRect& rect,
                                SkPaint& paint) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   TRACE_EVENT0("media", "WebMediaPlayerImpl:paint");
 
   // TODO(sandersd): Move this check into GetCurrentFrameFromCompositor() when
@@ skipping 380 matching lines @@
     // TODO(chcunningham): Monitor playback position vs buffered. Potentially
     // transition to HAVE_FUTURE_DATA here if not enough is buffered.
     SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
 
     // Let the DataSource know we have enough data. It may use this information
     // to release unused network connections.
     if (data_source_)
       data_source_->OnBufferingHaveEnough(false);
 
     // Blink expects a timeChanged() in response to a seek().
-    if (should_notify_time_changed_)
+    if (should_notify_time_changed_) {
+      should_notify_time_changed_ = false;
       client_->timeChanged();
+    }
 
     // Once we have enough, start reporting the total memory usage. We'll also
     // report once playback starts.
     ReportMemoryUsage();
 
     // Report the amount of time it took to leave the underflow state. Don't
     // bother to report this for MSE playbacks since it's out of our control.
     if (underflow_timer_ && data_source_) {
       UMA_HISTOGRAM_TIMES("Media.UnderflowDuration",
                           underflow_timer_->Elapsed());
@@ skipping 95 matching lines @@
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK_NE(ready_state_, WebMediaPlayer::ReadyStateHaveNothing);
 
   opaque_ = opaque;
   // Modify content opaqueness of cc::Layer directly so that
   // SetContentsOpaqueIsFixed is ignored.
   if (video_weblayer_)
     video_weblayer_->layer()->SetContentsOpaque(opaque_);
 }
 
-void WebMediaPlayerImpl::OnHidden() {
+void WebMediaPlayerImpl::OnFrameHidden() {

[whywhat, 2017/01/06 17:18:52]

nit: Following discussion with Dale around naming for visibility in
https://codereview.chromium.org/2552493002 -- maybe
OnForeground(ed)/OnBackground(ed)?

[sandersd (OOO until July 31), 2017/01/06 23:08:35]

I think I prefer to keep these names, since they are render frame concepts with
the same meaning. I'll wait for Dale to comment, though ;-)

The point is well taken though, we've got a bunch of related concepts with
confusingly similar names. (And some outright collisions: suspend/resume is also
used for media element network state.)

[DaleCurtis, 2017/01/07 00:31:03]

Saving the rename for later sgtm.

   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   if (IsBackgroundVideoTrackOptimizationEnabled())
     selectedVideoTrackChanged(nullptr);
 
   if (watch_time_reporter_)
     watch_time_reporter_->OnHidden();
 
   UpdatePlayState();
 
   // Schedule suspended playing media to be paused if the user doesn't come back
   // to it within some timeout period to avoid any autoplay surprises.
   ScheduleIdlePauseTimer();
 }
 
-void WebMediaPlayerImpl::OnShown() {
+void WebMediaPlayerImpl::OnFrameClosed() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
+  UpdatePlayState();
+}
+
+void WebMediaPlayerImpl::OnFrameShown() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  background_pause_timer_.Stop();
+
   if (watch_time_reporter_)
     watch_time_reporter_->OnShown();
 
   if (IsBackgroundVideoTrackOptimizationEnabled() &&
       client_->hasSelectedVideoTrack()) {
     WebMediaPlayer::TrackId trackId = client_->getSelectedVideoTrackId();
     selectedVideoTrackChanged(&trackId);
   }
 
-  must_suspend_ = false;
-  background_pause_timer_.Stop();
-
   UpdatePlayState();
 }
 
-bool WebMediaPlayerImpl::OnSuspendRequested(bool must_suspend) {
+void WebMediaPlayerImpl::OnIdleTimeout() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
-  if (must_suspend) {
-    must_suspend_ = true;
-    UpdatePlayState();
-    return true;
+  // If we are attempting preroll, clear the stale flag.
+  if (IsPrerollAttemptNeeded()) {
+    if (delegate_)
+      delegate_->ClearStaleFlag(delegate_id_);
+    return;
   }
 
-  // If we're beyond HaveFutureData, we can safely suspend at any time.
-  if (highest_ready_state_ >= WebMediaPlayer::ReadyStateHaveFutureData) {
-    is_idle_ = true;
-    UpdatePlayState();
-    return true;
-  }
-
-  // Before HaveFutureData blink will not call play(), so we must be careful to
-  // only suspend if we'll eventually receive an event that will trigger a
-  // resume. If the last time loading progressed was a while ago, and we still
-  // haven't reached HaveFutureData, we assume that we're waiting on more data
-  // to continue pre-rolling. When that data is loaded the pipeline will be
-  // resumed by didLoadingProgress().
-  if (last_time_loading_progressed_.is_null() ||
-      (tick_clock_->NowTicks() - last_time_loading_progressed_) >
-          kLoadingToIdleTimeout) {
-    is_idle_ = true;
-    UpdatePlayState();
-    return true;
-  }
-
-  return false;
+  UpdatePlayState();
 }
 
 void WebMediaPlayerImpl::OnPlay() {
   play();
   client_->playbackStateChanged();
 }
 
 void WebMediaPlayerImpl::OnPause() {
   pause();
   client_->playbackStateChanged();
@@ skipping 327 matching lines @@
   bool is_streaming = false;
 #else
   bool is_remote = false;
   bool is_streaming = data_source_ && data_source_->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);
-  SetDelegateState(state.delegate_state);
+  SetDelegateState(state.delegate_state, state.is_idle);
   SetMemoryReportingState(state.is_memory_reporting_enabled);
   SetSuspendState(state.is_suspended || pending_suspend_resume_cycle_);
 }
 
-void WebMediaPlayerImpl::SetDelegateState(DelegateState new_state) {
+void WebMediaPlayerImpl::SetDelegateState(DelegateState new_state,
+                                          bool is_idle) {
   if (!delegate_)
     return;
 
-  if (delegate_state_ == new_state) {
-    if (delegate_state_ != DelegateState::PLAYING ||
-        autoplay_muted_ == client_->isAutoplayingMuted()) {
-      return;
-    }
-  }
-
-  delegate_state_ = new_state;
-
-  switch (delegate_state_) {
+  switch (new_state) {
     case DelegateState::GONE:
       delegate_->PlayerGone(delegate_id_);
       break;
     case DelegateState::PLAYING: {
-      autoplay_muted_ = client_->isAutoplayingMuted();
-      bool has_audio = autoplay_muted_ ? false : hasAudio();
+      bool has_audio = hasAudio() && !client_->isAutoplayingMuted();
       delegate_->DidPlay(
-          delegate_id_, hasVideo(), has_audio, false,
+          delegate_id_, has_audio, hasVideo(),
           media::DurationToMediaContentType(pipeline_.GetMediaDuration()));
       break;
     }
     case DelegateState::PAUSED:
-      delegate_->DidPause(delegate_id_, false);
-      break;
-    case DelegateState::ENDED:
-      delegate_->DidPause(delegate_id_, true);
+      delegate_->DidPause(delegate_id_);
       break;
   }
+
+  delegate_->SetIdle(delegate_id_, is_idle);
 }
 
 void WebMediaPlayerImpl::SetMemoryReportingState(
     bool is_memory_reporting_enabled) {
   if (memory_usage_reporting_timer_.IsRunning() ==
       is_memory_reporting_enabled) {
     return;
   }
 
   if (is_memory_reporting_enabled) {
@@ skipping 25 matching lines @@
       can_suspend_state_ =
           frame->metadata()->IsTrue(VideoFrameMetadata::DECODER_OWNS_FRAME)
               ? CanSuspendState::NO
               : CanSuspendState::YES;
     }
   }
 #else
   can_suspend_state_ = CanSuspendState::YES;
 #endif
 
-  if (can_suspend_state_ == CanSuspendState::NO)
-    return;
-
-  if (is_suspended) {
+  if (is_suspended && can_suspend_state_ != CanSuspendState::NO) {
+    // If we were not resumed for long enough to satisfy the preroll attempt,
+    // reset the clock.
+    if (!preroll_attempt_pending_ && IsPrerollAttemptNeeded()) {
+      preroll_attempt_pending_ = true;
+      preroll_attempt_start_time_ = base::TimeTicks();
+    }
     pipeline_controller_.Suspend();
   } else {
+    // When resuming, start the preroll attempt clock.
+    if (preroll_attempt_pending_) {
+      preroll_attempt_pending_ = false;
+      preroll_attempt_start_time_ = tick_clock_->NowTicks();
+    }
     pipeline_controller_.Resume();
   }
 }
 
 WebMediaPlayerImpl::PlayState
 WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
                                                      bool is_streaming,
                                                      bool is_suspended,
                                                      bool is_backgrounded) {
   PlayState result;
 
+  bool must_suspend = delegate_ && delegate_->IsFrameClosed();
+  bool is_stale = delegate_ && delegate_->IsStale(delegate_id_);
+
   // This includes both data source (before pipeline startup) and pipeline
   // errors.
   bool has_error = IsNetworkStateError(network_state_);
 
   // After HaveMetadata, we know which tracks are present and the duration.
   bool have_metadata = ready_state_ >= WebMediaPlayer::ReadyStateHaveMetadata;
 
   // After HaveFutureData, Blink will call play() if the state is not paused;
   // prior to this point |paused_| is not accurate.
   bool have_future_data =
       highest_ready_state_ >= WebMediaPlayer::ReadyStateHaveFutureData;
 
   // Background suspend is not enabled for audio-only players unless paused,
   // though in the case of audio-only the session should be kept.
   // Videos are not suspended if the user resumed the playback via the remote
   // controls earlier and it's still playing.
   bool is_backgrounded_video = is_backgrounded && have_metadata && hasVideo();
   bool can_play_backgrounded = is_backgrounded_video && !is_remote &&
                                hasAudio() && IsResumeBackgroundVideosEnabled();
   bool is_background_playing =
-      delegate_ && delegate_->IsPlayingBackgroundVideo();
+      delegate_ && delegate_->IsBackgroundVideoPlaybackAllowed();

[whywhat, 2017/01/06 17:18:51]

nit: I'm a bit concerned that IsBgVideoPlaybackAllowed can be confused with
IsResumeBgVideosEnabled() (used above).
The latter is a feature flag (which we should just remove and replace with true
in a cleanup CL), the former is a per player bit set everytime the video is
resumed in bg by the user and reset every time the video comes to the foreground
(it's similar to a user gesture lock but bg specific).

So maybe IsBgVideoPlaybackUnlocked() ?

[sandersd (OOO until July 31), 2017/01/06 23:08:35]

Done.

   bool background_suspended = !is_streaming && is_backgrounded_video &&
                               !(can_play_backgrounded && is_background_playing);
   bool background_pause_suspended =
       !is_streaming && 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
   // the have future data state; see didLoadingProgress().
   //
   // TODO(sandersd): Make the delegate suspend idle players immediately when
   // hidden.
   bool idle_suspended =
-      !is_streaming && is_idle_ && paused_ && !seeking_ && !overlay_enabled_;
+      !is_streaming && 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_idle_| to remain suspended. |is_idle_|
+  // to HaveFutureData, we require |is_stale| to remain suspended. |is_stale|
   // will be cleared when we receive data which may take us to HaveFutureData.
   bool can_stay_suspended =
-      (is_idle_ || have_future_data) && is_suspended && paused_ && !seeking_;
+      (is_stale || have_future_data) && is_suspended && paused_ && !seeking_;
 
   // Combined suspend state.
-  result.is_suspended = is_remote || must_suspend_ || idle_suspended ||
+  result.is_suspended = is_remote || must_suspend || idle_suspended ||
                         background_suspended || background_pause_suspended ||
                         can_stay_suspended;
 
   // We do not treat |playback_rate_| == 0 as paused. For the media session,
   // being paused implies displaying a play button, which is incorrect in this
   // case. For memory usage reporting, we just use the same definition (but we
   // don't have to).
   //
   // Similarly, we don't consider |ended_| to be paused. Blink will immediately
   // call pause() or seek(), so |ended_| should not affect the computation.
   // Despite that, |ended_| does result in a separate paused state, to simplfy
   // the contract for SetDelegateState().
   //
   // |has_session| is used to decide when to create a media session. Idle
   // suspension does not destroy the media session, because we expect that the
   // notification controls (and audio focus) remain. We also require:
   //   - |have_metadata|, since the tracks and duration are passed to DidPlay().
   //   - |have_future_data|, since we need to know whether we are paused to
   //     correctly configure the session.
   //
   // TODO(sandersd): If Blink told us the paused state sooner, we could create
   // the media session sooner.
   bool can_play = !has_error && !is_remote && have_future_data;
-  bool has_session_playing =
-      can_play && !must_suspend_ && !background_suspended;
+  bool has_session_playing = can_play && !must_suspend && !background_suspended;
 
   // |has_session_suspended| means the player is suspended from the media
   // element point of view but paused and can be resumed from the delegate point
   // of view. Therefore it behaves like |paused_| for the delegate.
-  bool has_session_suspended = can_play && !must_suspend_ &&
+  bool has_session_suspended = can_play && !must_suspend &&
                                background_suspended && can_play_backgrounded;
 
   bool has_session = has_session_playing || has_session_suspended;
 
   if (!has_session) {
     result.delegate_state = DelegateState::GONE;
+    result.is_idle = delegate_ && delegate_->IsIdle(delegate_id_);
   } else if (paused_ || has_session_suspended) {
+    // TODO(sandersd): Is it possible to have a suspended session, be ended,
+    // and not be paused? If so we should be in a PLAYING state.
     result.delegate_state =
-        ended_ ? DelegateState::ENDED : DelegateState::PAUSED;
+        ended_ ? DelegateState::GONE : DelegateState::PAUSED;
+    result.is_idle = !seeking_;
   } else {
     result.delegate_state = DelegateState::PLAYING;
+    result.is_idle = false;
   }
 
   // It's not critical if some cases where memory usage can change are missed,
   // since media memory changes are usually gradual.
   result.is_memory_reporting_enabled =
-      can_play && !result.is_suspended && !paused_;
+      can_play && !result.is_suspended && (!paused_ || seeking_);
 
   return result;
 }
 
 void WebMediaPlayerImpl::ReportMemoryUsage() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   // About base::Unretained() usage below: We destroy |demuxer_| on the main
   // thread.  Before that, however, ~WebMediaPlayerImpl() posts a task to the
   // media thread and waits for it to finish.  Hence, the GetMemoryUsage() task
@@ skipping 65 matching lines @@
                                 this, &WebMediaPlayerImpl::OnPause);
 }
 
 void WebMediaPlayerImpl::CreateWatchTimeReporter() {
   // Create the watch time reporter and synchronize its initial state.
   watch_time_reporter_.reset(new WatchTimeReporter(
       hasAudio(), hasVideo(), !!chunk_demuxer_, is_encrypted_, media_log_,
       pipeline_metadata_.natural_size,
       base::Bind(&GetCurrentTimeInternal, this)));
   watch_time_reporter_->OnVolumeChange(volume_);
-  if (IsHidden())
+  if (delegate_ && delegate_->IsFrameHidden())
     watch_time_reporter_->OnHidden();
   else
     watch_time_reporter_->OnShown();
 }
 
 bool WebMediaPlayerImpl::IsHidden() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
-  return delegate_ && delegate_->IsHidden();
+  return delegate_ && delegate_->IsFrameHidden() && !delegate_->IsFrameClosed();
 }
 
 bool WebMediaPlayerImpl::DoesOverlaySupportMetadata() const {
   return pipeline_metadata_.video_rotation == VIDEO_ROTATION_0;
 }
 
 void WebMediaPlayerImpl::ActivateViewportIntersectionMonitoring(bool activate) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   client_->activateViewportIntersectionMonitoring(activate);
 }
 
 }  // namespace media