Closed Bug 1221881 Opened 9 years ago Closed 9 years ago

"ASSERTION: Should have state machine"

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox42 --- affected
firefox43 --- fixed
firefox44 --- fixed
firefox45 --- fixed

People

(Reporter: jruderman, Assigned: jwwang)

References

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase
###!!! ASSERTION: Should have state machine.: 'mDecoderStateMachine != nullptr', file dom/media/MediaDecoder.cpp, line 559

Testcase is the same as in bug 1221370, except for the last statement:
  - vid.mozCaptureStreamUntilEnded();
  + vid.autoplay = true
Attached file stack
Component: Audio/Video → Audio/Video: Playback
Priority: -- → P1
Assignee: nobody → jwwang
Here is what happened:
1. |ms.endOfStream("network")| propagates network to the media element.
https://hg.mozilla.org/mozilla-central/file/cc473fe5dc512c450634506f68cbacfb40a06a23/dom/media/mediasource/MediaSource.cpp#l329

2. MediaDecoder::Shutdown() is called, and |mDecoderStateMachine| is cleared later.

3. autoplay kicks in and MediaDecoder::Play() is called to hit the assertion.
https://hg.mozilla.org/mozilla-central/file/cc473fe5dc512c450634506f68cbacfb40a06a23/dom/html/HTMLMediaElement.cpp#l3989
Bug 1221881 - HTMLMediaElement::NetworkError() should clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.
Attachment #8685849 - Flags: review?(cpearce)
I wonder if this will fix bug 1221370
It does, but I would still like to know the root cause of bug 1221370. I think it might help to log the name of the task queue when debugging dispatch failures.
awesome.
should I mark 1221370 as a duplicate?
I think bug 1221370 might be resulted another root cause. It should be worth digging a bit to know the truth.

Also I think it is time to develop a tool to facilitate debugging dispatch failures.
(In reply to JW Wang [:jwwang] from comment #4)
> Created attachment 8685849 [details]
> MozReview Request: Bug 1221881 - HTMLMediaElement::NetworkError() should
> clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.
> 
> Bug 1221881 - HTMLMediaElement::NetworkError() should clear mDecoder. See
> bug 1221881 comment 2 for the root cause. r=cpearce.

Since MediaDecoder::NetworkError() calls MediaDecoder::Shutdown(), we should clear mDecoder for HTMLMediaElement to prevent it from calling into MediaDecoder functions.
Comment on attachment 8685849 [details]
MozReview Request: Bug 1221881 - HTMLMediaElement::NetworkError() should clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24915/diff/1-2/
(In reply to JW Wang [:jwwang] from comment #10)
> Comment on attachment 8685849 [details]
> MozReview Request: Bug 1221881 - HTMLMediaElement::NetworkError() should
> clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/24915/diff/1-2/

Hi Chris,
Can you review this patch? Thanks.
Flags: needinfo?(cpearce)
Comment on attachment 8685849 [details]
MozReview Request: Bug 1221881 - HTMLMediaElement::NetworkError() should clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.

https://reviewboard.mozilla.org/r/24915/#review22919
Attachment #8685849 - Flags: review?(cpearce) → review+
Done. Sorry for delay.
Flags: needinfo?(cpearce)
Thanks for the review!
https://hg.mozilla.org/mozilla-central/rev/c62903740717
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8685849 [details]
MozReview Request: Bug 1221881 - HTMLMediaElement::NetworkError() should clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.

Approval Request Comment
[Feature/regressing bug #]: 1221881
[User impact if declined]: Crashes. Bug 1221370 has information related to crash rates and test case to reproduce the crash 
[Describe test coverage new/current, TreeHerder]: in central, manually verified
[Risks and why]: Low, properly clearing a variable on shutdown
[String/UUID change made/needed]: none
Attachment #8685849 - Flags: approval-mozilla-beta?
Attachment #8685849 - Flags: approval-mozilla-aurora?
Does this affect 43?  Bug 1221370 only has 44/45 marked as affected. This seems good to uplift to 44 though.
Never mind, looks like it has been an issue since firefox 40.
Comment on attachment 8685849 [details]
MozReview Request: Bug 1221881 - HTMLMediaElement::NetworkError() should clear mDecoder. See bug 1221881 comment 2 for the root cause. r=cpearce.

Crash/assertion fix, recent regression. OK to uplift to aurora and beta.
Attachment #8685849 - Flags: approval-mozilla-beta?
Attachment #8685849 - Flags: approval-mozilla-beta+
Attachment #8685849 - Flags: approval-mozilla-aurora?
Attachment #8685849 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: