1259985 - Potentially invalid fix in bug 1258410

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

In bug 1258410, special exception was added to abort early should a member be null.

However, the fix was invalid as it didn't return !

bug 1258410 contained two fixes. I believe the 2nd fix https://hg.mozilla.org/releases/mozilla-beta/rev/baa3826260a0 prevent the problems from ever occurring, but better be safe than sorry.

the issue won't occur in nightly as the code was removed in bug 1259274

Comment 1

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

Attachment: MozReview Request: Bug 1259985 - Add missing return after null-check - r?jya

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

Comment 2

[Jean-Yves Avenard [:jya]]

Comment on attachment 8735224 [details]
MozReview Request: Bug 1259985 - Add missing return after null-check - r?jya

https://reviewboard.mozilla.org/r/42667/#review39125

Comment 3

[Pulsebot]

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

Comment 4

[Jean-Yves Avenard [:jya]]

https://crash-stats.mozilla.com/search/?product=Firefox&signature=%3Dmp4_demuxer%3A%3AMP4Metadata%3A%3AGetNumberTracks&build_id=%3E%3D20160325000000&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports

shows that the signature didn't show after the 25, which means that bug 1258410 may be sufficient

Comment 5

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

Comment on attachment 8735224 [details]
MozReview Request: Bug 1259985 - Add missing return after null-check - r?jya

Approval Request Comment
[Feature/regressing bug #]: bug 1258410
[User impact if declined]: Potential crashes due to null dereference
[Describe test coverage new/current, TreeHerder]: Lots of media mochitests, Aurora try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c15f18f244d , Beta try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1afdeaf94173
[Risks and why]: No risks that I can see, as it's just adding an early "return;", and there are other early returns in the same method.
[String/UUID change made/needed]: None.

Comment 6

[Liz Henry (:lizzard)]

Comment on attachment 8735224 [details]
MozReview Request: Bug 1259985 - Add missing return after null-check - r?jya

Prevent potential crashes, please uplift to aurora and beta.

Comment 7

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

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

Comment 8

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/f0bcd999646e

Comment 9

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

https://hg.mozilla.org/releases/mozilla-beta/rev/9200077814f6