Fixes play seek when user sets loop after ended.

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 2281 matching lines @@
 }
 
 void HTMLMediaElement::playInternal()
 {
     WTF_LOG(Media, "HTMLMediaElement::playInternal(%p)", this);
 
     // 4.8.10.9. Playing the media resource
     if (!m_player || m_networkState == NETWORK_EMPTY)
         scheduleDelayedAction(LoadMediaResource);
 
-    if (endedPlayback())
+    // Generally "ended" and "looping" are exclusive. Here, the loop attribute
+    // is ignored to seek back to start in case loop was set after playback
+    // ended. See http://crbug.com/364442
+    if (endedPlayback(/* ignore_loop_attr */ true))

[wolenetz, 2015/02/12 22:12:27]

nit: I don't see this style of commenting params much in Blink at all. philipj@
should this inline comment simply be dropped?

[chcunningham, 2015/02/13 03:37:29]

Perhaps this never should have been a bool. I'm not a fan of simply dropping the
comment... it looks like a setter and is generally much harder to read. 

I've replaced the bool with an enum to make its purpose/value self documenting.

         seek(0);
 
     if (m_mediaController)
         m_mediaController->bringElementUpToSpeed(this);
 
     if (m_paused) {
         m_paused = false;
         invalidateCachedTime();
         scheduleEvent(EventTypeNames::play);
 
@@ skipping 928 matching lines @@
 
 bool HTMLMediaElement::potentiallyPlaying() const
 {
     // "pausedToBuffer" means the media engine's rate is 0, but only because it had to stop playing
     // when it ran out of buffered data. A movie is this state is "potentially playing", modulo the
     // checks in couldPlayIfEnoughData().
     bool pausedToBuffer = m_readyStateMaximum >= HAVE_FUTURE_DATA && m_readyState < HAVE_FUTURE_DATA;
     return (pausedToBuffer || m_readyState >= HAVE_FUTURE_DATA) && couldPlayIfEnoughData() && !isBlockedOnMediaController();
 }
 
 bool HTMLMediaElement::couldPlayIfEnoughData() const

[wolenetz, 2015/02/12 22:12:27]

Should this method also ignore loopAttr w.r.t. endedPlayback calculation?

[chcunningham, 2015/02/13 03:37:29]

I don't think so. I've checked usage in 3 different places, and the usage of
ended here really means "stopped", like "played the whole video and I'm not
looping back to play it again". In this case, I think the default consideration
of 'loop' (where loop = true -> not ended, still playing) is correct.

[wolenetz, 2015/02/13 22:24:23]

Acknowledged.

 {
     return !paused() && !endedPlayback() && !stoppedDueToErrors();
 }
 
 bool HTMLMediaElement::endedPlayback() const
 {
+    // For general usage, presence of the loop attribute should be considered to mean playback
+    // has not "ended", as "ended" and "looping" are mutually exclusive. See
+    // http://dev.w3.org/html5/spec-preview/media-elements.html#ended-playback
+    return endedPlayback(/* ignore_loop_attr */ false);

[wolenetz, 2015/02/12 22:12:27]

nit: ditto: drop the inline comment?

[chcunningham, 2015/02/13 03:37:29]

Now an enum.

+}
+
+bool HTMLMediaElement::endedPlayback(bool ignoreLoopAttr) const
+{
     double dur = duration();
     if (!m_player || std::isnan(dur))
         return false;
 
     // 4.8.10.8 Playing the media resource
 
     // A media element is said to have ended playback when the element's
     // readyState attribute is HAVE_METADATA or greater,
     if (m_readyState < HAVE_METADATA)
         return false;
 
     // and the current playback position is the end of the media resource and the direction
     // of playback is forwards, Either the media element does not have a loop attribute specified,
     // or the media element has a current media controller.
     double now = currentTime();
     if (directionOfPlayback() == Forward)
-        return dur > 0 && now >= dur && (!loop() || m_mediaController);
+        return dur > 0 && now >= dur && (ignoreLoopAttr || !loop() || m_mediaController);
 
     // or the current playback position is the earliest possible position and the direction
     // of playback is backwards
     ASSERT(directionOfPlayback() == Backward);
     return now <= 0;
 }
 
 bool HTMLMediaElement::stoppedDueToErrors() const
 {
     if (m_readyState >= HAVE_METADATA && m_error) {
@@ skipping 685 matching lines @@
 
 #if ENABLE(WEB_AUDIO)
 void HTMLMediaElement::clearWeakMembers(Visitor* visitor)
 {
     if (!visitor->isAlive(m_audioSourceNode) && audioSourceProvider())
         audioSourceProvider()->setClient(nullptr);
 }
 #endif
 
 }