Closed Bug 1310162 Opened 4 years ago Closed 4 years ago

Crash in mozilla::dom::CompareSimpleTextTrackEvents::TrackChildPosition

Categories

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

50 Branch
All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox49 --- unaffected
firefox50 --- wontfix
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: philipp, Assigned: bechen)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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).
Flags: needinfo?(bechen)
Seems a duplicate of bug 1304948.
Flags: needinfo?(bechen)
Thanks, Benjamin.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1304948
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?
Flags: needinfo?(bechen)
Please reopen it if the signature is the same, thanks.
Flags: needinfo?(bechen)
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
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.
Assignee: nobody → bechen
Priority: -- → P1
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 on attachment 8822123 [details]
Bug 1310162 - mTrack in SimpleTextTrackEvent might be null.

https://reviewboard.mozilla.org/r/101128/#review101802
Attachment #8822123 - Flags: review?(jwwang) → review+
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9060d5b0d39a
mTrack in SimpleTextTrackEvent might be null. r=jwwang
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9060d5b0d39a
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Benjamin, could you consider an uplift to aurora & beta? This caused more than 10 000 crashes on release.
Flags: needinfo?(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
Flags: needinfo?(bechen)
Attachment #8822123 - Flags: approval-mozilla-beta?
Attachment #8822123 - Flags: approval-mozilla-aurora?
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.
Attachment #8822123 - Flags: approval-mozilla-beta?
Attachment #8822123 - Flags: approval-mozilla-beta+
Attachment #8822123 - Flags: approval-mozilla-aurora?
Attachment #8822123 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.