WebMediaPlayerImpl should notify ended event

Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 3055 matching lines @@
 
     // 4.8.10.9 steps 12-14. Needed if no ReadyState change is associated with the seek.
     if (m_seeking && m_readyState >= HAVE_CURRENT_DATA && !webMediaPlayer()->seeking())
         finishSeek();
 
     // Always call scheduleTimeupdateEvent when the media engine reports a time discontinuity,
     // it will only queue a 'timeupdate' event if we haven't already posted one at the current
     // movie time.
     scheduleTimeupdateEvent(false);
 
+    // TODO(amogh.bihani): Remove this once chromium change lands (codereview.chromium.org/533543004/).
     double now = currentTime();
     double dur = duration();
 
+    if (!std::isnan(dur) && dur && now >= dur && directionOfPlayback() == Forward)

[philipj_slow, 2014/09/10 13:11:08]

This is also temporary, to be removed after codereview.chromium.org/533543004/
right?

[amogh.bihani, 2014/09/10 13:39:22]

 Yes.

+        mediaPlayerEnded();
+
+    updatePlayState();
+}
+
+void HTMLMediaElement::mediaPlayerEnded()

[philipj_slow, 2014/09/10 13:24:35]

Do you intend to clamp duration to currentTime somewhere around here?

[amogh.bihani, 2014/09/10 13:39:22]

I am planning to clamp it in webmediaplayer_impl side.

+{
+    WTF_LOG(Media, "HTMLMediaElement::mediaPlayerEnded");

[philipj_slow, 2014/09/10 13:11:08]

There was a recent patch to add the this pointer to the logging, please do the
same when rebasing this.

[amogh.bihani, 2014/09/10 13:39:22]

Acknowledged.

+
     // When the current playback position reaches the end of the media resource when the direction of
     // playback is forwards, then the user agent must follow these steps:
-    if (!std::isnan(dur) && dur && now >= dur && directionOfPlayback() == Forward) {
+    if (!std::isnan(duration()) && directionOfPlayback() == Forward) {
         // If the media element has a loop attribute specified and does not have a current media controller,
         if (loop() && !m_mediaController) {
             m_sentEndEvent = false;
             //  then seek to the earliest possible position of the media resource and abort these steps.
             seek(0, IGNORE_EXCEPTION);
         } else {
             // If the media element does not have a current media controller, and the media element
             // has still ended playback, and the direction of playback is still forwards, and paused
             // is false,
             if (!m_mediaController && !m_paused) {
                 // changes paused to true and fires a simple event named pause at the media element.
                 m_paused = true;
                 scheduleEvent(EventTypeNames::pause);
             }
             // Queue a task to fire a simple event named ended at the media element.
             if (!m_sentEndEvent) {
                 m_sentEndEvent = true;
                 scheduleEvent(EventTypeNames::ended);
             }
             // If the media element has a current media controller, then report the controller state
             // for the media element's current media controller.
             updateMediaController();
         }
+    } else {
+        m_sentEndEvent = false;
     }
-    else
-        m_sentEndEvent = false;
 
     updatePlayState();
 }
 
 void HTMLMediaElement::mediaPlayerDurationChanged()
 {
     WTF_LOG(Media, "HTMLMediaElement::mediaPlayerDurationChanged(%p)", this);
+    refreshCachedTime();

[philipj_slow, 2014/09/10 13:11:08]

This looks correct, but is it related to the other changes or an independent
fix? The several layers of times are a bit confusing at present :)

[amogh.bihani, 2014/09/10 13:39:22]

It is related to the changes done in chromium side CL.

     // FIXME: Change MediaPlayerClient & WebMediaPlayer to convey
     // the currentTime when the duration change occured. The current
     // WebMediaPlayer implementations always clamp currentTime() to
     // duration() so the requestSeek condition here is always false.
     durationChanged(duration(), currentTime() > duration());
 }
 
 void HTMLMediaElement::durationChanged(double duration, bool requestSeek)
 {
     WTF_LOG(Media, "HTMLMediaElement::durationChanged(%p, %f, %d)", this, duration, requestSeek);
@@ skipping 845 matching lines @@
 
 #if ENABLE(WEB_AUDIO)
 void HTMLMediaElement::clearWeakMembers(Visitor* visitor)
 {
     if (!visitor->isAlive(m_audioSourceNode) && audioSourceProvider())
         audioSourceProvider()->setClient(0);
 }
 #endif
 
 }