827913 - tests/crashtests/801227.html | MOZ_ASSERT(mStream) in MediaManager::GetSourceStream() on tab-close with inactive stream

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P1)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Callstack + this

After hitting Start() I think I clicked on the page without accepting the cam popup (which dismisses it which is a little odd but hey), and then I closed the tab

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: remove no-longer-correct MOZ_ASSERT in getUserMedia

Comment 2

[Timothy B. Terriberry (:derf)]

Comment on attachment 699340 [details] [diff] [review]
remove no-longer-correct MOZ_ASSERT in getUserMedia

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

It would be better not to issue the MEDIA_STOP request at all in this case, I think, but this will work.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: remove no-longer-correct MOZ_ASSERT in getUserMedia

nicer fix

Comment 6

[Timothy B. Terriberry (:derf)]

Comment on attachment 699367 [details] [diff] [review]
remove no-longer-correct MOZ_ASSERT in getUserMedia

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

::: dom/media/MediaManager.h
@@ +103,5 @@
>      return mStream;
>    }
>    SourceMediaStream *GetSourceStream()
>    {
> +    NS_ASSERTION(mstream,"Getting stream from never-activated GUMCMSListener");

As instructed, I'm ignoring this typo :).

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2dd9714c57d3

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

This also causes a crash for tests/crashtests/801227.html which crashes in mozilla::GetUserMediaCallbackMediaStreamListener::GetSourceStream().

I haven't tested yet but if we also crash on alder we might want to backport. Randell, or is this mc only?

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2dd9714c57d3

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

The crash in 801227.html is gone. Thanks!

Comment 12

[Scoobidiver (away)]

It's #13 top browser crasher in 20.0a2.

Comment 13

[Jason Smith [:jsmith]]

(In reply to Scoobidiver from comment #12)
> It's #13 top browser crasher in 20.0a2.

Are you still seeing signatures after 1/9/2013? I'm wondering if this isn't fixed.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

The fix on that bug landed after the merge so it has never been made it into Firefox 20.

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 699367 [details] [diff] [review]
remove no-longer-correct MOZ_ASSERT in getUserMedia

[Approval Request Comment]
Bug caused by (feature/regressing bug #): a bug what landed shortly before FF20 uplift (I'd have to go back to remember exactly which one).  Probably 822956 or one that landed around the same time.

User impact if declined: crashes if you navigate away without answering a getUserMedia() UI request.

Testing completed (on m-c, etc.): On m-c for a week, verified fixed

Risk to taking this patch (and alternatives if risky): nil

String or UUID changes made by this patch: none

Comment 16

[Alex Keybl [:akeybl]]

Comment on attachment 699367 [details] [diff] [review]
remove no-longer-correct MOZ_ASSERT in getUserMedia

Crashes in a new feature in a fairly common user scenario. Approving for uplift to Aurora.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/31e9e2ae25e5