1310162 - Crash in mozilla::dom::CompareSimpleTextTrackEvents::TrackChildPosition

Status: RESOLVED FIXED

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-af5e0205-eb21-4f33-a2b5-bfd4b2161014.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::dom::CompareSimpleTextTrackEvents::TrackChildPosition(mozilla::dom::SimpleTextTrackEvent*) 	dom/html/TextTrackManager.cpp:467
1 	xul.dll 	mozilla::dom::CompareSimpleTextTrackEvents::LessThan(mozilla::dom::SimpleTextTrackEvent*, mozilla::dom::SimpleTextTrackEvent*) 	dom/html/TextTrackManager.cpp:494
2 	xul.dll 	mozilla::BinarySearchIf<nsTArray_Impl<RefPtr<mozilla::dom::SimpleTextTrackEvent>, nsTArrayInfallibleAllocator>, detail::ItemComparatorFirstElementGT<mozilla::dom::SimpleTextTrackEvent*&, mozilla::dom::CompareSimpleTextTrackEvents> >(nsTArray_Impl<RefPtr<mozilla::dom::SimpleTextTrackEvent>, nsTArrayInfallibleAllocator> const&, unsigned int, unsigned int, detail::ItemComparatorFirstElementGT<mozilla::dom::SimpleTextTrackEvent*&, mozilla::dom::CompareSimpleTextTrackEvents> const&, unsigned int*) 	obj-firefox/dist/include/mozilla/BinarySearch.h:80
3 	xul.dll 	nsTArray_Impl<RefPtr<mozilla::dom::SimpleTextTrackEvent>, nsTArrayInfallibleAllocator>::InsertElementSorted<mozilla::dom::SimpleTextTrackEvent*&, mozilla::dom::CompareSimpleTextTrackEvents, nsTArrayInfallibleAllocator>(mozilla::dom::SimpleTextTrackEvent*&, mozilla::dom::CompareSimpleTextTrackEvents const&) 	obj-firefox/dist/include/nsTArray.h:1444
4 	xul.dll 	mozilla::dom::HTMLMediaElement::FireTimeUpdate(bool) 	dom/html/HTMLMediaElement.cpp:5178

this is a new crash signature that started to pop up in firefox 50 pre-release builds and subsequent versions and is happening in the codepath added with bug 882718.
so far it's happening on various versions of windows with a rather low volume (0.07% of browser crashes in 50.0b6).

Comment 1

[Benjamin Chen [:bechen]]

Seems a duplicate of bug 1304948.

Comment 2

[Andrew Overholt [:overholt]]

Thanks, Benjamin.

Comment 3

[[:philipp]]

hi, crashes with the [@ mozilla::dom::CompareSimpleTextTrackEvents::TrackChildPosition] signature seem to continue in 50 and later even after the fix for bug 1304948 has landed.
do we need to reopen this bug or should a new one be filed for it?

Comment 4

[Benjamin Chen [:bechen]]

Please reopen it if the signature is the same, thanks.

Comment 5

[Benjamin Chen [:bechen]]

Now we can make sure the crash happened when creating |SimpleTextTrackEvent|, the mTrack is null.
https://dxr.mozilla.org/mozilla-central/source/dom/html/TextTrackManager.cpp?q=texttrackmanager.cpp&redirect_type=direct#731
But I don’t figure out the scenario or code sequence yet.

Comment 6

[Benjamin Chen [:bechen]]

Here is a crash scenario I presume:
At the end of playback, if the script remove a Cue from a TextTrack, the Cue will be removed immediately from TextTrack and MediaElement, but still alive because the |mLastActiveCues| holds its reference for exit event. Then seek happened, TimeMarchesOn will fire exit event at the Cue which already removed from TextTrack.

Comment 7

[Benjamin Chen [:bechen]]

Attachment: Bug 1310162 - mTrack in SimpleTextTrackEvent might be null.

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

Comment 8

[Benjamin Chen [:bechen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=16bf9f2ab863

Comment 9

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

Comment on attachment 8822123 [details]
Bug 1310162 - mTrack in SimpleTextTrackEvent might be null.

https://reviewboard.mozilla.org/r/101128/#review101802

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9060d5b0d39a
mTrack in SimpleTextTrackEvent might be null. r=jwwang

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/9060d5b0d39a

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Benjamin, could you consider an uplift to aurora & beta? This caused more than 10 000 crashes on release.

Comment 13

[Benjamin Chen [:bechen]]

Comment on attachment 8822123 [details]
Bug 1310162 - mTrack in SimpleTextTrackEvent might be null.

This patch should be able to clean apply to aurora and beta.

Approval Request Comment
[Feature/Bug causing the regression]: 882718
[User impact if declined]: video with subtitle might crash at the end of playback.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no, do not have reproduce step.
[Needs manual test from QE? If yes, steps to reproduce]: no, do not have reproduce step.
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: very safe
[Why is the change risky/not risky?]: Simple fix, null checking.
[String changes made/needed]: none

Approval Request Comment
[Feature/Bug causing the regression]: 882718
[User impact if declined]: video with subtitle might crash at the end of playback.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no, do not have reproduce step.
[Needs manual test from QE? If yes, steps to reproduce]: no, do not have reproduce step.
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: very safe
[Why is the change risky/not risky?]: Simple fix, null checking.
[String changes made/needed]: none

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8822123 [details]
Bug 1310162 - mTrack in SimpleTextTrackEvent might be null.

Fix for high volume crash, let's uplift to aurora and beta.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3707e4b400f8

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ab00cca642e4