Adding check for MIME types that do not take codecs parameter

net/base/mime_util.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_BASE_MIME_UTIL_H__
 #define NET_BASE_MIME_UTIL_H__
 
 #include <string>
 #include <vector>
 
@@ skipping 67 matching lines @@
 
 // Check to see if a particular MIME type is in our list which only supports a
 // certain subset of codecs. Returns true if and only if all codecs are
 // supported for that specific MIME type, false otherwise. If this returns
 // false you will still need to check if the media MIME tpyes and codecs are
 // supported.
 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/*".
 // Note that we do not erase the existing elements in the the provided vector.
 // Instead, we append the result to it.
 NET_EXPORT void GetExtensionsForMimeType(
     const std::string& mime_type,
     std::vector<base::FilePath::StringType>* extensions);
 
 // Test only method that removes proprietary media types and codecs from the
@@ skipping 26 matching lines @@
                                            std::string* post_data);
 
 // Adds the final delimiter to a multi-part upload request.
 NET_EXPORT void AddMultipartFinalDelimiterForUpload(
     const std::string& mime_boundary,
     std::string* post_data);
 
 }  // namespace net
 
 #endif  // NET_BASE_MIME_UTIL_H__