[webkit-reviews] review denied: [Bug 107046] Support non-WebVTT cues from in-band text tracks : [Attachment 183074] Another patch, another build system.

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Eric Carlson <eric.carlson at apple.com>'s
request for review:
Bug 107046: Support non-WebVTT cues from in-band text tracks
https://bugs.webkit.org/show_bug.cgi?id=107046

Attachment 183074: Another patch, another build system.
https://bugs.webkit.org/attachment.cgi?id=183074&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
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?