Stop using string-based enumerations in TextTrack, and eliminate support for string-based enumerations
Created attachment 277890 [details] Patch
Comment on attachment 277890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277890&action=review r=me with comments > Source/WebCore/html/HTMLTrackElement.cpp:124 > setAttribute(kindAttr, kind); setAttributeWithoutSynchronization() > Source/WebCore/html/HTMLTrackElement.h:44 > + const AtomicString& srclang() const; Do we still need this one now that the attribute has been marked as [Reflect]? Is this called from non-bindings code? > Source/WebCore/html/HTMLTrackElement.h:45 > + const AtomicString& label() const; ditto. > Source/WebCore/html/track/TextTrack.cpp:462 > + return (m_kind == TextTrackKind::Captions || m_kind == TextTrackKind::Subtitles || m_kind != TextTrackKind::Forced) I think this should be m_kind == TextTrackKind::Forced > Source/WebCore/html/track/TextTrack.h:198 > +#if !ENABLE(MEDIA_SOURCE) I would have done the #ifdefing inside the function instead of duplicating it.
Comment on attachment 277890 [details] Patch Attachment 277890 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1253882 New failing tests: imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
Created attachment 277891 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Comment on attachment 277890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277890&action=review >> Source/WebCore/html/HTMLTrackElement.cpp:124 >> setAttribute(kindAttr, kind); > > setAttributeWithoutSynchronization() Makes sense. I wish fastGetAttribute had a name that was more similar since both are basically the same concept! >> Source/WebCore/html/HTMLTrackElement.h:44 >> + const AtomicString& srclang() const; > > Do we still need this one now that the attribute has been marked as [Reflect]? Is this called from non-bindings code? Yes, it’s called in a couple places. Could change those to call fastGetAttribute directly but I thought I would leave it like this for now. >> Source/WebCore/html/HTMLTrackElement.h:45 >> + const AtomicString& label() const; > > ditto. Same here. >> Source/WebCore/html/track/TextTrack.cpp:462 >> + return (m_kind == TextTrackKind::Captions || m_kind == TextTrackKind::Subtitles || m_kind != TextTrackKind::Forced) > > I think this should be m_kind == TextTrackKind::Forced Oops, yes. I bet that’s why a test is failing. Fixing it. >> Source/WebCore/html/track/TextTrack.h:198 >> +#if !ENABLE(MEDIA_SOURCE) > > I would have done the #ifdefing inside the function instead of duplicating it. I duplicated it to avoid UNUSED_PARAM. What do you think?
Committed r200317: <http://trac.webkit.org/changeset/200317>