Closed Bug 1041362 Opened 6 years ago Closed 6 years ago

Intermittent test_mediarecorder_record_no_timeslice.html | uncaught exception - InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable at http://mochi.test:8888/tests/content/media/test/test_mediarecorder_record_no_ti

Categories

(Core :: Audio/Video: Recording, defect)

ARM
Android
defect
Not set
normal

Tracking

()

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

People

(Reporter: RyanVM, Assigned: bechen)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 5 obsolete files)

https://tbpl.mozilla.org/php/getParsedLog.php?id=44230148&tree=Mozilla-Inbound

Android 2.3 Emulator mozilla-inbound opt test mochitest-5 on 2014-07-20 13:29:23 PDT for push 131e77486177
slave: tst-linux64-spot-316

14:26:47     INFO -  2687 INFO SimpleTest START
14:26:47     INFO -  2688 INFO TEST-START | /tests/content/media/test/test_mediarecorder_record_no_timeslice.html
14:26:47     INFO -  2689 INFO dumping last 2 message(s)
14:26:47     INFO -  2690 INFO if you need more context, please use SimpleTest.requestCompleteLog() in your test
14:26:47     INFO -  2691 INFO Started Sun Jul 20 2014 13:37:32 GMT-0700 (PDT) (1405888652.128s) | undefined
14:26:47     INFO -  2692 INFO TEST-PASS | /tests/content/media/test/test_mediarecorder_record_no_timeslice.html | [started detodos.opus-0] Length of array should match number of running tests 
14:26:47     INFO -  2693 INFO TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_mediarecorder_record_no_timeslice.html | uncaught exception - InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable at http://mochi.test:8888/tests/content/media/test/test_mediarecorder_record_no_timeslice.html:95
14:26:47     INFO -  TEST-INFO | expected PASS
Looks like something wrong to the life cycle of |mediaRecorder|. Please have a check.
Assignee: nobody → bechen
Here is my guess:
|element.oncanplaythrough| will start a Session that holds a reference to MR.
Before the |element.oncanplaythrough| is called, the MR is destroyed because there is no Session inside.
So I guess the MR is destroyed at the gap between |element.play()| to |element.oncanplaythrough|.
Indeed, it looks like the test case must be modified in a more robust way.
test_mediarecorder_reocrd_immediate_stop.html
test_mediarecorder_record_session.html
test_mediarecorder_record_timeslice.html

These 3 testcases have the same problem.
update status:
The "uncaught exception - InvalidStateError" triggered by here.

http://dxr.mozilla.org/mozilla-central/source/content/media/MediaRecorder.cpp?from=Mediarecorder.cpp&case=true#736
(In reply to Benjamin Chen [:bechen] from comment #10)
> update status:
> The "uncaught exception - InvalidStateError" triggered by here.
> 
> http://dxr.mozilla.org/mozilla-central/source/content/media/MediaRecorder.
> cpp?from=Mediarecorder.cpp&case=true#736

// testcase:
if (element.ended) {

} else {
   mediaRecorder.start();
}
///
The code snip in testcase shows the MediaElement is not ended, neither the MediaDecoder.
Means that the MediaDecoder::PlaybackEnded function doesn't been invoked to finish/destroy the MediaStream.

But in the MediaRecorder::Start() function, we return error because the MediaStream is finished/destroyed...
Before MediaDecoder::PlaybackEnded() [1] is dispatched, StopAudioThread() is called which will end the audio/video tracks [2]. So, there is a race between stream end media element end on the main thread. Therefore, it is possible to see stream end on the main thread while media element is not ended.

[1] http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/MediaDecoderStateMachine.cpp#l2481
[2] http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/MediaDecoderStateMachine.cpp#l418
(In reply to JW Wang [:jwwang] from comment #14)
> Before MediaDecoder::PlaybackEnded() [1] is dispatched, StopAudioThread() is
> called which will end the audio/video tracks [2]. So, there is a race
> between stream end media element end on the main thread. Therefore, it is
> possible to see stream end on the main thread while media element is not
> ended.
> 
> [1]
> http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/
> MediaDecoderStateMachine.cpp#l2481
> [2]
> http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/
> MediaDecoderStateMachine.cpp#l418

Correction:
In the case of decoding to a stream, the state machine is not responsible for sending MediaDecoder::PlaybackEnded(). See [3].

[3] http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/MediaDecoderStateMachine.cpp#l2466
(In reply to JW Wang [:jwwang] from comment #18)
> (In reply to JW Wang [:jwwang] from comment #14)
> > Before MediaDecoder::PlaybackEnded() [1] is dispatched, StopAudioThread() is
> > called which will end the audio/video tracks [2]. So, there is a race
> > between stream end media element end on the main thread. Therefore, it is
> > possible to see stream end on the main thread while media element is not
> > ended.
> > 
> > [1]
> > http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/
> > MediaDecoderStateMachine.cpp#l2481
> > [2]
> > http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/
> > MediaDecoderStateMachine.cpp#l418
> 
> Correction:
> In the case of decoding to a stream, the state machine is not responsible
> for sending MediaDecoder::PlaybackEnded(). See [3].
> 
> [3]
> http://hg.mozilla.org/mozilla-central/file/5e704397529b/content/media/
> MediaDecoderStateMachine.cpp#l2466

Yeah, the MediaDecoder::PlaybackEnded() should come from :
MediaDecoderStateMachine::SendStreamData => ... =>
=> MediaStreamGraphImpl::UpdateCurrentTimeForStreams  =>  MediaDecoder::DecodedStreamGraphListener::DoNotifyFinished()
So our conclusion is that because the "finish event" comes earlier than MediaDecoder::PlaybackEnded(),
it is possible that the testcase check the "element.ended" returns false but the MediaStream had finished.

I'll try to modify these testcases to not rely on the "element.ended".
Attached patch bug-1041362.v01.patch (obsolete) — Splinter Review
Attached patch bug-1041362.enabletest.patch (obsolete) — Splinter Review
Comment on attachment 8497396 [details] [diff] [review]
bug-1041362.v01.patch

Review of attachment 8497396 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/test/test_mediarecorder_record_immediate_stop.html
@@ +107,5 @@
>         'Media recorder stream = element stream post recording');
>    };
>  
>    element.addEventListener('canplaythrough', canPlayThrough, false);
> +  mediaRecorder.start();

As my old memorize, this line would fail due to principle check fail...
Does this behavior change?
(In reply to Randy Lin [:rlin] from comment #25)
> Comment on attachment 8497396 [details] [diff] [review]
> bug-1041362.v01.patch
> 
> Review of attachment 8497396 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/test/test_mediarecorder_record_immediate_stop.html
> @@ +107,5 @@
> >         'Media recorder stream = element stream post recording');
> >    };
> >  
> >    element.addEventListener('canplaythrough', canPlayThrough, false);
> > +  mediaRecorder.start();
> 
> As my old memorize, this line would fail due to principle check fail...
> Does this behavior change?

http://dxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLMediaElement.cpp#1851

The principle should be set in HTMLMediaElement::CaptureStreamInternal?
(In reply to Randy Lin [:rlin] from comment #25)
> Comment on attachment 8497396 [details] [diff] [review]
> bug-1041362.v01.patch
> 
> Review of attachment 8497396 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/test/test_mediarecorder_record_immediate_stop.html
> @@ +107,5 @@
> >         'Media recorder stream = element stream post recording');
> >    };
> >  
> >    element.addEventListener('canplaythrough', canPlayThrough, false);
> > +  mediaRecorder.start();
> 
> As my old memorize, this line would fail due to principle check fail...
> Does this behavior change?

You are right, try server shows failures. The MediaElement doesn't have principle before we play it.
Now I'm going to set "preload metadata" and start the MediaElement when metadata loaded.
Attached patch bug-1041362.v02.patch (obsolete) — Splinter Review
Attachment #8497396 - Attachment is obsolete: true
Attachment #8498703 - Flags: feedback?(rlin)
Attached patch bug-1041362.enabletest.patch (obsolete) — Splinter Review
tryserver:
https://tbpl.mozilla.org/?tree=Try&rev=12e7b7f4ce5a
Attachment #8497397 - Attachment is obsolete: true
Comment on attachment 8498703 [details] [diff] [review]
bug-1041362.v02.patch

LGTM
Attachment #8498703 - Flags: feedback?(rlin) → feedback+
Comment on attachment 8498703 [details] [diff] [review]
bug-1041362.v02.patch

Review of attachment 8498703 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/test/test_mediarecorder_record_immediate_stop.html
@@ +116,1 @@
>    element.play();

You should play() in onloadedmetadata(), otherwise it is possible for the media stream  to finish before recording starts.
Attached patch bug-1041362.v02.patch (obsolete) — Splinter Review
Address comment 32.

try server:
https://tbpl.mozilla.org/?tree=Try&rev=2f8462d2cfbb
Attachment #8498703 - Attachment is obsolete: true
Attachment #8498725 - Flags: review?(roc)
r=roc
Attachment #8498725 - Attachment is obsolete: true
Attachment #8505367 - Flags: review+
Attachment #8498704 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5ab9da505d34
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
I just noticed the tests are still disabled.
Flags: needinfo?(bechen)
Blocks: 1090083
Flags: needinfo?(bechen)
You need to log in before you can comment on or make changes to this bug.