Bug 120873 - MediaStream API: Stop means stop
Summary: MediaStream API: Stop means stop
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
Keywords: BlinkMergeCandidate
Depends on:
Blocks: 121101
  Show dependency treegraph
Reported: 2013-09-06 11:48 PDT by Eric Carlson
Modified: 2013-09-10 10:30 PDT (History)
6 users (show)

See Also:

Proposed patch (1.49 KB, patch)
2013-09-09 08:36 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 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.
Comment 1 Eric Carlson 2013-09-09 08:36:47 PDT
Created attachment 211044 [details]
Proposed patch
Comment 2 Jer Noble 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.
Comment 3 WebKit Commit Bot 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>
Comment 4 WebKit Commit Bot 2013-09-09 11:18:48 PDT
All reviewed patches have been landed.  Closing bug.