1258562 - Crash in dom::MediaSource::Duration

Status: VERIFIED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Top crasher

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: MozReview Request: Bug 1258562: MSE] Abort if MediaSource has been shutdown. r?gerald

The assumption was made that this code was never called if MediaSourceDecoder::NextFrameBufferedStatus() had been called before. However, that assumption was incorrect as NextFrameBufferedStatus() is only called if we had determine we had no frame buffered.

Review commit: https://reviewboard.mozilla.org/r/41587/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41587/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8733129 [details]
MozReview Request: Bug 1258562: MSE] Abort if MediaSource has been shutdown. r?gerald

https://reviewboard.mozilla.org/r/41587/#review38009

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8733129 [details]
MozReview Request: Bug 1258562: MSE] Abort if MediaSource has been shutdown. r?gerald

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41587/diff/1-2/

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fad8fe6c7c90

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8733129 [details]
MozReview Request: Bug 1258562: MSE] Abort if MediaSource has been shutdown. r?gerald

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: this is a top crasher in current release, it will be in ESR for sure.
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/regressing bug #]: 1258562
[User impact if declined]: top crasher (> 3.5k in the last week)
[Describe test coverage new/current, TreeHerder]: Manual test, similar fix to bug 1245463
[Risks and why]: None. Only checking that the mediasource object still exists and hasn't been deleted underneath us
[String/UUID change made/needed]: none

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fad8fe6c7c90

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

This crash signature does not show up on Nightly 48.0a1 after build 03-21 which is good news. Based on that I would consider it verified.

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Jean-Yves, is ESR38 affected as well? Should we uplift this fix on that branch too?

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8733129 [details]
MozReview Request: Bug 1258562: MSE] Abort if MediaSource has been shutdown. r?gerald

Crash fix that has ceased on Nightly since it landed, Aurora47+, Beta46+, ESR45+

Comment 10

[Jean-Yves Avenard [:jya]]

it doesn't affect ESR38 no.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/964c7bcafb39

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8865b69792b1

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-esr45/rev/3cf954705245

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Based on comment 10 (thanks JYA).

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Jean-Yves/Anthony, we might do a new dot release for 45. I am considering taking this patch, what do you think?

Comment 16

[Jean-Yves Avenard [:jya]]

sounds like a good idea, patch should apply to 45 too.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8733129 [details]
MozReview Request: Bug 1258562: MSE] Abort if MediaSource has been shutdown. r?gerald

[Triage Comment]
For 45 too.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/026361c4c591

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-esr45/rev/b60e2c6c4086
Landed to GECKO4501esr_2016031618_RELBRANCH

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes with "Fix a crash impacting the video playback with Media Source Extension (1258562)" as wording