1180070 - [EME] Use MediaPromise for CDMProxy::Decrypt

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

In bug 1171257 Sotaro discovered that we can receive a decrypt result after flushing, resulting in test failures. When an EMEDecryptor flushes, we need to also ensure that the decrypts we're waiting on are flushed.

We can use MediaPromises to achieve this; we just need to disconnect all promises on flush or drain.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Use MediaPromises for CDMProxy::Decrypt

Use MediaPromises for CDMProxy::Decrypt, so that we can disconnect the promises we're waiting on when the EMEDecryptor flushes or drains.

This means we can't get a decrypt result that was in progress before a flush after the flush completes.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1efc5599cf23

Sotaro: Can you please check that this still fixes your bug? Thanks.

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8629172 [details] [diff] [review]
Patch: Use MediaPromises for CDMProxy::Decrypt

Review of attachment 8629172 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/platforms/agnostic/eme/EMEDecoderModule.cpp
@@ +73,2 @@
>      MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
> +    MOZ_ASSERT(aDecrypted.mSample);

Is it true that aDecrypted.mSample is non-null whether aDecrypted.mStatus returns an error or not?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8629172 [details] [diff] [review]
Patch: Use MediaPromises for CDMProxy::Decrypt

Review of attachment 8629172 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/platforms/agnostic/eme/EMEDecoderModule.cpp
@@ +73,2 @@
>      MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
> +    MOZ_ASSERT(aDecrypted.mSample);

Yes. We used to make mSample null in CDMProxy::DecryptJob::PostResult() in the error case, but I removed that in this patch.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/87c493cc1166

Comment 5

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8629172 [details] [diff] [review]
Patch: Use MediaPromises for CDMProxy::Decrypt

From the following, the test failures was fixed by the patch.
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4c3aaf7ce43

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/87c493cc1166