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)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: cpearce, Assigned: cpearce)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
5.06 KB,
patch
|
cpearce
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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)
Updated•9 years ago
|
Attachment #8606091 -
Flags: review?(jwwang) → review+
Comment 2•9 years ago
|
||
We have identical code in MediaKeys::MakePromise(). Someday we will have MediaKeysUtils to collect common functions.
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d148bd91115b
Assignee | ||
Comment 4•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=64c63327cac6
Assignee | ||
Comment 5•9 years ago
|
||
Update commit message. carry forward r=jwwang
Attachment #8606091 -
Attachment is obsolete: true
Attachment #8606228 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•9 years ago
|
||
Must remember to uplift.
Flags: needinfo?(cpearce)
Keywords: checkin-needed
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/44f38195b432
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 9•9 years ago
|
||
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 10•9 years ago
|
||
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+
Comment 11•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/a810a56634b1
status-firefox40:
--- → fixed
Comment 12•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/549b8e7903f5
status-firefox39:
--- → fixed
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(cpearce)
You need to log in
before you can comment on or make changes to this bug.
Description
•