1324335 - Make SeekJob::mTarget a Maybe<>

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

http://searchfox.org/mozilla-central/rev/f680e72cc6579f90b992b63ca14d923d2afea612/dom/media/MediaDecoderStateMachine.cpp#2041

It is invalid to read mSeekJob.mTarget after mSeekJob.Resolve() is called at #2032 because SeekJob::Resolve() resets mTarget.

Changing SeekJob::mTarget to a Maybe<> will prevent such a misuse.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

This issue is discovered by Dan.

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1324335. Part 1 - give MozPromiseHolder a move constructor.

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

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1324335. Part 2 - Make SeekJob::mTarget a Maybe<>.

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

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1324335. Part 3 - cache mTarget before resetting it.

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

Comment 5

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

Comment on attachment 8819808 [details]
Bug 1324335. Part 1 - give MozPromiseHolder a move constructor.

https://reviewboard.mozilla.org/r/99476/#review99768

Comment 6

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

Comment on attachment 8819809 [details]
Bug 1324335. Part 2 - Make SeekJob::mTarget a Maybe<>.

https://reviewboard.mozilla.org/r/99478/#review99776

Comment 7

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

Comment on attachment 8819810 [details]
Bug 1324335. Part 3 - cache mTarget before resetting it.

https://reviewboard.mozilla.org/r/99480/#review99778

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1c9fc080abf
Part 1 - give MozPromiseHolder a move constructor. r=gerald
https://hg.mozilla.org/integration/autoland/rev/3ef122da13fd
Part 2 - Make SeekJob::mTarget a Maybe<>. r=gerald
https://hg.mozilla.org/integration/autoland/rev/07eaefc44d99
Part 3 - cache mTarget before resetting it. r=gerald

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/b1c9fc080abf
https://hg.mozilla.org/mozilla-central/rev/3ef122da13fd
https://hg.mozilla.org/mozilla-central/rev/07eaefc44d99