1909826 - Crash on canalplus.com [@ mozilla::MediaDecoder::SetCDMProxy ]

Status: RESOLVED FIXED

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

Description

[Mayank Bansal]

I was on particularly slow network.
I was on canalplus, testing stuff. I had opened the page for one of the sport streams..
While a stream loads, I triggered a "Kill GPU process" from about:support

AR: Crash

https://crash-stats.mozilla.org/report/index/a24418de-83e1-4fb0-896b-410620240725#tab-bugzilla
https://crash-stats.mozilla.org/report/index/1a82b08c-500b-40bf-895f-ca5970240725#tab-bugzilla

Can repro intermittently

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Mayank Bansal]

Most of the time the tab recovers after killing the gpu process. But on these 2 occasions, the tab crashed.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Based on the crash reports in nightly/release, I don't believe this necessarily requires a GPU process crash to trigger, or at least it isn't obvious that is required.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

I've been looking at this but I'm not sure how it can happen. Given there are only two CDMProxy types, my assumption is that the switchover to the other state machine type fails, but the failure paths don't seem very likely. Any ideas alwu?

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

I'll continue to investigate in the meantime.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

If the prefs were changed to something other than 1 or 2, I see that we return false, but the CDM may or not match the state machine:
https://searchfox.org/mozilla-central/rev/19764d620c02025bdcc8d1f3c4fcf5a580407a01/dom/media/ExternalEngineStateMachine.cpp#1418

Can't tell from the crash reports if this is the case for those users.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1909826 - Improve MediaDecoder::SetCDMProxy to better support invalid CDM proxy types.

This patch makes it so that we more gracefully handle unsupported or
disabled CDM proxy types. It will diagnostic assert but barring that now
return a more appropriate error code instead of attempting to use the
CDM with the state machine.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0563ac8d038f
Improve MediaDecoder::SetCDMProxy to better support invalid CDM proxy types. r=alwu,media-playback-reviewers

Comment 10

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/0563ac8d038f

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9478642 [details]
Bug 1909826 - Improve MediaDecoder::SetCDMProxy to better support invalid CDM proxy types.

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Fixes another MFCDM related crash
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just checks for more errors to handle them properly
• String changes made/needed:
• Is Android affected?: No

Comment 12

[Dianna Smith [:diannaS]]

:aosmond this needs a rebased patch to beta

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1909826 - Improve MediaDecoder::SetCDMProxy to better support invalid CDM proxy types.

This patch makes it so that we more gracefully handle unsupported or
disabled CDM proxy types. It will diagnostic assert but barring that now
return a more appropriate error code instead of attempting to use the
CDM with the state machine.

Original Revision: https://phabricator.services.mozilla.com/D245284

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/f7650e1a6535