mediaKeySystemAccess.createMediaKeys() fails for dynamically generated iframes
Categories
(Core :: Audio/Video, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox83 | --- | wontfix |
firefox84 | --- | wontfix |
firefox85 | --- | fixed |
People
(Reporter: selcukg, Assigned: bryce)
References
(Depends on 1 open bug, Blocks 1 open bug, Regressed 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(3 files)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.183 Safari/537.36
Steps to reproduce:
- Create an iframe with src set to javacript:false
- Populate content of iframe with script that calls createMediaKeys
Actual results:
createMediaKeys fails with DOMException: Couldn't get channel in MediaKeys::Init
(The following change may have caused this regression:
https://github.com/mozilla/gecko-dev/commit/1a16e898b29ff439b3e82d6d74dc8fbc6a221666#diff-e5399c60b5868e1812563c18c4197628051fb710b0bd8d5a279a7eb86d922d55)
Expected results:
createMediaKeys() should succeed.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 1•4 years ago
|
||
Hi, bryce, could you take a look and assign the priority for this issue?
Thank you.
Assignee | ||
Comment 2•4 years ago
|
||
I can repro this and it looks like a regression from the fission handling changes. We use the channel on a document to grab principals, but my educated guess is that we don't have a channel for iframes lacking sources so we fall over.
Selcuk, is this something that is causing issues in the wild?
Assignee | ||
Comment 3•4 years ago
|
||
Assignee | ||
Comment 4•4 years ago
|
||
:kmag, is there any existing machinery we can use to work around this?
(In reply to Bryce Seager van Dyk (:bryce) from comment #2)
I can repro this and it looks like a regression from the fission handling changes. We use the channel on a document to grab principals, but my educated guess is that we don't have a channel for iframes lacking sources so we fall over.
Selcuk, is this something that is causing issues in the wild?
Yes, this is my observation as well.
Assignee | ||
Comment 6•4 years ago
|
||
(In reply to Selcuk from comment #5)
(In reply to Bryce Seager van Dyk (:bryce) from comment #2)
I can repro this and it looks like a regression from the fission handling changes. We use the channel on a document to grab principals, but my educated guess is that we don't have a channel for iframes lacking sources so we fall over.
Selcuk, is this something that is causing issues in the wild?
Yes, this is my observation as well.
Are you able to say where the issues in the wild are taking place?
The sample on codepen.io is an accurate representation of how we reproduced the issue; createMediaKeys fails for iframe without source while it works for iframe with a src URL.
Assignee | ||
Comment 8•4 years ago
|
||
Understood, but is this a problem that was noticed because it took place in a production website?
That's correct. We have noticed this issue on a client's DRM enabled video portal on production (requires authentication; can't share here). We have temporarily remedied the issue by converting dynamic iframe based workflow to shadow DOM using an existing flag on our side.
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 10•4 years ago
|
||
Looking at the code I wonder if it is possible to use parent document's channel as a fallback. Such iframes do not even require feature policy for encrypted-media (regarded as same domain).
Also is it possible to include the fix in the next Firefox release? (Multiple clients reported this issue; we are enhancing our workarounds for them. Static iframe (with a cross-domain src) workaround brings a burden on them to set Feature-Policy response headers.)
Assignee | ||
Comment 11•4 years ago
|
||
(In reply to Selcuk from comment #10)
Looking at the code I wonder if it is possible to use parent document's channel as a fallback. Such iframes do not even require feature policy for encrypted-media (regarded as same domain).
Also is it possible to include the fix in the next Firefox release? (Multiple clients reported this issue; we are enhancing our workarounds for them. Static iframe (with a cross-domain src) workaround brings a burden on them to set Feature-Policy response headers.)
I'm planning on investigating something like what you suggest. I've got some work in progress, but have some other work superseding it. I hope to be able to return to it by early next week. If you could email me to discuss in more detail the sites that are having problems that would be useful to make a case for uplifting the fix to release once it lands.
Assignee | ||
Comment 12•4 years ago
|
||
It looks like this is mitigated if we wait for the load on the load of the iframe https://codepen.io/SingingTree/pen/XWKoJKo?editors=1011
Looking further at what's going on I was incorrect above -- we will set the channel and load info with a blank src, but because of bug 543435 we actually do another load. This also means the mitigation mentioned in comment 10 and 11 will work initially, but you'll run into problems later because your media keys object will be associated with a document that disappears due to our async load.
Selcuk, can you confirm this is mitigated by waiting on the load event?
Reporter | ||
Comment 13•4 years ago
|
||
I gave a try, here is my observation:
Experiment #1:
after load event, injecting script (inline or with src) into the iframe makes the iframe's location.href about:blank. Possibly this causes DOMException: Key system is unsupported for even clearkey and widevine. Looks to be an expected behavior since chrome does the same.
Experiment #2:
after load event, injected script content using document.open -> write -> close sequence. This sets iframe's location to the parent window location. This works initially (4-5 seconds of playback) and then video element gets stuck in waiting state. In comparison, Chrome plays w/o getting stuck.
Might be irrelevant but firefox also does not seem to execute immediate async function calls in the injected script in either experiment while chrome executes as expected:
//inject a script
const newScript = iframe.contentDocument.createElement('script');
newScript.src = 'test.js';
frame.contentDocument.body.appendChild(newScript);
//test.js content:
console.log('this gets printed');
setTimeout(main, 100);
function main() {
console.log('this does not get called.');
}
Assignee | ||
Comment 14•4 years ago
|
||
If you can attach those test cases or link to them hosted somewhere, that would be useful. I've tried to repro the injected script logging from a timeout and that seems to work as expected in Fx and Chrome (everything gets printed).
injectScriptInIframe.html -
<html>
<head>
<script>
async function doOnLoad() {
let div = document.getElementById("myDiv");
let iframe = document.createElement("iframe");
let p = new Promise(r => {
iframe.onload = () => {
console.log("iframe loaded");
r();
};
});
iframe.src = "";
div.appendChild(iframe);
await p;
const newScript = iframe.contentDocument.createElement("script");
newScript.src = "timeoutLog.js";
iframe.contentDocument.body.appendChild(newScript);
}
window.onload = doOnLoad;
</script>
<body>
<div id="myDiv"></div>
</body>
</head>
</html>
timeoutLog.js -
console.log('this gets printed');
setTimeout(callback, 100);
function callback() {
console.log('this does not get called.');
}
I have another plan to see if we can mitigate this, will report back once I've tested it.
Reporter | ||
Comment 15•4 years ago
|
||
I will try to isolate that async method execution issue and provide a demo, obviously there are some contributing factors in my trials. Regardless, it might be irrelevant to the additional load event for iframe; I can report that with a separate ticket once isolated demo is ready on my end.
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 16•4 years ago
|
||
Provide coverage the ensures we can:
- Call navigator.requestMediaKeySystemAccess() and receive access
- Call createMediaKeys on the access object
in iframes that are same and different origin.
This should work when waiting for an iframe to fire the load event, but I also
provide a case for if we do not wait for load. It's undesirable to not wait for
the load, but we've historically worked in this case (if this was intentional is
not clear to me). So providing such a test allows for coverage of this case as
long as we want to continue supporting it. Said test will be red as of this
patch, but an immediate follow up will restore our compat with this case.
Assignee | ||
Comment 17•4 years ago
|
||
This restores some functionality to work more closely to how we'd done so in a
pre-fission world. Previously we'd associated MediaKeys we created with the top
level in process window's doc and used that document's principal as the top level
principal for the keys.
To make this more fission compatible, this was changed so the keys are
associated with the document they're created in, as well as using that
document's channel to determine the top level principal via the load info.
However, this caused us to regress in the case where MediaKeys are created
within an iframe that hasn't yet fired the load event.
To prevent that regression, we move back to associating the keys with a document
further up the hierarchy -- we use the top in process window's doc again.
Because this document may not actually be the top level doc in fission, we use
that document's load info to try and determine the top level principal.
Depends on D97321
Assignee | ||
Comment 18•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 19•4 years ago
|
||
Rework the MediaKeys class to shutdown when its parent inner window is destroyed
rather than the document it's created in. This is done to mitigate the case
where a MediaKeys is created in an about:blank document that has not yet
undergone its async load (i.e. blank document that will stay blank, blank
documents loading to other pages will still clobber their keys on load). This
specifically addresses cases where sites could create an iframe, not wait for
load, create a MediaKeys in the iframe, and then find the keys had become
unusable.
This is achieved by associating MediaKeys instances with their inner window and
having the window notify keys when a window is going to be destroyed. I decided
to use this approach rather than have MediaKeys observe for window destruction
for a few reasons:
- The keys would need to support weak references to use the observer service in
the desired way. Implementing this interface on the MediaKeys adds a
non-trivial level of complexity and means the keys would implement the WeakPtr
interface (already in place prior to this patch) and also the NS weak
reference interface, which I found confusing. - If the inner window stores pointers to MediaKeys created in it, it can be self
aware of if EME activity is taking place within it. The allows us to better
identify EME activity in documents. Historically one of the ways we'd
determined EME activity by checking if media elements have MediaKeys attached,
but this had lead to issues where if MediaKeys are created but not attached
to a media element we overlook them. With this patch's changes, we can also
have a document check its inner window to see if there are any MediaKeys. This
patch uses this to extend our check to avoid bfcaching pages with EME content. - There appears to be prior art using a similar approach for audio contexts and
peer connections, which I assume is sensible and I'm not reinventing the wheel
by following.
Assignee | ||
Comment 20•4 years ago
|
||
Try push with (hopefully) close to final patches https://treeherder.mozilla.org/jobs?repo=try&revision=449a075f9790d2a8f0b5b510efd0eabac0e0b06f&selectedTaskRun=ba3WsGoUQ5GhJWGvYZjTtQ.0
Comment 21•4 years ago
|
||
Comment 22•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b18095c8f8a
https://hg.mozilla.org/mozilla-central/rev/cd8c20e521f5
https://hg.mozilla.org/mozilla-central/rev/7b5facb4df3a
Assignee | ||
Comment 23•4 years ago
|
||
Follow ups
- bug 1681544 tracks adding a test to ensure MediaKeys are destroyed when we navigate away from a page.
- bug 1681741 tracks annotating
CDMProxy::Shutdown
withMOZ_CAN_RUN_SCRIPT
.
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•3 years ago
|
Description
•