Bug 124975

Summary:

Fix a crash in the webaudio source provider when the audio track is going away.

Product:

WebKit

Reporter:

Nick Diego Yamane (diegoyam) <nick.diego>

Component:

WebCore Misc.

Assignee:

Nick Diego Yamane (diegoyam) <nick.diego>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric.carlson, glenn, jer.noble, pnormand, rogerzanoni

Priority:

Keywords:

BlinkMergeCandidate

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

121101

Attachments:

Description	Flags
Patch	none
Improved ChangeLog	none

Description Nick Diego Yamane (diegoyam) 2013-11-28 09:13:17 PST

Consider merging:

https://chromium.googlesource.com/chromium/blink/+/b21838b32bf11b1a972dfc449ddde71115490c23

Before this, it was hitting a use-after-free crash on the renderer when the audio track in the media stream is going away and the webaudio MediaStreamSourceNode is still running.

Comment 1 Nick Diego Yamane (diegoyam) 2013-11-29 13:15:05 PST

Created attachment 218078 [details]
Patch

Comment 2 Philippe Normand 2013-11-30 01:16:39 PST

Comment on attachment 218078 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=218078&action=review

> Source/WebCore/ChangeLog:8
> +        Before this patch, it was hittinh a use-after-free crash  when the audio

Typo: hittinh. Also, I'm not sure it's a common thing to do when merging Blink changes but I think it'd be good to mention the blink commit and its author in the ChangeLog.

Comment 3 Eric Carlson 2013-11-30 14:05:30 PST

Comment on attachment 218078 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=218078&action=review

>> Source/WebCore/ChangeLog:8
>> +        Before this patch, it was hittinh a use-after-free crash  when the audio
> 
> Typo: hittinh. Also, I'm not sure it's a common thing to do when merging Blink changes but I think it'd be good to mention the blink commit and its author in the ChangeLog.

Yes, we typically include the Blink commit url and author in the ChangeLog.

Comment 4 Nick Diego Yamane (diegoyam) 2013-12-01 12:33:11 PST

(In reply to comment #3)
> (From update of attachment 218078 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218078&action=review
> 
> >> Source/WebCore/ChangeLog:8
> >> +        Before this patch, it was hittinh a use-after-free crash  when the audio
> > 
> > Typo: hittinh. Also, I'm not sure it's a common thing to do when merging Blink changes but I think it'd be good to mention the blink commit and its author in the ChangeLog.
> 
> Yes, we typically include the Blink commit url and author in the ChangeLog.

Yeah, you're right about that. I'll upload a patch with those i nformations in the changelog tomorrow.

Thanks

Comment 5 Nick Diego Yamane (diegoyam) 2013-12-02 03:59:01 PST

Created attachment 218160 [details]
Improved ChangeLog

Updated as per comments from Eric and Philip.

Comment 6 Philippe Normand 2013-12-02 04:09:28 PST

Comment on attachment 218160 [details]
Improved ChangeLog

Hum sorry the author is missing in the ChangeLog

Comment 7 Philippe Normand 2013-12-02 05:29:30 PST

Comment on attachment 218160 [details]
Improved ChangeLog

cq+ as other blink merges don't include author either. It can be found on the blink review link anyway.

Comment 8 WebKit Commit Bot 2013-12-02 05:55:57 PST

Comment on attachment 218160 [details]
Improved ChangeLog

Clearing flags on attachment: 218160

Committed r159931: <http://trac.webkit.org/changeset/159931>

Comment 9 WebKit Commit Bot 2013-12-02 05:55:59 PST

All reviewed patches have been landed.  Closing bug.