r137161 added support for in-band text tracks, and to created WebVTT cues from other types of cue. This translation is not always successful, so we should have a "generic" cue type internal to WebCore which does not use the WebVTT rendering rules.
Created attachment 183034 [details] Proposed patch
Attachment 183034 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/html/track/TextTrackCueGeneric.cpp:54: The parameter name "document" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/html/track/TextTrackCueGeneric.cpp:54: The parameter name "cue" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:302: An else should appear on the same line as the preceding } [whitespace/newline] [4] Source/WebCore/html/track/TextTrackCueGeneric.h:26: #ifndef header guard has wrong style, please use: TextTrackCueGeneric_h [build/header_guard] [5] Source/WebCore/html/track/TextTrackCueGeneric.h:69: The parameter name "context" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/html/track/TextTrack.h:108: The parameter name "cue" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 6 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 183037 [details] Update patch for my good friend the style bot.
Attachment 183037 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/html/track/TextTrack.h:108: The parameter name "cue" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 183037 [details] Update patch for my good friend the style bot. Attachment 183037 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15914508
Created attachment 183050 [details] Add new files to other port build systems.
Comment on attachment 183050 [details] Add new files to other port build systems. Attachment 183050 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15909617
Created attachment 183074 [details] Another patch, another build system.
Comment on attachment 183074 [details] Another patch, another build system. View in context: https://bugs.webkit.org/attachment.cgi?id=183074&action=review r- due to the leak. I am also not fond of the name TextTrackCueGeneric, since I don't understand what is generic about it. > Source/WebCore/html/track/TextTrackCueGeneric.cpp:103 > + :TextTrackCue(context, start, end, content) Missing space. > Source/WebCore/platform/graphics/InbandTextTrackPrivateClient.h:50 > + enum alignment { Types should start with a capital letter. > Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:335 > + m_cues.resize(0); This looks like a leak to me. > Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.h:66 > + Vector<GenericCueData*> m_cues; I don't see anyone deleting these cues. Can we use an OwnPtr?
(In reply to comment #9) > (From update of attachment 183074 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183074&action=review > > r- due to the leak. I am also not fond of the name TextTrackCueGeneric, since I don't understand what is generic about it. > A "generic" cue is a non-WebVTT cue, so it is one which should not be positioned/sized as if it was WebVTT. I agree that TextTrackCueGeneric isn't a great name, but I couldn't come up with anything better (TextTrackCueNonWebVTT, yuck). I added a short comment explaining what it is. > > Source/WebCore/html/track/TextTrackCueGeneric.cpp:103 > > + :TextTrackCue(context, start, end, content) > > Missing space. > Fixed. > > Source/WebCore/platform/graphics/InbandTextTrackPrivateClient.h:50 > > + enum alignment { > > Types should start with a capital letter. > Fixed. > > Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:335 > > + m_cues.resize(0); > > This looks like a leak to me. > > > Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.h:66 > > + Vector<GenericCueData*> m_cues; > > I don't see anyone deleting these cues. Can we use an OwnPtr? > Oops, it was a leak. Fixed by changing to Vector<OwnPtr<GenericCueData> >.
Created attachment 183826 [details] Updated patch.
Attachment 183826 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1 Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:307: Extra space after ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:327: Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] Total errors found: 2 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 183828 [details] Updated patch.
Comment on attachment 183828 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=183828&action=review > Source/WebCore/platform/graphics/InbandTextTrackPrivateClient.h:95 > +private: > + > + double m_startTime; Unnecessary newline.
Attachment 183828 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1 Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:307: Extra space after ( in function call [whitespace/parens] [4] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 183831 [details] One byte smaller this time.
Comment on attachment 183831 [details] One byte smaller this time. http://trac.webkit.org/changeset/140359
<rdar://problem/13055333>