[Webkit-unassigned] [Bug 230735] Add a module map file for PrivateFrameworks/WebKitLegacy

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=230735

--- Comment #17 from Ian Anderson <iana at apple.com> ---
Comment on attachment 441851
  --> https://bugs.webkit.org/attachment.cgi?id=441851
Patch

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

>> Source/WebCore/platform/ios/WebItemProviderPasteboard.h:-30
>> -struct CGSize;
> 
> Why not put the #Import inside #if TARGET_OS_IOS/#endif?

It needs to be outside because that's what's pulling in TargetConditionals and defining TARGET_OS_IOS.

>> Source/WebCore/platform/ios/wak/WAKAppKitStubs.h:-32
>> -#import <Foundation/Foundation.h>
> 
> Why does this need to go outside `#if TARGET_OS_IPHONE`?  This doesn't seem necessary since the entire contents is wrapped in #if TARGET_OS_IPHONE/#endif.

Same everywhere else, all of these TARGET_OS_ macros were being used undefined (and failing). We can include TargetConditionals explicitly everywhere, but you get it for free from Foundation and CoreGraphics and we need to include those headers anyway, seemed shorter to pull those out of the TARGET_OS_ chunks.

>> Source/WebKitLegacy/mac/DOM/WebDOMOperationsPrivate.h:31
>>  #import <JavaScriptCore/JSBase.h>
> 
> The header order should probably be this (since we include the non-private header first?  Or does that give a check-webkit-style error?
> 
> #import <WebKitLegacy/WebDOMOperations.h>
> #import <Foundation/Foundation.h>
> #import <JavaScriptCore/JSBase.h>

Yeah, check-webkit-style said no.

>> Source/WebKitLegacy/mac/Misc/WebDownload.h:33
>>  #define WebDownload_h
> 
> It's a bit strange that there are #import statements above this header guard.  Can they be moved inside the header guard?

Sure.

>> Source/WebKitLegacy/mac/Misc/WebDownload.h:40
>>  #import <WebKitLegacy/NSURLDownloadSPI.h>
> 
> Both WebDownload.h and NSURLDownloadSPI.h are marked as Private headers, so we should be able to replace all of this with just:
> 
> #import <WebKitLegacy/NSURLDownloadSPI.h>
> 
> Since that has al the same logic as this block of macros.

I did that in one version of this and I'm trying to remember why I didn't leave it that way. I thought someone had told me to not do it like that but I don't see that comment either. Let me try and see what happens.

>> Source/WebKitLegacy/mac/Storage/WebDatabaseQuotaManager.h:27
>> +#import <objc/NSObject.h>
> 
> Why <objc/NSObject.h> instead of <Foundation/NSObject.h>?

Just because it didn't use anything from Foundation. It's unintuitively worse for build performance to import <Foundation/NSObject.h> than it is to import <Foundation/Foundation.h>, but do you want me to switch all these to <Foundation/Foundation.h>?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211021/9a00ef55/attachment.htm>