1491889 - ChromiumCDM interface should allow for async init of Widevine CDM

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

For versions of the Widevine CDM up to 9, calling the initialization function would be sync, and does not appear fallible[0]. I.e. once the init function has returned we can expect the CDM to have initialized, and the function does not return a value that needs to be checked for error.

Starting at CDM 10 initialization is exposed differently by the CDM API[1]. The init function is similar, but the documentation for the function states that the CDM should not be considered initialized until the CDM calls an `OnInitialized`[2] function on the host.

To accommodate this new API we should:
- Wait until the OnInitialized callback has been made to signal the CDM is ready.
- Check the value passed to the callback to make sure we have success of init.

Looking at current code, I think this could be achieved by updating code around how the ChromiumCDMParent handles init. Right now, the parent will indicate success once it's sent the init message across IPC[3]. Once this happens, the CDM proxy indicates the CDM has been created.

Instead of the ChromiumCDMParent indicating success after the message is sent, we should update handling to only indicate success after the OnInitialized callback is made by the CDM. This callback will happen in the ChromiumCDMChild, which is turn should indicate to the parent it was made via IPC.  We should also check the value passed to the callback to handle CDM init failure.

[0]: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/media/gmp/widevine-adapter/content_decryption_module.h#657
[1]: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/media/gmp/widevine-adapter/content_decryption_module.h#870
[2]: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/media/gmp/widevine-adapter/content_decryption_module.h#1439
[3]: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/media/gmp/ChromiumCDMParent.cpp#69
[4]: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/media/gmp/ChromiumCDMProxy.cpp#117

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1491889 - Update chromium CDM interface to accommodate async init. r=cpearce

Starting at the Widevine CDM10 interface, the CDM is expected to make a callback
to an `OnInititalized` function to signal initialization has taken place. Prior
to this, it was sufficient to call the init function on the CDM, with no waiting
for a callback.

This changeset puts in place the IPDL to support async init, as well as the
handling for the ChromiumCDMParent and ChromiumCDMProxy. The code is not fully
updated to handle CDM10, so CDM9 is the only compatible CDM. Because CDM9 does
not perform the init callback, we manually call the callback in code. This also
accommodates the clearkey case, which uses the CDM9 interface.

Once CDM10 is implemented, the manual call to OnInitialized will be moved to the
CDM9 compat layer, so that CDMs 10+ are responsible for handling their own
callback.

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1491889 - Update ChromiumCDMChild to hold a promise to track CDM init. r=cpearce

This changeset extends the async initialize functionality added in the prior
changeset by wrapping the Initialize resolver in a promise. This allows us to
use familiar promise machinery to handle async init of the CDM. We do this by
creating the promise and setting up handling when we receive the init message on
the ChromiumCDMChild, but resolving the promise in the `OnInitialized` callback
from the CDM to the ChromiumCDMChild.

We still only support CDM9 as of this changeset. As such, we now manually call
`OnInitialized` to make sure the ChromiumCDMParent is notified that the CDM has
initialized. When we implement the CDM10 interface, these manual calls will be
moved to the CDM9 compat layer, and Widevine CDM10+ can perform its own
callback.

This changeset adds a failure path to initialization, as the `OnInitialized`
interface we implement allows for failure. However, since we manually call into
this path for CDM9 we shouldn't get any such failures. Once CDM10 is fully
implemented its possible that the init callback could indicate failure, and the
handling here would be invoked.

Depends on D6061

Comment 3

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

Comment on attachment 9009688 [details]
Bug 1491889 - Update chromium CDM interface to accommodate async init. r=cpearce

Chris Pearce (:cpearce) has approved the revision.

Comment 4

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

Comment on attachment 9009706 [details]
Bug 1491889 - Update ChromiumCDMChild to hold a promise to track CDM init. r=cpearce

Chris Pearce (:cpearce) has approved the revision.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8c53e2fdc7a
Update chromium CDM interface to accommodate async init. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/cb9048ebfec3
Update ChromiumCDMChild to hold a promise to track CDM init. r=cpearce

Comment 6

[Natalia Csoregi [:nataliaCs]]

Backed out for Logging.h:262 bustages

backout: https://hg.mozilla.org/integration/autoland/rev/8db8a228536dedac30934430b8c62fa889e5b7eb

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=cb9048ebfec36c62b47bc65f332559433ea88fce

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=201334550&repo=autoland&lineNumber=23945

[task 2018-09-25T03:01:57.080Z] 03:01:57     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/platforms/agnostic/gmp'
[task 2018-09-25T03:01:57.993Z] 03:01:57     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/gmp'
[task 2018-09-25T03:01:57.993Z] 03:01:57     INFO -  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/gcc/bin/g++ -o Unified_cpp_dom_media_gmp0.o -c  -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DGMP_SAFE_SHMEM -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/media/gmp -I/builds/worker/workspace/build/src/obj-firefox/dom/media/gmp -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/security/sandbox/chromium -I/builds/worker/workspace/build/src/security/sandbox/chromium-shim -I/builds/worker/workspace/build/src/media/mtransport -I/builds/worker/workspace/build/src/xpcom/base -I/builds/worker/workspace/build/src/xpcom/build -I/builds/worker/workspace/build/src/xpcom/threads -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow  -MD -MP -MF .deps/Unified_cpp_dom_media_gmp0.o.pp   /builds/worker/workspace/build/src/obj-firefox/dom/media/gmp/Unified_cpp_dom_media_gmp0.cpp
[task 2018-09-25T03:01:57.993Z] 03:01:57     INFO -  In file included from /builds/worker/workspace/build/src/dom/media/gmp/GMPLog.h:9:0,
[task 2018-09-25T03:01:57.993Z] 03:01:57     INFO -                   from /builds/worker/workspace/build/src/dom/media/gmp/CDMStorageIdProvider.cpp:7,
[task 2018-09-25T03:01:57.993Z] 03:01:57     INFO -                   from /builds/worker/workspace/build/src/obj-firefox/dom/media/gmp/Unified_cpp_dom_media_gmp0.cpp:2:
[task 2018-09-25T03:01:57.993Z] 03:01:57     INFO -  /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMChild.cpp: In lambda function:
[task 2018-09-25T03:01:57.994Z] 03:01:57     INFO -  /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/Logging.h:262:84: error: format '%d' expects argument of type 'int', but argument 4 has type 'nsresult' [-Werror=format=]
[task 2018-09-25T03:01:57.994Z] 03:01:57     INFO -         mozilla::detail::log_print(moz_real_module, _level, MOZ_LOG_EXPAND_ARGS _args); \
[task 2018-09-25T03:01:57.994Z] 03:01:57     INFO -                                                                                      ^
[task 2018-09-25T03:01:57.994Z] 03:01:57     INFO -  /builds/worker/workspace/build/src/dom/media/gmp/GMPLog.h:15:27: note: in expansion of macro 'MOZ_LOG'
[task 2018-09-25T03:01:57.995Z] 03:01:57     INFO -   #define GMP_LOG(msg, ...) MOZ_LOG(GetGMPLog(), LogLevel::Debug, (msg, ##__VA_ARGS__))
[task 2018-09-25T03:01:57.996Z] 03:01:57     INFO -                             ^~~~~~~
[task 2018-09-25T03:01:57.996Z] 03:01:57     INFO -  /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMChild.cpp:483:7: note: in expansion of macro 'GMP_LOG'
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -         GMP_LOG("ChromiumCDMChild::RecvInit() init promise rejected with rv=%d",
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -         ^
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -  cc1plus: all warnings being treated as errors
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1122: recipe for target 'Unified_cpp_dom_media_gmp0.o' failed
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -  make[4]: *** [Unified_cpp_dom_media_gmp0.o] Error 1
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/gmp'
[task 2018-09-25T03:01:57.997Z] 03:01:57     INFO -  make[4]: *** Waiting for unfinished jobs....

Comment 7

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c36fcb22c35e048846b8df42435a896a13b0f0b

Comment 8

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

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

Comment 9

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

All right, let's try this again.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53dfb7e2bc86
Update chromium CDM interface to accommodate async init. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/8a1695f0c08c
Update ChromiumCDMChild to hold a promise to track CDM init. r=cpearce

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/53dfb7e2bc86
https://hg.mozilla.org/mozilla-central/rev/8a1695f0c08c

Comment 12

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 9009688 [details]
Bug 1491889 - Update chromium CDM interface to accommodate async init. r=cpearce

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Please see Bug 1518525 for details and motivation.

Further notes: this is the third bug in a series tracked by bug 1518525. Could we please uplift all patches on this bug? Please see https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9976fb4de94bf0933f1faa8118154961fac20fb&selectedJob=220243631 for a try push where these patches have been grafted onto esr60.

User impact if declined: Failure to playback premium media.

Fix Landed on Version: 64

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This has landed and baked since 64. I have verified that the patches work when grafted onto esr60.

String or UUID changes made by this patch: None.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9009688 [details]
Bug 1491889 - Update chromium CDM interface to accommodate async init. r=cpearce

Needed for ESR60 to continue to support Widevine video playback in the very near future. Approved for 60.5.0esr.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/01c06ac0606e
https://hg.mozilla.org/releases/mozilla-esr60/rev/0faad9929481