<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add a module map file for PrivateFrameworks/WebKitLegacy"
   href="https://bugs.webkit.org/show_bug.cgi?id=230735#c17">Comment # 17</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add a module map file for PrivateFrameworks/WebKitLegacy"
   href="https://bugs.webkit.org/show_bug.cgi?id=230735">bug 230735</a>
              from <span class="vcard"><a class="email" href="mailto:iana@apple.com" title="Ian Anderson <iana@apple.com>"> <span class="fn">Ian Anderson</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=441851&action=diff" name="attach_441851" title="Patch">attachment 441851</a> <a href="attachment.cgi?id=441851&action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=441851&action=review">https://bugs.webkit.org/attachment.cgi?id=441851&action=review</a>

<span class="quote">>> Source/WebCore/platform/ios/WebItemProviderPasteboard.h:-30
>> -struct CGSize;

> Why not put the #Import inside #if TARGET_OS_IOS/#endif?</span >

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

<span class="quote">>> 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.</span >

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.

<span class="quote">>> 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></span >

Yeah, check-webkit-style said no.

<span class="quote">>> 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?</span >

Sure.

<span class="quote">>> 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.</span >

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.

<span class="quote">>> Source/WebKitLegacy/mac/Storage/WebDatabaseQuotaManager.h:27
>> +#import <objc/NSObject.h>

> Why <objc/NSObject.h> instead of <Foundation/NSObject.h>?</span >

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>?</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>