Closed Bug 760770 Opened 13 years ago Closed 10 years ago

Intermittent test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm or test/gizmo.mp4 got unexpected stalled after ended

Categories

(Core :: Audio/Video, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
firefox36 --- fixed
firefox-esr31 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: philor, Assigned: jwwang)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

https://tbpl.mozilla.org/php/getParsedLog.php?id=12300738&tree=Mozilla-Inbound Rev3 WINNT 6.1 mozilla-inbound opt test mochitests-1/5 on 2012-06-01 23:44:38 PDT for push 26b8f2292405 slave: talos-r3-w7-062 95581 INFO TEST-INFO | /tests/content/media/test/test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm got timeupdate nsBuiltinDecoderStateMachine::RunStateMachine queuing nsBuiltinDecoder::PlaybackEnded nsBuiltinDecoder::PlaybackEnded mPlayState=3 95582 INFO TEST-PASS | /tests/content/media/test/test_timeupdate_small_files.html | seek.webm - shouldn't get timeupdate after ended 95583 INFO TEST-INFO | /tests/content/media/test/test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm got timeupdate 95584 INFO TEST-PASS | /tests/content/media/test/test_timeupdate_small_files.html | seek.webm - should see at least one timeupdate: 4 95585 INFO TEST-INFO | /tests/content/media/test/test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm got ended 95586 INFO TEST-PASS | /tests/content/media/test/test_timeupdate_small_files.html | [finished detodos.opus-4] Length of array should match number of running tests - 1 should equal 1 95587 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm got unexpected stalled after ended 95588 INFO TEST-PASS | /tests/content/media/test/test_timeupdate_small_files.html | [finished seek.webm-3] Length of array should match number of running tests - 0 should equal 0 95589 INFO TEST-INFO | /tests/content/media/test/test_timeupdate_small_files.html | Finished at Sat Jun 02 2012 00:01:57 GMT-0700 (Pacific Daylight Time) (1338620517.262s)
Blocks: 760325
kinetik: Do you have any ideas about this? Currently the #4 orange failure on mozilla-inbound.
kinetik, ping for comment 145
kinetik, re-ping for comment 145 / comment 338.
Chris, since you wrote this test, can you tell us if this failure is valid? In bug 760325, roc seemed to agree with me that it's the original intent of the test, but due to the premature 'finished' call the failure was ignored by tinderbox (bug 677964).
(In reply to Mats Palmgren [:mats] (on vacation) from comment #571) > Chris, since you wrote this test, can you tell us if this failure > is valid? In bug 760325, roc seemed to agree with me that it's the > original intent of the test, but due to the premature 'finished' > call the failure was ignored by tinderbox (bug 677964). cpearce pinged via direct email, in case this was lost in the tbplbot spam.
Following on from the email, I've tried to disable on just the failing case (ie event.type stalled & just for seek.webm). Hope that's ok? :-)
Attachment #648331 - Flags: review?(cpearce)
Attachment #648331 - Flags: review?(cpearce) → review+
Whiteboard: [orange] → [orange][test disabled][leave open]
Whiteboard: [orange][test disabled][leave open] → [orange][part of test disabled][leave open]
Comment on attachment 648331 [details] [diff] [review] Mark as todo for 'stalled' && seek.webm [Approval Request Comment] Bug caused by (feature/regressing bug #): Unknown User impact if declined: Greater intermittent orange Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): test-only change String or UUID changes made by this patch: None
Attachment #648331 - Flags: approval-mozilla-beta?
Attachment #648331 - Flags: approval-mozilla-aurora?
Comment on attachment 648331 [details] [diff] [review] Mark as todo for 'stalled' && seek.webm low risk, test-only change, approving.
Attachment #648331 - Flags: approval-mozilla-beta?
Attachment #648331 - Flags: approval-mozilla-beta+
Attachment #648331 - Flags: approval-mozilla-aurora?
Attachment #648331 - Flags: approval-mozilla-aurora+
Mark as todo for 'stalled' && seek.webm: https://hg.mozilla.org/releases/mozilla-aurora/rev/f2e923c88812 https://hg.mozilla.org/releases/mozilla-beta/rev/90cded313be6 (Not marking fixed on branch since only disables part of the test)
Blocks: 791105
Whiteboard: [orange][part of test disabled][leave open] → [part of test disabled][leave open]
Comment on attachment 648331 [details] [diff] [review] Mark as todo for 'stalled' && seek.webm (To get this off of the bugzilla-todos "to check in" list)
Attachment #648331 - Flags: checkin+
Summary: Intermittent test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm got unexpected stalled after ended → Intermittent test_timeupdate_small_files.html | http://mochi.test:8888/tests/content/media/test/seek.webm or test/gizmo.mp4 got unexpected stalled after ended
Disabling on Linux for too many intermittent failures. Couldn't find any reference to test_timeupdate_small_files.html in http://mxr.mozilla.org/mozilla-central/source/content/media/test/manifest.js so nuking the whole test on Linux for now - feel free to follow up with a more targeted disabling if desired.
Whiteboard: [part of test disabled][leave open] → [test disabled][leave open]
ChannelMediaResource::CacheClientNotifyDataEnded() will post an event to call HTMLMediaElement::DownloadSuspended() in the next cycle in the main thread. Meanwhile, it is possible for MediaDecoderStateMachine to decode to the end of the stream and post MediaDecoder::PlaybackEnded() to the main thread. Therefore, there is a race between HTMLMediaElement::DownloadSuspended() and MediaDecoder::PlaybackEnded(). I think we can modify the test to allow at most one 'progress' and one 'suspend' (triggered by HTMLMediaElement::DownloadSuspended()) after 'ended'. Of cos, 'stalled' should not be allowed after 'ended' event.
Flags: needinfo?(roc)
How about, if playback has ended then we make HTMLMediaElement::DownloadSuspended not do anything?
Flags: needinfo?(roc)
HTMLMediaElement::DownloadSuspended() is needed to change the network state to NETWORK_IDLE. Otherwise, the network state will stay in NETWORK_LOADING.
Flags: needinfo?(roc)
(In reply to JW Wang [:jwwang] from comment #930) > I think we can modify the test to allow at most one 'progress' and one > 'suspend' (triggered by HTMLMediaElement::DownloadSuspended()) after > 'ended'. Of cos, 'stalled' should not be allowed after 'ended' event. OK but I think we should allow any number of 'progress' and 'suspend' events after 'ended'.
Flags: needinfo?(roc)
Sorry, I don't understand how more than one 'progress' is possible after 'ended' when HTMLMediaElement::DownloadSuspended() only fires one.
Flags: needinfo?(roc)
It's just that per-spec I don't think it's a bug to fire multiple progress and suspend events after 'ended'.
Flags: needinfo?(roc)
Per discussion from comment 930 to comment 947, we would allow 'progress' and 'suspend' events after 'ended' (but no others).
Assignee: nobody → jwwang
Attachment #8505287 - Flags: review?(roc)
Please check in 760770_fix_test_timeupdate_small_files_html.patch only. Thanks.
Keywords: checkin-needed
Whiteboard: [test disabled][leave open] → [test disabled]
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: