[webkit-reviews] review granted: [Bug 110798] Factor SourceBuffer methods out of MediaSourcePrivate & WebMediaSource into SourceBufferPrivate & WebSourceBuffer respectively. : [Attachment 190606] Patch

[bugzilla-daemon at webkit.org]

Jer Noble <jer.noble at apple.com> has granted Aaron Colwell
<acolwell at chromium.org>'s request for review:
Bug 110798: Factor SourceBuffer methods out of MediaSourcePrivate &
WebMediaSource into SourceBufferPrivate & WebSourceBuffer respectively.
https://bugs.webkit.org/show_bug.cgi?id=110798

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

------- Additional Comments from Jer Noble <jer.noble at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=190606&action=review


This is much better.  I really appreciate the link to the specific version of
the spec matching the implementation. Just one nit:

> Source/WebCore/platform/graphics/MediaSourcePrivate.h:36
> +#include <wtf/Forward.h>
> +#include <wtf/Vector.h>

Nit: it shouldn't be necessary to include <wtf/Vector.h> here as you're only
using a const Vector<String>& in the header. I believe you might be getting the
compile error below...:

> Source/WebCore/platform/graphics/MediaSourcePrivate.h:48
> +    virtual AddStatus addSourceBuffer(const String& type, const
Vector<String>& codecs, OwnPtr<SourceBufferPrivate>*) = 0;

...because Forward.h doesn't define the default value for the second template
parameter for Vector.  Change this to a "const Vector<String, 0>&", and I think
that will fix the compile error when you remove the #include <wtf/Vector.h>.

That said, this is just a nit.