MSE: Fix rare flaky changeType-play-* failures

Unless the app explicitly sets `mediaSource.duration`,
`HTMLMediaElement.duration` remains NaN until initial HAVE_METADATA is
reached, even if the attached mediaSource has already buffered media
well beyond the initialization segment(s) necessary to begin transition
to HAVE_METADATA. In Chromium, that transition is begun asynchronously,
letting thread hop through the pipeline thread complete while letting
the app continue. Eventually, the media element transitions, but in the
interim, its value for duration could still be NaN, even if mediaSource
has a duration value.

This change relaxes the changeType-play-* utility's reliance on strict
matching of mediaElement.duration and mediaSource.duration, instead
relying on the latter for use in trimming the buffered duration.

I've filed MSE spec issue #275 to discuss this behavior:
w3c/media-source#275

This change also reports more details in changeType web-test assertion
failures, which also enabled finding the suspected root cause of the
tests' flakiness.

Bug: 1184745

Change-Id: I208cbfbbc60776366a16b6a3e79f52480df5be37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2906776
Auto-Submit: Matthew Wolenetz <[email protected]>
Reviewed-by: Ted Meyer <[email protected]>
Commit-Queue: Ted Meyer <[email protected]>
Cr-Commit-Position: refs/heads/master@{#886148}

Author: wolenetz

media-source/mediasource-changetype-util.js

@@ -116,10 +116,12 @@ function appendBuffer(test, sourceBuffer, data) {
   sourceBuffer.appendBuffer(data);
 }
 
-function trimBuffered(test, mediaElement, sourceBuffer, minimumPreviousDuration, newDuration, skip_duration_prechecks) {
+function trimBuffered(test, mediaSource, sourceBuffer, minimumPreviousDuration, newDuration, skip_duration_prechecks) {
   if (!skip_duration_prechecks) {
-    assert_less_than(newDuration, minimumPreviousDuration);
-    assert_less_than(minimumPreviousDuration, mediaElement.duration);
+    assert_less_than(newDuration, minimumPreviousDuration,
+        "trimBuffered newDuration must be less than minimumPreviousDuration");
+    assert_less_than(minimumPreviousDuration, mediaSource.duration,
+        "trimBuffered minimumPreviousDuration must be less than mediaSource.duration");
   }
   test.expectEvent(sourceBuffer, "update");
   test.expectEvent(sourceBuffer, "updateend");
@@ -128,7 +130,8 @@ function trimBuffered(test, mediaElement, sourceBuffer, minimumPreviousDuration,
 
 function trimDuration(test, mediaElement, mediaSource, newDuration, skip_duration_prechecks) {
   if (!skip_duration_prechecks) {
-    assert_less_than(newDuration, mediaElement.duration);
+    assert_less_than(newDuration, mediaSource.duration,
+        "trimDuration newDuration must be less than mediaSource.duration");
   }
   test.expectEvent(mediaElement, "durationchange");
   mediaSource.duration = newDuration;
@@ -158,7 +161,8 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
   // implicit_changetype and negative_test.
 
   function findSafeOffset(targetTime, overlappedMediaMetadata, overlappedStartTime, overlappingMediaMetadata) {
-    assert_greater_than_equal(targetTime, overlappedStartTime);
+    assert_greater_than_equal(targetTime, overlappedStartTime,
+        "findSafeOffset targetTime must be greater than or equal to overlappedStartTime");
 
     let offset = targetTime;
     if ("start_time" in overlappingMediaMetadata) {
@@ -177,7 +181,8 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
     let gopsToRetain = Math.ceil((targetTime - overlappedStartTime) / overlappedMediaMetadata["keyframe_interval"]);
     let adjustedTime = overlappedStartTime + gopsToRetain * overlappedMediaMetadata["keyframe_interval"];
 
-    assert_greater_than_equal(adjustedTime, targetTime);
+    assert_greater_than_equal(adjustedTime, targetTime,
+        "findSafeOffset adjustedTime must be greater than or equal to targetTime");
     offset += adjustedTime - targetTime;
     return { "offset": offset, "adjustedTime": adjustedTime };
   }
@@ -221,7 +226,8 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
   // changeType B->B and append B starting at 1.0 seconds (or at the first
   // keyframe in B at or after 1.0 seconds if it has keyframe_interval defined).
   test.waitForExpectedEvents(() => {
-    assert_less_than(lastStart, 1.0);
+    assert_less_than(lastStart, 1.0,
+        "changeType B->B lastStart must be less than 1.0");
     let safeOffset = findSafeOffset(1.0, metadataB, lastStart, metadataB);
     lastStart = safeOffset["adjustedTime"];
     if (!implicit_changetype) {
@@ -239,7 +245,8 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
   // changeType B->A and append A starting at 1.5 seconds (or at the first
   // keyframe in B at or after 1.5 seconds if it has keyframe_interval defined).
   test.waitForExpectedEvents(() => {
-    assert_less_than(lastStart, 1.5);
+    assert_less_than(lastStart, 1.5,
+        "changeType B->A lastStart must be less than 1.5");
     let safeOffset = findSafeOffset(1.5, metadataB, lastStart, metadataA);
     // Retain the previous lastStart because the next block will append data
     // which begins between that start time and this block's start time.
@@ -258,7 +265,8 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
   // changeType A->A and append A starting at 1.3 seconds (or at the first
   // keyframe in B at or after 1.3 seconds if it has keyframe_interval defined).
   test.waitForExpectedEvents(() => {
-    assert_less_than(lastStart, 1.3);
+    assert_less_than(lastStart, 1.3,
+        "changeType A->A lastStart must be less than 1.3");
     // Our next append will begin by overlapping some of metadataB, then some of
     // metadataA.
     let safeOffset = findSafeOffset(1.3, metadataB, lastStart, metadataA);
@@ -277,7 +285,7 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
   // Trim duration to 2 seconds, then play through to end.
   test.waitForExpectedEvents(() => {
     // If negative testing, then skip fragile assertions.
-    trimBuffered(test, mediaElement, sourceBuffer, 2.1, 2, negative_test);
+    trimBuffered(test, mediaSource, sourceBuffer, 2.1, 2, negative_test);
   });
 
   test.waitForExpectedEvents(() => {
@@ -286,7 +294,7 @@ function runChangeTypeTest(test, mediaElement, mediaSource, metadataA, typeA, da
   });
 
   test.waitForExpectedEvents(() => {
-    assert_equals(mediaElement.currentTime, 0);
+    assert_equals(mediaElement.currentTime, 0, "currentTime must be 0");
     test.expectEvent(mediaSource, "sourceended");
     test.expectEvent(mediaElement, "play");
     test.expectEvent(mediaElement, "ended");