Have WebMediaPlayerImpl track ended state.

content/renderer/media/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 "content/renderer/media/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 #include <vector>
@@ skipping 137 matching lines @@
       main_task_runner_(base::MessageLoopProxy::current()),
       media_task_runner_(
           RenderThreadImpl::current()->GetMediaThreadTaskRunner()),
       media_log_(new RenderMediaLog()),
       pipeline_(media_task_runner_, media_log_.get()),
       load_type_(LoadTypeURL),
       opaque_(false),
       paused_(true),
       seeking_(false),
       playback_rate_(0.0f),
+      ended_(false),
       pending_seek_(false),
       pending_seek_seconds_(0.0f),
       should_notify_time_changed_(false),
       client_(client),
       delegate_(delegate),
       defer_load_cb_(params.defer_load_cb()),
       gpu_factories_(RenderThreadImpl::current()->GetGpuFactories()),
       supports_save_(true),
       chunk_demuxer_(NULL),
       // Threaded compositing isn't enabled universally yet.
@@ skipping 137 matching lines @@
 
 bool WebMediaPlayerImpl::supportsSave() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   return supports_save_;
 }
 
 void WebMediaPlayerImpl::seek(double seconds) {
   DVLOG(1) << __FUNCTION__ << "(" << seconds << ")";
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
+  ended_ = false;
+
   if (ready_state_ > WebMediaPlayer::ReadyStateHaveMetadata)
     SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
 
   base::TimeDelta seek_time = ConvertSecondsToTimestamp(seconds);
 
   if (seeking_) {
     pending_seek_ = true;
     pending_seek_seconds_ = seconds;
     if (chunk_demuxer_)
       chunk_demuxer_->CancelPendingSeek(seek_time);
@@ skipping 113 matching lines @@
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   if (pipeline_metadata_.timeline_offset.is_null())
     return std::numeric_limits<double>::quiet_NaN();
 
   return pipeline_metadata_.timeline_offset.ToJsTime();
 }
 
 double WebMediaPlayerImpl::currentTime() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
+

[acolwell GONE FROM CHROMIUM, 2014/08/30 01:00:28]

nit: Add a DCHECK here to make sure you are never called in the HAVE_NOTHING
ready state since duration is invalid in that case. You should have cover from
HTMLMediaElement above, but it doesn't hurt to have this precondition explicit.

[scherkus (not reviewing), 2014/09/02 20:58:25]

Sadly Blink does call this when we HAVE_NOTHING due to line 252 :\

#2 0x7fcf451534a6 content::WebMediaPlayerImpl::currentTime()
#3 0x7fcf41928e97 blink::HTMLMediaElement::refreshCachedTime()
#4 0x7fcf4192eb23 blink::HTMLMediaElement::updatePlayState()
#5 0x7fcf4192cd3e blink::HTMLMediaElement::setReadyState()
#6 0x7fcf4192e138 blink::HTMLMediaElement::mediaPlayerReadyStateChanged()
#7 0x7fcf4192e15c blink::HTMLMediaElement::mediaPlayerReadyStateChanged()
#8 0x7fcf3ffc56ba blink::WebMediaPlayerClientImpl::readyStateChanged()
#9 0x7fcf3ffc56dc blink::WebMediaPlayerClientImpl::readyStateChanged()
#10 0x7fcf45150201 content::WebMediaPlayerImpl::SetReadyState()
#11 0x7fcf4514fa41 content::WebMediaPlayerImpl::DoLoad()

I've replaced with a check from the spec text that returns 0 when we're in that
state.

+  // TODO(scherkus): Replace with an explicit ended signal to HTMLMediaElement,
+  // see http://crbug.com/409280
+  if (ended_)
+    return duration();
+
   return (paused_ ? paused_time_ : pipeline_.GetMediaTime()).InSecondsF();
 }
 
 WebMediaPlayer::NetworkState WebMediaPlayerImpl::networkState() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   return network_state_;
 }
 
 WebMediaPlayer::ReadyState WebMediaPlayerImpl::readyState() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
@@ skipping 235 matching lines @@
   // Update our paused time.
   if (paused_)
     paused_time_ = pipeline_.GetMediaTime();
 
   should_notify_time_changed_ = time_changed;
 }
 
 void WebMediaPlayerImpl::OnPipelineEnded() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(main_task_runner_->BelongsToCurrentThread());
+  ended_ = true;

[acolwell GONE FROM CHROMIUM, 2014/08/30 01:00:28]

I don't think you can blindly update it here. At a minimum I think you need to
return w/o updating if seeking_ is true. Otherwise I think you can get in a
situation where the ended event and the seek request pass eachother and ended_
gets in the wrong state.

[scherkus (not reviewing), 2014/09/02 20:58:25]

Good catch -- done. We have a similar check in OnBufferingStateChanged().

   client_->timeChanged();
 }
 
 void WebMediaPlayerImpl::OnPipelineError(PipelineStatus error) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK_NE(error, media::PIPELINE_OK);
 
   if (ready_state_ == WebMediaPlayer::ReadyStateHaveNothing) {
     // Any error that occurs before reaching ReadyStateHaveMetadata should
     // be considered a format error.
@@ skipping 300 matching lines @@
   compositor_task_runner_->PostTask(FROM_HERE,
                                     base::Bind(&GetCurrentFrameAndSignal,
                                                base::Unretained(compositor_),
                                                &video_frame,
                                                &event));
   event.Wait();
   return video_frame;
 }
 
 }  // namespace content