Closed Bug 1276530 Opened 4 years ago Closed 4 years ago

Intermittent test_texttrackevents_video.html | TrackElement should fire cue change 6 times. - got 2, expected 6

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: aryx, Assigned: bechen)

Details

(Keywords: intermittent-failure)

Attachments

(4 files)

https://treeherder.mozilla.org/logviewer.html#?job_id=28990022&repo=mozilla-inbound

13:23:42     INFO -  1836 INFO TEST-START | dom/media/test/test_texttrackevents_video.html
13:23:42     INFO -  [376] WARNING: no triggering principal available via loadInfo, assuming load is cross-origin: file c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/src/netwerk/protocol/http/HttpBaseChannel.cpp, line 1410
13:23:42     INFO -  [376] WARNING: no triggering principal available via loadInfo, assuming load is cross-origin: file c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/src/netwerk/protocol/http/HttpBaseChannel.cpp, line 1410
13:23:42     INFO -  ++DOMWINDOW == 28 (175B8400) [pid = 376] [serial = 694] [outer = 1CB82400]
13:23:45     INFO -  TEST-INFO | started process screenshot
13:23:45     INFO -  TEST-INFO | screenshot: exit 0
13:23:45     INFO -  1837 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | Track::ReadyState should be set to LOADED.
13:23:45     INFO -  1838 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | Track::OnCueChange should exist.
13:23:45     INFO -  1839 INFO TEST-UNEXPECTED-FAIL | dom/media/test/test_texttrackevents_video.html | TrackElement should fire cue change 6 times. - got 2, expected 6
13:23:45     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:268:5
13:23:45     INFO -      @dom/media/test/test_texttrackevents_video.html:56:7
13:23:45     INFO -      EventListener.handleEvent*@dom/media/test/test_texttrackevents_video.html:53:5
13:23:45     INFO -      Async*@dom/media/test/test_texttrackevents_video.html:17:1
13:23:45     INFO -  Not taking screenshot here: see the one that was previously logged
13:23:45     INFO -  1840 INFO TEST-UNEXPECTED-FAIL | dom/media/test/test_texttrackevents_video.html | TrackElement.track should fire cue change 6 times. - got 2, expected 6
13:23:45     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:268:5
13:23:45     INFO -      @dom/media/test/test_texttrackevents_video.html:57:7
13:23:45     INFO -      EventListener.handleEvent*@dom/media/test/test_texttrackevents_video.html:53:5
13:23:45     INFO -      Async*@dom/media/test/test_texttrackevents_video.html:17:1
Assignee: nobody → bechen
See https://html.spec.whatwg.org/multipage/embedded-content.html#time-marches-on
If the playback position is not smooth, step 15 will remove the duplicates if there are multiple oncuechange events on the same TextTrack object.
I will modify the testcase by adding onenter/onexit events.
INFO -  1934 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | Track::ReadyState should be set to LOADED.
INFO -  1935 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | Track::OnCueChange should exist.
INFO -  1936 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | textTrack.cues.length should 3.
INFO -  1937 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | TrackElement should fire cue change at least one times.
INFO -  1938 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | trackElementCueChangeCount should <= 6
INFO -  1939 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | TrackElement.track should fire cue change at least one times.
INFO -  1940 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | trackCueChangeCount should <= 6
INFO -  1941 INFO TEST-PASS | dom/media/test/test_texttrackevents_video.html | cueEnterCount should fire three times.
INFO -  1942 INFO TEST-UNEXPECTED-FAIL | dom/media/test/test_texttrackevents_video.html | cueExitCount should fire three times. - got 1, expected 3

There are 3 onenter events but only 1 onexit events, maybe there is a bug in TimeMarchesOn().
(In reply to Benjamin Chen [:bechen] from comment #5)

> 
> There are 3 onenter events but only 1 onexit events, maybe there is a bug in
> TimeMarchesOn().

https://dxr.mozilla.org/mozilla-central/source/dom/html/TextTrackManager.cpp#663

The cue might not has id.
Attachment #8762462 - Flags: review?(giles) → review+
Comment on attachment 8762462 [details]
Bug 1276530 - Fix test_texttrackevents_video.html.

https://reviewboard.mozilla.org/r/59138/#review56178

Valgrind failure on try looks unrelated.

::: dom/media/test/test_texttrackevents_video.html:80
(Diff revision 1)
>  
>      video.addEventListener('ended', function() {
>        // Should be fired 6 times, as there are 3 cues, with a change event
>        // for when it is activated/deactivated (6 events total)
> -      is(trackElementCueChangeCount, 6, "TrackElement should fire cue change 6 times.");
> -      is(trackCueChangeCount, 6, "TrackElement.track should fire cue change 6 times.");
> +      isnot(trackElementCueChangeCount, 0, "TrackElement should fire cue change at least one times.");
> +      ok(trackElementCueChangeCount <= 6, 'trackElementCueChangeCount should <= 6');

Should be "at least one time." Singular 'time' to agree with 'one'.
Comment on attachment 8762462 [details]
Bug 1276530 - Fix test_texttrackevents_video.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59138/diff/1-2/
Comment on attachment 8762621 [details]
Bug 1276530 - part2: The id of cue might be empty, so don't use it for checking.

https://reviewboard.mozilla.org/r/59200/#review56384
Attachment #8762621 - Flags: review?(giles) → review+
Keywords: checkin-needed
Comment on attachment 8762462 [details]
Bug 1276530 - Fix test_texttrackevents_video.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59138/diff/2-3/
Comment on attachment 8762621 [details]
Bug 1276530 - part2: The id of cue might be empty, so don't use it for checking.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59200/diff/1-2/
failed to apply:

applying fa5b89817264
patching file dom/media/TextTrackCue.h
Hunk #1 FAILED at 297
1 out of 1 hunks FAILED -- saving rejects to file dom/media/TextTrackCue.h.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue

could you take a look, thanks!
Flags: needinfo?(bechen)
Comment on attachment 8762462 [details]
Bug 1276530 - Fix test_texttrackevents_video.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59138/diff/3-4/
Comment on attachment 8762621 [details]
Bug 1276530 - part2: The id of cue might be empty, so don't use it for checking.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59200/diff/2-3/
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9b7466d93f7
Fix test_texttrackevents_video.html. r=rillian
https://hg.mozilla.org/integration/mozilla-inbound/rev/21e3d0701b3d
part2: The id of cue might be empty, so don't use it for checking. r=rillian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c9b7466d93f7
https://hg.mozilla.org/mozilla-central/rev/21e3d0701b3d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Flags: needinfo?(bechen)
Can you please nominate this for Aurora uplift when you get a chance? Also, Part 2 will need rebasing if/when it's approved.
Flags: needinfo?(bechen)
Approval Request Comment
[Feature/regressing bug #]: none
[User impact if declined]: none
[Describe test coverage new/current, TreeHerder]: test_texttrackevents_video.html
[Risks and why]: low risk because testcase is wrong
[String/UUID change made/needed]: none
Flags: needinfo?(bechen)
Attachment #8765353 - Flags: approval-mozilla-aurora?
Approval Request Comment
[Feature/regressing bug #]:882718
[User impact if declined]: onenter/onexit events might lost.
[Describe test coverage new/current, TreeHerder]:test_texttrackevents_video.html
[Risks and why]: low.
[String/UUID change made/needed]: none
Attachment #8765356 - Flags: approval-mozilla-aurora?
Comment on attachment 8765353 [details] [diff] [review]
bug1276530part1fixtestcase.aurora.patch

Test fixes, ok to uplift
Attachment #8765353 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8765356 [details] [diff] [review]
bug1276530part2cueid.aurora.patch

Fix for video events, ok to uplift to aurora along with the changed tests.
Attachment #8765356 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.