[webkit-reviews] review granted: [Bug 125245] [MSE] Refactor MediaSourceBase back into MediaSource : [Attachment 218436] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 125245: [MSE] Refactor MediaSourceBase back into MediaSource
https://bugs.webkit.org/show_bug.cgi?id=125245

Attachment 218436: Patch
https://bugs.webkit.org/attachment.cgi?id=218436&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=218436&action=review


> Source/WebCore/Modules/mediasource/MediaSource.cpp:86
> +    DEFINE_STATIC_LOCAL(const AtomicString, open, ("open",
AtomicString::ConstructFromLiteral));

I have seen more NeverDestroyed<> used more often than DEFINE_STATIC_LOCAL()
recently. Do you know if there is a reason for using one or the other?

> Source/WebCore/Modules/mediasource/MediaSource.cpp:639
> +    for (unsigned long i = 0; i < m_sourceBuffers->length(); ++i)

Nit: m_sourceBuffers won't change length during the loop so you can cache
length() in a variable during initialization.

> Source/WebCore/Modules/mediasource/MediaSource.cpp:649
> +    for (size_t i = 0; i < m_activeSourceBuffers->length(); ++i)

Ditto.