Skip to content

Commit 79ffad6

Browse files
wolenetzchromium-wpt-export-bot
authored andcommitted
MSE-in-Workers: Switch getHandle() to simpler [SameObject] handle attr
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. BUG=878133 Change-Id: Ic07095d6d1dc95b8e6be818027984600aa7ab334
1 parent 2829b7f commit 79ffad6

7 files changed

+88
-24
lines changed

media-source/dedicated-worker/mediasource-worker-detach-element.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ util.mediaSource.addEventListener("sourceopen", () => {
3232
err => { postMessage({ subject: messageSubject.ERROR, info: err }) } );
3333
}, { once : true });
3434

35-
let handle = util.mediaSource.getHandle();
35+
let handle = util.mediaSource.handle;
3636

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

media-source/dedicated-worker/mediasource-worker-duration.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ function processPhase(isResponseToAck = false) {
180180
case testPhase.kInitial:
181181
assert(Number.isNaN(util.mediaSource.duration), "Initial unattached MediaSource duration must be NaN, but instead is " + util.mediaSource.duration);
182182
phase = testPhase.kAttaching;
183-
let handle = util.mediaSource.getHandle();
183+
let handle = util.mediaSource.handle;
184184
postMessage({ subject: messageSubject.HANDLE, info: handle }, { transfer: [handle] } );
185185
break;
186186

media-source/dedicated-worker/mediasource-worker-handle-transfer-to-main.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ importScripts('mediasource-message-util.js');
55
// harness, and would confuse the test case message parsing there.
66

77
// Just obtain a MediaSourceHandle and transfer it to creator of our context.
8-
let handle = new MediaSource().getHandle();
8+
let handle = new MediaSource().handle;
99
postMessage(
1010
{subject: messageSubject.HANDLE, info: handle}, {transfer: [handle]});

media-source/dedicated-worker/mediasource-worker-handle-transfer.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
importScripts('/resources/testharness.js');
22

33
test(t => {
4-
let handle = new MediaSource().getHandle();
4+
let handle = new MediaSource().handle;
55
assert_true(handle instanceof MediaSourceHandle);
66
assert_throws_dom('DataCloneError', function() {
77
postMessage(handle);
88
}, 'serializing handle without transfer');
99
}, 'MediaSourceHandle serialization without transfer must fail, tested in worker');
1010

1111
test(t => {
12-
let handle = new MediaSource().getHandle();
12+
let handle = new MediaSource().handle;
1313
assert_true(handle instanceof MediaSourceHandle);
1414
assert_throws_dom('DataCloneError', function() {
1515
postMessage(handle, [handle, handle]);

media-source/dedicated-worker/mediasource-worker-handle.html

+11
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@
3333
});
3434
}, "Test main context receipt of postMessage'd MediaSourceHandle from DedicatedWorker MediaSource");
3535

36+
test(t => {
37+
assert_true(window.hasOwnProperty("MediaSourceHandle"), "window must have MediaSourceHandle visibility");
38+
39+
// Note, MSE spec may eventually describe how a main-thread MediaSource can
40+
// attach to an HTMLMediaElement using a MediaSourceHandle. For now, we
41+
// ensure that the implementation of this is not available.
42+
assert_throws_dom('NotSupportedError', function() {
43+
let h = new MediaSource().handle;
44+
}, 'main thread MediaSource instance cannot (yet) create a usable MediaSourceHandle');
45+
}, "Test main-thread-owned MediaSource instance cannot create a MediaSourceHandle");
46+
3647
if (MediaSource.hasOwnProperty("canConstructInDedicatedWorker") && MediaSource.canConstructInDedicatedWorker === true) {
3748
// If implementation claims support for MSE-in-Workers, then fetch and run
3849
// some tests directly in another dedicated worker and get their results

media-source/dedicated-worker/mediasource-worker-handle.js

+41-16
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ test(t => {
1313
}, "MediaSource in DedicatedWorker context must have true-valued canConstructInDedicatedWorker if Window context had it");
1414

1515
test(t => {
16-
assert_true("getHandle" in MediaSource.prototype, "dedicated worker MediaSource must have getHandle");
16+
assert_true(
17+
'handle' in MediaSource.prototype,
18+
'dedicated worker MediaSource must have handle in prototype');
1719
assert_true(self.hasOwnProperty("MediaSourceHandle"), "dedicated worker must have MediaSourceHandle visibility");
18-
}, "MediaSource prototype in DedicatedWorker context must have getHandle, and worker must have MediaSourceHandle");
20+
}, 'MediaSource prototype in DedicatedWorker context must have \'handle\', and worker must have MediaSourceHandle');
1921

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

2527
test(t => {
2628
const ms = new MediaSource();
27-
const handle = ms.getHandle();
28-
assert_not_equals(handle, null, "must have a non-null getHandle result");
29+
const handle = ms.handle;
30+
assert_not_equals(handle, null, 'must have a non-null \'handle\' attribute');
2931
assert_true(handle instanceof MediaSourceHandle, "must be a MediaSourceHandle");
30-
}, "mediaSource.getHandle() in DedicatedWorker returns a MediaSourceHandle");
32+
}, 'mediaSource.handle in DedicatedWorker returns a MediaSourceHandle');
3133

3234
test(t => {
33-
const ms = new MediaSource();
34-
const handle1 = ms.getHandle();
35-
let handle2 = null;
36-
assert_throws_dom("InvalidStateError", function()
37-
{
38-
handle2 = ms.getHandle();
39-
}, "getting second handle from MediaSource instance");
40-
assert_equals(handle2, null, "getting second handle from same MediaSource must have failed");
41-
assert_not_equals(handle1, null, "must have a non-null result of the first getHandle");
42-
assert_true(handle1 instanceof MediaSourceHandle, "first getHandle result must be a MediaSourceHandle");
43-
}, "mediaSource.getHandle() must not succeed more than precisely once for a MediaSource instance");
35+
const msA = new MediaSource();
36+
const msB = new MediaSource();
37+
const handleA1 = msA.handle;
38+
const handleA2 = msA.handle;
39+
const handleA3 = msA['handle'];
40+
const handleB1 = msB.handle;
41+
const handleB2 = msB.handle;
42+
assert_true(
43+
handleA1 === handleA2 && handleB1 === handleB2 && handleA1 != handleB1,
44+
'SameObject is observed for mediaSource.handle, and different MediaSource instances have different handles');
45+
assert_true(
46+
handleA1 === handleA3,
47+
'SameObject is observed even when accessing handle differently');
48+
assert_true(
49+
handleA1 instanceof MediaSourceHandle &&
50+
handleB1 instanceof MediaSourceHandle,
51+
'handle property returns MediaSourceHandles');
52+
}, 'mediaSource.handle observes SameObject property correctly');
53+
54+
test(t => {
55+
const ms1 = new MediaSource();
56+
const handle1 = ms1.handle;
57+
const ms2 = new MediaSource();
58+
const handle2 = ms2.handle;
59+
assert_true(
60+
handle1 !== handle2,
61+
'distinct MediaSource instances must have distinct handles');
62+
63+
// Verify attempt to change value of the handle property does not succeed.
64+
ms1.handle = handle2;
65+
assert_true(
66+
ms1.handle === handle1 && ms2.handle === handle2,
67+
'MediaSource handle is readonly, so should not have changed');
68+
}, 'Attempt to set MediaSource handle property should fail to change it, since it is readonly');
4469

4570
done();

media-source/dedicated-worker/mediasource-worker-play.js

+31-3
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,37 @@ onmessage = function(evt) {
99
};
1010

1111
let util = new MediaSourceWorkerUtil();
12+
let handle = util.mediaSource.handle;
13+
14+
util.mediaSource.addEventListener('sourceopen', () => {
15+
// Immediately re-verify the SameObject property of the handle we transferred.
16+
if (handle !== util.mediaSource.handle) {
17+
postMessage({
18+
subject: messageSubject.ERROR,
19+
info: 'mediaSource.handle changed from the original value'
20+
});
21+
}
22+
23+
// Also verify that transferring the already-transferred handle instance is
24+
// prevented correctly.
25+
try {
26+
postMessage(
27+
{
28+
subject: messageSubject.ERROR,
29+
info:
30+
'This postMessage should fail: the handle has already been transferred',
31+
extra_info: util.mediaSource.handle
32+
},
33+
{transfer: [util.mediaSource.handle]});
34+
} catch (e) {
35+
if (e.name != 'DataCloneError') {
36+
postMessage({
37+
subject: messageSubject.ERROR,
38+
info: 'Expected handle retransfer exception did not occur'
39+
});
40+
}
41+
}
1242

13-
util.mediaSource.addEventListener("sourceopen", () => {
1443
sourceBuffer = util.mediaSource.addSourceBuffer(util.mediaMetadata.type);
1544
sourceBuffer.onerror = (err) => {
1645
postMessage({ subject: messageSubject.ERROR, info: err });
@@ -40,7 +69,6 @@ util.mediaSource.addEventListener("sourceopen", () => {
4069
};
4170
util.mediaLoadPromise.then(mediaData => { sourceBuffer.appendBuffer(mediaData); },
4271
err => { postMessage({ subject: messageSubject.ERROR, info: err }) });
43-
}, { once : true });
72+
}, {once: true});
4473

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

0 commit comments

Comments
 (0)