[webkit-reviews] review granted: [Bug 102830] In-band text tracks infrastructure : [Attachment 175276] Updated patch

[bugzilla-daemon at webkit.org]

Philippe Normand <pnormand at igalia.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 102830: In-band text tracks infrastructure
https://bugs.webkit.org/show_bug.cgi?id=102830

Attachment 175276: Updated patch
https://bugs.webkit.org/attachment.cgi?id=175276&action=review

------- Additional Comments from Philippe Normand <pnormand at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=175276&action=review


Looks pretty good! I just have a few nits to consider before landing.

> Source/WebCore/ChangeLog:42
> +	   (TextTrackList::invalidateTrackIndexesAfterTrack): New, invalide the
cached track indexes of

Typo: invalide

> Source/WebCore/html/track/InbandTextTrack.cpp:40
> +InbandTextTrack::InbandTextTrack(ScriptExecutionContext* context,
TextTrackClient* client, PassRefPtr<InbandTextTrackPrivate> playerPrivate)

Nit: playerPrivate made me think of MediaPlayerPrivate here, maybe a different
name can be used, something like trackPrivate?

> Source/WebCore/html/track/InbandTextTrack.cpp:94
> +void InbandTextTrack::addCue(InbandTextTrackPrivate*, double start, double
end, const String& id, const String& content, const String& settings)

I don't see this method used yet, unless I missed it :) Maybe the first
argument is not necessary at all? We already have the InbandTextTrackPrivate
pointer as a private member.