Closed Bug 1165163 Opened 9 years ago Closed 9 years ago

[EME] Firefox crash in MediaKeySession.remove() if CDM crashes

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

If the CDM crashes, and MediaKeySession.remove() is called, we can crash.

For example:
https://crash-stats.mozilla.com/report/index/57febf58-278e-4706-8926-0a8682150515

We don't null check MediaKeySession::mKeys in MediaKeySession.remove(). mKeys is nulled out if the CDM crashes.
In MediaKeySession, instead of relying MediaKeySession::mKeys to create our promise objects, create them directly. This means we can still create promises after our pointer to the MediaKeys has been cleared, so we can always reject our promises.
Attachment #8606091 - Flags: review?(jwwang)
Attachment #8606091 - Flags: review?(jwwang) → review+
We have identical code in MediaKeys::MakePromise(). Someday we will have MediaKeysUtils to collect common functions.
Update commit message. carry forward r=jwwang
Attachment #8606091 - Attachment is obsolete: true
Attachment #8606228 - Flags: review+
Must remember to uplift.
Flags: needinfo?(cpearce)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/44f38195b432
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8606228 [details] [diff] [review]
Patch v2: Don't rely on mediaKeys to create MediaKeySession's promises

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: If the CDM crashes during EME session startup, the browser can crash. Adobe are fixing a particular crash in their CDM that causes us to crash, but we should still not crash on our side.
[Describe test coverage new/current, TreeHerder]: Lots of EME mochitests, though our test CDM doesn't crash in when we exercise the case where Adobe's crashes.
[Risks and why]: Low; basically we just change the parent object that we use to create some promises; pretty safe.
[String/UUID change made/needed]: None.
Attachment #8606228 - Flags: approval-mozilla-beta?
Attachment #8606228 - Flags: approval-mozilla-aurora?
Comment on attachment 8606228 [details] [diff] [review]
Patch v2: Don't rely on mediaKeys to create MediaKeySession's promises

Approved for uplift to aurora (40) and beta (39). May help with EME crashes.
Attachment #8606228 - Flags: approval-mozilla-beta?
Attachment #8606228 - Flags: approval-mozilla-beta+
Attachment #8606228 - Flags: approval-mozilla-aurora?
Attachment #8606228 - Flags: approval-mozilla-aurora+
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.