[webkit-reviews] review granted: [Bug 73466] HTMLTrackElement.readyState should return TextTrack "readiness state". : [Attachment 117225] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 73466: HTMLTrackElement.readyState should return TextTrack "readiness
state".
https://bugs.webkit.org/show_bug.cgi?id=73466

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=117225&action=review


This code is OK but I am not happy about having two enums that have to always
match. Can we fix that?

> Source/WebCore/html/HTMLTrackElement.cpp:223
> +   
ensureTrack()->setReadinessState(static_cast<TextTrack::ReadinessState>(state))
;

This cast isn’t safe if we ever change one of the two enums without the other.
There’s not even a comment to guarantee we keep them in sync.

> Source/WebCore/html/HTMLTrackElement.cpp:230
> +    return
static_cast<HTMLTrackElement::ReadyState>(ensureTrack()->readinessState());

No need here for HTMLTrackElement:: prefix.

This cast isn’t safe if we ever change one of the two enums without the other.
There’s not even a comment to guarantee we keep them in sync.

> Source/WebCore/html/TextTrack.h:113
> +    TextTrack::ReadinessState m_readinessState;

There is no need for TextTrack:: prefix here.