Revert "MSE-in-Workers: srcObject part 5: Conditionally fail worker o…

…bjectURL" This reverts commit 6315549b8c2ece3dbbf3062c1a87347589a5e115. Reason for revert: This is causing failures on the WebKit Linux Leak builder i.e. https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Linux%20Leak/39394/overview Original change's description: > MSE-in-Workers: srcObject part 5: Conditionally fail worker objectURL > > If the MediaSourceInWorkersUsingHandle feature is enabled, this change > prevents successful ability of obtaining an objectURL that would succeed > in loading a worker-owned MediaSource. > > It changes the wpt tests to use handle for attachment and verifies > expected new behavior of getHandle and that worker objectURL attachment > fails (these tests run on experimental builds of Chromium with > currently-experimental MediaSourceInWorkersUsingHandle feature enabled, > just like the currently-experimental MediaSourceInWorkers feature.) > > References: > Full prototype CL for the parts 1-4 that have already landed: > https://chromium-review.googlesource.com/c/chromium/src/+/3515334 > MSE spec issue: > w3c/media-source#175 > MSE spec feature updates switching from worker MSE attachment via > object URL to attachment via srcObject MediaSourceHandle: > * w3c/media-source#305 > * further clarifications in discussion at > w3c/media-source#306 (comment) > > BUG=878133 > > Change-Id: I60ddca79ee0f95c87b6d84e4f44ad9c283f359a3 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3698231 > Commit-Queue: Matthew Wolenetz <[email protected]> > Auto-Submit: Matthew Wolenetz <[email protected]> > Reviewed-by: Will Cassella <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1012712} Bug: 878133 Change-Id: I1e405ae1de146d1f3183592b00a43bd3c38d849d No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3695890 Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Nidhi Jaju <[email protected]> Owners-Override: Nidhi Jaju <[email protected]> Cr-Commit-Position: refs/heads/main@{#1012823}
web-platform-tests · Jun 10, 2022 · 9589b7d · 9589b7d
1 parent 65f6556
commit 9589b7d
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 146 deletions.
diff --git a/media-source/dedicated-worker/mediasource-message-util.js b/media-source/dedicated-worker/mediasource-message-util.js
@@ -4,7 +4,6 @@
 const messageSubject = {
   ERROR: "error",  // info field may contain more detail
   OBJECT_URL: "object url", // info field contains object URL
-  HANDLE: "handle", // info field contains the MediaSourceHandle
   STARTED_BUFFERING: "started buffering",
   FINISHED_BUFFERING: "finished buffering",
   VERIFY_DURATION: "verify duration", // info field contains expected duration

diff --git a/media-source/dedicated-worker/mediasource-worker-detach-element.html b/media-source/dedicated-worker/mediasource-worker-detach-element.html
@@ -7,11 +7,11 @@
 <body>
 <script>
 
-const AFTER_SETTING_SRCOBJECT = "after setting srcObject";
+const AFTER_SETTING_SRC = "after setting src";
 const AFTER_STARTED_BUFFERING = "after receiving Started Buffering message from worker";
 const AFTER_FINISHED_BUFFERING = "after receiving Finished Buffering message from worker";
 
-[ AFTER_SETTING_SRCOBJECT, AFTER_STARTED_BUFFERING, AFTER_FINISHED_BUFFERING ].forEach(when => {
+[ AFTER_SETTING_SRC, AFTER_STARTED_BUFFERING, AFTER_FINISHED_BUFFERING ].forEach(when => {
   for (let timeouts = 0; timeouts < 5; ++timeouts) {
     async_test(test => { startWorkerAndDetachElement(test, when, timeouts); },
         "Test element detachment from worker MediaSource after at least " + timeouts +
@@ -22,8 +22,9 @@
 function detachElementAfterMultipleSetTimeouts(test, element, timeouts_remaining) {
   if (timeouts_remaining <= 0) {
     // While not the best way to detach, this triggers interoperable logic that
-    // includes detachment.
-    element.srcObject = null;
+    // includes detachment, though also begins a failed load(). We don't handle
+    // video error event, so the latter is not an issue in this test.
+    element.src='';
     test.step_timeout(() => { test.done(); }, 10);
   } else {
     test.step_timeout(() => {
@@ -50,10 +51,11 @@
       case messageSubject.ERROR:
         assert_unreached("Worker error: " + e.data.info);
         break;
-      case messageSubject.HANDLE:
-        const handle = e.data.info;
-        video.srcObject = handle;
-        if (when_to_start_timeouts == AFTER_SETTING_SRCOBJECT) {
+      case messageSubject.OBJECT_URL:
+        const url = e.data.info;
+        assert_true(url.match(/^blob:.+/) != null);
+        video.src = url;
+        if (when_to_start_timeouts == AFTER_SETTING_SRC) {
           detachElementAfterMultipleSetTimeouts(test, video, timeouts_to_await);
         }
         break;

diff --git a/media-source/dedicated-worker/mediasource-worker-detach-element.js b/media-source/dedicated-worker/mediasource-worker-detach-element.js
@@ -14,6 +14,7 @@ let util = new MediaSourceWorkerUtil();
 let sentStartedBufferingMessage = false;
 
 util.mediaSource.addEventListener("sourceopen", () => {
+  URL.revokeObjectURL(util.mediaSourceObjectUrl);
   let sourceBuffer;
   try {
     sourceBuffer = util.mediaSource.addSourceBuffer(util.mediaMetadata.type);
@@ -32,7 +33,7 @@ util.mediaSource.addEventListener("sourceopen", () => {
                              err => { postMessage({ subject: messageSubject.ERROR, info: err }) } );
 }, { once : true });
 
-postMessage({ subject: messageSubject.HANDLE, info: util.mediaSource.getHandle() } );
+postMessage({ subject: messageSubject.OBJECT_URL, info: util.mediaSourceObjectUrl} );
 
 // Append increasingly large pieces at a time, starting/continuing at |position|.
 // This allows buffering the test media without timeout, but also with enough

diff --git a/media-source/dedicated-worker/mediasource-worker-duration.html b/media-source/dedicated-worker/mediasource-worker-duration.html
@@ -46,12 +46,13 @@
       case messageSubject.ERROR:
         assert_unreached("Worker error: " + e.data.info);
         break;
-      case messageSubject.HANDLE:
-        const handle = e.data.info;
+      case messageSubject.OBJECT_URL:
+        const url = e.data.info;
+        assert_true(url.match(/^blob:.+/) !== null);
         assert_equals(video.duration, NaN, "initial video duration before attachment should still be NaN");
         assert_equals(video.readyState, HTMLMediaElement.HAVE_NOTHING,
                       "initial video readyState before attachment should still be HAVE_NOTHING");
-        video.srcObject = handle;
+        video.src = url;
         break;
       case messageSubject.VERIFY_DURATION:
         assert_equals(video.duration, e.data.info, "duration should match expectation");
@@ -72,7 +73,7 @@
         // This test is a worker-driven set of verifications, and it will send
         // this message when it is complete. See comment in the worker script
         // that describes the phases of this test case.
-        assert_not_equals(video.srcObject, null, "test should at least have set srcObject.");
+        assert_not_equals(video.src, "", "test should at least have set src.");
         t.done();
         break;
       default:

diff --git a/media-source/dedicated-worker/mediasource-worker-duration.js b/media-source/dedicated-worker/mediasource-worker-duration.js
@@ -16,9 +16,9 @@ const testPhase = {
   // main thread.
   kInitial: "Initial",
 
-  // Main thread receives MediaSourceHandle, re-verifies that the media element
+  // Main thread receives object URL, re-verifies that the media element
   // duration is still NaN and readyState is still HAVE_NOTHING, and then sets
-  // the handle as the srcObject of the media element, eventually causing worker
+  // the URL as the src of the media element, eventually causing worker
   // mediaSource 'onsourceopen' event dispatch.
   kAttaching: "Awaiting sourceopen event that signals attachment is setup",
 
@@ -58,6 +58,7 @@ let phase = testPhase.kInitial;
 
 // Setup handler for receipt of attachment completion.
 util.mediaSource.addEventListener("sourceopen", () => {
+  URL.revokeObjectURL(util.mediaSourceObjectUrl);
   assert(phase === testPhase.kAttaching, "Unexpected sourceopen received by Worker mediaSource.");
   phase = testPhase.kRequestNaNDurationCheck;
   processPhase();
@@ -180,7 +181,7 @@ function processPhase(isResponseToAck = false) {
     case testPhase.kInitial:
       assert(Number.isNaN(util.mediaSource.duration), "Initial unattached MediaSource duration must be NaN, but instead is " + util.mediaSource.duration);
       phase = testPhase.kAttaching;
-      postMessage({ subject: messageSubject.HANDLE, info: util.mediaSource.getHandle() });
+      postMessage({ subject: messageSubject.OBJECT_URL, info: util.mediaSourceObjectUrl });
       break;
 
     case testPhase.kAttaching:

diff --git a/media-source/dedicated-worker/mediasource-worker-get-objecturl.js b/media-source/dedicated-worker/mediasource-worker-get-objecturl.js
diff --git a/media-source/dedicated-worker/mediasource-worker-handle.html b/media-source/dedicated-worker/mediasource-worker-handle.html
diff --git a/media-source/dedicated-worker/mediasource-worker-handle.js b/media-source/dedicated-worker/mediasource-worker-handle.js
diff --git a/media-source/dedicated-worker/mediasource-worker-objecturl.html b/media-source/dedicated-worker/mediasource-worker-objecturl.html
@@ -1,6 +1,6 @@
 <!DOCTYPE html>
 <html>
-<title>Test MediaSource object and objectUrl creation and load via that url should fail, with MediaSource in dedicated worker</title>
+<title>Test MediaSource object and objectUrl creation and revocation, with MediaSource in dedicated worker</title>
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="mediasource-message-util.js"></script>
@@ -11,7 +11,7 @@
   assert_true(MediaSource.hasOwnProperty("canConstructInDedicatedWorker"), "MediaSource hasOwnProperty 'canConstructInDedicatedWorker'");
   assert_true(MediaSource.canConstructInDedicatedWorker, "MediaSource.canConstructInDedicatedWorker");
 
-  let worker = new Worker("mediasource-worker-get-objecturl.js");
+  let worker = new Worker("mediasource-worker-play.js");
   worker.onmessage = t.step_func(e => {
     let subject = e.data.subject;
     assert_true(subject != undefined, "message must have a subject field");
@@ -21,21 +21,19 @@
         break;
       case messageSubject.OBJECT_URL:
         const url = e.data.info;
-        const video = document.createElement("video");
-        document.body.appendChild(video);
-        video.onerror = t.step_func((target) => {
-          assert_true(video.error != null);
-          assert_equals(video.error.code, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
-          t.done();
-        });
-        video.onended = t.unreached_func("video should not have successfully loaded and played to end");
-        video.src = url;
+        assert_true(url.match(/^blob:.+/) != null);
+        URL.revokeObjectURL(url);
+        // TODO(crbug.com/1196040): Revoking MediaSource objectURLs on thread
+        // that didn't create them is at best a no-op. This test case is
+        // possibly obsolete.
+        t.done();
         break;
       default:
         assert_unreached("Unexpected message subject: " + subject);
+
     }
   });
-}, "Test main context load of a DedicatedWorker MediaSource object URL should fail");
+}, "Test main context revocation of DedicatedWorker MediaSource object URL");
 
 if (MediaSource.hasOwnProperty("canConstructInDedicatedWorker") && MediaSource.canConstructInDedicatedWorker === true) {
   // If implementation claims support for MSE-in-Workers, then fetch and run

diff --git a/media-source/dedicated-worker/mediasource-worker-objecturl.js b/media-source/dedicated-worker/mediasource-worker-objecturl.js
@@ -20,7 +20,9 @@ test(t => {
 test(t => {
   const ms = new MediaSource();
   const url = URL.createObjectURL(ms);
-}, "URL.createObjectURL(mediaSource) in DedicatedWorker does not throw exception");
+  assert_true(url != null);
+  assert_true(url.match(/^blob:.+/) != null);
+}, "URL.createObjectURL(mediaSource) in DedicatedWorker returns a Blob URI");
 
 test(t => {
   const ms = new MediaSource();

diff --git a/media-source/dedicated-worker/mediasource-worker-play-terminate-worker.html b/media-source/dedicated-worker/mediasource-worker-play-terminate-worker.html
@@ -40,7 +40,7 @@
     video.loop = true;
   }
 
-  if (when_to_start_timeouts == "before setting srcObject") {
+  if (when_to_start_timeouts == "before setting src") {
     terminateWorkerAfterMultipleSetTimeouts(test, worker, timeouts_to_await);
   }
 
@@ -51,10 +51,11 @@
       case messageSubject.ERROR:
         assert_unreached("Worker error: " + e.data.info);
         break;
-      case messageSubject.HANDLE:
-        const handle = e.data.info;
-        video.srcObject = handle;
-        if (when_to_start_timeouts == "after setting srcObject") {
+      case messageSubject.OBJECT_URL:
+        const url = e.data.info;
+        assert_true(url.match(/^blob:.+/) != null);
+        video.src = url;
+        if (when_to_start_timeouts == "after setting src") {
           terminateWorkerAfterMultipleSetTimeouts(test, worker, timeouts_to_await);
         }
         video.play().catch(error => {
@@ -72,7 +73,7 @@
   });
 }
 
-[ "before setting srcObject", "after setting srcObject", "after first ended event" ].forEach(when => {
+[ "before setting src", "after setting src", "after first ended event" ].forEach(when => {
   for (let timeouts = 0; timeouts < 10; ++timeouts) {
     async_test(test => { startWorkerAndTerminateWorker(test, when, timeouts); },
         "Test worker MediaSource termination after at least " + timeouts +

diff --git a/media-source/dedicated-worker/mediasource-worker-play.html b/media-source/dedicated-worker/mediasource-worker-play.html
@@ -26,9 +26,10 @@
       case messageSubject.ERROR:
         assert_unreached("Worker error: " + e.data.info);
         break;
-      case messageSubject.HANDLE:
-        const handle = e.data.info;
-        video.srcObject = handle;
+      case messageSubject.OBJECT_URL:
+        const url = e.data.info;
+        assert_true(url.match(/^blob:.+/) != null);
+        video.src = url;
         video.play();
         break;
       default:

diff --git a/media-source/dedicated-worker/mediasource-worker-play.js b/media-source/dedicated-worker/mediasource-worker-play.js
@@ -11,6 +11,7 @@ onmessage = function(evt) {
 let util = new MediaSourceWorkerUtil();
 
 util.mediaSource.addEventListener("sourceopen", () => {
+  URL.revokeObjectURL(util.mediaSourceObjectUrl);
   sourceBuffer = util.mediaSource.addSourceBuffer(util.mediaMetadata.type);
   sourceBuffer.onerror = (err) => {
     postMessage({ subject: messageSubject.ERROR, info: err });
@@ -42,4 +43,4 @@ util.mediaSource.addEventListener("sourceopen", () => {
                              err => { postMessage({ subject: messageSubject.ERROR, info: err }) });
 }, { once : true });
 
-postMessage({ subject: messageSubject.HANDLE, info: util.mediaSource.getHandle() });
+postMessage({ subject: messageSubject.OBJECT_URL, info: util.mediaSourceObjectUrl });
diff --git a/media-source/dedicated-worker/mediasource-worker-util.js b/media-source/dedicated-worker/mediasource-worker-util.js
@@ -22,6 +22,7 @@ let MEDIA_LIST = [
 class MediaSourceWorkerUtil {
   constructor() {
     this.mediaSource = new MediaSource();
+    this.mediaSourceObjectUrl = URL.createObjectURL(this.mediaSource);
 
     // Find supported test media, if any.
     this.foundSupportedMedia = false;