774357 - crash in nsHTMLMediaElement::EndMediaStreamPlayback

Status: VERIFIED FIXED

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

Description

[Scoobidiver (away)]

It first appeared in 16.0a1/20120716. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=57abb5f70e01&tochange=4cffe2b37d0c
It's likely a regression from bug 598868.

Signature 	nsHTMLMediaElement::EndMediaStreamPlayback() More Reports Search
UUID	36691628-1eff-4059-b10a-bb6e02120716
Date Processed	2012-07-16 14:25:11
Uptime	92
Last Crash	more than 3 months before submission
Install Age	3.1 minutes since version was first installed.
Install Time	2012-07-16 14:20:51
Product	Firefox
Version	16.0a1
Build ID	20120716030546
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 37 stepping 2
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x8
User Comments	Just testing the WebRTC demo page; Sequence: - start video - stop video - start audio - stop audio => crash
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x68c1, AdapterSubsysID: 00000000, AdapterDriverVersion: 8.723.8.2000
D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- 
EMCheckCompatibility	True
Adapter Vendor ID	0x1002
Adapter Device ID	0x68c1
Total Virtual Memory	4294836224
Available Virtual Memory	3967201280
System Memory Use Percentage	72
Available Page File	4346949632
Available Physical Memory	1155796992

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsHTMLMediaElement::EndMediaStreamPlayback 	content/html/content/src/nsHTMLMediaElement.cpp:2579
1 	xul.dll 	nsHTMLMediaElement::AbortExistingLoads 	content/html/content/src/nsHTMLMediaElement.cpp:581
2 	xul.dll 	nsHTMLMediaElement::Load 	content/html/content/src/nsHTMLMediaElement.cpp:701
3 	xul.dll 	nsHTMLMediaElement::SetAttr 	content/html/content/src/nsHTMLMediaElement.cpp:1843
4 	xul.dll 	nsGenericHTMLElement::SetAttr 	content/html/content/src/nsGenericHTMLElement.h:168
5 	xul.dll 	nsGenericHTMLElement::SetAttrHelper 	content/html/content/src/nsGenericHTMLElement.cpp:2829
6 	xul.dll 	nsHTMLMediaElement::SetSrc 	content/html/content/src/nsHTMLMediaElement.cpp:475
7 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70
8 	xul.dll 	XPCWrappedNative::CallMethod 	js/xpconnect/src/XPCWrappedNative.cpp:2382
9 	xul.dll 	XPCWrappedNative::SetAttribute 	js/xpconnect/src/xpcprivate.h:2815
10 	xul.dll 	XPC_WN_GetterSetter 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1514
11 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:344
12 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:387
13 	mozjs.dll 	js::Shape::set 	js/src/jsscopeinlines.h:296
14 	mozjs.dll 	js::baseops::SetPropertyHelper 	js/src/jsobj.cpp:4942
15 	mozjs.dll 	js::SetPropertyOperation 	js/src/jsinterpinlines.h:350
16 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2355
17 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:355
18 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:387
19 	mozjs.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:5564
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsHTMLMediaElement%3A%3AEndMediaStreamPlayback%28%29
https://crash-stats.mozilla.com/report/list?signature=nsHTMLMediaElement%3A%3AEndMediaStreamPlayback

Comment 1

[Jason Smith [:jsmith]]

Anant - Any ideas?

Comment 2

[Marcia Knous [:marcia]]

One of the crash reports had this comment: Just testing the WebRTC demo page; Sequence: - start video - stop video - start audio - stop audio => crash - but I wasn't able to reproduce.

Comment 3

[Paul Adenot (:padenot)]

I was able to reproduce using the WebRTC demo page, using the step in comment 2.

Comment 4

[Paul Adenot (:padenot)]

Attachment: Fix a crash in nsHTMLMediaElement::EndMediaStreamPlayback r=

|mVideoFrameContainer| is not null-checked, and probably should be.

Here is a possible fix. Feel free to forward if you think someone is more
appropritate for the review.

Comment 5

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

Comment on attachment 643499 [details] [diff] [review]
Fix a crash in nsHTMLMediaElement::EndMediaStreamPlayback r=

Switching review to roc and/or cjones (who reviewed the patch that last changed this line); revision 99337, bug 598868

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 643499 [details] [diff] [review]
Fix a crash in nsHTMLMediaElement::EndMediaStreamPlayback r=

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

::: content/html/content/src/nsHTMLMediaElement.cpp
@@ +2576,5 @@
>    if (mPaused) {
>      GetMediaStream()->ChangeExplicitBlockerCount(-1);
>    }
> +  if (mVideoFrameContainer) {
> +    mVideoFrameContainer->GetImageContainer()->SetCurrentImage(nsnull);

Move this up be inside if (container) and use 'container'

Comment 7

[Paul Adenot (:padenot)]

Attachment: Fix a crash in nsHTMLMediaElement::EndMediaStreamPlayback r=

Addressed roc's comments.

Comment 8

[Jason Smith [:jsmith]]

Can we land the fix for this? I'm blocked on some of my testing due to this bug.

Comment 9

[Paul Adenot (:padenot)]

Jason, sorry about that. I'll rebase it tomorrow, and ask someone to land it.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/39ab0e54330a

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/39ab0e54330a

Comment 12

[Jason Smith [:jsmith]]

Busted. I tested this with my USB headset and tried to access the audio piece of Anant's page https://people.mozilla.com/~anarayanan/gum_test.html and repeatedly was getting NO_DEVICES_FOUND. I tested this on the 8/1/2012 build before this fix landed, did not get this error, and successfully could hear and could playback sound I generated. Filed a followup bug in bug 781705.

Comment 13

[Jason Smith [:jsmith]]

Disregard. I realized that the issue I hit is already a known issue. Will retest now.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We do not have an attached testcase here and the one from the demo page has been changed a lot since then. So I don't think we can have a crashtest here.