Skip to content

Commit

Permalink
MSE-in-Workers: Switch getHandle() to simpler [SameObject] handle attr
Browse files Browse the repository at this point in the history
Following discussion in the spec PR #306 [1], this change updates the
way to obtain a MediaSourceHandle to be via a "handle" attribute on the
MediaSource instance that is both readonly and always returns the same
object (or throws exception, if for instance, it is attempted to be read
from a main-thread-owned MediaSource instance instead of a dedicated-
worker-owned MediaSource instance).

Also included is the removal of the readyState check when attempting to
obtain this handle (since it is now never expected to be changeable; no
sequence of distinct handles is ever expected to be obtainable from a
worker MediaSource). Also removed is the prevention of retrieving such a
handle from an instance more than once.

Multiple tests are added or updated to ensure correct behavior.

[1] w3c/media-source#306

BUG=878133

Change-Id: Ic07095d6d1dc95b8e6be818027984600aa7ab334
  • Loading branch information
wolenetz authored and chromium-wpt-export-bot committed Jul 13, 2022
1 parent f5f78fe commit a8bf0c5
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ util.mediaSource.addEventListener("sourceopen", () => {
err => { postMessage({ subject: messageSubject.ERROR, info: err }) } );
}, { once : true });

let handle = util.mediaSource.getHandle();
let handle = util.mediaSource.handle;

postMessage({ subject: messageSubject.HANDLE, info: handle }, { transfer: [handle] } );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,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;
let handle = util.mediaSource.getHandle();
let handle = util.mediaSource.handle;
postMessage({ subject: messageSubject.HANDLE, info: handle }, { transfer: [handle] } );
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ importScripts('mediasource-message-util.js');
// harness, and would confuse the test case message parsing there.

// Just obtain a MediaSourceHandle and transfer it to creator of our context.
let handle = new MediaSource().getHandle();
let handle = new MediaSource().handle;
postMessage(
{subject: messageSubject.HANDLE, info: handle}, {transfer: [handle]});
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
importScripts('/resources/testharness.js');

test(t => {
let handle = new MediaSource().getHandle();
let handle = new MediaSource().handle;
assert_true(handle instanceof MediaSourceHandle);
assert_throws_dom('DataCloneError', function() {
postMessage(handle);
}, 'serializing handle without transfer');
}, 'MediaSourceHandle serialization without transfer must fail, tested in worker');

test(t => {
let handle = new MediaSource().getHandle();
let handle = new MediaSource().handle;
assert_true(handle instanceof MediaSourceHandle);
assert_throws_dom('DataCloneError', function() {
postMessage(handle, [handle, handle]);
Expand Down
11 changes: 11 additions & 0 deletions media-source/dedicated-worker/mediasource-worker-handle.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@
});
}, "Test main context receipt of postMessage'd MediaSourceHandle from DedicatedWorker MediaSource");

test(t => {
assert_true(window.hasOwnProperty("MediaSourceHandle"), "window must have MediaSourceHandle visibility");

// Note, MSE spec may eventually describe how a main-thread MediaSource can
// attach to an HTMLMediaElement using a MediaSourceHandle. For now, we
// ensure that the implementation of this is not available.
assert_throws_dom('NotSupportedError', function() {
let h = new MediaSource().handle;
}, 'main thread MediaSource instance cannot (yet) create a usable MediaSourceHandle');
}, "Test main-thread-owned MediaSource instance cannot create a MediaSourceHandle");

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
Expand Down
57 changes: 41 additions & 16 deletions media-source/dedicated-worker/mediasource-worker-handle.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ test(t => {
}, "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(
'handle' in MediaSource.prototype,
'dedicated worker MediaSource must have handle in prototype');
assert_true(self.hasOwnProperty("MediaSourceHandle"), "dedicated worker must have MediaSourceHandle visibility");
}, "MediaSource prototype in DedicatedWorker context must have getHandle, and worker must have MediaSourceHandle");
}, 'MediaSource prototype in DedicatedWorker context must have \'handle\', and worker must have MediaSourceHandle');

test(t => {
const ms = new MediaSource();
Expand All @@ -24,22 +26,45 @@ test(t => {

test(t => {
const ms = new MediaSource();
const handle = ms.getHandle();
assert_not_equals(handle, null, "must have a non-null getHandle result");
const handle = ms.handle;
assert_not_equals(handle, null, 'must have a non-null \'handle\' attribute');
assert_true(handle instanceof MediaSourceHandle, "must be a MediaSourceHandle");
}, "mediaSource.getHandle() in DedicatedWorker returns a MediaSourceHandle");
}, 'mediaSource.handle 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");
const msA = new MediaSource();
const msB = new MediaSource();
const handleA1 = msA.handle;
const handleA2 = msA.handle;
const handleA3 = msA['handle'];
const handleB1 = msB.handle;
const handleB2 = msB.handle;
assert_true(
handleA1 === handleA2 && handleB1 === handleB2 && handleA1 != handleB1,
'SameObject is observed for mediaSource.handle, and different MediaSource instances have different handles');
assert_true(
handleA1 === handleA3,
'SameObject is observed even when accessing handle differently');
assert_true(
handleA1 instanceof MediaSourceHandle &&
handleB1 instanceof MediaSourceHandle,
'handle property returns MediaSourceHandles');
}, 'mediaSource.handle observes SameObject property correctly');

test(t => {
const ms1 = new MediaSource();
const handle1 = ms1.handle;
const ms2 = new MediaSource();
const handle2 = ms2.handle;
assert_true(
handle1 !== handle2,
'distinct MediaSource instances must have distinct handles');

// Verify attempt to change value of the handle property does not succeed.
ms1.handle = handle2;
assert_true(
ms1.handle === handle1 && ms2.handle === handle2,
'MediaSource handle is readonly, so should not have changed');
}, 'Attempt to set MediaSource handle property should fail to change it, since it is readonly');

done();
34 changes: 31 additions & 3 deletions media-source/dedicated-worker/mediasource-worker-play.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,37 @@ onmessage = function(evt) {
};

let util = new MediaSourceWorkerUtil();
let handle = util.mediaSource.handle;

util.mediaSource.addEventListener('sourceopen', () => {
// Immediately re-verify the SameObject property of the handle we transferred.
if (handle !== util.mediaSource.handle) {
postMessage({
subject: messageSubject.ERROR,
info: 'mediaSource.handle changed from the original value'
});
}

// Also verify that transferring the already-transferred handle instance is
// prevented correctly.
try {
postMessage(
{
subject: messageSubject.ERROR,
info:
'This postMessage should fail: the handle has already been transferred',
extra_info: util.mediaSource.handle
},
{transfer: [util.mediaSource.handle]});
} catch (e) {
if (e.name != 'DataCloneError') {
postMessage({
subject: messageSubject.ERROR,
info: 'Expected handle retransfer exception did not occur'
});
}
}

util.mediaSource.addEventListener("sourceopen", () => {
sourceBuffer = util.mediaSource.addSourceBuffer(util.mediaMetadata.type);
sourceBuffer.onerror = (err) => {
postMessage({ subject: messageSubject.ERROR, info: err });
Expand Down Expand Up @@ -40,7 +69,6 @@ util.mediaSource.addEventListener("sourceopen", () => {
};
util.mediaLoadPromise.then(mediaData => { sourceBuffer.appendBuffer(mediaData); },
err => { postMessage({ subject: messageSubject.ERROR, info: err }) });
}, { once : true });
}, {once: true});

let handle = util.mediaSource.getHandle();
postMessage({ subject: messageSubject.HANDLE, info: handle }, { transfer: [handle] });

0 comments on commit a8bf0c5

Please sign in to comment.