MediaSource.readyState should use an IDL enum as per specification (https://www.w3.org/TR/2016/CR-media-source-20160503/#idl-def-ReadyState).
Created attachment 309147 [details] Patch
Comment on attachment 309147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309147&action=review > Source/WebCore/Modules/mediasource/MediaSource.cpp:56 > +static const char* dumpReadyState(WebCore::MediaSource::ReadyState readyState) Nit: "dumpReadyState" doesn't describe what this function does. Maybe "toString" instead? > Source/WebCore/Modules/mediasource/MediaSource.cpp:61 > + case WebCore::MediaSource::ReadyState::Closed: return "closed"; > + case WebCore::MediaSource::ReadyState::Ended: return "ended"; > + case WebCore::MediaSource::ReadyState::Open: return "open"; Nit: I think having the "return" on the same line as the "case" makes these slightly harder to read. > Source/WebCore/Modules/mediasource/MediaSource.cpp:528 > + ReadyState oldState = readyState(); Nit: "auto" please.
Comment on attachment 309147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309147&action=review >> Source/WebCore/Modules/mediasource/MediaSource.cpp:56 >> +static const char* dumpReadyState(WebCore::MediaSource::ReadyState readyState) > > Nit: "dumpReadyState" doesn't describe what this function does. Maybe "toString" instead? WebCore:: is not needed. > Source/WebCore/Modules/mediasource/MediaSource.cpp:85 > + , m_readyState(ReadyState::Closed) Could be inline initialization of the member in the header. > Source/WebCore/Modules/mediasource/MediaSource.cpp:526 > + ASSERT(state == ReadyState::Open || state == ReadyState::Closed || state == ReadyState::Ended); This assertion is no longer useful I believe. > Source/WebCore/Modules/mediasource/MediaSource.h:123 > + void setReadyState(const ReadyState); No const please. > Source/WebCore/Modules/mediasource/MediaSource.h:124 > + void onReadyStateChange(const ReadyState oldState, const ReadyState newState); No const please.
Created attachment 309490 [details] Patch
I made the corrections you suggested in uploaded patch. Thanks for the reviews.
Comment on attachment 309490 [details] Patch Clearing flags on attachment: 309490 Committed r216509: <http://trac.webkit.org/changeset/216509>
All reviewed patches have been landed. Closing bug.