Windows: Convert mime types to lowercase in GetPlatformMimeTypeFromExtension.

Windows: Convert mime types to lowercase in GetPlatformMimeTypeFromExtension.

PluginList::GetPluginInfoArray() expects all incoming mime types to be lowercase,
but this is not always the case when reading them from the Windows registry.

One such offender is a .potm file extension:
"Content Type"="application/vnd.ms-powerpoint.template.macroEnabled.12"

BUG=

[themblsha, 2016-04-01 12:06:22]

Description was changed from

==========
Windows: Convert mime types to lowercase in GetPlatformMimeTypeFromExtension.

PluginList::GetPluginInfoArray() expects all incoming mime types to be
lowercase,
but this is not always the case when reading them from the Windows registry.

One such offender is a .potm file extension:
"Content Type"="application/vnd.ms-powerpoint.template.macroEnabled.12"

BUG=
==========

to

==========
Windows: Convert mime types to lowercase in GetPlatformMimeTypeFromExtension.

PluginList::GetPluginInfoArray() expects all incoming mime types to be
lowercase,
but this is not always the case when reading them from the Windows registry.

One such offender is a .potm file extension:
"Content Type"="application/vnd.ms-powerpoint.template.macroEnabled.12"

BUG=
==========

[themblsha, 2016-04-01 12:06:22]

mblsha@yandex-team.ru changed reviewers:
+ mattm@chromium.org, rsleevi@chromium.org

[Ryan Sleevi, 2016-04-01 16:58:16]

Could you explain more why this isn't done at the PluginList ingesting, rather
than here at output?

[themblsha, 2016-04-07 12:20:44]

On 2016/04/01 16:58:16, Ryan Sleevi wrote:
> Could you explain more why this isn't done at the PluginList ingesting, rather
> than here at output?

I thought that a DCHECK in PluginList::GetPluginInfoArray() means that
everyone's already supplying lowercase mime types.

Do you think this solution would be better?

diff --git a/content/common/plugin_list.cc b/content/common/plugin_list.cc
index 8043881..411119a 100644
--- a/content/common/plugin_list.cc
+++ b/content/common/plugin_list.cc
@@ -314,13 +314,13 @@ bool
PluginList::GetPluginsNoRefresh(std::vector<WebPluginInfo>* plugins) {
 
 void PluginList::GetPluginInfoArray(
     const GURL& url,
-    const std::string& mime_type,
+    const std::string& _mime_type,
     bool allow_wildcard,
     bool* use_stale,
     bool include_npapi,
     std::vector<WebPluginInfo>* info,
     std::vector<std::string>* actual_mime_types) {
-  DCHECK(mime_type == base::ToLowerASCII(mime_type));
+  const std::string mime_type = base::ToLowerASCII(_mime_type);
   DCHECK(info);
 
   if (!use_stale)

[Ryan Sleevi, 2016-04-08 00:41:52]

On 2016/04/07 12:20:44, themblsha wrote:
> I thought that a DCHECK in PluginList::GetPluginInfoArray() means that
> everyone's already supplying lowercase mime types.

Well, they're obviously not ;)

The DCHECK means that it's an expected precondition - so the question is where
is the mimetype that is being passed coming from, and deciding where the
mimetype should be lower-cased.

I don't think marking all mimetypes to lower is correct, and I'm not sure that
your change is correct either. If perhaps you had a BUG filed (*polite cough*
:P), we could continue discussion on the bug as to where the appropriate checks
should be made. From digging through the source history, I stopped at
https://chromium.googlesource.com/chromium/src/+/20a793eb518ccc6503f6521e0ebc...
in trying to figure out when that DCHECK was introduced and why, but it's
sufficient to say it's been there for a number of years.

So your next step is to work upwards in the callstack (from GetPluginInfoArray)
to see where that mime type is coming from. I can understand you think changing
this in //net will change, but the //net code isn't calling GetPluginInfoArray -
someone else is, and they're using info (presumably) they obtained from //net.
So there's the next logical place to do the conversion.

Would you file a bug that includes your investigation. Most importantly for this
discussion, include a full backtrace of when this DCHECK fires, and a way to
reproduce. I think it'd be super easy to have a more meaningful discussion on
that bug as to how to fix :)

[themblsha, 2016-04-08 14:27:30]

Thanks, created https://bugs.chromium.org/p/chromium/issues/detail?id=601776
with relevant stacktraces.

[Ryan Sleevi, 2016-04-27 20:43:50]

I'm going to close this issue, based on the discussion in
https://bugs.chromium.org/p/chromium/issues/detail?id=601776

Would you like to continue working on that solution? Otherwise, I can always
mark it Available and see about getting a fix in eventually.

[themblsha, 2016-04-29 10:15:24]

On 2016/04/27 20:43:50, Ryan Sleevi wrote:
> I'm going to close this issue, based on the discussion in
> https://bugs.chromium.org/p/chromium/issues/detail?id=601776
> 
> Would you like to continue working on that solution? Otherwise, I can always
> mark it Available and see about getting a fix in eventually.

I won't have the time for this for the next two-three weeks. I'll try going back
to this later if it's still not resolved by then.