Issue 165177: Merge 22027 - Fix a DCHECK we hit when a mime type isn't ASCII....

Issue 165177: Merge 22027 - Fix a DCHECK we hit when a mime type isn't ASCII.... (Closed)

Created:
11 years, 4 months ago by laforge

Modified:
9 years, 7 months ago

Reviewers:
Evan Martin

CC:
chromium-reviews_googlegroups.com, darin (slow to review)

Base URL:
svn://chrome-svn/chrome/branches/195/src/

Visibility:
Public.

Description

Merge 22027 - Fix a DCHECK we hit when a mime type isn't ASCII. I have a layout test for this, but Tony says I should send it upstream so that will be a separate commit. BUG=17958 Review URL: http://codereview.chromium.org/160363 TBR=evan@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22813

Patch Set 1 #

Created: 11 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+18 lines, -6 lines)			Patch
	MM	webkit/glue/simple_webmimeregistry_impl.cc	View		4 chunks	+18 lines, -6 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

laforge

I reverted this after looking at further revisions. Kind Regards, Anthony Laforge Technical Program Manager ...

11 years, 4 months ago (2009-08-07 22:55:45 UTC) #2

I reverted this after looking at further revisions.
Kind Regards,

Anthony Laforge
Technical Program Manager
Mountain View, CA


On Fri, Aug 7, 2009 at 3:47 PM, <laforge@chromium.org> wrote:

> Reviewers: Evan Martin,
>
> Description:
> Merge 22027 - Fix a DCHECK we hit when a mime type isn't ASCII.
>
> I have a layout test for this, but Tony says I should send it upstream
> so that will be a separate commit.
>
> BUG=17958
>
> Review URL: http://codereview.chromium.org/160363
>
> TBR=evan@chromium.org
>
>
> Please review this at http://codereview.chromium.org/165177
>
> SVN Base: svn://chrome-svn/chrome/branches/195/src/
>
> Affected files:
>  MM    webkit/glue/simple_webmimeregistry_impl.cc
>
>
> Index: webkit/glue/simple_webmimeregistry_impl.cc
> ===================================================================
> --- webkit/glue/simple_webmimeregistry_impl.cc  (revision 22812)
> +++ webkit/glue/simple_webmimeregistry_impl.cc  (working copy)
> @@ -14,18 +14,30 @@
>  using WebKit::WebString;
>  using WebKit::WebMimeRegistry;
>
> +namespace {
> +
> +// Convert a WebString to ASCII, falling back on an empty string in the
> case
> +// of a non-ASCII string.
> +const std::string& AsASCII(const WebString& string) {
> +  if (!IsStringASCII(string))
> +    return EmptyString();
> +  return UTF16ToASCII(string);
> +}
> +
> +}  // namespace
> +
>  namespace webkit_glue {
>
>  WebMimeRegistry::SupportsType
> SimpleWebMimeRegistryImpl::supportsImageMIMEType(
>     const WebString& mime_type) {
> -  if (!net::IsSupportedImageMimeType(UTF16ToASCII(mime_type).c_str()))
> +  if (!net::IsSupportedImageMimeType(AsASCII(mime_type).c_str()))
>     return WebMimeRegistry::IsNotSupported;
>   return WebMimeRegistry::IsSupported;
>  }
>
>  WebMimeRegistry::SupportsType
> SimpleWebMimeRegistryImpl::supportsJavaScriptMIMEType(
>     const WebString& mime_type) {
> -  if
> (!net::IsSupportedJavascriptMimeType(UTF16ToASCII(mime_type).c_str()))
> +  if (!net::IsSupportedJavascriptMimeType(AsASCII(mime_type).c_str()))
>     return WebMimeRegistry::IsNotSupported;
>   return WebMimeRegistry::IsSupported;
>  }
> @@ -33,12 +45,12 @@
>  WebMimeRegistry::SupportsType
> SimpleWebMimeRegistryImpl::supportsMediaMIMEType(
>     const WebString& mime_type, const WebString& codecs) {
>   // Not supporting the container is a flat-out no.
> -  if (!net::IsSupportedMediaMimeType(UTF16ToASCII(mime_type).c_str()))
> +  if (!net::IsSupportedMediaMimeType(AsASCII(mime_type).c_str()))
>     return IsNotSupported;
>
>   // If we don't recognize the codec, it's possible we support it.
>   std::vector<std::string> parsed_codecs;
> -  net::ParseCodecString(UTF16ToASCII(codecs).c_str(), &parsed_codecs);
> +  net::ParseCodecString(AsASCII(codecs).c_str(), &parsed_codecs);
>   if (!net::AreSupportedMediaCodecs(parsed_codecs))
>     return MayBeSupported;
>
> @@ -48,7 +60,7 @@
>
>  WebMimeRegistry::SupportsType
> SimpleWebMimeRegistryImpl::supportsNonImageMIMEType(
>     const WebString& mime_type) {
> -  if (!net::IsSupportedNonImageMimeType(UTF16ToASCII(mime_type).c_str()))
> +  if (!net::IsSupportedNonImageMimeType(AsASCII(mime_type).c_str()))
>     return WebMimeRegistry::IsNotSupported;
>   return WebMimeRegistry::IsSupported;
>  }
> @@ -72,7 +84,7 @@
>  WebString SimpleWebMimeRegistryImpl::preferredExtensionForMIMEType(
>     const WebString& mime_type) {
>   FilePath::StringType file_extension;
> -  net::GetPreferredExtensionForMimeType(UTF16ToASCII(mime_type),
> +  net::GetPreferredExtensionForMimeType(AsASCII(mime_type),
>                                         &file_extension);
>   return FilePathStringToWebString(file_extension);
>  }
>
> Property changes on: webkit\glue\simple_webmimeregistry_impl.cc
> ___________________________________________________________________
> Modified: svn:mergeinfo
>   Merged /trunk/src/webkit/glue/simple_webmimeregistry_impl.cc:r22027
>
>
>
>

Evan Martin

11 years, 4 months ago (2009-08-09 05:31:38 UTC) #3

I think it ended up being fine (I think you ended up including it
anyway), but I think this also doesn't affect release builds (since
it's only a DCHECK).  I will try to make a more descriptive commit
description in the future.

On Fri, Aug 7, 2009 at 3:55 PM, Anthony LaForge<laforge@chromium.org> wrote=
:
> I reverted this after looking at further revisions.
> Kind Regards,
>
> Anthony Laforge
> Technical Program Manager
> Mountain View, CA
>
>
> On Fri, Aug 7, 2009 at 3:47 PM, <laforge@chromium.org> wrote:
>>
>> Reviewers: Evan Martin,
>>
>> Description:
>> Merge 22027 - Fix a DCHECK we hit when a mime type isn't ASCII.
>>
>> I have a layout test for this, but Tony says I should send it upstream
>> so that will be a separate commit.
>>
>> BUG=3D17958
>>
>> Review URL: http://codereview.chromium.org/160363
>>
>> TBR=3Devan@chromium.org
>>
>>
>> Please review this at http://codereview.chromium.org/165177
>>
>> SVN Base: svn://chrome-svn/chrome/branches/195/src/
>>
>> Affected files:
>> =A0MM =A0 =A0webkit/glue/simple_webmimeregistry_impl.cc
>>
>>
>> Index: webkit/glue/simple_webmimeregistry_impl.cc
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- webkit/glue/simple_webmimeregistry_impl.cc =A0(revision 22812)
>> +++ webkit/glue/simple_webmimeregistry_impl.cc =A0(working copy)
>> @@ -14,18 +14,30 @@
>> =A0using WebKit::WebString;
>> =A0using WebKit::WebMimeRegistry;
>>
>> +namespace {
>> +
>> +// Convert a WebString to ASCII, falling back on an empty string in the
>> case
>> +// of a non-ASCII string.
>> +const std::string& AsASCII(const WebString& string) {
>> + =A0if (!IsStringASCII(string))
>> + =A0 =A0return EmptyString();
>> + =A0return UTF16ToASCII(string);
>> +}
>> +
>> +} =A0// namespace
>> +
>> =A0namespace webkit_glue {
>>
>> =A0WebMimeRegistry::SupportsType
>> SimpleWebMimeRegistryImpl::supportsImageMIMEType(
>> =A0 =A0 const WebString& mime_type) {
>> - =A0if (!net::IsSupportedImageMimeType(UTF16ToASCII(mime_type).c_str())=
)
>> + =A0if (!net::IsSupportedImageMimeType(AsASCII(mime_type).c_str()))
>> =A0 =A0 return WebMimeRegistry::IsNotSupported;
>> =A0 return WebMimeRegistry::IsSupported;
>> =A0}
>>
>> =A0WebMimeRegistry::SupportsType
>> SimpleWebMimeRegistryImpl::supportsJavaScriptMIMEType(
>> =A0 =A0 const WebString& mime_type) {
>> - =A0if
>> (!net::IsSupportedJavascriptMimeType(UTF16ToASCII(mime_type).c_str()))
>> + =A0if (!net::IsSupportedJavascriptMimeType(AsASCII(mime_type).c_str())=
)
>> =A0 =A0 return WebMimeRegistry::IsNotSupported;
>> =A0 return WebMimeRegistry::IsSupported;
>> =A0}
>> @@ -33,12 +45,12 @@
>> =A0WebMimeRegistry::SupportsType
>> SimpleWebMimeRegistryImpl::supportsMediaMIMEType(
>> =A0 =A0 const WebString& mime_type, const WebString& codecs) {
>> =A0 // Not supporting the container is a flat-out no.
>> - =A0if (!net::IsSupportedMediaMimeType(UTF16ToASCII(mime_type).c_str())=
)
>> + =A0if (!net::IsSupportedMediaMimeType(AsASCII(mime_type).c_str()))
>> =A0 =A0 return IsNotSupported;
>>
>> =A0 // If we don't recognize the codec, it's possible we support it.
>> =A0 std::vector<std::string> parsed_codecs;
>> - =A0net::ParseCodecString(UTF16ToASCII(codecs).c_str(), &parsed_codecs)=
;
>> + =A0net::ParseCodecString(AsASCII(codecs).c_str(), &parsed_codecs);
>> =A0 if (!net::AreSupportedMediaCodecs(parsed_codecs))
>> =A0 =A0 return MayBeSupported;
>>
>> @@ -48,7 +60,7 @@
>>
>> =A0WebMimeRegistry::SupportsType
>> SimpleWebMimeRegistryImpl::supportsNonImageMIMEType(
>> =A0 =A0 const WebString& mime_type) {
>> - =A0if (!net::IsSupportedNonImageMimeType(UTF16ToASCII(mime_type).c_str=
()))
>> + =A0if (!net::IsSupportedNonImageMimeType(AsASCII(mime_type).c_str()))
>> =A0 =A0 return WebMimeRegistry::IsNotSupported;
>> =A0 return WebMimeRegistry::IsSupported;
>> =A0}
>> @@ -72,7 +84,7 @@
>> =A0WebString SimpleWebMimeRegistryImpl::preferredExtensionForMIMEType(
>> =A0 =A0 const WebString& mime_type) {
>> =A0 FilePath::StringType file_extension;
>> - =A0net::GetPreferredExtensionForMimeType(UTF16ToASCII(mime_type),
>> + =A0net::GetPreferredExtensionForMimeType(AsASCII(mime_type),
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 &file_extension);
>> =A0 return FilePathStringToWebString(file_extension);
>> =A0}
>>
>> Property changes on: webkit\glue\simple_webmimeregistry_impl.cc
>> ___________________________________________________________________
>> Modified: svn:mergeinfo
>> =A0 Merged /trunk/src/webkit/glue/simple_webmimeregistry_impl.cc:r22027
>>
>>
>>
>
>

Expand Messages | Collapse Messages