Revert "MSE-in-Workers: srcObject part 5: Conditionally fail worker objectURL"
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:
> https://github.com/w3c/media-source/issues/175
> MSE spec feature updates switching from worker MSE attachment via
> object URL to attachment via srcObject MediaSourceHandle:
> * https://github.com/w3c/media-source/pull/305
> * further clarifications in discussion at
> https://github.com/w3c/media-source/pull/306#issuecomment-1144180822
>
> BUG=878133
>
> Change-Id: I60ddca79ee0f95c87b6d84e4f44ad9c283f359a3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3698231
> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
> Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
> Reviewed-by: Will Cassella <cassew@chromium.org>
> 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 <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Nidhi Jaju <nidhijaju@chromium.org>
Owners-Override: Nidhi Jaju <nidhijaju@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1012823}
diff --git a/media-source/dedicated-worker/mediasource-message-util.js b/media-source/dedicated-worker/mediasource-message-util.js
index c62eb8e..247071d 100644
--- 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
index 0f74d95..7b00c59 100644
--- 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
index 10c2f84..a4df32e 100644
--- 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 sentStartedBufferingMessage = false;
util.mediaSource.addEventListener("sourceopen", () => {
+ URL.revokeObjectURL(util.mediaSourceObjectUrl);
let sourceBuffer;
try {
sourceBuffer = util.mediaSource.addSourceBuffer(util.mediaMetadata.type);
@@ -32,7 +33,7 @@
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
index c195775..2cb834a 100644
--- 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
index e490134..8060594 100644
--- a/media-source/dedicated-worker/mediasource-worker-duration.js
+++ b/media-source/dedicated-worker/mediasource-worker-duration.js
@@ -16,9 +16,9 @@
// 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 @@
// 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 @@
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
deleted file mode 100644
index e9a5af6..0000000
--- a/media-source/dedicated-worker/mediasource-worker-get-objecturl.js
+++ /dev/null
@@ -1,13 +0,0 @@
-importScripts("mediasource-worker-util.js");
-
-// Note, we do not use testharness.js utilities within the worker context
-// because it also communicates using postMessage to the main HTML document's
-// harness, and would confuse the test case message parsing there.
-
-onmessage = function(evt) {
- postMessage({ subject: messageSubject.ERROR, info: "No message expected by Worker"});
-};
-
-let util = new MediaSourceWorkerUtil();
-
-postMessage({ subject: messageSubject.OBJECT_URL, info: URL.createObjectURL(util.mediaSource) });
diff --git a/media-source/dedicated-worker/mediasource-worker-handle.html b/media-source/dedicated-worker/mediasource-worker-handle.html
deleted file mode 100644
index b084fb6..0000000
--- a/media-source/dedicated-worker/mediasource-worker-handle.html
+++ /dev/null
@@ -1,48 +0,0 @@
-<!DOCTYPE html>
-<html>
-<title>Test MediaSource object and handle creation, 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>
-<script>
-
-async_test(t => {
- // Fail fast if MSE-in-Workers is not supported.
- assert_true(MediaSource.hasOwnProperty("canConstructInDedicatedWorker"), "MediaSource hasOwnProperty 'canConstructInDedicatedWorker'");
- assert_true(MediaSource.canConstructInDedicatedWorker, "MediaSource.canConstructInDedicatedWorker");
- assert_true(window.hasOwnProperty("MediaSourceHandle"), "window must have MediaSourceHandle visibility");
-
- 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");
- switch (subject) {
- case messageSubject.ERROR:
- assert_unreached("Worker error: " + e.data.info);
- break;
- case messageSubject.HANDLE:
- const handle = e.data.info;
- assert_not_equals(handle, null, "must have a non-null MediaSourceHandle");
- assert_true(handle instanceof MediaSourceHandle, "must be a MediaSourceHandle");
- t.done();
- break;
- default:
- assert_unreached("Unexpected message subject: " + subject);
-
- }
- });
-}, "Test main context receipt of postMessage'd MediaSourceHandle from DedicatedWorker MediaSource");
-
-if (MediaSource.hasOwnProperty("canConstructInDedicatedWorker") && MediaSource.canConstructInDedicatedWorker === true) {
- // If implementation claims support for MSE-in-Workers, then fetch and run
- // some tests directly in another dedicated worker and get their results
- // merged into those from this page.
- fetch_tests_from_worker(new Worker("mediasource-worker-handle.js"));
-} else {
- // Otherwise, fetch and run a test that verifies lack of support of
- // MediaSource construction in another dedicated worker.
- fetch_tests_from_worker(new Worker("mediasource-worker-must-fail-if-unsupported.js"));
-}
-
-</script>
-</html>
diff --git a/media-source/dedicated-worker/mediasource-worker-handle.js b/media-source/dedicated-worker/mediasource-worker-handle.js
deleted file mode 100644
index 577b1fa..0000000
--- a/media-source/dedicated-worker/mediasource-worker-handle.js
+++ /dev/null
@@ -1,45 +0,0 @@
-importScripts("/resources/testharness.js");
-
-test(t => {
- // The Window test html conditionally fetches and runs these tests only if the
- // implementation exposes a true-valued static canConstructInDedicatedWorker
- // attribute on MediaSource in the Window context. So, the implementation must
- // agree on support here in the dedicated worker context.
-
- // Ensure we're executing in a dedicated worker context.
- assert_true(self instanceof DedicatedWorkerGlobalScope, "self instanceof DedicatedWorkerGlobalScope");
- assert_true(MediaSource.hasOwnProperty("canConstructInDedicatedWorker", "DedicatedWorker MediaSource hasOwnProperty 'canConstructInDedicatedWorker'"));
- assert_true(MediaSource.canConstructInDedicatedWorker, "DedicatedWorker MediaSource.canConstructInDedicatedWorker");
-}, "MediaSource in DedicatedWorker context must have true-valued canConstructInDedicatedWorker if Window context had it");
-
-test(t => {
- assert_true("getHandle" in MediaSource.prototype, "dedicated worker MediaSource must have getHandle");
- assert_true(self.hasOwnProperty("MediaSourceHandle"), "dedicated worker must have MediaSourceHandle visibility");
-}, "MediaSource prototype in DedicatedWorker context must have getHandle, and worker must have MediaSourceHandle");
-
-test(t => {
- const ms = new MediaSource();
- assert_equals(ms.readyState, "closed");
-}, "MediaSource construction succeeds with initial closed readyState in DedicatedWorker");
-
-test(t => {
- const ms = new MediaSource();
- const handle = ms.getHandle();
- assert_not_equals(handle, null, "must have a non-null getHandle result");
- assert_true(handle instanceof MediaSourceHandle, "must be a MediaSourceHandle");
-}, "mediaSource.getHandle() in DedicatedWorker returns a MediaSourceHandle");
-
-test(t => {
- const ms = new MediaSource();
- const handle1 = ms.getHandle();
- let handle2 = null;
- assert_throws_dom("InvalidStateError", function()
- {
- handle2 = ms.getHandle();
- }, "getting second handle from MediaSource instance");
- assert_equals(handle2, null, "getting second handle from same MediaSource must have failed");
- assert_not_equals(handle1, null, "must have a non-null result of the first getHandle");
- assert_true(handle1 instanceof MediaSourceHandle, "first getHandle result must be a MediaSourceHandle");
-}, "mediaSource.getHandle() must not succeed more than precisely once for a MediaSource instance");
-
-done();
diff --git a/media-source/dedicated-worker/mediasource-worker-objecturl.html b/media-source/dedicated-worker/mediasource-worker-objecturl.html
index ae60199..5553b5c 100644
--- 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
index 2e70d99..9a7195f 100644
--- a/media-source/dedicated-worker/mediasource-worker-objecturl.js
+++ b/media-source/dedicated-worker/mediasource-worker-objecturl.js
@@ -20,7 +20,9 @@
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
index d6496af..ca8b697 100644
--- 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
index 0292b13..07317e6 100644
--- 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
index e29b1b8..0312f16 100644
--- a/media-source/dedicated-worker/mediasource-worker-play.js
+++ b/media-source/dedicated-worker/mediasource-worker-play.js
@@ -11,6 +11,7 @@
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 @@
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
index 7adaf82..695d117 100644
--- a/media-source/dedicated-worker/mediasource-worker-util.js
+++ b/media-source/dedicated-worker/mediasource-worker-util.js
@@ -22,6 +22,7 @@
class MediaSourceWorkerUtil {
constructor() {
this.mediaSource = new MediaSource();
+ this.mediaSourceObjectUrl = URL.createObjectURL(this.mediaSource);
// Find supported test media, if any.
this.foundSupportedMedia = false;
