Bug 120873

Summary: MediaStream API: Stop means stop
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebCore Misc.Assignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, glenn, hta, jer.noble, tommyw
Priority: P2 Keywords: BlinkMergeCandidate
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 121101    
Attachments:
Description Flags
Proposed patch none

Eric Carlson
Reported 2013-09-06 11:48:25 PDT
Consider merging https://chromium.googlesource.com/chromium/blink/+/9a97b0f3892320fa5741485e06a78183b77c3635 Removing notification functionality from MediaStreamTrack::stop because the world is being torn down.
Attachments
Proposed patch (1.49 KB, patch)
2013-09-09 08:36 PDT, Eric Carlson
no flags
Eric Carlson
Comment 1 2013-09-09 08:36:47 PDT
Created attachment 211044 [details] Proposed patch
Jer Noble
Comment 2 2013-09-09 10:55:59 PDT
Comment on attachment 211044 [details] Proposed patch This seems weird. MediaStreamTrack is an ActiveDOMObject, but doesn't override any of ActiveDOMObject's methods besides ::stop(), and doesn't call setPendingActivity(). Why isn't MediaStream an ActiveDOMObject? After all, it has a m_stopped ivar too. If anything, MediaStreamSource is the active object, but MediaStreamTrack::stop() doesn't try to stop it's associated source. This seems highly weird, and I'm not entirely comfortable with it. But my problems with this method don't have much to do with this change specifically, so LGTM with reservations.
WebKit Commit Bot
Comment 3 2013-09-09 11:18:45 PDT
Comment on attachment 211044 [details] Proposed patch Clearing flags on attachment: 211044 Committed r155364: <http://trac.webkit.org/changeset/155364>
WebKit Commit Bot
Comment 4 2013-09-09 11:18:48 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.