Bug 136035 – Use NSURLFileTypeMappings directly instead of depending on WebKitSystemInterface wrappers for it

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Maciej Stachowiak]

Use NSURLFileTypeMappings directly instead of depending on WebKitSystemInterface wrappers for it

[Maciej Stachowiak]

Created attachment 236746 [details]
Patch

[Maciej Stachowiak]

Created attachment 236751 [details]
Attempt #2

Try again to see if this actually builds without internal headers

[Maciej Stachowiak]

Created attachment 236777 [details]
Attempt #3

Trying yet another version (can't easily test locally because precompiled headers).

[Mark Rowe (bdash)]

Comment on attachment 236777 [details]
Attempt #3

View in context: https://bugs.webkit.org/attachment.cgi?id=236777&action=review

> Source/WebCore/platform/spi/cocoa/NSURLFileTypeMappings.h:27
> +#ifndef NSURLFileTypeMappings_h
> +#define NSURLFileTypeMappings_h

It seems a bit sketchy to define NS-prefixed macros.

[Maciej Stachowiak]

Created attachment 236797 [details]
Patch

[Maciej Stachowiak]

Created attachment 236801 [details]
New version to avoid breaking non-clang platforms

Comment on attachment 236797 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236797&action=review

> Source/WTF/wtf/Platform.h:1046
> +#if PLATFORM(COCOA) &&  defined __has_include && __has_include(<CoreFoundation/CFPriv.h>)

Extra space after the &&. I see that sometimes we use parentheses after the 'defined' keyword and sometimes we don’t.

I wonder if instead of checking for the presence of a specific header we shouldn’t be using something that’s directly tied to how our configuration files define SDKROOT, but this seems fine for now.

> Source/WebCore/platform/ios/MIMETypeRegistryIOS.mm:36
>      return wkGetMIMETypeForExtension(ext);
> +    return [[NSURLFileTypeMappings sharedMappings] MIMETypeForExtension:(NSString *)ext];

You forgot to delete the first return statement!

> Source/WebCore/platform/mac/MIMETypeRegistryMac.mm:37
> +String MIMETypeRegistry::getMIMETypeForExtension(const String& ext)

Good opportunity to rename the parameter to “extension”.

Comment on attachment 236801 [details]
New version to avoid breaking non-clang platforms

View in context: https://bugs.webkit.org/attachment.cgi?id=236801&action=review

r=me assuming you fix MIMETypeRegistryIOS.mm prior to landing.

> Source/WebCore/platform/ios/MIMETypeRegistryIOS.mm:35
>      return wkGetMIMETypeForExtension(ext);

Need to delete this!

[Maciej Stachowiak]

(In reply to comment #8)
> (From update of attachment 236801 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236801&action=review
> 
> r=me assuming you fix MIMETypeRegistryIOS.mm prior to landing.

I fixed it and confirmed it builds on iOS now.

> 
> > Source/WebCore/platform/ios/MIMETypeRegistryIOS.mm:35
> >      return wkGetMIMETypeForExtension(ext);
> 
> Need to delete this!

[Maciej Stachowiak]

Committed r172749: <http://trac.webkit.org/changeset/172749>