[b2g] The repeat deleting to the port of the OutputStreamData

RESOLVED FIXED in Firefox 37

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: alwu, Assigned: alwu)

Tracking

unspecified
mozilla37
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 wontfix, firefox37 fixed)

Details

(crash signature)

Attachments

(2 attachments, 8 obsolete attachments)

STR.
1. goto http://people.mozilla.org/~rlin/MediaRecorder.html
2. play VP9/720WebM/352WebM
3. refresh browser
4. continually repeat step 2 & 3, crash
dump the callstack on flame 2.2 

(gdb) bt
#0  mozilla::MediaInputPort::Destroy (this=0x0) at ../../../../firefox/dom/media/MediaStreamGraph.cpp:2615
#1  0xb55390ea in mozilla::MediaDecoder::DestroyDecodedStream (this=0xb31d9f20) at ../../../../firefox/dom/media/MediaDecoder.cpp:294
#2  0xb553c818 in Shutdown (this=0xb31d9f20) at ../../../../firefox/dom/media/MediaDecoder.cpp:484
#3  mozilla::MediaDecoder::Shutdown (this=0xb31d9f20) at ../../../../firefox/dom/media/MediaDecoder.cpp:473
#4  0xb54f72ea in mozilla::dom::HTMLMediaElement::ShutdownDecoder (this=this@entry=0xb1c3d880) at ../../../../firefox/dom/html/HTMLMediaElement.cpp:609
#5  0xb54f7b42 in mozilla::dom::HTMLMediaElement::~HTMLMediaElement (this=0xb1c3d880, __in_chrg=<optimized out>) at ../../../../firefox/dom/html/HTMLMediaElement.cpp:2053
#6  0xb5508e7a in mozilla::dom::HTMLVideoElement::~HTMLVideoElement (this=0xb1c3d880, __in_chrg=<optimized out>) at ../../../../firefox/dom/html/HTMLVideoElement.cpp:51
#7  0xb5508e8c in mozilla::dom::HTMLVideoElement::~HTMLVideoElement (this=0xb1c3d880, __in_chrg=<optimized out>) at ../../../../firefox/dom/html/HTMLVideoElement.cpp:51
#8  0xb4f9a0d2 in InMemoryDataSource::DeleteCycleCollectable (this=<optimized out>) at ../../../../firefox/rdf/base/nsRDFXMLDataSource.cpp:453
#9  0xb514831c in mozilla::dom::FragmentOrElement::cycleCollection::DeleteCycleCollectable (this=<optimized out>, p=<optimized out>) at ../../dist/include/mozilla/dom/FragmentOrElement.h:235
#10 0xb4cf88f2 in SnowWhiteKiller::~SnowWhiteKiller (this=0xbe9c6f44, __in_chrg=<optimized out>) at ../../../../firefox/xpcom/base/nsCycleCollector.cpp:2646
#11 0xb4cf899c in nsCycleCollector::FreeSnowWhite (this=0xb6aaa000, aUntilNoSWInPurpleBuffer=aUntilNoSWInPurpleBuffer@entry=false) at ../../../../firefox/xpcom/base/nsCycleCollector.cpp:2820
#12 0xb4cf89c6 in nsCycleCollector_doDeferredDeletion () at ../../../../firefox/xpcom/base/nsCycleCollector.cpp:4201
#13 0xb4f6490c in AsyncFreeSnowWhite::Run (this=0xb6a810c0) at ../../../../../firefox/js/xpconnect/src/XPCJSRuntime.cpp:226
#14 0xb4d14e78 in nsThread::ProcessNextEvent (this=0xb6a02710, aMayWait=<optimized out>, aResult=0xbe9c7017) at ../../../../firefox/xpcom/threads/nsThread.cpp:830
#15 0xb4d22aa6 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=aMayWait@entry=false) at /home/alastor/firefox/xpcom/glue/nsThreadUtils.cpp:265
#16 0xb4e66d2c in mozilla::ipc::MessagePump::Run (this=0xb6a01eb0, aDelegate=0xbe9c7118) at ../../../../firefox/ipc/glue/MessagePump.cpp:99
#17 0xb4e5b654 in MessageLoop::RunInternal (this=this@entry=0xbe9c7118) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:233
#18 0xb4e5b708 in RunHandler (this=0xbe9c7118) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:226
#19 MessageLoop::Run (this=0xbe9c7118) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:200
#20 0xb56f9e06 in nsBaseAppShell::Run (this=0xb3930520) at ../../../firefox/widget/nsBaseAppShell.cpp:164
#21 0xb5a150d2 in XRE_RunAppShell () at ../../../../firefox/toolkit/xre/nsEmbedFunctions.cpp:731
#22 0xb4e5b654 in MessageLoop::RunInternal (this=this@entry=0xbe9c7118) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:233
#23 0xb4e5b708 in RunHandler (this=0xbe9c7118) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:226
#24 MessageLoop::Run (this=this@entry=0xbe9c7118) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:200
#25 0xb5a1550a in XRE_InitChildProcess (aArgc=<optimized out>, aArgv=<optimized out>, aGMPLoader=<optimized out>) at ../../../../firefox/toolkit/xre/nsEmbedFunctions.cpp:568
#26 0xb4f3d810 in content_process_main (argc=8, argc@entry=9, argv=argv@entry=0xb6a01ca0) at ../../../../firefox/ipc/contentproc/plugin-container.cpp:190
#27 0xb4e67f8e in mozilla::ipc::ProcLoaderLoadRunner::DoWork (this=<optimized out>) at ../../../../firefox/ipc/glue/ProcessUtils_linux.cpp:415
#28 0xb4e65ea6 in ProcLoaderServiceRun (aReservedFds=..., aArgv=<optimized out>, aArgc=1, aFd=<optimized out>, aPeerPid=32340) at ../../../../firefox/ipc/glue/ProcessUtils_linux.cpp:574
#29 XRE_ProcLoaderServiceRun (aPeerPid=32340, aFd=<optimized out>, aArgc=1, aArgv=<optimized out>, aReservedFds=...) at ../../../../firefox/ipc/glue/ProcessUtils_linux.cpp:608
#30 0x00011314 in RunProcesses (aReservedFds=..., argv=0xbe9c7c84, argc=1) at ../../../../firefox/b2g/app/B2GLoader.cpp:219
#31 main (argc=1, argv=0xbe9c7c84) at ../../../../firefox/b2g/app/B2GLoader.cpp:290
It seems that there are another crashes related to this bug. They all have similar error backtrace. 

Case 1.
STR.
1. goto http://people.mozilla.org/~rlin/MediaRecorder.html
2. Play 720PWebM, then put the phone to sleep
3. Wake up the phone & unlock the device
4. The video becomes a loading state (It seems abnormal), then put the phone to sleep
5. Repeat step 3, then crash happens

Case 2.
STR.
1. goto http://people.mozilla.org/~rlin/MediaRecorder.html
2. Play 720PWebM, then put the phone to sleep
3. Wake up the phone & unlock the device
4. The video becomes a loading state , then play 720PWebM again
5. Crash
Here are their callback, and there are some differences after the #27. 

#0  mozilla::MediaInputPort::Destroy (this=0x0) at ../../../../firefox/dom/media/MediaStreamGraph.cpp:2619
#1  0xb597f474 in mozilla::MediaDecoder::DestroyDecodedStream (this=0xb1246040) at ../../../../firefox/dom/media/MediaDecoder.cpp:301
#2  0xb597f4d6 in mozilla::MediaDecoder::SetDormantIfNecessary (this=0xb1246040, aDormant=<optimized out>) at ../../../../firefox/dom/media/MediaDecoder.cpp:142
#3  0xb59405e6 in mozilla::dom::HTMLMediaElement::NotifyOwnerDocumentActivityChanged (this=this@entry=0xb43bdda0) at ../../../../firefox/dom/html/HTMLMediaElement.cpp:3470
#4  0xb59565cc in mozilla::dom::HTMLVideoElement::NotifyOwnerDocumentActivityChanged (this=0xb43bdda0) at ../../../../firefox/dom/html/HTMLVideoElement.cpp:188
#5  0xb55c5f9a in NotifyActivityChanged (aSupports=0xb43bdda0, aUnused=<optimized out>) at ../../../../firefox/dom/base/nsDocument.cpp:4492
#6  0xb55bad1c in EnumerateObservers (aEntry=<optimized out>, aData=<optimized out>) at ../../../../firefox/dom/base/nsDocument.cpp:9881
#7  0xb55baf9e in nsTHashtable<nsPtrHashKey<nsISupports> >::s_EnumStub (aTable=<optimized out>, aEntry=<optimized out>, aNumber=<optimized out>, aArg=<optimized out>)
    at ../../dist/include/nsTHashtable.h:496
#8  0xb516e058 in Enumerate (aArg=0xbebf2f28, aEtor=0xb55baf95 <nsTHashtable<nsPtrHashKey<nsISupports> >::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*)>, this=0xb1a538a0)
    at /home/alastor/firefox/xpcom/glue/pldhash.cpp:723
#9  PL_DHashTableEnumerate (aTable=0xb1a538a0, aEtor=0xb55baf95 <nsTHashtable<nsPtrHashKey<nsISupports> >::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*)>, aArg=aArg@entry=0xbebf2f28)
    at /home/alastor/firefox/xpcom/glue/pldhash.cpp:774
#10 0xb55c0e08 in EnumerateEntries (aUserArg=0xbebf2f20, aEnumFunc=0xb55bad13 <EnumerateObservers(nsPtrHashKey<nsISupports>*, void*)>, this=<optimized out>) at ../../dist/include/nsTHashtable.h:215
#11 nsIDocument::EnumerateActivityObservers (this=this@entry=0xb1a8f800, aEnumerator=<optimized out>, aData=aData@entry=0x0) at ../../../../firefox/dom/base/nsDocument.cpp:9892
#12 0xb55c125a in UpdateVisibilityState (this=0xb1a8f800) at ../../../../firefox/dom/base/nsDocument.cpp:12142
#13 nsDocument::UpdateVisibilityState (this=0xb1a8f800) at ../../../../firefox/dom/base/nsDocument.cpp:12128
#14 0xb513b60e in nsRunnableMethodImpl<tag_nsresult (nsMemoryReporterManager::*)(), void, true>::Run (this=<optimized out>) at ../../dist/include/nsThreadUtils.h:388
#15 0xb51609b0 in nsThread::ProcessNextEvent (this=0xb4302710, aMayWait=<optimized out>, aResult=0xbebf2fc7) at ../../../../firefox/xpcom/threads/nsThread.cpp:830
#16 0xb516e5de in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=aMayWait@entry=false) at /home/alastor/firefox/xpcom/glue/nsThreadUtils.cpp:265
#17 0xb52ada0c in mozilla::ipc::MessagePump::Run (this=0xb4301dc0, aDelegate=0xbebf30c8) at ../../../../firefox/ipc/glue/MessagePump.cpp:99
#18 0xb52a2334 in MessageLoop::RunInternal (this=this@entry=0xbebf30c8) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:233
#19 0xb52a23e8 in RunHandler (this=0xbebf30c8) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:226
#20 MessageLoop::Run (this=0xbebf30c8) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:200
#21 0xb5b40286 in nsBaseAppShell::Run (this=0xb222e5e0) at ../../../firefox/widget/nsBaseAppShell.cpp:164
#22 0xb5e5b7d2 in XRE_RunAppShell () at ../../../../firefox/toolkit/xre/nsEmbedFunctions.cpp:731
#23 0xb52a2334 in MessageLoop::RunInternal (this=this@entry=0xbebf30c8) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:233
#24 0xb52a23e8 in RunHandler (this=0xbebf30c8) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:226
#25 MessageLoop::Run (this=this@entry=0xbebf30c8) at ../../../../firefox/ipc/chromium/src/base/message_loop.cc:200
#26 0xb5e5bc0a in XRE_InitChildProcess (aArgc=<optimized out>, aArgv=<optimized out>, aGMPLoader=<optimized out>) at ../../../../firefox/toolkit/xre/nsEmbedFunctions.cpp:568
#27 0x00009114 in content_process_main (argc=7, argv=0xbebf3a54) at ../../../../firefox/ipc/app/../contentproc/plugin-container.cpp:190

[Case1]
#28 0xb4e6ac6e in mozilla::ipc::ProcLoaderLoadRunner::DoWork (this=<optimized out>) at ../../../../firefox/ipc/glue/ProcessUtils_linux.cpp:415
#29 0xb4e68b86 in ProcLoaderServiceRun (aReservedFds=..., aArgv=<optimized out>, aArgc=1, aFd=<optimized out>, aPeerPid=206) at ../../../../firefox/ipc/glue/ProcessUtils_linux.cpp:574
#30 XRE_ProcLoaderServiceRun (aPeerPid=206, aFd=<optimized out>, aArgc=1, aArgv=<optimized out>, aReservedFds=...) at ../../../../firefox/ipc/glue/ProcessUtils_linux.cpp:608
#31 0x00011314 in RunProcesses (aReservedFds=..., argv=0xbeb1ec84, argc=1) at ../../../../firefox/b2g/app/B2GLoader.cpp:219
#32 main (argc=1, argv=0xbeb1ec84) at ../../../../firefox/b2g/app/B2GLoader.cpp:290

[Case2]
#28 0xb6e704a4 in __libc_init (raw_args=0xbebf3a50, onexit=<optimized out>, slingshot=0x9131 <main(int, char**)>, structors=<optimized out>) at bionic/libc/bionic/libc_init_dynamic.cpp:112
#29 0x00009014 in _start ()
Assignee: nobody → alwu
Let me describe more about this crash.

========================================================================
Reason : Due to the repeat deleting to the port of the OutputStreamData. 

Something you should know : When the media went into the dormant state, the source and ports were released by |DestroyDecodedStream()|. 

[Crash case]
1. Play video & refresh it 
  Step1: Play a video
  Step2: Refreshing made the media into the dormant state
  Step3: Destroy the ports and decoded-source
  Step4: The decoder shutdown, call |DestroyDecodedStream()| to destroy the ports again 
  Step5: Crash! repeat deleting!
 

2. Play video -> sleep the phone -> wake up the phone -> sleep the phone
  Step1: Play a video
  Step2: Sleep the phone, media into dormant state
  Step3: Destroy the ports and decoded-source
  Step4: Wake up, but not recreate the ports (because the decoded-source didn't exist)
  Step5: Sleep the phone, media into dormant state
  Step6: Destroy the ports and decoded-source (the ports were already released)
  Step7: Crash! repeat deleting!
Posted patch solveForRepeatDeleting.patch (obsolete) — Splinter Review
Now I have a simple patch could fix the crash problem, but there still has a problem. The video does not play after waking up the phone, and it stays in the seek state.
(In reply to Alastor Wu [:alwu] from comment #5)
> Created attachment 8528260 [details] [diff] [review]
> solveForRepeatDeleting.patch
> 
> Now I have a simple patch could fix the crash problem, but there still has a
> problem. The video does not play after waking up the phone, and it stays in
> the seek state.
 
And there has an error message , "Unexpected error when seeking ", which is resultd from OmxDecoder::ReadVideo(). The result of |mVideoSource->read(..)| is "INFO_FORMAT_CHANGED".
Summary: [b2g] crash when continually play & refresh the video playback → [b2g] The repeat deleting to the port of the OutputStreamData
I am about to introduce a test to bug 879717 that reproduces this crash at a high rate on B2G.

What it does is essentially shutting down the decoder immediately after loadedmetadata.

See my try here:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2da1d9869681

And the relevant log on B2G emulator OPT here:
http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/pehrsons@gmail.com-2da1d9869681/try-emulator/try_ubuntu64_vm-b2g-emulator_test-mochitest-6-bm117-tests1-linux64-build315.txt.gz
Blocks: 879717
Because of blocking another bug, I think that I should fixed this crash first. So I move the problem, video can't playback after waking from the dormant state, to another bug. See Bug 1105913.
Posted patch Bug1100803_v1.patch (obsolete) — Splinter Review
Hi, JW,
Could you give my patch some suggestions?
I describe some details in the comment4, thanks a lot for your help :)
Attachment #8528260 - Attachment is obsolete: true
Attachment #8530055 - Flags: feedback?(jwwang)
Comment on attachment 8530055 [details] [diff] [review]
Bug1100803_v1.patch

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

::: dom/media/MediaDecoder.cpp
@@ +290,5 @@
>      // its Destroy message before this decoder is destroyed. So we have to
>      // be careful not to send any messages after the Destroy().
>      if (os.mStream->IsDestroyed()) {
>        // Probably the DOM MediaStream was GCed. Clean up.
> +      if (os.mPort) {

You should also do so in MediaDecoder::PlaybackEnded().

@@ +307,2 @@
>    }
> +  if (mShuttingDown) {

What is this for?
(In reply to Alastor Wu [:alwu] from comment #4)
> 2. Play video -> sleep the phone -> wake up the phone -> sleep the phone
>   Step1: Play a video
>   Step2: Sleep the phone, media into dormant state
>   Step3: Destroy the ports and decoded-source

What is decoded-source? Can you show me the code?
(In reply to JW Wang [:jwwang] from comment #10)
> @@ +307,2 @@
> >    }
> > +  if (mShuttingDown) {
> 
> What is this for?

If we release |mDecodedStream| in the dormant state, we can't arouse the function|RecreateDecodedStream(...)| in the |MediaDecoderStateMachine::StartSeek()|.

> What is decoded-source? Can you show me the code?

What I mean is |nsAutoPtr<DecodedStreamData> mDecodedStream| which is a member variable of the MediaDecoder.
Comment on attachment 8530055 [details] [diff] [review]
Bug1100803_v1.patch

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

::: dom/media/MediaDecoder.cpp
@@ +307,2 @@
>    }
> +  if (mShuttingDown) {

It will leak mDecodedStream if MediaDecoder::RecreateDecodedStream is called when |mShuttingDown| is false.
Attachment #8530055 - Flags: feedback?(jwwang) → feedback-
Alastor, are you still looking into this? I guess you might be busy in meetings all of this week.

It is the last bit left blocking bug 879717 after bug 1106963 lands (I hope). Would be great to close it soon.
Flags: needinfo?(alwu)
Hi, Andreas, 
Sorry for that slow process. I would try to finish it ASAP.
Flags: needinfo?(alwu)
Posted patch Bug1100803_v2.patch (obsolete) — Splinter Review
Hi, JW,
Could you give my patch some suggestions?

The previous starting condition of the |RecreateDecodedStream()| leads to the error that we can't recreate the stream and ports after waking from the dormant state. Because we release the |mDecodedStream| in the dormant state, but we need it to arouse the |RecreateDecodedStream()|.

So I change the condition to ensure we can arouse the |RecreateDecodedStream()| successfully.
Attachment #8530055 - Attachment is obsolete: true
Attachment #8533462 - Flags: feedback?(jwwang)
Comment on attachment 8533462 [details] [diff] [review]
Bug1100803_v2.patch

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

::: dom/media/MediaDecoder.cpp
@@ +279,5 @@
>  }
> +
> +bool MediaDecoder::IsNeedToRecreateDecodedStream()
> +{
> +  if (mIsExitingDormant || mDecodedStream) {

It returns true when |mIsExitingDormant| is true even when the media element is not captured at all. We should't create a DecodedStreamData when not captured.
Attachment #8533462 - Flags: feedback?(jwwang) → feedback-
Posted patch Bug1100803_v3.patch (obsolete) — Splinter Review
Hi, JW,

Now I arouse the |RecreateDecodedStream()| depend on whether the media tracks have been created. I use the variable |mMediaTracksConstructed| as a condition. This variable is set to true only when the state is |PLAY_STATE_PLAYING|. Therefore, we could seek the streams when it is true, because we extactly have the media element and played it before.

Thanks your help :)
Attachment #8533462 - Attachment is obsolete: true
Attachment #8534104 - Flags: feedback?(jwwang)
Comment on attachment 8534104 [details] [diff] [review]
Bug1100803_v3.patch

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

You will still create a DecodedStreamData when not captured.
Attachment #8534104 - Flags: feedback?(jwwang) → feedback-
Posted patch Bug1100803_v4.patch (obsolete) — Splinter Review
Hi, JW,
I use the variables, |mSrcStream| and |mOutputStream|, in the HTMLMediaElement to ensure the media element does not read data from files.
Very thanks for your help :)
Attachment #8534104 - Attachment is obsolete: true
Attachment #8534271 - Flags: feedback?(jwwang)
Comment on attachment 8534271 [details] [diff] [review]
Bug1100803_v4.patch

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

::: dom/media/MediaDecoder.cpp
@@ +280,5 @@
> +
> +void MediaDecoder::RecreateDecodedStreamIfNecessary(int64_t aStartTimeUSecs)
> +{
> +  HTMLMediaElement* mediaElement = mOwner->GetMediaElement();
> +  if (mediaElement->IsNecessaryToCreateStream()) {

I think you just need to check |mInitialAudioCaptured| to know if it is captured or not without resorting to the media element.
Attachment #8534271 - Flags: feedback?(jwwang) → feedback-
Posted patch Bug1100803_v5.patch (obsolete) — Splinter Review
Hi, JW,
Here is my revised patch,
Thanks a lot :)
Attachment #8534271 - Attachment is obsolete: true
Attachment #8534296 - Flags: feedback?(jwwang)
Comment on attachment 8534296 [details] [diff] [review]
Bug1100803_v5.patch

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

The patch should be separated into 2 parts. One is for fixing duplicated deletion in comment 5, the other for the timing issue in checking mDecoder->GetDecodedStream() in MediaDecoderStateMachine::StartSeek().

::: dom/media/MediaDecoder.h
@@ +866,5 @@
>    bool IsLogicallyPlaying();
>  
> +  // To Ensure the media element has a need to capture the streams. In somecase,
> +  // it just reads data from the file.
> +  void RecreateDecodedStreamIfNecessary(int64_t aStartTimeUSecs);

The comment is confusing. It should be "Re-create a decoded stream if audio being captured".
Attachment #8534296 - Flags: feedback?(jwwang) → feedback+
Hi, Robert.
Sorry to bother you, could you help me review my patches?
Thanks a lots :)
Attachment #8534296 - Attachment is obsolete: true
Attachment #8534762 - Flags: review?(roc)
Attachment #8534766 - Flags: review?(roc)
(In reply to Alastor Wu [:alwu] from comment #25)
> Created attachment 8534766 [details] [diff] [review]
> Part2 : Recreate media streams

Please describe the problem and what this patch tries to fix. It will make the life of the reviewer easier.
Hi, Robert,
Here are some descriptions for this bug.
Hope it could be helpful :)

==========================================================
[Problem] 
Media ports repeated deletion

[Situation] 
That could happen in two situations, see Comment4.

[Solution]
Check the existence of media ports, before delete them.

==========================================================

[Problem] 
Media stream can't be recreated 

[Situation] 
When the media go to the dormant state, the |mDecodedStream| would be released. It causes that we can't arouse the |RecreateDecodedStream()| in the |MediaDecoderStateMachine::StartSeek()|.

[Solution]
We use |mInitialAudioCaptured| to ensure that we wouldn't recreate a stream without be captured by media element, and ensure that we indeed have a streams which can be played, be seeked ..e.t.c.
Comment on attachment 8534766 [details] [diff] [review]
Part2 : Recreate media streams

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

Please add tests for both of these bugs. Thanks!
Attachment #8534766 - Flags: review?(roc) → review+
With this bug landing soon, I was trying to be proactive on bug 879717 with a try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=61090a7a7e25

> TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_peerConnection_basicH264Video.html | Test timed out. - expected PASS

That's an issue I haven't seen before. Alastor, do you know if it's triggered by your patches here or mine on bug 879717?
Flags: needinfo?(alwu)
Hi, Andreas,
Sorry for that I didn't get this problem before, and here is my try result.
Hope it would be helpful :)
https://tbpl.mozilla.org/?tree=Try&rev=b2e256f98679
Flags: needinfo?(alwu)
Attachment #8534762 - Attachment is obsolete: true
Attachment #8534766 - Attachment is obsolete: true
Attachment #8536267 - Flags: review+
Keywords: checkin-needed
Please also file a following bug to add test cases for this bug. Thanks.
(In reply to JW Wang [:jwwang] from comment #33)
> Please also file a following bug to add test cases for this bug. Thanks.

Should I land this patch before writing test cases?
Because I am solving the Bug 1105913 now, I could write the test cases after fixing this bug.
Sure, you can have test cases  later.
https://hg.mozilla.org/mozilla-central/rev/333eda1b0099
https://hg.mozilla.org/mozilla-central/rev/19313a129f6d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8536267 [details] [diff] [review]
[checkin] Part1 : Avoid media ports repeated deletion

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Blocking uplift of bug 879717, see bug 879717 comment 200 for its approval request comment.
[Describe test coverage new/current, TBPL]:Landed on m-c in 37. Covered by all media tests capturing an element with mozCaptureStream().
[Risks and why]: Low. Only adds guards to protect from referencing null pointers. Only ever seen on B2G but could potentially appear in other slow situations.
[String/UUID change made/needed]: None.

This request applies to all patches on this bug.
Attachment #8536267 - Flags: approval-mozilla-beta?
Hi, Andreas, 
Bug1105913 is highly related with this bug, I think it should be uplift together if needed.
Ah, didn't know that one. So it would be enough to uplift bug 1105913 part 1?
Flags: needinfo?(alwu)
Hi, Andreas,
I think what you need is the patch 1 in Bug1105913 and Bug1100803. 
They can ensure the correct releasing order of the media port to avoid the repeated deletion.
Flags: needinfo?(alwu)
Comment on attachment 8536267 [details] [diff] [review]
[checkin] Part1 : Avoid media ports repeated deletion

If it affects only b2g, not taking it.
Attachment #8536267 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Crash Signature: [@ android_atomic_inc ]
You need to log in before you can comment on or make changes to this bug.