Have WebMediaPlayerImpl track ended state.

content/renderer/media/webmediaplayer_impl.cc

Index: content/renderer/media/webmediaplayer_impl.cc
diff --git a/content/renderer/media/webmediaplayer_impl.cc b/content/renderer/media/webmediaplayer_impl.cc
index 5b1cf2bdeb3e7ebb26dce33351f2ec07533c498c..6db95e7277843ced8655598b7027666a203721ab 100644
--- a/content/renderer/media/webmediaplayer_impl.cc
+++ b/content/renderer/media/webmediaplayer_impl.cc
@@ -155,6 +155,7 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       paused_(true),
       seeking_(false),
       playback_rate_(0.0f),
+      ended_(false),
       pending_seek_(false),
       pending_seek_seconds_(0.0f),
       should_notify_time_changed_(false),
@@ -312,6 +313,8 @@ void WebMediaPlayerImpl::seek(double seconds) {
   DVLOG(1) << __FUNCTION__ << "(" << seconds << ")";
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
+  ended_ = false;
+
   if (ready_state_ > WebMediaPlayer::ReadyStateHaveMetadata)
     SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
 
@@ -445,6 +448,12 @@ double WebMediaPlayerImpl::timelineOffset() const {
 
 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();
 }
 
@@ -700,6 +709,7 @@ void WebMediaPlayerImpl::OnPipelineSeeked(bool 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();
 }