[webkit-reviews] review granted: [Bug 86308] <video> won't load when URL ends with .php : [Attachment 141600] Updated patch - this time with not as many uninitialized variables.

[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 86308: <video> won't load when URL ends with .php
https://bugs.webkit.org/show_bug.cgi?id=86308

Attachment 141600: Updated patch - this time with not as many uninitialized
variables.
https://bugs.webkit.org/attachment.cgi?id=141600&action=review

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


> Source/WebCore/platform/graphics/MediaPlayer.cpp:328
> +    , m_typeInferredFromExtension(false)

This variable name sounds like it would be a type, not a boolean.
m_typeWasInferredFromExtension should like a boolean. Or for additional
clarity, m_contentMIMETypeWasInferredFromExtension.

> Source/WebCore/platform/graphics/MediaPlayer.cpp:354
> +    m_typeInferredFromExtension = false;

Seems strange that for m_url, m_contentMIMEType, and m_contentTypeCodecs we use
local variables and set the data member only at the end, but for
m_typeInferredFromExtension we set the data member directly. The code would be
easier to read if all worked the same way.

> Source/WebCore/platform/graphics/MediaPlayer.cpp:369
>		   String mediaType =
MIMETypeRegistry::getMediaMIMETypeForExtension(extension);
> -		   if (!mediaType.isEmpty())
> +		   if (!mediaType.isEmpty()) {
>		       type = mediaType;
> +		       m_typeInferredFromExtension = true;
> +		   }

Seems like for clarity mediaType could be named inferredType.