Closed Bug 1433309 Opened 2 years ago Closed 2 years ago

[EME] Annotate whether CDM init fails due to shutdown

Categories

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

57 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox59 + fixed
firefox60 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

Details

Attachments

(1 file)

The past 3 times we've shipped a new Firefox release, I've gotten emails from Netflix saying they'd had a spike in failures of MediaKeySystemAccess.createMediaKeys()'s promise rejecting with detail "GetCDM failed". The failures spike in the old version of Firefox, but don't occur much in the new version.

I'm wondering whether this is caused by the browser restarting to apply a Firefox 
update while Netflix is loading its player.

To test this theory, I'd like to add more detail to the promise reject to denote whether the browser was shutting down.
Assignee: nobody → cpearce
Comment on attachment 8945673 [details]
Bug 1433309 - Annotate createMediaKeys promise reject with whether failure occurred due to pending shutdown.

https://reviewboard.mozilla.org/r/215766/#review221854
Attachment #8945673 - Flags: review?(gsquelart) → review+
Note to self: consider this patch for uplift.
Flags: needinfo?(cpearce)
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1373fe3d25c
Annotate createMediaKeys promise reject with whether failure occurred due to pending shutdown. r=gerald
https://hg.mozilla.org/mozilla-central/rev/f1373fe3d25c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment on attachment 8945673 [details]
Bug 1433309 - Annotate createMediaKeys promise reject with whether failure occurred due to pending shutdown.

Approval Request Comment
[Feature/Bug causing the regression]: Netflix playback

[User impact if declined]: Netflix see a spike in failures in the outgoing Firefox version every time we ship a new Firefox version. I suspect what we're seeing is Firefox users restarting their browser to apply an update while Netflix's player is loading. To help prove that theory, this patch adds more descriptive error messages to our promise rejects so that Netflix's telemetry can collect the failure messages and verify whether this in happening. The hope is that we can show that this is not a failure worth trying to fix.

[Is this code covered by automated tests?]: Not directly, but we have plenty of test coverage for media playback already.

[Has the fix been verified in Nightly?]: Yes

[Needs manual test from QE? If yes, steps to reproduce]: No, QE not needed.

[List of other uplifts needed for the feature/fix]: None.

[Is the change risky?]: No.

[Why is the change risky/not risky?]: This patch adds logging on a failure path.

[String changes made/needed]: None.
Flags: needinfo?(cpearce)
Attachment #8945673 - Flags: approval-mozilla-beta?
Chris, just want to double check with you, is anything here logging or sending data that we should get a privacy/data review for?
Flags: needinfo?(cpearce)
We're reporting to JS whether the CDM crashed while starting up the GMP process, or whether we're shutting down on said code path. I don't think that warrants a privacy/data review.
Flags: needinfo?(cpearce)
Comment on attachment 8945673 [details]
Bug 1433309 - Annotate createMediaKeys promise reject with whether failure occurred due to pending shutdown.

Diagnostic patch to help with shutdown issues, let's uplift for beta 8 later this week.
Attachment #8945673 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Chris Pearce (:cpearce) from comment #7)
> [Is this code covered by automated tests?]: Not directly, but we have plenty of test coverage for media playback already.
> [Needs manual test from QE? If yes, steps to reproduce]: No, QE not needed.

Setting qe-verify- based on Chris's assessment on manual testing needs.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.