Adding check for MIME types that do not take codecs parameter

net/base/mime_util.h

Index: net/base/mime_util.h
diff --git a/net/base/mime_util.h b/net/base/mime_util.h
index 9662e9656e397de2d48adedbfae590a3249ff4a7..cff021455f79b3ebb533f350fb848240f153d593 100644
--- a/net/base/mime_util.h
+++ b/net/base/mime_util.h
@@ -85,6 +85,10 @@ NET_EXPORT bool IsSupportedStrictMediaMimeType(
     const std::string& mime_type,
     const std::vector<std::string>& codecs);
 
+// Check to see if a particular MIME type specifically requires codecs parameter
+// not to be present with it.

[Ryan Sleevi, 2014/04/10 19:04:16]

The comment here needs to be improved, but unfortunately, I'm not intimately
familiar enough to know how to suggest improving it.

My naive response was first just fixing the grammar, to something like
// Check to see if a particular MIME type specifically requires the 'codecs'
parameter be absent.

But that doesn't describe what happens for mime types where no codecs are
supported (say, application/x-microsoft-word or the like)

Alternatively, it might be posed as
"SupportedStrictMediaMimeTypeSupportsCodecs", which indicates if the given
|mime_type| (which is a strict media mime type, presumably?) supports the
"codecs" parameter. If it doesn't, returns false.

However, when thinking about that, I'm not sure why you can't use
IsSupportedStrictMediaMimeType(), which an empty "codecs" used to indicate no
codecs parameter is supported. Is there a case where we would support a strict
media mime type, but "codecs" would be empty AND "codecs" be a valid parameter?
It doesn't seem like it.

[amogh.bihani, 2014/04/11 05:24:07]

There are two things:
[a] Browser will never reach the IsSupportedStrictMediaMimeType() if the
'codecs' is null[1]. It will return a "maybe" as soon as it sees a null 'codecs'
parameter.

[b] Strict mime type can have empty 'codecs' parameter, in that case we return a
"maybe"[2].

That's why we need the check.
-------
[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

[2]
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element...

[Ryan Sleevi, 2014/04/14 23:52:25]

The naming here still bothers me, in that it doesn't seem the substance of the
concern was addressed.

Please understand, I'm not familiar with the mime-type rules - I'm deferring to
scherkus and such here, but the fact that this code continues to "surprise" me
(at least as far as function name/comment/implementation) is part of the
concern.

Again, using the examples above, if you pass some random mime type, like
"application/x-microsoft-word", then I cannot reasonably guess what this
function will return without _also_ looking into the implementation.

That is, this function 'implies' it's authoritative for all mime types, but it's
not - it only applies to the supported strict mime types.

Minimally, something like

DoesStrictMediaMimeTypeAllowEmptyCodecs() or something similar, so that someone
reading the net/ code can understand.

Alternatively, it would seem you could change how webkitplatformsupport is
structured and avoid the issue entirely

if (net::IsStrictMediaMimeType(...)) {
  if (net::IsSupportedStrictMediaMimeType(...))
    return IsSupported;
  if (codecs.isNull())
    return MayBeSupported;
  return IsNotSupported;
}

[amogh.bihani, 2014/04/15 13:26:50]

Done. :)

+NET_EXPORT bool MimeTypeDoesNotNeedCodecs(const std::string& mime_type);
+
 // Get the extensions associated with the given mime type. This should be passed
 // in lower case. There could be multiple extensions for a given mime type, like
 // "html,htm" for "text/html", or "txt,text,html,..." for "text/*".