Fixes play seek when user sets loop after ended.

LayoutTests/media/video-loop.html

Index: LayoutTests/media/video-loop.html
diff --git a/LayoutTests/media/video-loop.html b/LayoutTests/media/video-loop.html
index 4e44416754b58ee0508ab8abe7788dd0ffd2da00..b8d1341faf8b281515c0378217381afb820ba9d7 100644
--- a/LayoutTests/media/video-loop.html
+++ b/LayoutTests/media/video-loop.html
@@ -54,14 +54,25 @@
                         consoleWrite("");
                         break;
                     case 3:
-                        consoleWrite("<br><em>++ third seek completed, beginning playback for the last time.</em>");
+                        consoleWrite("<br><em>++ third seek completed, unset loop and let play to the end.</em>");

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

nit: s/loop/'loop'

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

Done.

                         testExpected("video.paused", true);
                         testExpected("video.ended", false);
                         run("video.play()");
                         consoleWrite("");
                         break;
+                    case 4:
+                        consoleWrite("<br><em>++ fourth seek completed.</em>");
+                        // don't use "testExpected()" so we won't log the actual duration as the floating point result may differ with different engines
+                        reportExpected(mediaElement.currentTime < mediaElement.duration, "mediaElement.currentTime", "<", "mediaElement.duration", mediaElement.currentTime);
+                        testExpected("video.loop", true);
+                        testExpected("video.paused", false);
+                        testExpected("video.ended", false);
+                        // Pause now that test is over to prevent re-firing ended and annoying debugging experience.

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

nit: change comment to a consoleWrite to make expectation even clearer (perhaps
like ..."pausing playback to prevent another 'ended' while ending test")

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

Done.

+                        run("video.pause()");
+                        consoleWrite("");
+                        endTest();
                     default:
-                        failTest("Video should have only seeked three times.");
+                        failTest("Video should have only seeked four times.");
                         break;
                         
                 }
@@ -75,8 +86,16 @@
                 // don't use "testExpected()" so we won't log the actual duration as the floating point result may differ with different engines
                 reportExpected(mediaElement.currentTime == mediaElement.duration, "mediaElement.currentTime", "==", "mediaElement.duration", mediaElement.currentTime);
 
+                // Testing edge case where we set loop after playback ends. Verifies fix for crbug.com/364442

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

nit: usually we don't spend code to reference a fixed bug in the [test] code
(except in chromium for cases like test media files specific to particular
regressions). Curious folks can always see the commit history to figure out
which bug these lines were added to verify. (also s/loop/'loop'/ if you keep
some reference to the attribute in comment here)

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

Know what you mean, but this comment is pretty tiny and the significance of this
test is not obvious. I'd like to keep it so no one comes along and deletes what
might initially appear to be an over-engineered test case.

[fs, 2015/02/13 11:49:22]

This whole interchange seems to suggest that you really just should make a
separate TC for the bug you're fixing. That ought to address both of your
concerns (and it's quite possible that this TC shouldn't have been allowed to
balloon like this in the first place.)

[chcunningham, 2015/02/13 23:01:43]

Done.

+                consoleWrite("<br><em>++ with ended == true, set loop and play again.</em>");

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

nit: s/loop/'loop'

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

Done.

+                testExpected("video.loop", false);
+                run("video.loop = true");
+                testExpected("video.loop", true);
+                // Playback cannot be "ended" when loop attribute is set; It is simply paused.

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

nit: move this part of this comment to the consoleWrite to make expectation even
more clear.

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

Done.

+                // See http://dev.w3.org/html5/spec-preview/media-elements.html#ended-playback

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

nit: No strong feeling here, but I suspect we don't really need to clarify with
link to spec definition of HTMLMediaElement.ended here.

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

Done.

+                testExpected("video.ended", false);

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

nit: there is no explicit check that the video is indeed paused, though comment,
above, asserts this. add check please.

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

Done.

+                run("video.play()");
                 consoleWrite("");
-                endTest();
             }
     
             function start()
@@ -115,6 +134,8 @@
             <li>When 'seeked' event fires, verify that time has jumped back and movie is playing.</li>

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

nit: seeked() case 2 doesn't explicitly check that time has jumped back relative
to the last known currentTime from app's perspective. (e.g. instead maybe check
currentTime < duration - 0.4). Strengthen case 2's verification?

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

Done.

             <li>Set 'loop' to false and play again.</li>
             <li>Verify that 'ended' event fires.</li>
+            <li>Set 'loop' to true now that video is ended. Call play again.</li>
+            <li>Verify 'seeked' event fires, verify that time has jumped back and movie is playing.</li>
         </ol>
         </p>
         <script>start()</script>