Summary: | MediaStreamTrack now tracks its own state | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Thiago de Barros Lacerda <thiago.lacerda> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson, glenn, hta, jer.noble, tommyw | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 124288 | ||||||||
Attachments: |
|
Description
Thiago de Barros Lacerda
2013-10-18 09:27:49 PDT
Created attachment 214581 [details]
Patch
Comment on attachment 214581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214581&action=review > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:80 > + m_state = m_source ? (TrackState) m_source->readyState() : MediaStreamTrack::New; The space between the cast and variable is unnecessary, but I'm more worried about the cast itself. There should only be one instance of this enum, either in MediaStreamTrack, or MediaStreamSource. So, if you reverted the m_readyState -> m_state change, removed the MediaStreamTrack::TrackState enum, and kept the new setState() method (but renamed it setReadyState()), this patch would be a lot simpler. > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:307 > + setState((TrackState) m_source->readyState()); Ditto. (In reply to comment #2) > (From update of attachment 214581 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214581&action=review > > > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:80 > > + m_state = m_source ? (TrackState) m_source->readyState() : MediaStreamTrack::New; > > The space between the cast and variable is unnecessary, but I'm more worried about the cast itself. There should only be one instance of this enum, either in MediaStreamTrack, or MediaStreamSource. > > So, if you reverted the m_readyState -> m_state change, removed the MediaStreamTrack::TrackState enum, and kept the new setState() method (but renamed it setReadyState()), this patch would be a lot simpler. > > > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:307 > > + setState((TrackState) m_source->readyState()); > > Ditto. Sounds good. Created attachment 214772 [details]
Patch
Comment on attachment 214772 [details]
Patch
Looks good. r=me.
Comment on attachment 214772 [details] Patch Clearing flags on attachment: 214772 Committed r157733: <http://trac.webkit.org/changeset/157733> All reviewed patches have been landed. Closing bug. |