1330889 - Remove some mState checks from MediaDecoderStateMachine

Status: RESOLVED FIXED

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

Comment 1

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

Attachment: Bug 1330889. Part 1 - remove the assertion in MDSM::FinishShutdown.

http://searchfox.org/mozilla-central/rev/3f614bdf91a2379a3e2c822da84e524f5e742121/dom/media/MediaDecoderStateMachine.cpp#1825
This assertion ensures we never escape the SHTUDOWN state.

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

Comment 2

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

Attachment: Bug 1330889. Part 2 - remove some mState checks from MDSM::ResetDecode.

We want to separate mechanism from policy. The policy should be encoded in
the state objects which will decided whether to call ResetDecode(). It is
also possible to add a new state which has to call ResetDecode().

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

Comment 3

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

Attachment: Bug 1330889. Part 3 - remove the mState check from HasLowBufferedData().

We have |mBuffered.Ref().IsInvalid()| below to check if mBuffered is valid.

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

Comment 4

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

Attachment: Bug 1330889. Part 4 - remove the mState checks from MaybeStartPlayback().

The same reason as P2.

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

Comment 5

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

Attachment: Bug 1330889. Part 5 - remove MDSM::mState.

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

Comment 6

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8826534 [details]
Bug 1330889. Part 1 - remove the assertion in MDSM::FinishShutdown.

https://reviewboard.mozilla.org/r/104486/#review105612

Comment 7

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8826535 [details]
Bug 1330889. Part 2 - remove some mState checks from MDSM::ResetDecode.

https://reviewboard.mozilla.org/r/104488/#review105618

Comment 8

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8826536 [details]
Bug 1330889. Part 3 - remove the mState check from HasLowBufferedData().

https://reviewboard.mozilla.org/r/104490/#review105630

Comment 9

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8826537 [details]
Bug 1330889. Part 4 - remove the mState checks from MaybeStartPlayback().

https://reviewboard.mozilla.org/r/104492/#review105638

Comment 10

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8826538 [details]
Bug 1330889. Part 5 - remove MDSM::mState.

https://reviewboard.mozilla.org/r/104494/#review105642

Comment 11

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

Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/359372e06f84
Part 1 - remove the assertion in MDSM::FinishShutdown. r=kikuo
https://hg.mozilla.org/integration/autoland/rev/befabe73c225
Part 2 - remove some mState checks from MDSM::ResetDecode. r=kikuo
https://hg.mozilla.org/integration/autoland/rev/4971ca440d80
Part 3 - remove the mState check from HasLowBufferedData(). r=kikuo
https://hg.mozilla.org/integration/autoland/rev/9584a95515b3
Part 4 - remove the mState checks from MaybeStartPlayback(). r=kikuo
https://hg.mozilla.org/integration/autoland/rev/e4de8d9250ac
Part 5 - remove MDSM::mState. r=kikuo

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/359372e06f84
https://hg.mozilla.org/mozilla-central/rev/befabe73c225
https://hg.mozilla.org/mozilla-central/rev/4971ca440d80
https://hg.mozilla.org/mozilla-central/rev/9584a95515b3
https://hg.mozilla.org/mozilla-central/rev/e4de8d9250ac