[webkit-reviews] review granted: [Bug 51195] MediaPlayer should look for MIME type in data: url : [Attachment 76786] 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 51195: MediaPlayer should look for MIME type in data: url
https://bugs.webkit.org/show_bug.cgi?id=51195

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

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

Can we make tests for this?

> WebCore/platform/graphics/MediaPlayer.cpp:306
> +    // If the MIME type is missing or is not meaningful, try to figure it
out from the url.
> +    if (type.isEmpty() && protocolIs(url, "data"))

The comment says “is not meaningful”, but the code only checks for empty
strings. Aren’t there other non-meaningful MIME types?

I suggest capitalizing URL in the comment.

I suggest we add protocolIsData to KURL.h. We already have it for actual KURL
objects and I’d like to have it for strings. It’s not good to have the actual
"data" string in the code. We should also adopt protocolIsData in KURL.cpp,
KURLGoogle.cpp, DataURL.cpp, ResourceHandleSoup.cpp. None of that needs to be
done right away or by you.