Closed Bug 1298307 Opened 8 years ago Closed 6 years ago

Intermittent dom/media/test/test_webvtt_empty_displaystate.html | Cue's displayState shouldn't be empty.

Categories

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

Product:
Core
Component:
Audio/Video: Playback
Type:
defect

Tracking

()

Status:
RESOLVED INCOMPLETE
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr52 --- wontfix
firefox55 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: bechen)

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

Bug 1298307 - Check cue's active state first, then verify displaystate.
59 bytes, text/x-review-board-request
alwu
: review+
smaug
: review+
Details
bug1298307avoidnullframe.patch
2.52 KB, patch
bechen
: review+
Details | Diff | Splinter Review
Component: Audio/Video → Audio/Video: Playback
Assignee: nobody → bechen
After discuss with :alwu offline, we find something we need to modify or investigate. 1. The testcase assume that when a cue receive an onenter event, the displayState object should exist. This assumption is wrong because there is "missing cue" whose displayState is null but still receive onenetr event. Missing cue comes from the bad performance playback, for example: if the timeupdate are 1s, 5s, 10s, once a cue whose time is 2-3s, it is a missing cue, break the testcase. 2. In the testcase, the cue whose time is 0s to 1s, if it is a missing cue, means that the timeupdate of the MediaElement doesn't have 0s. It is weird that we don't receive the timeupdate whose time is 0s.
Comment on attachment 8867628 [details] Bug 1298307 - Check cue's active state first, then verify displaystate. https://reviewboard.mozilla.org/r/139202/#review142952
Attachment #8867628 - Flags: review?(alwu) → review+
Keywords: checkin-needed
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 2 changes to 2 files remote: remote: WebIDL file dom/webidl/VTTCue.webidl altered in changeset e3e52ed72862 without DOM peer review remote: remote: remote: remote: ************************** ERROR **************************** remote: remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=... remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding.. remote: remote: ************************************************************* remote: remote: remote: transaction abort! remote: rollback completed remote: pretxnchangegroup.d_webidl hook failed abort: push failed on remote
Comment on attachment 8867628 [details] Bug 1298307 - Check cue's active state first, then verify displaystate. https://reviewboard.mozilla.org/r/139202/#review143742 Hi :smaug, please help to review the patch. The testcase wants to verify the displayState of the TextTrackCue object, and the problem is that when TextTrackCue receive the onenter event, the displayState might be empty because it is a missing cue, details in comment 18. There is a boolean member |mActive| in TextTrackCue which indicate the cue is showing or not. When the mActive is true, the displayState should be true. So I modify the VTTCue.webidl. (Note: the displayState is a div, the root of a subtree parsed in vtt.jsm)
Comment on attachment 8867628 [details] Bug 1298307 - Check cue's active state first, then verify displaystate. https://reviewboard.mozilla.org/r/139202/#review144134
Attachment #8867628 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fd0137b3448c Check cue's active state first, then verify displaystate. r=alwu,smaug
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fd0137b3448c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
This isn't fixed.
Status: RESOLVED → REOPENED
status-firefox55: fixed → affected
Flags: needinfo?(bechen)
Resolution: FIXED → ---
Target Milestone: mozilla55 → ---
I guess we hit the timing issue. From the log, fail seems only occurs at opt build. The nsVideoFrame be constructed after append to dom tree, not immediately. Once the main thread and decode thread run faster than "next refresh driver tick", the TextTrackManager will early return here http://searchfox.org/mozilla-central/source/dom/html/TextTrackManager.cpp#270 I still thinking about changing the testcase or our implementation.
Flags: needinfo?(bechen)
Attached patch bug1298307avoidnullframe.patch (obsolete) — — Splinter Review
Attachment #8874288 - Flags: review?(alwu)
Comment on attachment 8874288 [details] [diff] [review] bug1298307avoidnullframe.patch Review of attachment 8874288 [details] [diff] [review]: ----------------------------------------------------------------- Please also modify the comment in [1] in order to capture the case of unexpected missing cue. [1] https://goo.gl/38QBcU ::: dom/media/test/test_webvtt_empty_displaystate.html @@ +58,5 @@ > info("--- check the type of cue ---"); > isnot(window.TextTrackCue, undefined, "TextTrackCue should be defined."); > isnot(window.VTTCue, undefined, "VTTCue should be defined."); > > + var cue = new VTTCue(1, 2, "Test cue"); Add the check to ensure the ending time is always small than the video duration, in case someday the video duration changes.
Attachment #8874288 - Flags: review?(alwu) → review+
Attachment #8874288 - Attachment is obsolete: true
Attachment #8874366 - Flags: review+
Hi sheriff, please help to check-in attachment 8874366 [details] [diff] [review].
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/028a9e5e1355 Change the cue's startTime/endTime to avoid the videoframe was not created. r=alwu
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/028a9e5e1355
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I hit this again on my Fennec devices, and the reproduce rate is about 0.1%. However I have some idea to increase the reproduce rate. I'll try to reproduce it at local first.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Here is the root cause: https://searchfox.org/mozilla-central/source/dom/html/TextTrackManager.cpp#273 We early return here because the videoFrame is not created at the beginning.
Flags: needinfo?(alwu)
Since its failure rate is super low, I think it's not worth to spend time investigating it. I'll prefer to wait for a long while to see its trend (still happen or disappear).
Flags: needinfo?(alwu)
https://wiki.mozilla.org/Bugmasters#Intermittent_Test_Failure_Cleanup
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → INCOMPLETE

Fail reappeared on m-c
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=223828377&repo=mozilla-central&lineNumber=3426

Log snippet:

19:16:14 INFO - TEST-PASS | dom/media/test/test_webvtt_empty_displaystate.html | video.duration should larger than 2
19:16:14 INFO - --- video.currentTime is 0.000047
19:16:14 INFO - --- video.currentTime is 0.28021
19:16:14 INFO - --- video.currentTime is 0.563409
19:16:14 INFO - --- video.currentTime is 0.843682
19:16:14 INFO - Buffered messages logged at 19:15:46
19:16:14 INFO - cueChrome.getActive true
19:16:14 INFO - Buffered messages finished
19:16:14 INFO - TEST-UNEXPECTED-FAIL | dom/media/test/test_webvtt_empty_displaystate.html | Cue's displayState shouldn't be empty.
19:16:14 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:275:7
19:16:14 INFO - checkCueDisplayState@dom/media/test/test_webvtt_empty_displaystate.html:28:5
19:16:14 INFO - runTest/cue.onenter@dom/media/test/test_webvtt_empty_displaystate.html:81:7
19:16:14 INFO - EventHandlerNonNullrunTest@dom/media/test/test_webvtt_empty_displaystate.html:75:3
19:16:14 INFO - EventHandlerNonNull@dom/media/test/test_webvtt_empty_displaystate.html:95:1
19:16:14 INFO - --- video.currentTime is 1.125941
19:16:14 INFO - --- video.currentTime is 1.407174
19:16:14 INFO - --- video.currentTime is 1.688404
19:16:14 INFO - --- video.currentTime is 1.972608
19:16:14 INFO - TEST-PASS | dom/media/test/test_webvtt_empty_displaystate.html | Cue's displayState should be empty.
19:16:14 INFO - --- video.currentTime is 2.012647

Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---

https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup

Status: REOPENED → RESOLVED
Closed: 7 years ago → 6 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: