| Summary: | Use NSURLFileTypeMappings directly instead of depending on WebKitSystemInterface wrappers for it | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Maciej Stachowiak <mjs> | ||||||||||||
| Component: | New Bugs | Assignee: | Maciej Stachowiak <mjs> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | aestes, benjamin, cmarcelo, commit-queue, dbates, ddkilzer, mitz | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Maciej Stachowiak
2014-08-17 23:25:23 PDT
Created attachment 236746 [details]
Patch
Created attachment 236751 [details]
Attempt #2
Try again to see if this actually builds without internal headers
Created attachment 236777 [details]
Attempt #3
Trying yet another version (can't easily test locally because precompiled headers).
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. Created attachment 236797 [details]
Patch
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! (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! Committed r172749: <http://trac.webkit.org/changeset/172749> |